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

feat: add yuv support #78

Merged
merged 3 commits into from
Feb 27, 2022
Merged

feat: add yuv support #78

merged 3 commits into from
Feb 27, 2022

Conversation

wep21
Copy link
Collaborator

@wep21 wep21 commented Feb 20, 2022

Signed-off-by: wep21 border_goldenmarket@yahoo.co.jp

Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
@wep21
Copy link
Collaborator Author

wep21 commented Feb 20, 2022

@jbohren @clydemcqueen Could you review this PR? This PR supports yuv image encoding.

@clydemcqueen
Copy link

Nice improvement! Looks good to me.

I tested this on Foxy, Galactic and Rolling.

I noticed that the image_encoding parameter is missing from the README. (This is true for ROS1 as well.) Can you add it to the README and list the supported encodings?

Possible future work: it would be nice to have a set of tests that quickly run through the possible encodings and check for output.

@wep21
Copy link
Collaborator Author

wep21 commented Feb 23, 2022

@jbohren friendly ping

@wep21 wep21 requested a review from jbohren February 26, 2022 18:24
Signed-off-by: wep21 <border_goldenmarket@yahoo.co.jp>
@wep21
Copy link
Collaborator Author

wep21 commented Feb 27, 2022

@clydemcqueen I added the parameter description at e3fc18c

@clydemcqueen
Copy link

@wep21 LGTM++

@wep21 wep21 merged commit 1a260ae into ros-drivers:ros2 Feb 27, 2022
@wep21 wep21 deleted the add-yuv-support branch February 27, 2022 22:53
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.

None yet

2 participants