[Bug 721953] pango: basetextoverlay: handle video/x-raw(ANY) if downstream supports the GstVideoOverlayCompositionMeta API

[GStreamer (bugzilla.gnome.org)]

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

Nicolas Dufresne <nicolas.dufresne> changed:

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

--- Comment #19 from Nicolas Dufresne <nicolas.dufresne at collabora.co.uk> 2014-01-29 21:36:45 UTC ---
Review of attachment 267572:
 --> (https://bugzilla.gnome.org/review?bug=721953&attachment=267572)

My only concert is in the videosink_query, we need not to hide that the sink
has the feature support to compose in order to allow having multiple
textoverlay one after the other. Would it be possible to check if that is
handled correctly ?

::: ext/pango/gstbasetextoverlay.c
@@ +552,3 @@
+    gst_caps_unref (overlay->sw_template_caps);
+    overlay->sw_template_caps = NULL;
+  }

Should be a static global caps instead of per-instance allocated.

@@ +796,3 @@
+      GST_DEBUG_OBJECT (overlay, "unsupported caps %" GST_PTR_FORMAT, caps);
+      gst_pad_push_event (overlay->video_sinkpad,
+          gst_event_new_custom (GST_EVENT_RECONFIGURE, NULL));

I think this is the the right use of RECONFIGURE, and this should be removed.

@@ +1083,3 @@
 static GstCaps *
+gst_base_text_overlay_add_caps_feature_support (GstCaps * caps,
+    const gchar * feature, GstCaps * filter)

This is very confusing, as the filter does not match the query filter, but it
the sw caps supported by the renderer, while the caps match the query filter. I
would better name that helper and it's parameter to reflect what it's used for.

@@ +1103,3 @@
+static GstCaps *
+gst_base_text_overlay_filter_caps_by_feature (GstCaps * caps,
+    const gchar * feature, GstCaps * filter)

Again the use of filter is a bit confusing here

@@ +1145,3 @@
+  GstCaps *peer_caps = NULL, *caps = NULL, *overlay_filter = NULL;
+
+  if (G_UNLIKELY (!overlay))

Is this really possible ?

@@ +1175,3 @@
+      GST_DEBUG_OBJECT (pad, "our template  %" GST_PTR_FORMAT, templ);
+      caps =
+          gst_caps_intersect_full (peer_caps, templ,
GST_CAPS_INTERSECT_FIRST);

If peer_caps is ANY, this will return a copy of the template, why doing an
intersection ? We know thought that this is no filter or the filter is ANY, so
no need to intersect with the filter.

@@ +1242,3 @@
+      GST_DEBUG_OBJECT (pad, "our template  %" GST_PTR_FORMAT, templ);
+      caps =
+          gst_caps_intersect_full (peer_caps, templ,
GST_CAPS_INTERSECT_FIRST);

Same.

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