audiobasesink: get time is racy

Submitted by Philippe Renon

Link to original bug (#788562)

Description

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.

Version: 1.12.x