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 773099 - decodebin2 ignores the GST_CHANGE_STATE_ASYNC returned by the child element.
decodebin2 ignores the GST_CHANGE_STATE_ASYNC returned by the child element.
Status: RESOLVED INVALID
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
git master
Other Linux
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2016-10-17 12:54 UTC by y.bandou
Modified: 2017-06-23 16:26 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
decodebin2 don't ignore the GST_CHANGE_STATE_ASYNC (1.10 KB, patch)
2016-10-17 12:54 UTC, y.bandou
none Details | Review

Description y.bandou 2016-10-17 12:54:24 UTC
Created attachment 337847 [details] [review]
decodebin2 don't ignore the GST_CHANGE_STATE_ASYNC

1/ The application changes the pipeline state from paused to playing state,
   the change state paused_to_playing is propagate to  playbin, then to
   uridecodebin then to decodebin2 then to the child elements 

2/ My videodecoder element changes his state in asynchronous mode, so returns GST_CHANGE_STATE_ASYNC in video_decoder_change_state function.

3/ decodebin2 ignores this return (GST_CHANGE_STATE_ASYNC) and forwards  to uridecodebin  GST_CHANGE_STATE_SUCCESS.


here is the patch that I propose in attached
Comment 1 Wonchul Lee 2016-10-18 02:18:05 UTC
The patch looks good to me.
When a pipeline consisted with decodebin2(without Uridecodebin or Playbin) go to PAUSED state, It always returns async state and waits to finish auto-plugging.
If the Decodebin exposed earlier than the child element which needs more buffers to ready to PAUSED, it will be ignored.
Comment 2 Wonchul Lee 2016-10-18 02:43:24 UTC
No, above comment was wrong.
Even if the decoder is not ready, the sink element will wait to be prerolled in the normal case unless set async-enabled to the sink, so It seems to happen in a specific situation.
Comment 3 y.bandou 2016-10-27 10:19:31 UTC
Thank you Wonchul for your comments.

Our videodecoder plugin go to playing state from paused state in async mode.

My issue is when the pipeline go to playing state from paused state the decodebin2 don't forward the async transition of the videodecoder plugin to uridecodebin,because the decodebin2 ignores the GST_CHANGE_STATE_ASYNC return of the videodecoder and forward the GST_CHANGE_STATE_SUCCES  to uridecodebin.

What do you think about the attached patch ?
Comment 4 Wonchul Lee 2016-10-28 02:46:05 UTC
afaik, a decoder produces output once it received caps and buffers regardless of its state PAUSED or PLAYING, so no need to change state PAUSED to PLAYING asynchronously generally. If you don't specify certain behaviour in your video decoder when state changing PAUSED to PLAYING.
I think the patch is acceptable but It would be good to add more details about your decoder.
Comment 5 y.bandou 2016-10-28 13:08:00 UTC
The videodecoder plugin is based on HAL SoftAtHome, the video decoder device in this HAL starts asynchronously. so i can't do otherway.
Comment 6 Sebastian Dröge (slomo) 2016-10-31 12:01:29 UTC
A videodecoder that handles the state change from PAUSED->PLAYING sounds like a design problem. Decoders, parsers, demuxers, ... and everything that does not sync to the clock really, should do exactly the same in PAUSED as in PLAYING.
Comment 7 Romain 2016-11-03 14:58:19 UTC
(In reply to Sebastian Dröge (slomo) from comment #6)
> A videodecoder that handles the state change from PAUSED->PLAYING sounds
> like a design problem. Decoders, parsers, demuxers, ... and everything that
> does not sync to the clock really, should do exactly the same in PAUSED as
> in PLAYING.

Yes, we agree that this is not the way that these elements usually behave. this behavior is the one that seems to be the best balance between what GStreamer expects and how our drivers behave:

This video decoder cannot start in pause mode, so our element has different behaviors when going from READY->PAUSED and PLAYING->PAUSED. 

On the READY->PAUSED transition, nothing happens. This state change is synchronous.
On the PAUSED->PLAYING, the video driver is actually started, which is asynchronous (i.e. the preroll is done here).
On the PLAYING->PAUSED transition the video driver is paused.
Comment 8 Sebastian Dröge (slomo) 2016-11-04 15:14:55 UTC
Why does it matter for your decoder, why can't it produce data while in PAUSED? This breaks lots of assumptions in GStreamer and would basically mean that your decoder would have to work as a live element. (which decodebin also does not handle well, and generally seems weird)
Comment 9 Romain 2016-11-14 16:18:27 UTC
(In reply to Sebastian Dröge (slomo) from comment #8)
> Why does it matter for your decoder, why can't it produce data while in
> PAUSED? This breaks lots of assumptions in GStreamer and would basically
> mean that your decoder would have to work as a live element. (which
> decodebin also does not handle well, and generally seems weird)

As said before we have to cope with the drivers that are available on the chipsets we use and the abstraction layers that we use. We work on improving this but it does not depend only on us.

Concerning decodebin2 it seems that async state changes are fully supported except that they are currently ignored in the paused to playing transition. Would you agree to add support for such async transitions or is this something that you prefer to prevent ?
Comment 10 Sebastian Dröge (slomo) 2016-11-14 16:25:45 UTC
It might "work" sometimes, but it's definitely not going to work in general.

I would prefer to instead solve it in your elements. Sure, the drivers and API is like it is, but that should be solveable at the GStreamer layer now already. Without more details I can't give any suggestions though. Maybe what you have their is conceptionally more a sink than a decoder.
Comment 11 Romain 2016-11-15 10:03:52 UTC
ok. Thank you Sebastian. We will keep this internal until we can do some evolutions on our decode element.
Comment 12 Sebastian Dröge (slomo) 2016-11-15 12:04:03 UTC
Let me know if you want to discuss the design of your elements and maybe want to find a different way of making things work :) Or maybe after that I actually agree with your approach, who knows ;)
Comment 13 Tim-Philipp Müller 2016-12-23 17:52:01 UTC
What shall we do with this? Sounds like it should be closed?