GNOME Bugzilla – Bug 769894
validate: Race when checking whether some actions have completed
Last modified: 2016-09-30 07:04:08 UTC
This race happens quite quickly with the GES scrubbing validate tests. The core reason is because of the way GES timeline posts its own ASYNC messages and handles seeks asynchronously. Note : This is *not* an issue in GES. Take the following scenario (the direction of the seek doesn't matter): pause, playback-time=0.0 seek, playback-time=0.0, start="duration - 0.5", flags=accurate+flush * The pause action is executed * The pipeline prerolls * The STATE_CHANGED message (READY=>PAUSED) is posted on the bus from a streaming thread * The ASYNC_DONE message is posted on the bus from a streaming thread * The main thread pops out the STATE_CHANGED message from the bus ** The pause action is marked as done ** The next action is executed (flushing seek) ** ges-pipeline posts ASYNC_START and returns TRUE for the seek event * The main thread pops out the next message (ASYNC_DONE from the initial preroll) ** it checks for the current action, sees that it's a seek, and marks it as done .. but the seek wasn't executed by then The visible result of this is seeing such scenarios failing with: "event: position after a seek is wrong : Reported position after accurate seek in PAUSED state should be exactly what the user asked for" Because validate checks the position after it *thinks* the seek event was executed ... but it wasn't really.
Should we just also wait for ASYNC_DONE to happen when going to PAUSED? Other idea would be to check the message seqnum for the seek events but I am afraid this is not well supported. What do you prefer?
Created attachment 333302 [details] [review] validate:scenario: Wait for ASYNC_DONE to set async state change DONE Fixes
If you wait for *both* that should do the job here. Note however that you can't assume to get the same number of async-done as seeks and state changes you submitted. If there are multiple happening at the same time, you will only receive a single async-done. This could be problematic with things automatically changing states, or the handling of the BUFFERING messages.
One might be able to look at the sticky SEGMENT event on a pad and correlate the seqnum of that to the seqnum of SEEK events created.
(In reply to Sebastian Dröge (slomo) from comment #3) > If you wait for *both* that should do the job here. Note however that you > can't assume to get the same number of async-done as seeks and state changes > you submitted. If there are multiple happening at the same time, you will > only receive a single async-done. > This could be problematic with things automatically changing states, or the > handling of the BUFFERING messages. I am trying to think about issues it can cause issues but in GstValidate everything is currently very sequential and I can not think of a scenario failling with that approach. Basically in the patch I proposed I am just making sure that when set_state returns ASYNC, we always wait for 1- the new state to be reached 2- an ASYNC_DONE message on the bus (is there a particular order for those events to happen?). I can't reproduce that bug with it.
ASYNC_DONE always comes after the STATE_CHANGED message IIRC. I think it can cause issues in gst-validate in combination with scrub seeking. Are you always waiting for ASYNC_DONE before sending the next seek? If not, you might run into problems if you later wait for ASYNC_DONE, depending on the circumstances. Should be good to go though, it's definitely improving things and more correct.
Attachment 333302 [details] pushed as 6e9c672 - validate:scenario: Wait for ASYNC_DONE to set async state change DONE