[vlc-commits] aout: reduce output lock scope

Rémi Denis-Courmont git at videolan.org
Fri May 4 15:50:50 CEST 2018


vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Fri May  4 15:58:37 2018 +0300| [00f4188b9e325405a997e6cfe86811a7fa122c8b] | committer: Rémi Denis-Courmont

aout: reduce output lock scope

The all audio output calls from the decoder must be (and actually are)
serialized by the decoder.

The output lock needs to protect against concurrent accesses to the
request parameters (owner->req) and the audio output module callbacks,
both of which can be accessed by interface threads.

It does not need to protect anything else, notably synchronization
parameters, the filter chain and the volume mixer.

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

 src/audio_output/dec.c    | 18 +-----------------
 src/audio_output/output.c | 39 ++++++++++++++++++++++++---------------
 2 files changed, 25 insertions(+), 32 deletions(-)

diff --git a/src/audio_output/dec.c b/src/audio_output/dec.c
index afb38ff5dc..363dc62afc 100644
--- a/src/audio_output/dec.c
+++ b/src/audio_output/dec.c
@@ -75,9 +75,6 @@ int aout_DecNew( audio_output_t *p_aout,
 
     aout_owner_t *owner = aout_owner(p_aout);
 
-    /* TODO: reduce lock scope depending on decoder's real need */
-    aout_OutputLock (p_aout);
-
     /* Create the audio output stream */
     owner->volume = aout_volume_New (p_aout, p_replay_gain);
 
@@ -101,7 +98,6 @@ int aout_DecNew( audio_output_t *p_aout,
 error:
         aout_volume_Delete (owner->volume);
         owner->volume = NULL;
-        aout_OutputUnlock (p_aout);
         return -1;
     }
 
@@ -109,7 +105,6 @@ error:
     owner->sync.end = VLC_TS_INVALID;
     owner->sync.resamp_type = AOUT_RESAMPLING_NONE;
     owner->sync.discontinuity = true;
-    aout_OutputUnlock (p_aout);
 
     atomic_init (&owner->buffers_lost, 0);
     atomic_init (&owner->buffers_played, 0);
@@ -124,7 +119,6 @@ void aout_DecDelete (audio_output_t *aout)
 {
     aout_owner_t *owner = aout_owner (aout);
 
-    aout_OutputLock (aout);
     if (owner->mixer_format.i_format)
     {
         aout_FiltersDelete (aout, owner->filters);
@@ -132,7 +126,6 @@ void aout_DecDelete (audio_output_t *aout)
     }
     aout_volume_Delete (owner->volume);
     owner->volume = NULL;
-    aout_OutputUnlock (aout);
 }
 
 static int aout_CheckReady (audio_output_t *aout)
@@ -363,7 +356,6 @@ int aout_DecPlay(audio_output_t *aout, block_t *block)
     block->i_length = CLOCK_FREQ * block->i_nb_samples
                                  / owner->input_format.i_rate;
 
-    aout_OutputLock (aout);
     int ret = aout_CheckReady (aout);
     if (unlikely(ret == AOUT_DEC_FAILED))
         goto drop; /* Pipeline is unrecoverably broken :-( */
@@ -411,15 +403,13 @@ int aout_DecPlay(audio_output_t *aout, block_t *block)
     owner->sync.discontinuity = false;
     aout_OutputPlay (aout, block);
     atomic_fetch_add_explicit(&owner->buffers_played, 1, memory_order_relaxed);
-out:
-    aout_OutputUnlock (aout);
     return ret;
 drop:
     owner->sync.discontinuity = true;
     block_Release (block);
 lost:
     atomic_fetch_add_explicit(&owner->buffers_lost, 1, memory_order_relaxed);
-    goto out;
+    return ret;
 }
 
 void aout_DecGetResetStats(audio_output_t *aout, unsigned *restrict lost,
@@ -437,7 +427,6 @@ void aout_DecChangePause (audio_output_t *aout, bool paused, mtime_t date)
 {
     aout_owner_t *owner = aout_owner (aout);
 
-    aout_OutputLock (aout);
     if (owner->sync.end != VLC_TS_INVALID)
     {
         if (paused)
@@ -447,23 +436,19 @@ void aout_DecChangePause (audio_output_t *aout, bool paused, mtime_t date)
     }
     if (owner->mixer_format.i_format)
         aout_OutputPause (aout, paused, date);
-    aout_OutputUnlock (aout);
 }
 
 void aout_DecChangeRate(audio_output_t *aout, float rate)
 {
     aout_owner_t *owner = aout_owner(aout);
 
-    aout_OutputLock(aout);
     owner->sync.rate = rate;
-    aout_OutputUnlock(aout);
 }
 
 void aout_DecFlush (audio_output_t *aout, bool wait)
 {
     aout_owner_t *owner = aout_owner (aout);
 
-    aout_OutputLock (aout);
     owner->sync.end = VLC_TS_INVALID;
     if (owner->mixer_format.i_format)
     {
@@ -477,7 +462,6 @@ void aout_DecFlush (audio_output_t *aout, bool wait)
             aout_FiltersFlush (owner->filters);
         aout_OutputFlush (aout, wait);
     }
-    aout_OutputUnlock (aout);
 }
 
 void aout_ChangeViewpoint(audio_output_t *aout,
diff --git a/src/audio_output/output.c b/src/audio_output/output.c
index bc77e09c68..4e6194e6e5 100644
--- a/src/audio_output/output.c
+++ b/src/audio_output/output.c
@@ -530,13 +530,12 @@ static void aout_PrepareStereoMode (audio_output_t *aout,
 /**
  * Starts an audio output stream.
  * \param fmt audio output stream format [IN/OUT]
- * \warning The caller must hold the audio output lock.
+ * \warning The caller must NOT hold the audio output lock.
  */
 int aout_OutputNew (audio_output_t *aout, audio_sample_format_t *restrict fmt,
                     aout_filters_cfg_t *filters_cfg)
 {
     aout_owner_t *owner = aout_owner (aout);
-    aout_OutputAssertLocked (aout);
 
     audio_channel_type_t input_chan_type = fmt->channel_type;
     unsigned i_nb_input_channels = fmt->i_channels;
@@ -577,7 +576,10 @@ int aout_OutputNew (audio_output_t *aout, audio_sample_format_t *restrict fmt,
 
     aout->current_sink_info.headphones = false;
 
-    if (aout->start (aout, fmt))
+    aout_OutputLock(aout);
+    int ret = aout->start(aout, fmt);
+    aout_OutputUnlock(aout);
+    if (ret)
     {
         msg_Err (aout, "module not functional");
         return -1;
@@ -595,33 +597,36 @@ int aout_OutputNew (audio_output_t *aout, audio_sample_format_t *restrict fmt,
 /**
  * Stops the audio output stream (undoes aout_OutputNew()).
  * \note This can only be called after a successful aout_OutputNew().
- * \warning The caller must hold the audio output lock.
+ * \warning The caller must NOT hold the audio output lock.
  */
 void aout_OutputDelete (audio_output_t *aout)
 {
-    aout_OutputAssertLocked (aout);
-
+    aout_OutputLock(aout);
     if (aout->stop != NULL)
         aout->stop (aout);
+    aout_OutputUnlock(aout);
 }
 
 int aout_OutputTimeGet (audio_output_t *aout, mtime_t *delay)
 {
-    aout_OutputAssertLocked (aout);
+    int ret;
 
     if (aout->time_get == NULL)
         return -1;
-    return aout->time_get (aout, delay);
+
+    aout_OutputLock(aout);
+    ret = aout->time_get (aout, delay);
+    aout_OutputUnlock(aout);
+    return ret;
 }
 
 /**
  * Plays a decoded audio buffer.
  * \note This can only be called after a successful aout_OutputNew().
- * \warning The caller must hold the audio output lock.
+ * \warning The caller must NOT hold the audio output lock.
  */
 void aout_OutputPlay (audio_output_t *aout, block_t *block)
 {
-    aout_OutputAssertLocked (aout);
 #ifndef NDEBUG
     aout_owner_t *owner = aout_owner (aout);
     assert (owner->mixer_format.i_frame_length > 0);
@@ -629,13 +634,15 @@ void aout_OutputPlay (audio_output_t *aout, block_t *block)
             owner->mixer_format.i_bytes_per_frame /
             owner->mixer_format.i_frame_length);
 #endif
+    aout_OutputLock(aout);
     aout->play (aout, block);
+    aout_OutputUnlock(aout);
 }
 
 static void PauseDefault (audio_output_t *aout, bool pause, mtime_t date)
 {
     if (pause)
-        aout_OutputFlush (aout, false);
+        aout->flush(aout, false);
     (void) date;
 }
 
@@ -644,12 +651,13 @@ static void PauseDefault (audio_output_t *aout, bool pause, mtime_t date)
  * This enables the output to expedite pause, instead of waiting for its
  * buffers to drain.
  * \note This can only be called after a successful aout_OutputNew().
- * \warning The caller must hold the audio output lock.
+ * \warning The caller must NOT hold the audio output lock.
  */
 void aout_OutputPause( audio_output_t *aout, bool pause, mtime_t date )
 {
-    aout_OutputAssertLocked (aout);
+    aout_OutputLock(aout);
     ((aout->pause != NULL) ? aout->pause : PauseDefault) (aout, pause, date);
+    aout_OutputUnlock(aout);
 }
 
 /**
@@ -658,12 +666,13 @@ void aout_OutputPause( audio_output_t *aout, bool pause, mtime_t date )
  * \param wait if true, wait for buffer playback (i.e. drain),
  *             if false, discard the buffers immediately (i.e. flush)
  * \note This can only be called after a successful aout_OutputNew().
- * \warning The caller must hold the audio output lock.
+ * \warning The caller must NOT hold the audio output lock.
  */
 void aout_OutputFlush( audio_output_t *aout, bool wait )
 {
-    aout_OutputAssertLocked( aout );
+    aout_OutputLock(aout);
     aout->flush (aout, wait);
+    aout_OutputUnlock(aout);
 }
 
 static int aout_OutputVolumeSet (audio_output_t *aout, float vol)



More information about the vlc-commits mailing list