GNOME Bugzilla – Bug 788362
directsoundsrc: provided clock is not monotonic after pause and resume
Last modified: 2018-11-03 15:22:41 UTC
Video playback with audio jumps forwards visibly when going from pause to plauing. This is a log extract of a video playback going from playing to pause and back to playing. The shows the out of audiobasesink gstaudiobasesink.c:545:gst_audio_base_sink_get_time:<directsoundsink0> The time displayed on the far right is the time that will feed the audio clock that drives the pipeline. 0:00:02.432044715 processed samples: raw 30240, delay 9600, real 20640, time 0:00:00.430000000 0:00:02.437322934 processed samples: raw 30720, delay 9600, real 21120, time 0:00:00.440000000 Pausing... 0:00:02.437831207 processed samples: raw 30720, delay 9600, real 21120, time 0:00:00.440000000 0:00:02.438205430 processed samples: raw 30720, delay 9600, real 21120, time 0:00:00.440000000 0:00:02.438512938 processed samples: raw 31200, delay 0, real 31200, time 0:00:00.650000000 (1) Playing... 0:00:05.773609712 processed samples: raw 31200, delay 0, real 31200, time 0:00:00.650000000 0:00:05.774203007 processed samples: raw 33600, delay 2880, real 30720, time 0:00:00.640000000 (2) 0:00:05.774348849 processed samples: raw 40800, delay 9600, real 31200, time 0:00:00.650000000 You can see at (1) that the clock jumps forward by more than 200ms. At (2) it goes back in time by 10ms. The audio clock time is computed by GstAudioBaseSink based on where it thinks it is (number of samples sent to the device) minus the delay (number of samples not yet played by the device). The delay is provided by the directsoundsink. The problem is that when pausing, directsoundsink resets the DirectSoundBuffer and the delay becomes zero. This explains the jump forward of the clock. Fixing this is not too complex: don't reset the DirectSoundBuffer and pause it instead. But that requires that directsoundsink can distinguish a pause from a stop. Currently pause and stop are not exposed to audiosink children and are handled by the same reset vmethod. A patch to expose the pause and stop vmethod is provided here https://bugzilla.gnome.org/show_bug.cgi?id=788361
More compact and hopefully more readable version of the log extract: 02.432044715 : raw 30240, delay 9600, real 20640, time 00.430000000 02.437322934 : raw 30720, delay 9600, real 21120, time 00.440000000 Pausing... 02.437831207 : raw 30720, delay 9600, real 21120, time 00.440000000 02.438205430 : raw 30720, delay 9600, real 21120, time 00.440000000 02.438512938 : raw 31200, delay 0, real 31200, time 00.650000000 (1) Playing... 05.773609712 : raw 31200, delay 0, real 31200, time 00.650000000 05.774203007 : raw 33600, delay 2880, real 30720, time 00.640000000 (2) 05.774348849 : raw 40800, delay 9600, real 31200, time 00.650000000
Created attachment 360685 [details] [review] directsoundsink: don't reset DirectSoundBuffer when pausing This patch fixes the 200ms forward jump of the clock. But it does not fix the clock going backward. Don't know where that comes from but i am looking into it and have some leads.
In its current state the patch is not correct so please don't merge it. Also don't merge the linked audiosink patch. The patch breaks flushing seeks. The flush does not happen... There is no simple way (afaik) to know if a flush is needed in directsoundsink's pause method. Note that audioringbuffer calls its pause vmethod to do a flush expecting it to reset (i.e. stop+reset). The patch removed that behavior to make pause do only a stop and not a reset. So now I need to do a pause that resets too but I don't known how to detect the need for a reset.
One way to address the flush issue it to add a flush vmethod to audioringbuffer class. This will make the flush explicit instead of relying on pause doing it. But that's even more API changes + retro-compatibility shims (in addition to what was done in audiosink) so would like to get feedback before going down that road.
Flushing was fixed by using the audio ring buffer clear_all vmethod. I had to expose it to AudioBaseSink child classes (in addition to pause/stop) : https://bugzilla.gnome.org/show_bug.cgi?id=788361 The new attached directsoundsink patch makes use of the pause, stop and clear_all to properly pause (without flushing), stop and flush.
Created attachment 360809 [details] [review] directsoundsink: don't reset DirectSoundBuffer when pausing
To sum up where I stand: 1/ GstAudioSink has been updated to expose more GstAudioRingBuffer vmethods to child classes (see https://bugzilla.gnome.org/show_bug.cgi?id=788361). The pause/resume/stop/clear_all vmethods have been exposed and the reset vmethod has been deprecated. These changes are ABI compatible and don't affect existing audio sinks. 2/ Using the above changes, I was able to fix directsoundsink so that pause does not flush the IDirectSoundBuffer. The flush was the cause of the clock jumping forward. I'd like to get these changes reviewed and merged before I proceed with the next step: find why the clock sometimes goes back by 10ms when unpausing.
Will attach latest patch. Current one is not the latest.
Created attachment 360849 [details] [review] directsoundsink: don't reset DirectSoundBuffer when pausing
I think the clock going backwards is explained by https://bugzilla.gnome.org/show_bug.cgi?id=788562
Current patch still has issue with seeking. A better/simpler solution is on its way.
Created attachment 361288 [details] [review] directsoundsink: don't reset DirectSoundBuffer when pausing Final patch for now. directsoundsink can now do a real pause without resetting the audio device. This removes a visible video jump when pausing and resuming. Seeking works well too. The original issue was reproducible with gst-play and (any) video. With the fix, gst-play behaves well when pausing/resuming/seeking.
Had a look at alsasink to see if it has the same issue as directsoundsink. It does not but because, I believe, it's delay method is wrong. Afaik, audio sink compute audio clock time by taking the number of samples sent to the audio device and deduct from it how much remains to be played (as reported by the audio device itself through the delay method). So time = samples - delay. The alsasink delay() method returns the value of snd_pcm_delay() which, according to the documentation [1] is the intrinsic latency of the audio device (i.e. how long it takes for a first sample to become audible). I think it should return the value of snd_pcm_avail() [2] which return the current fill level of the device. directsoundsink delay() method return how much sound remains to be played and not the internal latency of the audio device (which is not available afaik). I am not sure who is correct there, directsoundsink or alsasink ? [1] https://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m.html#ga012e8b999070e72ab23514f25e7d6482 [2] https://www.alsa-project.org/alsa-doc/alsa-lib/group___p_c_m.html#ga577b4d51e08d94930a05bbe73291ed2a
Got it wrong : snd_pcm_avail() returns how much can be written. So it looks like alsasink could have the same time issue as directsoundsink.
This alsasink issue looks similar to the directsoundsink issue: https://bugzilla.gnome.org/show_bug.cgi?id=750694
Any chance to get this reviewed ?
-- GitLab Migration Automatic Message -- This bug has been migrated to freedesktop.org's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.freedesktop.org/gstreamer/gst-plugins-good/issues/408.