[Bug 725248] videobox fails to negotiate in an environment with possible RGB input.

[GStreamer (bugzilla.gnome.org)]

https://bugzilla.gnome.org/show_bug.cgi?id=725248
  GStreamer | gst-plugins-good | git

Thiago Sousa Santos <thiagossantos> changed:

           What    |Removed                     |Added
----------------------------------------------------------------------------
 Attachment #278525|none                        |needs-work
             status|                            |

--- Comment #5 from Thiago Sousa Santos <thiagossantos at gmail.com> 2014-06-16 15:14:52 UTC ---
Review of attachment 278525:
 --> (https://bugzilla.gnome.org/review?bug=725248&attachment=278525)

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:
RGBA : AYUV
AYUV : RGBA
I420 : AYUV
AYUV : I420
AYUV : AYUV

::: 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.

-- 
Configure bugmail: https://bugzilla.gnome.org/userprefs.cgi?tab=email
------- You are receiving this mail because: -------
You are the QA contact for the bug.
You are the assignee for the bug.