[Bug 704950] videomixer: add colorspace conversion

[GStreamer (bugzilla.gnome.org)]

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

Sebastian Dröge (slomo) <slomo> changed:

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

--- Comment #18 from Sebastian Dröge (slomo) <slomo at circular-chaos.org> 2013-07-29 08:02:51 UTC ---
Review of attachment 250307:
 --> (https://bugzilla.gnome.org/review?bug=704950&attachment=250307)

::: gst/videomixer/videomixer2.c
@@ +315,3 @@
+    /* If we want alpha, disregard all the other formats */
+    if (need_alpha && !(pad->info.finfo->flags & GST_VIDEO_FORMAT_FLAG_ALPHA))
+      continue;

What if downstream does not support an alpha format at all?

@@ +317,3 @@
+      continue;
+
+    formats[GST_VIDEO_INFO_FORMAT (&pad->info)] += 1;

As said on IRC this is not going to work because GST_VIDEO_FORMAT_LAST can
change at any point and we can't just require videomixer to be recompiled
because of this.

@@ +333,3 @@
+  mix->info = best_info;
+  mix->info.width = new_width;
+  mix->info.height = new_height;

You should also check if downstream even supports what you chose here, I think
this code should be inside update_src_caps() for that reason

@@ +451,1 @@
+    gst_structure_set_value (s, "format", v);

I think what you want here is first the configured caps (i.e. format,
colorimetry, etc but not width/height/framerate) and afterwards almost blank
raw video caps to allow anything we can convert into.

@@ +451,3 @@
+    gst_structure_set_value (s, "format", v);
+
+    gst_structure_remove_field (s, "colorimetry");

There are also other fields that we can convert between. Like chroma-siting.
Check videoconvert for the fields you can remove

@@ +474,3 @@
+  template_caps = gst_pad_get_pad_template_caps (GST_PAD (mix->srcpad));
+  s = gst_caps_get_structure (template_caps, 0);
+  v = gst_structure_get_value (s, "format");

Same goes here, also the *template* caps probably don't have a single format
field. You need to intersect the template caps with the downstream accepted
caps, and then somehow select suitable caps from those that remain.

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