GNOME Bugzilla – Bug 702282
basesink: makes element go to PLAYING without PAUSED_TO_PLAYING transition
Last modified: 2013-07-24 09:37:23 UTC
Created attachment 246846 [details] Test code to reproduce basesink bug It looks like basesink is somehow making the element go to PLAYING state but it doesn't go through the PAUSED state.. or at least, it doesn't get the PAUSED_TO_PLAYING transition which makes pulsesink not work. In my code, using autoaudiosink, it's working (uses pulsesink internally), but if I replace it with pulsesink directly, it stops working... The difference I noticed in the log is that with autoaudiosink, it says "commiting state to PAUSED" then the whole preroll, etc.. while with pulsesink it directly says "commiting state to PLAYING". The actual issue with the sound is that it doesn't do the latency query, so when it does a get_calibration on PulseSinkClock, it gets 'cexternal = 0' which makes it wait days before rendering the first buffer, while the one with autoaudiosink does do the latency query, and sets calibration on the clock (in sync_latency) which makes audiobasesink report the right buffer timestamps because PulseSinkClock doesn't return a wrong external calibration value in audiobasesink, on the state change PAUSED_TO_PLAYING, it does this : GST_DEBUG_OBJECT (sink, "ringbuffer may start now"); sink->priv->sync_latency = TRUE; In my log, when using pulsesink directly, there isn't any "ringbuffer may start now", that's why it doesn't call sync_latency when the first buffer arrives and why the PulseSinkClock is not calibrated and reports wrong values, and I believe that issue is caused by the missing PAUSED_TO_PLAYING transition. If I force the sink to go to PAUSED before going to PLAYING, then it works. I wrote a small test code to reproduce this bug, and I've attached it to this bug report. You will see I set my pipeline to playing, then I add the source and sink to it and set them to PLAYING. When you run it, no sound will play and the g_debug will output that the sink is in the state PLAYING. If you enable GST_DEBUG=audiobasesink:5, you will notice that the debug message "ringbuffer may start now" does not appear, even though the element went to the PLAYING state. If you uncomment the line that uses autoaudiosink, it works and on stdout, it will report that the state of the sink is PAUSED, and the "ringbuffer may start now" message does appear. If you use the alternative code that sets the element to PAUSED first, it will work with pulsesink.
I know about this issue for some time. It happens when you set a sink to PLAYING, it will got to PAUSED async, when it gets the first buffer it commits to PAUSED and then go to PLAYING. When it goes to PLAYING, it doesn't call the state change function with PAUSED_TO_PLAYING (see gst_base_sink_commit_state()) This is tricky to solve, I have tried some things already but they are all racy: 1 call state change function. We should ideally call this with the STATE_LOCK but that would deadlock easily. 2 call state change without the state lock. This deadlocks because the PREROLL lock is taken in the state change 3 release PREROLL lock and call state change. It is possible that the two state change functions are called at the same time. This could be fixed by taking locks in the subclass. I think I will try 3 some more.
commit 124b8e38afa5a7e18b01d8f5969b7b20b9854030 Author: Wim Taymans <wim.taymans@collabora.co.uk> Date: Mon Jun 17 10:25:20 2013 +0200 basesink: call state change in all cases When we asynchronously go from READY to PLAYING, also call the state change function so that subclasses can update their state for PLAYING. Because the PREROLL lock is not recursive, we can't make this without races and we must assume for now that the subclass can handle concurrent calls to PAUSED->PLAYING and PLAYING->PAUSED. We can make this assumption because not many elements actually do something in those state changes and the ones that did would be broken even more without this change. https://bugzilla.gnome.org/show_bug.cgi?id=702282
cherry-picked into 1.0 branch.
*** Bug 696675 has been marked as a duplicate of this bug. ***