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 721950 - assrender: handle video/x-raw(ANY) if downstream supports the GstVideoOverlayCompositionMeta API
assrender: handle video/x-raw(ANY) if downstream supports the GstVideoOverlay...
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-bad
git master
Other Linux
: Normal enhancement
: 1.3.1
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2014-01-10 17:45 UTC by Matthieu Bouron
Modified: 2014-03-05 19:41 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
assrender: handle video/x-raw(ANY) if downstream supports the GstVideoOverlayCompositionMeta API (2.29 KB, patch)
2014-01-10 17:46 UTC, Matthieu Bouron
needs-work Details | Review
assrender: handle video/x-raw(ANY) if downstream supports the GstVideoOverlayCompositionMeta API (12.05 KB, patch)
2014-02-04 15:21 UTC, Matthieu Bouron
needs-work Details | Review
assrender: handle video/x-raw(ANY) if downstream supports the GstVideoOverlayCompositionMeta API (12.10 KB, patch)
2014-03-03 13:31 UTC, Matthieu Bouron
committed Details | Review

Description Matthieu Bouron 2014-01-10 17:45:24 UTC
This patch makes the assrender element handle video/x-raw(ANY) if downstream supports the GstVideoOverlayCompositionMeta API since in this case it only places the correct meta on outgoing buffers.

This is useful when an hardware decoder is used and a downstream element supports the overlay composition meta.
Comment 1 Matthieu Bouron 2014-01-10 17:46:21 UTC
Created attachment 265955 [details] [review]
assrender: handle video/x-raw(ANY) if downstream supports the GstVideoOverlayCompositionMeta API
Comment 2 Tim-Philipp Müller 2014-01-10 19:08:20 UTC
This doesn't look right to me. You can't just claim to support everything on your template caps and error out if you get something you don't like. Defeats negotiation.
Comment 3 Sebastian Dröge (slomo) 2014-01-14 09:11:02 UTC
Review of attachment 265955 [details] [review]:

See https://bugzilla.gnome.org/show_bug.cgi?id=721953#c4
Also same as in videobalance and deinterlace applies to both.
Comment 4 Matthieu Bouron 2014-02-04 15:21:12 UTC
Created attachment 268076 [details] [review]
assrender: handle video/x-raw(ANY) if downstream supports the GstVideoOverlayCompositionMeta API

Patch updated:
  * based on https://bugzilla.gnome.org/show_bug.cgi?id=721953 method.
Comment 5 Nicolas Dufresne (ndufresne) 2014-02-05 19:37:45 UTC
Review of attachment 268076 [details] [review]:

Looks good. Is the bug in gstpad fix ? if not, make this bug blocked please, and only push when only fixed.
Comment 6 Sebastian Dröge (slomo) 2014-02-05 20:27:21 UTC
Also only push once I reviewed the textoverlay patch
Comment 7 Sebastian Dröge (slomo) 2014-03-02 20:04:41 UTC
Review of attachment 268076 [details] [review]:

Which GstPad fix?

But the patch is almost good to be merged, some minor comments here

::: ext/assrender/gstassrender.c
@@ +185,3 @@
       "Sebastian Dröge <sebastian.droege@collabora.co.uk>");
+
+  klass->sw_template_caps = gst_static_caps_get (&sw_template_caps);

There's no point in storing static caps in the class, just get them
whenever needed.

@@ +573,3 @@
+          GST_CAPS_INTERSECT_FIRST);
+    }
+

If you first unref and then append it is more efficient as the simple caps
can then just be taken directly instead of copying them

@@ +611,3 @@
+
+      /* if peer returns ANY caps, return src pad template caps */
+      caps = gst_caps_copy (gst_pad_get_pad_template_caps (srcpad));

Apply filter here

@@ +674,2 @@
+      /* if peer returns ANY caps, return sink pad template caps */
+      caps = gst_caps_copy (gst_pad_get_pad_template_caps (sinkpad));

Apply filter here
Comment 8 Matthieu Bouron 2014-03-03 13:31:14 UTC
Created attachment 270797 [details] [review]
assrender: handle video/x-raw(ANY) if downstream supports the GstVideoOverlayCompositionMeta API

Patch updated, according to latest comments. Thanks for the reviews.
Comment 9 Sebastian Dröge (slomo) 2014-03-03 19:33:00 UTC
Comment on attachment 270797 [details] [review]
assrender: handle video/x-raw(ANY) if downstream supports the GstVideoOverlayCompositionMeta API

Good to go after we have the second textoverlay test. I want to be sure that this concept works.
Comment 10 Sebastian Dröge (slomo) 2014-03-05 19:41:48 UTC
commit 4da4dfc4bfd8ecc51236ad872687fd21a4056274
Author: Matthieu Bouron <matthieu.bouron@collabora.com>
Date:   Fri Jan 10 17:38:09 2014 +0000

    assrender: handle video/x-raw(ANY) if downstream supports the GstVideoOverla
    
    https://bugzilla.gnome.org/show_bug.cgi?id=721950