After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 725248 - videobox fails to negotiate in an environment with possible RGB input.
videobox fails to negotiate in an environment with possible RGB input.
Product: GStreamer
Classification: Platform
Component: gst-plugins-good
git master
Other All
: Normal normal
: 1.2.5
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Reported: 2014-02-26 18:02 UTC by Stirling Westrup
Modified: 2014-06-17 12:47 UTC
See Also:
GNOME target: ---
GNOME version: ---

Fix for the capnego problem (1.01 KB, patch)
2014-06-11 12:28 UTC, RaviKiran
needs-work Details | Review
Corrected patch (2.17 KB, patch)
2014-06-16 10:26 UTC, RaviKiran
needs-work Details | Review
Capsnego fix (1.93 KB, patch)
2014-06-17 07:48 UTC, RaviKiran
committed Details | Review

Description Stirling Westrup 2014-02-26 18:02:13 UTC
I have an element that only outputs a small range of RGB formats. I want to videoconvert to a YUV format, clip the results and output the final video to an xvimagesink.

However, I find that in this scenario, videobox fails to negotiate a format it can work with, and ends up rejecting all caps, causing the pipeline to fail. Here is the smallest reproduction of the error I've been able to create:

gst-launch-1.0 videotestsrc  ! "video/x-raw,format={BGRx,BGRA,RGB,RGB16,RGB15}" ! videoconvert ! videobox ! xvimagesink display=:0

I have this issue both with the 1.2 version of GStreamer I'm developing on, and the latest 1.3.x version in git.
Comment 1 Tim-Philipp Müller 2014-02-26 18:07:34 UTC
Could reproduce that.
Comment 2 RaviKiran 2014-06-11 12:28:07 UTC
Created attachment 278262 [details] [review]
Fix for the capnego problem

Looks like the issue is in  gst_video_box_transform_caps(). "AYUV" is treated as rgb. 

       if (strstr (str, "RGB") || strstr (str, "BGR") ||
            strcmp (str, "AYUV") == 0)  /* <== problem could here */
           seen_rgb = TRUE;
         else if (strcmp (str, "I420") == 0 || strcmp (str, "YV12") == 0 ||
             strcmp (str, "AYUV") == 0)

Pipeline worked with attached patch
Comment 3 Thiago Sousa Santos 2014-06-12 14:29:49 UTC
Review of attachment 278262 [details] [review]:

Thanks for spotting the issue, it is indeed related to the AYUV format as videobox can convert from both AYUV to other YUV and also to RGB formats.

Unfortunately your patch introduces a regression, the pipeline: "gst-launch-1.0 videotestsrc  ! "video/x-raw,format={AYUV}" ! videoconvert ! videobox ! "video/x- raw,format={RGBA}" ! videoconvert ! xvimagesink" won't work anymore as videobox now claims it can't convert AYUV -> RGBA while it can (and could before your patch).

I guess we need to handle AYUV differently as it means that videobox can produce both RGB and YUV formats. We need to handle AYUV separately on that if to set both seen_rgb and seen_yuv to true. (In the loop case you can even break the loop after this case as both seen_yuv/rgb are set anyway)

::: gst/videobox/gstvideobox.c
@@ -2877,1 @@
           seen_rgb = TRUE;

This same issue appears another time when iterating the list just above this occurence
Comment 4 RaviKiran 2014-06-16 10:26:42 UTC
Created attachment 278525 [details] [review]
Corrected patch

Thanks for pointing out the issue. I have corrected the patch. Please review.
Comment 5 Thiago Sousa Santos 2014-06-16 15:14:52 UTC
Review of attachment 278525 [details] [review]:

This still doesn't fix it. To test you should make sure that all the following pipelines still work:

gst-launch-1.0 videotestsrc  ! "video/x-raw,format={X}" ! videobox ! "video/x-raw,format={Y}" ! videoconvert ! xvimagesink

replace X and Y with:
I420 : AYUV
AYUV : I420

::: gst/videobox/gstvideobox.c
@@ +2863,3 @@
         lval = gst_value_list_get_value (fval, j);
         if ((str = g_value_get_string (lval))) {
+          if (strstr (str, "RGB") || strstr (str, "BGR"))

for AYUV you need to set both "seen_rgb" and "seen_yuv" to true as with AYUV input videobox can convert to both RGB and YUV formats.

The variable names "seen_yuv" and "seen_rgb" are a bit misleading as it is not representing what it has 'seen' anymore but what it can produce. So when you set seen_rgb to true it actually means that videobox has seen a format that it can use to produce RGB output, the same applies for seen_yuv.

As AYUV can be used for producing both rgb and yuv, it needs to set both variables to true. Something like:

if (ayuv) { seen_rgb=true, seen_yuv=true }
else if (other rgb) { seen_rgb=true }
else if (other yuv) { seen_yuv=true }

@@ +2872,3 @@
     } else if (fval && G_VALUE_HOLDS_STRING (fval)) {
       if ((str = g_value_get_string (fval))) {
+        if (strstr (str, "RGB") || strstr (str, "BGR"))

Same here.

@@ -2888,3 @@
-      g_value_set_string (&val, "AYUV");
-      gst_value_list_append_value (&list, &val);
-      g_value_unset (&val);

This is not correct. AYUV should be kept being set for both seen_rgb and seen_yuv as it can be converted from both format sets.

If you look at the comment a bit above in the code you will see:

    /* Supported conversions:
     * I420->AYUV
     * I420->YV12
     * YV12->AYUV
     * YV12->I420
     * AYUV->I420
     * AYUV->YV12
     * AYUV->xRGB (24bpp, 32bpp, incl. alpha)
     * xRGB->xRGB (24bpp, 32bpp, from/to all variants, incl. alpha)
     * xRGB->AYUV (24bpp, 32bpp, incl. alpha)
     * Passthrough only for everything else.

As you can see, AYUV can be converted from the RGB formats and to RGB formats as well as for another YUV formats.
Comment 6 RaviKiran 2014-06-17 07:48:35 UTC
Created attachment 278573 [details] [review]
Capsnego fix

Corrected per your suggestion. It works for all supported conversions now.
Comment 7 Thiago Sousa Santos 2014-06-17 12:45:36 UTC
Comment on attachment 278573 [details] [review]
Capsnego fix

Commited this patch with a style fix for using {} on all of those if/else if expressions as using on some and not on others made it harder to read IMHO. Also added a better commit message.
Comment 8 Thiago Sousa Santos 2014-06-17 12:47:59 UTC
commit 3c4c130c5e11524b0ea7a849ec1dd213741e93cf
Author: Ravi Kiran K N <>
Date:   Tue Jun 17 13:16:27 2014 +0530

    videobox: Fix caps negotiation issue
    Make sure that if AYUV is received it will detect that it can produce
    both RGB and YUV formats
    Signed-off-by: Ravi Kiran K N <>

and pushed for 1.2 branch:
commit daf888a6d80af3d64c5618470e2b12111b7657e3
Author: Ravi Kiran K N <>
Date:   Tue Jun 17 13:16:27 2014 +0530

    videobox: Fix caps negotiation issue
    Make sure that if AYUV is received it will detect that it can produce
    both RGB and YUV formats
    Signed-off-by: Ravi Kiran K N <>