GNOME Bugzilla – Bug 759624
v4l2: Exposes colorimetry for RGB format which confuses videoconvert
Last modified: 2016-04-16 14:43:52 UTC
While testing with v4l2src and videoconvert I came across some unexpected behavior. By changing the frame size i could cause the output frame colors to break. The following example can be run in vivid to verify that it doesn't work: this is wrong: gst-launch-1.0 -v v4l2src device=/dev/video3 ! video/x-raw,width=640,height=360,format=RGB ! videoconvert ! video/x-raw,format=I420 ! videoconvert ! ximagesink this is right:
-- CONTINUING FROM ABOVE -- gst-launch-1.0 -v v4l2src device=/dev/video3 ! 'video/x-raw,width=1280,height=720,format=RGB,colorimetry=(string)1:4:7:1' ! videoconvert ! video/x-raw,format=I420 ! videoconvert ! ximagesink Running the same pipelines with videotest src would not break the results in the first case. Comparing the two pipelines I was able to notice that the colorimetry was being fixed to "1:4:7:1", whereas with videotestsrc the colorimetry was not set. After even further investigation I was able to get this debug info for the above pipelines: For the bad one: 00:00.511107821 1342 0x743e30 DEBUG video-converter video-converter.c:1501:chain_scale: 230400 <> 230400 0:00:00.511127805 1342 0x743e30 DEBUG video-converter video-converter.c:1545:chain_convert: matrix 4 -> 4 (1) 0:00:00.511146296 1342 0x743e30 DEBUG video-converter video-converter.c:1547:chain_convert: bits 8 -> 8 (1) 0:00:00.511168155 1342 0x743e30 DEBUG video-converter video-converter.c:1549:chain_convert: primaries 1 -> 4 (1) For the good one: 0:00:00.588955810 1348 0x743e30 DEBUG video-converter video-converter.c:1545:chain_convert: matrix 4 -> 3 (0) 0:00:00.588978150 1348 0x743e30 DEBUG video-converter video-converter.c:1547:chain_convert: bits 8 -> 8 (1) 0:00:00.588996163 1348 0x743e30 DEBUG video-converter video-converter.c:1549:chain_convert: primaries 1 -> 1 (1) 0:00:00.589018669 1348 0x743e30 DEBUG video-converter video-converter.c:1593:chain_convert: to RGB matrix It seems to verify that the colorimetries are different for different frame sizes. At this point the subject is outside of my knowledge so I am not even aware if this is a problem of v4l2, v4l2src or videoconvert. Anyone know what is going on here?
Which version of GStreamer is this ? Can you check if a simple pipeline like this one sets the colorimetry in your case: gst-launch-1.0 -v v4l2src num-buffers=1 ! fakesink
Thanks for the tip Nicolas, it seems that the problem is between the two videoconverts, they seem to negotiate the colorimetry to 1:3:8:1 rather than 1:4:8:1. to replicate: run the following pipelines with vivid wrong pipeline: gst-launch-1.0 -v v4l2src device=/dev/video3 ! video/x-raw,width=640,height=360,format=RGB ! videoconvert ! 'video/x-raw,format=I420,colorimetry=(string)1:4:8:1' ! videoconvert ! ximagesink right pipeline: gst-launch-1.0 -v v4l2src device=/dev/video3 ! video/x-raw,width=640,height=360,format=RGB ! videoconvert ! 'video/x-raw,format=I420,colorimetry=(string)1:3:8:1' ! videoconvert ! ximagesink
I am running on 1.6.0
I'm not sure what to do here. There is a bug in vivid driver, since it pretends having a BT601 color matrix for the RGB format. I would expect the identity matrix here (hence something starting with "1:1:...". At the same time, I can't verify if videoconvert is doing exactly the right thing. Here's a pipeline that produce the same output without v4l2src: gst-launch-1.0 -v videotestsrc ! capssetter join=0 replace=1 caps="video/x-raw,width=640,height=360,format=RGB,colorimetry=(string)1:4:7:1,framerate=30/1,pixel-aspect-ratio=1/1,interlace-mode=progressive" ! videoconvert ! 'video/x-raw,format=I420,colorimetry=(string)1:4:7:1' ! videoconvert ! gtksink In this scenario, we have videotestsrc that produce RGB color in a way that no transformation is needed, but using a capssetter I make it pretend it produces 1:4:7:1 (like vivid do). This lead to similar artefact. Another pipeline with similar output, but only one converter: gst-launch-1.0 -v videotestsrc ! capssetter join=0 replace=1 caps="video/x-raw,width=640,height=360,format=RGB,colorimetry=(string)1:4:7:1,framerate=30/1,pixel-aspect-ratio=1/1,interlace-mode=progressive" ! videoconvert ! 'video/x-raw,format=I420,colorimetry=(string)1:4:7:1' ! glimagsink Let's ask Wim if this is the right behaviour for the bogus input. Is so, I'll have to close as invalid, and we should report/fix vivid that reports bogus colorimetry in RGB.
Just some more information. This is the output from vivid driver: root@qt5022:~# v4l2-ctl -V -d /dev/video0 Format Video Capture: Width/Height : 640/360 Pixel Format : 'RGB3' Field : None Bytes per Line : 1920 Size Image : 691200 Colorspace : sRGB Transfer Function : Default YCbCr Encoding : Default Quantization : Default Flags : On gstv4l2object.c I found the following lines: case V4L2_COLORSPACE_SRGB: case V4L2_COLORSPACE_JPEG: /* This is in fact sYCC */ cinfo->range = GST_VIDEO_COLOR_RANGE_0_255; cinfo->matrix = GST_VIDEO_COLOR_MATRIX_BT601; cinfo->transfer = GST_VIDEO_TRANSFER_SRGB; cinfo->primaries = GST_VIDEO_COLOR_PRIMARIES_BT709; are we sure that we need to set the matrix value to bt601 for srgb colorspace? I have contacted also the vivid developer (Thanks Hans!) and he has replied the following: """ Just to be clear, the ycbcr_encoding value is only valid for YCbCr formats and should be ignored for others. I actually think vivid returns V4L2_XFER_FUNC_DEFAULT whenever the format isn't YCbCr, but I'm not 100% certain of that. The ycbcr_encoding obviously makes no sense if the format isn't YCbCr. """ I dont think that this is the issue here, but I believe that this is not handled properly in the code neither. Thanks!
Created attachment 319332 [details] [review] Patch to fix colorspace for RGB images I am attaching a patch with the RGB colorspace mapping with the RGB colorspace matrix. Testing it under vivid using the following pipeline seems to work: gst-launch-1.0 -v v4l2src device=/dev/video3 ! video/x-raw,width=640,height=360,format=RGB ! videoconvert ! 'video/x-raw,format=I420,colorimetry=(string)1:4:8:1' ! videoconvert ! ximagesink We have also tested under a UVD camera integrated on a Thinkpad W520, vivid and a proprietary camera (that uses vb2)
I've made it match what is documented here: http://linuxtv.org/downloads/v4l-dvb-apis/ch02s06.html#col-srgb
Review of attachment 319332 [details] [review]: I don't think this is correct, or at least the comment in your patch does not explain how using the identity matrix is correct here. sRGB is usually applied to YUY2 color format. The odd thing is that vivid applies it to an RGB pixel formats. Normally all RGB pixel format comes with identity matrix. I'd like some input from real experts here, I'll ask attention from Wim and Hans.
In my analyses I found that the matrix in GStreamer is called encoding in V4L2. V4L2 API lack the notion of identity encoding. I'd like to know from Hans what this means for non-YUV formats, as this does not seem documented. V4L2 isn't clear about what colorspace shall be used for format that require no transformation.
Hello. I have been hyper busy lately, so I have not been able to dig further. But maybe this is useful for you! I got this response from Hans the 5th of January: Just to be clear, the ycbcr_encoding value is only valid for YCbCr formats and should be ignored for others. I actually think vivid returns V4L2_XFER_FUNC_DEFAULT whenever the format isn't YCbCr, but I'm not 100% certain of that. The ycbcr_encoding obviously makes no sense if the format isn't YCbCr. Regards, Hans Since the pixelformat is RGB the ycbcr encoding is undefined (Default as reported by vivid). I don't know what gstreamer expects in that case. Is the gstreamer code perhaps confusing pixelformat and colorspace? For non-ycbcr formats the matrix should be a unity matrix. Anyway, what vivid reports looks OK to me: RGB pixelformat and sRGB colorspace with the other colorspace-related values set to Default (there are V4L2_MAP_*_DEFAULT macros in videodev2.h to calculate how to map a Default value to the actual format). Regards, Hans Can you check first if I am right in my assumption that gstreamer doesn't handle the new colorspace fields? (i.e. the ycbcr_enc, quantization and xfer_func fields of v4l2_pix_format). All I am basing it on is the code snippet you pasted in your mail and a suspicion, but I could be very wrong about that. A grep for those fields in the source code would be sufficient. Note that xfer_func was added last, so there is a chance that they support ycbcr_enc and quantization, but not yet xfer_func. Anyway, feel free to add my mail to their bugzilla if I am right. Thanks!
Not that I'm an expert, but it seems to make sense, no, since there are no YUV components involved here? enum GstVideoColorMatrix: The color matrix is used to convert between Y'PbPr and non-linear RGB (R'G'B') - GST_VIDEO_COLOR_MATRIX_RGB = identity matrix.
Then dimitrios patch is incorrect according to you exchange with Hans. Please CC linux-media next time, so we don't ask Hans the same question over and over.
Review of attachment 319332 [details] [review]: So this patch is incorrect. Instead the code should check the select format, and ignore the encoding if it's not a YUV pixel format.
Btw, the problem in Hans documentation is that it does not say what happens when this isn't not a YUV (it's not fully specified). On GStreamer side, even if we have the identity matrix, we still don't have identity for the other function, so better not set colorimetry at all in the caps in this case.
Master: commit fd3244060900ece017ea22c3a7e12bd767defc00 Author: Nicolas Dufresne <nicolas.dufresne@collabora.com> Date: Tue Jan 19 15:14:59 2016 -0500 v4l2object: Don't set colorimetry for non YUV formats Setting colormetry in caps for RGB have no meaning, but worst it confuses the converters downstream. https://bugzilla.gnome.org/show_bug.cgi?id=759624 And 1.6 as this is where the regression was introduced. commit f96c72d1106f9fbd0c794884f35f2bc22c4567d4
Had some more feedback from Hans. It does not fully clarify my mind about what those colorimetry value really means for the RGB data we received. Some statement in the doc seems to be in favour the the R'G'B component found in the frames could even be in limited range. That would indicate a partial color conversion having taken place. I have no idea if this is actually supported by GStreamer. Hans is then making a guess on what GStreamer should do, though I'm adding Wim in CC here, as I'm missing information. Note that the change I just did still holds since it fixes the regression by bringing back the old behaviour. Wim, if you feel this is incorrectly fixed, let me know, and I'll re-open and revisit this. On 01/19/2016 07:00 PM, Nicolas Dufresne wrote: > Hi Hans, > > we are having issues in GStreamer with the colorspace in V4L2. The API > does not provide any encoding for RGB formats. Which API? GStreamer or V4L2? > The encoding matrix for > those is usually the identity matrix, anything else makes very little > sense to me. While normally RGB formats use the sRGB colorspace, this is by no means always the case. HDMI for example also supports AdobeRGB and BT2020 RGB. > For example, vivid will declare a stream with RGB based > pixel format as having the default for sRGB colorspace, which lead to > non-identity syCC encoding. I don't follow. sYCC is for YCbCr formats. RGB formats do not contain any information about YCbCr (i.e. the ycbcr_enc field should be ignored). If gstreamer wants to convert RGB formats to YCbCr formats, then it can choose whatever RGB->YCbCr conversion it wants. The colorspace (i.e. the chromaticities), xfer_func and quantization fields as reported by V4L2 are all still valid for RGB pixelformats. You need those as well: take an HDMI receiver that converts Y'CbCr to R'G'B' (let's be precise here and use the quote). If the input is HDTV using Rec.709, then the colorspace is set to V4L2_COLORSPACE_REC709 and the other fields are all 0 (DEFAULT). These map to XFER_FUNC_709 for the transfer function, QUANTIZATION_FULL_RANGE for the quantization and ycbcr_enc is ignored since there is nothing to do here (the Y'CbCr to R'G'B' conversion is already done in hardware using the Rec. 709 Y'CbCr encoding). If you would just ignore all fields and use COLORSPACE_SRGB, then you would be using the wrong transfer function (XFER_FUNC_SRGB instead of XFER_FUNC_709). > Shall we simply ignore the encoding set by drivers when the pixel > format is RGB based ? To me it makes very little sense, but the code in > GStreamer is very generic and this wrong information lead to errors > when the data is converted to YUV and back to RGB. It seems to me that for RGB formats GStreamer should just set cinfo->matrix (which I assume is the Y'CbCr to R'G'B' matrix) to the unity matrix and everything else follows the normal rules. Regards, Hans ====
(In reply to Nicolas Dufresne (stormer) from comment #17) > Had some more feedback from Hans. It does not fully clarify my mind about > what those colorimetry value really means for the RGB data we received. Some > statement in the doc seems to be in favour the the R'G'B component found in > the frames could even be in limited range. That would indicate a partial > color conversion having taken place. I have no idea if this is actually > supported by GStreamer. Hans is then making a guess on what GStreamer should > do, though I'm adding Wim in CC here, as I'm missing information. Note that > the change I just did still holds since it fixes the regression by bringing > back the old behaviour. > > Wim, if you feel this is incorrectly fixed, let me know, and I'll re-open > and revisit this. > It looks incorrect. colorimetry for RGB is needed to get the range, transfer function and primaries right. You should ignore the YUV->RGB function because it is unused. The bug is that videoconvert does not ignore the matrix for RGB formats. I'll have a look at that. I see some things that probably also need to be done to make things nicer: 1) Always try to set the matrix function to RGB when dealing with RGB colorimetry. This should probably be done in gst_video_info_to_caps() 2) set the matrix function in the colorimetry to RGB when parsing RGB caps in gst_video_info_from_caps()
commit 83fe1c7705172ae0a59ce388edd3dc3f2fd916e7 Author: Wim Taymans <wtaymans@redhat.com> Date: Wed Jan 20 10:02:20 2016 +0100 video-converter: ignore matrix for RGB formats For RGB formats, the matrix in the colorimetry (conversion from YUV to RGB) is irrelevant and we should ignore it and assume the identity transform for everything we do. Fixes https://bugzilla.gnome.org/show_bug.cgi?id=759624
commit 773e2476e64011474f1503b184180d1fa81f1162 Author: Wim Taymans <wtaymans@redhat.com> Date: Wed Jan 20 10:19:34 2016 +0100 video-info: enfore RGB matrix for RGB formats In gst_video_info_to_caps(), make sure we end up with an RGB matrix for RGB formats and warn when the GstVideoInfo colorimetry is wrong. In gst_video_info_from_caps(), fix the GstVideoInfo with an RGB matrix for RGB formats and warn about inconsistent caps. See https://bugzilla.gnome.org/show_bug.cgi?id=759624
So now, you could set colorimetry on v4l2src unconditionally. It could make sense to set the matrix to RGB for RGB formats, otherwise, video-info will fix that for you.
> It seems to verify that the colorimetries are different for different frame > sizes. At this point the subject is outside of my knowledge so I am not even > aware if this is a problem of v4l2, v4l2src or videoconvert. Anyone know what > is going on here? When no colorimetry is specified, we take defaults. The default depends on the resolution, we take BT601 for SD, BT709 for HD and BT2020 for UHD resolutions. You can see a color change in the blue and purple when resizing with this example pipeline: gst-launch-1.0 videotestsrc ! video/x-raw,format=AYUV ! videoconvert ! ximagesink
Ok, he're the next step. I'll backport: commit 83fe1c7705172ae0a59ce388edd3dc3f2fd916e7 commit 773e2476e64011474f1503b184180d1fa81f1162 And produce a new patch that moves the YUV check in v4l2 into the function that converts to GstVideoColorimetry structure. So from now on, we'll expose the colorimetry field for RGB too, but set the matrix to identity. As said, this is not longer requires, as the matrix have no meaning, but it will make it less weird when debugging the caps transformation.
This is fixed and backported now.