GNOME Bugzilla – Bug 620441
[video] Add support for Y800 and Y16 formats
Last modified: 2010-06-07 12:12:41 UTC
Created attachment 162618 [details] [review] A patch implementing the requested feature The fourcc formats Y800 (aka Y8 or GREY http://www.fourcc.org/yuv.php#Y800) and Y16 (http://www.fourcc.org/yuv.php#Y16) don't have a GstVideoFormat defined in gst-libs/gst/video/video.h. Y800 actually is GST_VIDEO_FORMAT_GRAY8 and Y16 is actually GST_VIDEO_FORMAT_GRAY16_LE. However, if the function gst_video_format_from_fourcc is called with either 'Y800', 'Y8 ' or 'Y16 ', it will return GST_VIDEO_FORMAT_UNKNOWN. These formats should be recognized. I submitted a patch that fixes this by considering these YUV formats the same as their YUV equivalent. The only drawback I see from this patch is the fact that the function gst_video_format_new_caps will return a YUV caps when the formats GST_VIDEO_FORMAT_GRAY{8,16_LE} are given. If this is a problem, it could easily be changed to prioritize video/x-raw-gray caps by changing the order of the checks in the function. As soon as I will have submitted this, I will submit a second patch with a different approach.
Created attachment 162619 [details] [review] A second patch implementing the same feature with a different approach This patch is a second approach to the problem of recognizing Y800 and Y16 formats. Instead of considering that they are the same as GST_VIDEO_FORMAT_GRAY8 and GST_VIDEO_FORMAT_GRAY16_LE, new types are defined. This doesn't clash with previous definitions, but causes a little bit of code duplication. I'm not sure which approach is to be prefered.
I think we should get rid of Y800, Y8 and Y16 and use the GRAY formats instead.
Are they actually the same formats? Or might they have different value ranges? (16-235 vs. 0-255)
You are right, they are NOT in the same range. So this rules out my first patch where I considered gray formats the same as Y8 and Y16. The second should be fine though, and it doesn't change existing behavior, just adds new formats.
Then we should definitely add support for them in ffmpegcolorspace too. Do you know if Y16 is little or big endian?
According to http://www.fourcc.org/yuv.php#Y16, Y16 is little endian.
While working on a patch for ffmpegcolorspace, I noticed that the function gst_ffmpeg_caps_to_pixfmt considers that the pix format is set to PIX_FMT_GRAY8 when the fourcc format is Y800. That's what ffmpegcolorspace does when it encounters Y800: consider it the same as GRAY8. This is a wrong, althought hard to notice, assumption (range [16-235] VS [0-255]), which means that a fix should be made. I'm working on it.
Created attachment 162787 [details] [review] A patch implementing the requested feature with addition to ffmpegcolorspace This patch is a more complete addition of the two new formats: Y800 and Y16. It also adds the required conversion routines in ffmpegcolorspace.
Created attachment 162788 [details] [review] A patch necessary for the proper parsing of the caps This patch has to be made to gstreamer so that patch "162787: A patch implementing the requested feature with addition to ffmpegcolorspace" works. It just modifies the way fourcc is parsed into a integer so that the fourcc formats that don't have 4 caracters (like Y8 or Y16) are parsed properly.
Created attachment 162790 [details] A batch file with test pipelines for the submitted patches This file contains a bunch of pipelines that I've used to test the conversions between the new added formats and other formats.
Thanks but please use git format-patch style patches in the future :) commit 39b68dc2a89ff412ac91e23f2d01ffe5bc7275dc Author: Sebastian Dröge <sebastian.droege@collabora.co.uk> Date: Mon Jun 7 08:31:25 2010 +0200 ffmpegcolorspace: Map "Y8 " and "GREY" to "Y800" and add it to the template commit 4c0b39b68014b3c934d9723933bae1dd31d7787f Author: Martin Bisson <martin.bisson@gmail.com> Date: Mon Jun 7 08:17:13 2010 +0200 ffmpegcolorspace: Add support for Y800 and Y16 Fixes bug #620441. commit b0d15133d7a6af36a5632e1fd9278c04c32d29b8 Author: Martin Bisson <martin.bisson@gmail.com> Date: Mon Jun 7 08:16:01 2010 +0200 video: Add support for Y800 and Y16 Fixes bug #620441. commit 28fdbee35a216555bd4a0c814e0e520e41e9c2e7 Author: Martin Bisson <martin.bisson@gmail.com> Date: Mon Jun 7 08:18:40 2010 +0200 value: Add support for parsing short fourccs from strings For example "Y16 " and "Y8 ".