core: aout_TimingReport() uses a vlc_mutex
Hello,
I was kind of lazy when I implemented vlc_aout_stream_NotifyTiming:
void vlc_aout_stream_NotifyTiming(vlc_aout_stream *stream, vlc_tick_t system_ts,
vlc_tick_t audio_ts)
{
/* This function might be called from high priority audio threads (so, no
* mutexes, allocation, IO, debug, wait...). That is why we use a circular
* buffer of points. The vlc_aout_stream user will read these points and
* update the clock from vlc_aout_stream_Play() and
* vlc_aout_stream_UpdateLatency(). */
/* VLC mutexes use atomic and the reader will only do very fast
* operations (copy of the timing_point data). */
vlc_mutex_lock(&stream->timing_points.lock);
...
}
Our vlc_mutex_t implementation is fast, it will just use an atomic operation more than 95% of times, or wait in a futex. The stream->timing_points.lock mutex is only held for setting 2 variables. Even if our vlc_mutex_t are fast, nothing prevent a thread to be pre-empted while holding this lock, causing the audio data callback to wait a little more (but nothing critical IMHO).
My question: Should we fix it, or we don't care ?
If we do care, I propose to add a lock-free, concurrent, generic SPCP queue like this one https://nullprogram.com/blog/2022/05/14/ in the core API. Indeed, the android AAudio and coreaudio modules might also need it.
More context why we should not lock, from AAudio: https://developer.android.com/ndk/reference/group/audio#group___audio_1gad88ad53a723807879ba75d3b5e07c073
Also note that this callback function should be considered a "real-time" function. It must not do anything that could cause an unbounded delay because that can cause the audio to glitch or pop.
These are things the function should NOT do:
- use any mutexes or other synchronization primitives
I investigated chromium and firefox to see how they do it:
- Gecko (firefox): lockless usage of AAudio
- Chromium: use a basic mutex from the data callback.