Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Update tracker_node.py #15

Closed
wants to merge 1 commit into from

Conversation

Gowresh7
Copy link

@Gowresh7 Gowresh7 commented Aug 7, 2023

Added a logic to handle 'bgr8a' encoding, usually found in simulation image sensors

Added a logic to handle 'bgr8a' encoding, usually found in simulation image sensors
Copy link
Owner

@Alpaca-zip Alpaca-zip left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you @Gowresh7 for your Pull request!

Upon reviewing the code, I am contemplating the implications of merging this PR.

The current implementation I've written does not explicitly handle the encoding of the subscribed topics; it simply passes the images to the track function as a numpy array. However, as per the official Ultralytics documentation, images passed to this function are expected to be in BGR encoding.

Given this, I would strongly advise ensuring that the image topics are in BGR encoding before utilizing my repository.

I acknowledge the need for conversion from various encodings to BGR, and I will certainly consider this as a future enhancement for the repository.

Comment on lines +46 to +47
numpy_image = cv2.cvtColor(numpy_image, cv2.COLOR_RGBA2RGB)
encoding = "bgr8"
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Your code uses tab for indentation, but please unify it with 4 spaces.
  • cv2.COLOR_RGBA2RGB converts from RGBA to RGB and outputs in RGB order. However, encoding = "bgr8" indicates BGR order.
Suggested change
numpy_image = cv2.cvtColor(numpy_image, cv2.COLOR_RGBA2RGB)
encoding = "bgr8"
numpy_image = cv2.cvtColor(numpy_image, cv2.COLOR_RGBA2RGB)
encoding = "rgb8"

@@ -40,6 +40,12 @@ def image_callback(self, msg):
header = msg.header
encoding = msg.encoding
numpy_image = ros_numpy.numpify(msg)

# Convert RGBA to RGB
if numpy_image.shape[2] == 4: # Check if the image has 4 channels
Copy link
Owner

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This section assumes that all 4-channel images are RGBA. If the original encoding is bgra8 or bgra16, this could lead to bugs.

@Alpaca-zip
Copy link
Owner

Added a logic to handle 'bgr8a' encoding, usually found in simulation image sensors

You commented above, but looking at the code, I think you are assuming rgba8. Which is correct?

@Gowresh7
Copy link
Author

Gowresh7 commented Aug 8, 2023

Hello,

Thanks for the feedback.

I just implemented this as a workaround because I was not able to make the code work for CARLA Simulation. I did not actually consider the ultralytics documentation.

For some reason, I was able to make it work only with 'bgr8' encoding. Will check it out further and let you know.

@Alpaca-zip
Copy link
Owner

Hello, @Gowresh7 .

This has already been improved in the #29.
Feel free to re-open this PR if you have any more improvements. Thank you for your cooperation.

Best regards,

@Alpaca-zip Alpaca-zip closed this Sep 8, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants