GNOME Bugzilla – Bug 757376
playbin: have-context and need-context messages are lost
Last modified: 2015-11-10 09:15:42 UTC
Working on bug 754820, I have had to revamp the context handling in gstreamer-vaapi. WIP branch: https://github.com/ceyusa/gstreamer-vaapi/commits/gstcontext I'm facing a problem that in a simple playbin (either using gst-launch or gst-play) the display context, created by vaapisink it is not shared in the pipeline, and the vaapidecode ends creating its own. As far as I understand this is problematic flow: 1\ playbin instantiates the video sink (vaapisink) and set it in READY state. NOTE: at this moment the video sink is not attached to any bin. 2\ When turning into READY state, the start() vmethod is called, and it looks for a display context. As the element is isolated, the upstream and downstream messages fail. Then the element creates a need-context message and post it in the bus. 3\ As in the bus there is no a handler assigned yet, the message is queued. 4\ The video sink then creates its own display, and post a have-context message in to the bus, which is queued too. 5\ When the application assign the bus watcher, the bus dispatches all the queue messages. The applications usually doesn't handle the context messages, since they are processed sync and the watcher do async operations. Thus, the need-context and have-context messages are lost. 6\ decodebin instantiates the decoder (vaapidecoder) which in its open() vmethod looks for a display context. At this moment the video sink is not plugged, so the downstream query is lost. 7\ the decoder post the need-context message in the bus, which the bin forwards up to the application and the message is finally ignored. 8\ the decoder creates it own display context and post a have-context. 9\ An we have duplicated display (one in the video sink and one in decoder), and the video might never get rendered. I have written a proof-of-concept patch for gst-play.c which, such as in gstbin, collects the incoming have-context messages, and when a need-context message arrives, it is looked in the context list and set to the source element if found. Obviously, as the application watcher dispatches the messages async, I needed to add a hackish usleep after sending the need-context message by the decoder.
Created attachment 314508 [details] [review] proof-of-concept hack for handling have-context and need-context messages in the application
playbin should handle this messages, but playbin, somehow, it hasn't set its bin's sync_handler to the bus, and the messages are sent directly to the application. Possible solutions that pop out of my mind are: 1\ I don't know if it is possible to set to NULL the video sink after it was instantiated and set it to READY and set it to READY again when it is already attached to a bin. 2\ delay the display context lookup when the sink is attached to a bin 3\ the bus filters out the need-context and have-context messages and don't queue them to the application, but queued them separately and inject them to a bin when the bus is attached to a bin. 4\ create the video sink in playbin and attach it to a bin before setting it to READY state. 5\ ????
gstbin should automatically be taking care of that now since http://cgit.freedesktop.org/gstreamer/gstreamer/commit/?id=d5ded1588920c4471eefe055d09095d9e5e989b5 i.e. when adding an element to a bin, the gstcontext is propagated so that the added element always gets the latest or propagates its context. Playbin doesn't need to handle need-context/have-context messages as that's all handled by gstbin/gstelement. Also since http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/?id=d50b713f44eb17ebf69aa771c26f8eb19e226319 the autoplug-query on decodebin/uridecodebin is signalled and playbin correctly forwards the context query to the sink.
(In reply to Matthew Waters from comment #3) > gstbin should automatically be taking care of that now since > http://cgit.freedesktop.org/gstreamer/gstreamer/commit/ > ?id=d5ded1588920c4471eefe055d09095d9e5e989b5 > > i.e. when adding an element to a bin, the gstcontext is propagated so that > the added element always gets the latest or propagates its context. Perhaps I am missing something in gstreamer-vaapi code, I need to follow up what is done with this patch. Though, I need to say, that I update my gstreamer setup daily, so I have been using this patch since it was committed, and I don't see the context been "re-posted" to the bus when the video sink is added in to a bin. But I need to look a it more closer. Thanks for the cue, Matthew! > > Playbin doesn't need to handle need-context/have-context messages as that's > all handled by gstbin/gstelement. > > Also since > http://cgit.freedesktop.org/gstreamer/gst-plugins-base/commit/ > ?id=d50b713f44eb17ebf69aa771c26f8eb19e226319 the autoplug-query on > decodebin/uridecodebin is signalled and playbin correctly forwards the > context query to the sink.
OK. I found the error: gstreamer-vaapi don't chain up the set_context() vmethod. But still, I see some odd behavior in the context sharing: vaapisink creates a display but it is not shared because it is added into a bin much more later. Then vaapidecode creates a new display (vaapsink is not "bined" yet). The display created by vaapidecode is asigned to vaapsink, instead of the way around.
I found something that might be interesting. Printing the order of the bin adding: Inserting streamsynchronizer0 into (null) Inserting playsink into (null) Inserting uridecodebin0 into playbin Inserting source into uridecodebin0 Inserting typefind into (null) Inserting decodebin0 into uridecodebin0 Inserting qtdemux0 into decodebin0 Inserting multiqueue0 into decodebin0 Inserting vaapih264parse0 into decodebin0 Inserting capsfilter0 into decodebin0 Inserting aacparse0 into decodebin0 Inserting h264parse0 into decodebin0 Inserting capsfilter1 into decodebin0 Inserting avdec_aac0 into decodebin0 Inserting vaapidecode0 into decodebin0 <---- Inserting inputselector0 into playbin Inserting inputselector1 into playbin Inserting audiotee into playsink Inserting vaapisink0 into vbin <---- Inserting vqueue into vbin Inserting identity into (null) Inserting conv into vconv Inserting scale into vconv Inserting vconv into vbin Inserting vdconv into vdbin Inserting deinterlace into vdbin Inserting vdbin into playsink Inserting vbin into playsink Inserting pulsesink0 into abin Inserting aqueue into abin Inserting identity into (null) Inserting conv into aconv Inserting resample into aconv Inserting aconv into abin Inserting abin into playsink First, vaapisink is instantiated and set to READY. Hence it negotiates its context and, since the element is isolated, it creates its own VA Display. Then vaapidecode is created and inserted into the decodebin. It negotiates the context, but in the pipeline there is not! so it creates its own VA Display. We have two VA Display. Afterwards vaapisink is inserted into the vbin, and the context is negotiated, and the one created by the vaapidecode replaces the old one created by the vaapisink. Although this simply works, it is far from optimal, since we are creating twice a VA Display, and, IMO (wrong, perhaps), the video sink's context should be preferred.
The application context shall be preferred. Because the decodebin pipelines are constructed src to sink, unlike the static pipeline where the state change is sink to src, both ways need to be supported. So one or the other should make little difference, if anything "special is needed" with the context, the application will provide one. It's unfortunate though, since it would seem that the way we plug the decoders these days introduce side effect. A question, is the decoder added to the pipeline when it's state is changed to READY ? I'm a bit out of date maybe, but the mechanism in place for context sharing would only work if it was. Other then that, there is active work to implement the query proxying in playbin, maybe that work will help a little. It will let an unlinked decoder query the sink context I believe.
(In reply to Nicolas Dufresne (stormer) from comment #7) > The application context shall be preferred. Because the decodebin pipelines > are constructed src to sink, unlike the static pipeline where the state > change is sink to src, both ways need to be supported. So one or the other > should make little difference, if anything "special is needed" with the > context, the application will provide one. If a bin already has a context set, the element to add will receive its context. Thus the requirement that the application context is preferred holds. > It's unfortunate though, since it would seem that the way we plug the > decoders these days introduce side effect. A question, is the decoder added > to the pipeline when it's state is changed to READY ? I'm a bit out of date > maybe, but the mechanism in place for context sharing would only work if it > was. Other then that, there is active work to implement the query proxying > in playbin, maybe that work will help a little. It will let an unlinked > decoder query the sink context I believe. Decodebin adds the decoder to the decodebin, sets the target pad (so the context query is forwarded with autplug-query) and sets the decoder to PAUSED (from NULL). Despite this, an element has always needed to deal with context replacements in GstElement::set_context. As mentioned by the commit message in http://cgit.freedesktop.org/gstreamer/gstreamer/commit/?id=d5ded1588920c4471eefe055d09095d9e5e989b5 if you want the context to be propagated between the context query and the context messages, you have to manually call gst_element_set_context upon successfully performing a context query.
Thanks a lot for your replies. They have provided me a great insight. Since now the picture is clearer in my head, I consider this is no longer a bug, so I'm closing it as NOTABUG.