[vlc-commits] coreaudio: fix unlikely but possible deadlock on pause

Thomas Guillem git at videolan.org
Wed Mar 1 17:49:52 CET 2017


vlc | branch: master | Thomas Guillem <thomas at gllm.fr> | Fri Feb 24 17:40:33 2017 +0100| [61686a07d128fbba5c676d7cbc371113cd07a881] | committer: Thomas Guillem

coreaudio: fix unlikely but possible deadlock on pause

The pause state need to be known by ca_Play in order to don't wait indefinitely
when the RenderCallback is paused. A deadlock could happen with asynchronous
audio decoders.

> http://git.videolan.org/gitweb.cgi/vlc.git/?a=commit;h=61686a07d128fbba5c676d7cbc371113cd07a881
---

 modules/audio_output/audiounit_ios.m    |  7 +++++-
 modules/audio_output/auhal.c            | 40 +++++----------------------------
 modules/audio_output/coreaudio_common.c | 23 +++++++++++++++++++
 modules/audio_output/coreaudio_common.h |  3 +++
 4 files changed, 37 insertions(+), 36 deletions(-)

diff --git a/modules/audio_output/audiounit_ios.m b/modules/audio_output/audiounit_ios.m
index 74c2590..451d3c1 100644
--- a/modules/audio_output/audiounit_ios.m
+++ b/modules/audio_output/audiounit_ios.m
@@ -103,7 +103,6 @@ avas_SetActive(audio_output_t *p_aout, bool active, NSUInteger options)
 static void
 Pause (audio_output_t *p_aout, bool pause, mtime_t date)
 {
-    VLC_UNUSED(date);
     struct aout_sys_t * p_sys = p_aout->sys;
 
     /* We need to start / stop the audio unit here because otherwise the OS
@@ -127,10 +126,16 @@ Pause (audio_output_t *p_aout, bool pause, mtime_t date)
         {
             err = AudioOutputUnitStart(p_sys->au_unit);
             if (err != noErr)
+            {
                 msg_Err(p_aout, "AudioOutputUnitStart failed [%4.4s]",
                         (const char *) &err);
+                /* Do not un-pause, the Render Callback won't run, and next call
+                 * of ca_Play will deadlock */
+                return;
+            }
         }
     }
+    ca_Pause(p_aout, pause, date);
 }
 
 static int
diff --git a/modules/audio_output/auhal.c b/modules/audio_output/auhal.c
index cb2a9d6..256a94a 100644
--- a/modules/audio_output/auhal.c
+++ b/modules/audio_output/auhal.c
@@ -121,7 +121,6 @@ struct aout_sys_t
 
     float                       f_volume;
     bool                        b_mute;
-    atomic_bool                 b_paused;
 
     bool                        b_ignore_streams_changed_callback;
 };
@@ -928,15 +927,8 @@ RenderCallbackAnalog(void *p_data, AudioUnitRenderActionFlags *ioActionFlags,
     VLC_UNUSED(inBusNumber);
     VLC_UNUSED(inNumberFrames);
 
-    audio_output_t * p_aout = p_data;
-    aout_sys_t *p_sys = p_aout->sys;
-    uint8_t *p_output = ioData->mBuffers[0].mData;
-    size_t i_size = ioData->mBuffers[0].mDataByteSize;
-
-    if (!atomic_load(&p_sys->b_paused))
-        ca_Render(p_aout, p_output, i_size);
-    else
-        memset(p_output, 0, i_size);
+    ca_Render(p_data, ioData->mBuffers[0].mData,
+              ioData->mBuffers[0].mDataByteSize);
 
     return noErr;
 }
@@ -962,10 +954,7 @@ RenderCallbackSPDIF(AudioDeviceID inDevice, const AudioTimeStamp * inNow,
     uint8_t *p_output = outOutputData->mBuffers[p_sys->i_stream_index].mData;
     size_t i_size = outOutputData->mBuffers[p_sys->i_stream_index].mDataByteSize;
 
-    if (!atomic_load(&p_sys->b_paused))
-        ca_Render(p_data, p_output, i_size);
-    else
-        memset(p_output, 0, i_size);
+    ca_Render(p_aout, p_output, i_size);
 
     return noErr;
 }
@@ -973,15 +962,6 @@ RenderCallbackSPDIF(AudioDeviceID inDevice, const AudioTimeStamp * inNow,
 #pragma mark -
 #pragma mark initialization
 
-static void
-Pause(audio_output_t *p_aout, bool pause, mtime_t date)
-{
-    struct aout_sys_t * p_sys = p_aout->sys;
-    VLC_UNUSED(date);
-
-    atomic_store(&p_sys->b_paused, pause);
-}
-
 /*
  * StartAnalog: open and setup a HAL AudioUnit to do PCM audio output
  */
@@ -1811,7 +1791,6 @@ Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt)
     p_sys->i_stream_index = -1;
     p_sys->b_revert = false;
     p_sys->b_changed_mixing = false;
-    p_sys->b_paused = false;
     p_sys->c.i_device_latency = 0;
 
     vlc_mutex_lock(&p_sys->selected_device_lock);
@@ -1928,15 +1907,13 @@ Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt)
         p_sys->c.i_device_latency = 0;
     }
 
-    bool b_success = false;
-
     /* Check for Digital mode or Analog output mode */
     if (AOUT_FMT_SPDIF (fmt) && b_start_digital)
     {
         if (StartSPDIF (p_aout, fmt) == VLC_SUCCESS)
         {
             msg_Dbg(p_aout, "digital output successfully opened");
-            b_success = true;
+            return VLC_SUCCESS;
         }
     }
     else
@@ -1944,16 +1921,10 @@ Start(audio_output_t *p_aout, audio_sample_format_t *restrict fmt)
         if (StartAnalog(p_aout, fmt) == VLC_SUCCESS)
         {
             msg_Dbg(p_aout, "analog output successfully opened");
-            b_success = true;
+            return VLC_SUCCESS;
         }
     }
 
-    if (b_success)
-    {
-        p_aout->pause = Pause;
-        return VLC_SUCCESS;
-    }
-
 error:
     /* If we reach this, this aout has failed */
     msg_Err(p_aout, "opening auhal output failed");
@@ -2031,7 +2002,6 @@ static int Open(vlc_object_t *obj)
     p_sys->b_selected_dev_is_default = false;
     memset(&p_sys->sfmt_revert, 0, sizeof(p_sys->sfmt_revert));
     p_sys->i_stream_id = 0;
-    atomic_init(&p_sys->b_paused, false);
 
     p_aout->sys = p_sys;
     p_aout->start = Start;
diff --git a/modules/audio_output/coreaudio_common.c b/modules/audio_output/coreaudio_common.c
index 910546a..0ca4986 100644
--- a/modules/audio_output/coreaudio_common.c
+++ b/modules/audio_output/coreaudio_common.c
@@ -42,6 +42,12 @@ ca_Render(audio_output_t *p_aout, uint8_t *p_output, size_t i_requested)
 {
     struct aout_sys_common *p_sys = (struct aout_sys_common *) p_aout->sys;
 
+    if (atomic_load_explicit(&p_sys->b_paused, memory_order_relaxed))
+    {
+        memset(p_output, 0, i_requested);
+        return;
+    }
+
     /* Pull audio from buffer */
     int32_t i_available;
     void *p_data = TPCircularBufferTail(&p_sys->circular_buffer,
@@ -107,6 +113,15 @@ ca_Flush(audio_output_t *p_aout, bool wait)
 }
 
 void
+ca_Pause(audio_output_t * p_aout, bool pause, mtime_t date)
+{
+    struct aout_sys_common *p_sys = (struct aout_sys_common *) p_aout->sys;
+    VLC_UNUSED(date);
+
+    atomic_store_explicit(&p_sys->b_paused, pause, memory_order_relaxed);
+}
+
+void
 ca_Play(audio_output_t * p_aout, block_t * p_block)
 {
     struct aout_sys_common *p_sys = (struct aout_sys_common *) p_aout->sys;
@@ -128,6 +143,12 @@ ca_Play(audio_output_t * p_aout, block_t * p_block)
             assert(false);
             break;
         }
+        if (atomic_load_explicit(&p_sys->b_paused, memory_order_relaxed))
+        {
+            msg_Warn(p_aout, "dropping block because the circular buffer is "
+                     "full and paused");
+            break;
+        }
 
         /* Wait for the render buffer to play the remaining data */
         int32_t i_avalaible_bytes;
@@ -162,12 +183,14 @@ ca_Init(audio_output_t *p_aout, const audio_sample_format_t *fmt,
         return VLC_EGENERIC;
 
     atomic_init(&p_sys->i_underrun_size, 0);
+    atomic_init(&p_sys->b_paused, false);
 
     p_sys->i_rate = fmt->i_rate;
     p_sys->i_bytes_per_frame = fmt->i_bytes_per_frame;
     p_sys->i_frame_length = fmt->i_frame_length;
 
     p_aout->play = ca_Play;
+    p_aout->pause = ca_Pause;
     p_aout->flush = ca_Flush;
     p_aout->time_get = ca_TimeGet;
     return VLC_SUCCESS;
diff --git a/modules/audio_output/coreaudio_common.h b/modules/audio_output/coreaudio_common.h
index 1b6f908..f6a532a 100644
--- a/modules/audio_output/coreaudio_common.h
+++ b/modules/audio_output/coreaudio_common.h
@@ -49,6 +49,7 @@ struct aout_sys_common
     /* circular buffer to swap the audio data */
     TPCircularBuffer    circular_buffer;
     atomic_uint         i_underrun_size;
+    atomic_bool         b_paused;
     int                 i_rate;
     unsigned int        i_bytes_per_frame;
     unsigned int        i_frame_length;
@@ -67,6 +68,8 @@ int  ca_TimeGet(audio_output_t *p_aout, mtime_t *delay);
 
 void ca_Flush(audio_output_t *p_aout, bool wait);
 
+void ca_Pause(audio_output_t * p_aout, bool pause, mtime_t date);
+
 void ca_Play(audio_output_t * p_aout, block_t * p_block);
 
 int  ca_Init(audio_output_t *p_aout, const audio_sample_format_t *fmt,



More information about the vlc-commits mailing list