Add support for NV12 encoding in cv_bridge #544
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
- Update getCvType to return CV_8UC1 for NV12 encoding. - Extend Encoding enum to include NV12. - Enhance getEncoding to recognize NV12. - Add conversion codes for NV12 to GRAY, RGB, BGR, RGBA, and BGRA. - Modify conversion logic to handle NV12 as a color format. - Adjust matFromImage to account for height scaling in NV12 images. Signed-off-by: Zhaoyuan Cheng <[email protected]>
- Corrected the error message to include height scaling in the size calculation. - Adjusted the formatting for better readability. Signed-off-by: Zhaoyuan Cheng <[email protected]>
Author
|
@ahcorde could you please have a review for this PR? |
There was a problem hiding this comment.
Pull Request Overview
This PR adds support for the NV12 image encoding to the cv_bridge package, enabling conversion and proper handling of NV12-formatted data.
- Extended
getCvTypeandgetEncodingto recognize NV12. - Added NV12 conversion codes for GRAY, RGB, BGR, RGBA, and BGRA.
- Updated
matFromImageto apply height scaling for NV12’s planar layout.
Comments suppressed due to low confidence (2)
cv_bridge/src/cv_bridge.cpp:299
- NV12-specific logic was introduced here (height scaling check) and in conversion paths; please add unit tests that cover NV12
matFromImagebehavior andcvtColorconversions to ensure correctness.
if (source.height * source.step * enc::getHeightScaling(source.encoding) != source.data.size()) {
cv_bridge/src/cv_bridge.cpp:102
- [nitpick] Consider adding a comment explaining why NV12 is mapped to
CV_8UC1(i.e., single-channel Mat with interleaved Y and UV planes) to help future maintainers understand this choice.
if (encoding == enc::NV12) {return CV_8UC1;}
Comment on lines
+301
to
+302
| ss << "Image is wrongly formed: height * step * HeightScaling != size or " << source.height << " * " << | ||
| source.step << "*" << enc::getHeightScaling(source.encoding) << " != " << source.data.size(); |
There was a problem hiding this comment.
In the error message, HeightScaling is capitalized and concatenated; consider using a user-friendly phrase like "height scaling" (lowercase, with space) for clarity and consistency.
Suggested change
| ss << "Image is wrongly formed: height * step * HeightScaling != size or " << source.height << " * " << | |
| source.step << "*" << enc::getHeightScaling(source.encoding) << " != " << source.data.size(); | |
| ss << "Image is wrongly formed: height * step * height scaling != size or " << source.height << " * " << | |
| source.step << " * " << enc::getHeightScaling(source.encoding) << " != " << source.data.size(); |
Member
|
Thanks for the PR, and sorry for the delay in reviewing. Could you please update |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
This PR adds support for the NV12 encoding in the
cv_bridgepackage. Change depends on common_interfaces PR: ros2/common_interfaces#264The following changes have been made:
getCvTypeandgetEncodingfunctions to handle NV12 encoding.matFromImageto correctly handle height scaling for NV12 encoding.Changes
Encoding Support:
NV12to theEncodingenum.getCvTypeandgetEncodingfunctions to recognizeNV12.Conversion Codes:
NV12toGRAY,RGB,BGR,RGBA, andBGRAingetConversionCodes.Height Scaling:
matFromImageto account for height scaling when the encoding isNV12.