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 788562 - audiobasesink: get time is racy
audiobasesink: get time is racy
Status: RESOLVED OBSOLETE
Product: GStreamer
Classification: Platform
Component: gst-plugins-base
1.12.x
Other All
: Normal normal
: git master
Assigned To: GStreamer Maintainers
GStreamer Maintainers
Depends on:
Blocks:
 
 
Reported: 2017-10-05 15:46 UTC by Philippe Renon
Modified: 2018-11-03 12:00 UTC
See Also:
GNOME target: ---
GNOME version: ---



Description Philippe Renon 2017-10-05 15:46:18 UTC
Here is a simplified version of the get_time method:


/* we call this function without holding the lock on sink for performance
 * reasons. Try hard to not deal with and invalid ringbuffer and rate. */
static GstClockTime
gst_audio_base_sink_get_time (GstClock * clock, GstAudioBaseSink * sink)
{
  [SNIP]

  /* our processed samples are always increasing */
  samples = gst_audio_ring_buffer_samples_done (ringbuffer); (1)

  /** racy if a new sample is written when we are here... */

  /* the number of samples not yet processed, this is still queued in the
   * device (not played for playback). */
  delay = gst_audio_ring_buffer_delay (ringbuffer); (2)

  samples -= delay;

  result = gst_util_uint64_scale_int (samples, GST_SECOND, rate);

  return result;
}

It computes the time as time = samples - delay.
Where samples is the number of samples that have been written to the audio device and delay represents how much remains to be played by the device.

The race condition happens like this:
- samples value is gotten at line (1)
- a new sample is written to the device (but the samples variable does not account for it)
- delay value is gotten at (2) and is bigger than expected because of new sample that was written

If this happens then the delay is too big and when it gets subtracted to samples, the resulting time goes backwards (by 10ms in my case which corresponds I believe to the duration of a sample).

I don't know how to fix this issue because there are three classes involved : the audiobasesink, the audioringbugger and the specific sink (a directsoundsink in my case). The specific sink is involved because the get_delay method is implemented there.

There are solutions to mitigate the issue:

1/ Get the delay before getting the samples
This will not solve the race condition but will make it less likely to happen.
If it happens, the clock will jump forward (and might jump backwards on a subsequent get_time 
invocation).

2/ Remember the last time value returned by get_time.
If the new time is before the last time then return the last time.

On a side note, I believe that directsoundsink delay method is also racy in a similar way.
Comment 1 GStreamer system administrator 2018-11-03 12:00:39 UTC
-- 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-base/issues/391.