-
Notifications
You must be signed in to change notification settings - Fork 105
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 YUV format codes and documentation #214
Conversation
995a3cf
to
3d472ab
Compare
Use format codes instead of ambiguous names. Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de>
3d472ab
to
e51d356
Compare
The naming of the format was discussed in #76. As mentioned there, one of the reasons of adding the format was to be able to then extend cv_bridge to be able to perform the right conversion. OpenCV defines: cv::COLOR_YUV2RGB_YUY2 = 115,
cv::COLOR_YUV2BGR_YUY2 = 116, and then the aliases: cv::COLOR_YUV2RGB_YUYV = COLOR_YUV2RGB_YUY2,
cv::COLOR_YUV2BGR_YUYV = COLOR_YUV2BGR_YUY2,
cv::COLOR_YUV2RGB_YUNV = COLOR_YUV2RGB_YUY2,
cv::COLOR_YUV2BGR_YUNV = COLOR_YUV2BGR_YUY2, so hinting they treat YUY2 as the canonical name, but also YUYV and YUNV as alternative names, seemingly following the table that used to be at fourcc.org. It is true that this doesn't follow the V4L2 codes and that V4L2 drivers return the code FWIW, Windows seems to use YUY2 as well and does not use YUYV as an alias AFAICT. Also see RFC2361. FFmpeg mixes things a bit: running
so they use the YUY2 FourCC, which they then map to the internal yuyv422 type. With all that said, I'd agree that YUYV would be clearer and I wouldn't be against adopting that per se, I had that name in mind initially as well. But for almost everywhere I look except for V4L2, YUY2 is the common default name to refer to this encoding. |
Thanks for the detailed response. That means that I wouldn't so much rely on OpenCV for choosing those names. It is more important that they properly specify the precise memory layout so that the image can be reconstructed manually from the raw data. When you get a data stream from the camera with a given fourcc, then the mapping to the ROS "encodings" should be unambiguous (see also #204). But in any case, I find the |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks cleaner and more clearly documented.
Use format codes instead of ambiguous names. Signed-off-by: Christian Rauch <Rauch.Christian@gmx.de> Signed-off-by: Tom Noble <tom@rivelinrobotics.com>
**User-Facing Changes** - Renamed raw image encoding `uyuv` to `uyvy` for accuracy. - Added support for raw image encoding `yuv422_yuy2` as an alias for `yuyv`. **Description** - Fixes #6271, fixes FG-3953 - Schemas PR: foxglove/schemas#121 All [YUV 4:2:2 formats](https://en.wikipedia.org/wiki/Chroma_subsampling) encode one U and one V value for every two Y values. The byte orders supported by Studio and by other apps in the ecosystem are `[U, Y1, V, Y2]` and `[Y1, U, Y2, V]`. - Replaces `uyuv`, which seems to have been a typo introduced in #5292, with `uyvy`, which accurately represents the byte order we are decoding. - Adds `yuv422_yuy2` as an encoding alias for `yuyv`, taken from ROS: - [sensor_msgs/image_encodings.hpp](https://github.com/ros2/common_interfaces/blob/366eea24ffce6c87f8860cbcd27f4863f46ad822/sensor_msgs/include/sensor_msgs/image_encodings.hpp) - ros2/common_interfaces#76 - ros2/common_interfaces#214 - `yuv422_yuy2` is the default encoding used by [usb_cam](https://github.com/ros-drivers/usb_cam/tree/ros2) - Remove incorrect and useless `Int8Array` type casting - Update YUV-to-RGB conversion formula to be faster and match RViz code - Added story for both UYVY and YUYV - Confirmed compatibility with rqt_image_view and rviz (although those two look slightly different from each other): <img width="1730" alt="Screenshot 2023-07-13 at 5 02 16 PM" src="https://github.com/foxglove/studio/assets/14237/c7be26e5-3bee-417b-8e67-c3a305e8453c"> Other references: - https://www.kernel.org/doc/html/latest/userspace-api/media/v4l/pixfmt-packed-yuv.html
The YUV formats seem to be ambiguous. With this PR, I would like to clarify this and properly document their memory layout. There is no "yuv422_yuy2" format and the "yuv422" format refers to the format code
YU16
. The documentation mentions a "UYUV" format, which does not exist. Additionally, the website http://www.fourcc.org is not reachable anymore.With the wayback machine, I compared the documented memory layout from http://www.fourcc.org with the one from the Linux kernel documentation from https://www.kernel.org/doc/html/latest/. I found that for the
yuv422
the memory layout described at fourcc.org matchesV4L2_PIX_FMT_UYVY
at kernel.org andyuv422_yuy2
matchesV4L2_PIX_FMT_YUYV
.The NV21 and NV24 formats have unambiguous format codes across the kernel documentation. For the YUV 4:2:2 formats, different sources seem to use different names, hence I propose to use the fourcc instead of an ambiguous name.
See https://git.kernel.org/pub/scm/linux/kernel/git/torvalds/linux.git/tree/include/uapi/linux/videodev2.h for a list of codes used with video devices.
@sgvandijk Since you added the "yuv422_yuy2" format via #78, can you comment if my changes make sense to you?