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 753850 - gl: overlay composition negotation with the sink fails
gl: overlay composition negotation with the sink fails
Status: RESOLVED FIXED
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal blocker
: 1.5.91
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2015-08-19 23:21 UTC by Arnaud Vrac
Modified: 2015-08-28 18:27 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
glcolorconvert: Relay overlay composition meta api (3.00 KB, patch)
2015-08-21 00:32 UTC, Nicolas Dufresne (ndufresne)
none Details | Review
gl: Let base transform relay the meta api for us (6.85 KB, patch)
2015-08-21 18:54 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review
basetransform: Reconfigure before propose_allocation (3.87 KB, patch)
2015-08-21 21:29 UTC, Nicolas Dufresne (ndufresne)
none Details | Review
basetransform: Reconfigure before propose_allocation (4.14 KB, patch)
2015-08-21 22:18 UTC, Nicolas Dufresne (ndufresne)
committed Details | Review

Description Arnaud Vrac 2015-08-19 23:21:36 UTC
The overlay composition with the sink fails in textoverlay when using playsink. When using the sink directly the query allocation is successful. For comparison here are both pipelines:

GST_DEBUG='pango:5' ~/Work/gst/bin/gst-launch-1.0 videotestsrc ! queue ! t.video_sink http://absolut.zogzog.org/goinfre/share/samples/srt/tags.srt ! queue ! subparse ! t.text_sink textoverlay name=t ! glimagesink 2>&1 | grep 'sink alloc'

prints "sink alloc has overlay meta 1"

GST_DEBUG='pango:5' ~/Work/gst/bin/gst-launch-1.0 videotestsrc ! queue ! t.video_raw_sink http://absolut.zogzog.org/goinfre/share/samples/srt/tags.srt ! subparse ! queue ! t.text_sink playsink name=t flags=0x5 2>&1 | grep 'sink alloc'

prints "sink alloc has overlay meta 0"
Comment 1 Arnaud Vrac 2015-08-20 07:13:15 UTC
As discussed on IRC, this seems to be a bug in the gl elements. Updating the title.
Comment 2 Nicolas Dufresne (ndufresne) 2015-08-21 00:10:39 UTC
I'm starting to have an idea ;-P

This does not work (though using RGBA works ;-P)

gst-launch-1.0 videotestsrc ! video/x-raw,format=I420 ! textoverlay text=YEAH font-desc="Arial 60" ! glimagesink
Comment 3 Nicolas Dufresne (ndufresne) 2015-08-21 00:32:45 UTC
Created attachment 309776 [details] [review]
glcolorconvert: Relay overlay composition meta api

During allocation query, when this element is not passthrough, it must
relay the overlay compostion meta and it's parameters.
Comment 4 Nicolas Dufresne (ndufresne) 2015-08-21 00:35:53 UTC
That's only a part of the solution. This seems to work, if you do multiple resize, but fails if you do 1 step resize (e.g. maximize). I wonder if we don't need to somehow force the downstream allocation query to happen. I need to read more glfilter and GstBaseTransform code. I'll resume that tomorrow.
Comment 5 Arnaud Vrac 2015-08-21 13:45:50 UTC
I can also make the negotiation fail when using a simple pipeline with a non-passthrough videoconvert... For example the following pipeline fails with vaapisink (with the following patch: http://pastebin.com/0NGpC79P) and my custom sink:

gst-launch-1.0 videotestsrc ! video/x-raw,format=YUV9 ! textoverlay text=YEAH font-desc="Arial 60" ! videoconvert ! vaapisink
Comment 6 Arnaud Vrac 2015-08-21 13:46:27 UTC
And this is a regression, it was working fine before.
Comment 7 Nicolas Dufresne (ndufresne) 2015-08-21 16:26:24 UTC
(In reply to Arnaud Vrac from comment #6)
> And this is a regression, it was working fine before.

Was it ? Which version ?
Comment 8 Nicolas Dufresne (ndufresne) 2015-08-21 16:28:30 UTC
Hmm, looks like I miss that, gstvideoconvert implements filter_meta() virtual, and always return true. This is expected to forward all metas. I need to have a look at that, since this is what we should do in glupload and glcolorconvert.
Comment 9 Nicolas Dufresne (ndufresne) 2015-08-21 18:54:32 UTC
Created attachment 309837 [details] [review]
gl: Let base transform relay the meta api for us

During allocation query, when this element is not passthrough, it must
relay the overlay compostion meta and it's parameters. Fortunatly, base
transform can do this for us.
Comment 10 Nicolas Dufresne (ndufresne) 2015-08-21 18:56:42 UTC
This new patch is much nicer, as it let GstBaseTransform do the meta api forwarding. Remains one bug still, for some reason, one of the reconfigure triggers only after the renegotiation sequence causing the meta parameters to be outdated. I need to understand why.
Comment 11 Nicolas Dufresne (ndufresne) 2015-08-21 21:29:29 UTC
Created attachment 309844 [details] [review]
basetransform: Reconfigure before propose_allocation

There exist cases where a reconfigure event was propagated from
downstream, but caps didn't change. In this case, we would
reconfigure only when the next buffer arrives. The problem is that
due to the allocation query being cached, the return query parameters
endup outdated.

In this patch we refactor the reconfigurating code into a function, and
along with reconfiguring when a new buffer comes in, we also reconfigure
when a query allocation arrives.
Comment 12 Nicolas Dufresne (ndufresne) 2015-08-21 22:18:01 UTC
Created attachment 309849 [details] [review]
basetransform: Reconfigure before propose_allocation

There exist cases where a reconfigure event was propagated from
downstream, but caps didn't change. In this case, we would
reconfigure only when the next buffer arrives. The problem is that
due to the allocation query being cached, the return query parameters
endup outdated.

In this patch we refactor the reconfigurating code into a function, and
along with reconfiguring when a new buffer comes in, we also reconfigure
when a query allocation arrives.
Comment 13 Nicolas Dufresne (ndufresne) 2015-08-21 22:19:49 UTC
I had a small bug in the implementation, I would save the query and then reconfigure, obviously we need to do the opposite to get the latest query. Works in playbin, with video convert, etc. Please review/test provide feedback.
Comment 14 Sebastian Dröge (slomo) 2015-08-22 07:30:21 UTC
Comment on attachment 309837 [details] [review]
gl: Let base transform relay the meta api for us

I don't think this is correct. The elements can't handle every possible meta, unless they *are* in passthrough mode. E.g. the elements can rescale, but things like the ROI meta then need to be handled explicitly.
Comment 15 Nicolas Dufresne (ndufresne) 2015-08-22 15:38:15 UTC
Unlike glcolorscale, glcolorconvert only do color conversion, that's why I simply picked the method used in videoconvert here instead of filtering. As it does not scale, ROI remain properly located.
Comment 16 Nicolas Dufresne (ndufresne) 2015-08-22 15:45:42 UTC
Another point, unlike videoconvert, this element does not support video/x-raw(ANY), only meta::GstVideoOverlayComposition is, so filterting here would be a bit pointless.
Comment 17 Nicolas Dufresne (ndufresne) 2015-08-22 16:11:48 UTC
Attachment 309849 [details] pushed as 2534e39 - basetransform: Reconfigure before propose_allocation
Comment 18 Sebastian Dröge (slomo) 2015-08-22 16:52:00 UTC
Ah there's transform_meta and filter_meta... which are both doing slightly different things. I guess your code is correct then, or at least consistent. We should review the use of those two vfuncs after 1.6.0.
Comment 19 Nicolas Dufresne (ndufresne) 2015-08-23 05:14:49 UTC
Noted, for the record (and for others), filter_meta is for the meta that should be copied from downstream allocation query to upstream allocation query, while transform_meta is for the metas attached to the buffer (so the other direction).
Comment 20 Nicolas Dufresne (ndufresne) 2015-08-23 05:16:57 UTC
Attachment 309837 [details] pushed as 206638c - gl: Let base transform relay the meta api for us