[vlc-commits] aout: restart output on the decoder thread, safely

[Rémi Denis-Courmont]

vlc | branch: master | Rémi Denis-Courmont <remi at remlab.net> | Thu Aug  4 19:34:51 2011 +0300| [d263cd6d2403f4ec72705e00b951d74f2ebe5fe5] | committer: Rémi Denis-Courmont

aout: restart output on the decoder thread, safely

This should fix the remaining crashes and deadlocks when changing
the audio device or the channel map.

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

 src/audio_output/aout_internal.h |    2 +
 src/audio_output/dec.c           |   42 ++++++++++++++++++++++++++-
 src/audio_output/intf.c          |   60 ++++++-------------------------------
 3 files changed, 53 insertions(+), 51 deletions(-)

diff --git a/src/audio_output/aout_internal.h b/src/audio_output/aout_internal.h
index 2b53fd7..0a4ac0d 100644
--- a/src/audio_output/aout_internal.h
+++ b/src/audio_output/aout_internal.h
@@ -108,6 +108,8 @@ typedef struct
     /* Filters between mixer and output */
     filter_t *filters[AOUT_MAX_FILTERS];
     int       nb_filters;
+
+    bool need_restart;
 } aout_owner_t;
 
 typedef struct
diff --git a/src/audio_output/dec.c b/src/audio_output/dec.c
index cd09fac..d5e3727 100644
--- a/src/audio_output/dec.c
+++ b/src/audio_output/dec.c
@@ -36,6 +36,7 @@
 #include <vlc_input.h>
 
 #include "aout_internal.h"
+#include "libvlc.h"
 
 #undef aout_DecNew
 /**
@@ -147,6 +148,43 @@ void aout_DecDelete( audio_output_t * p_aout, aout_input_t * p_input )
     free( p_input );
 }
 
+static void aout_CheckRestart (audio_output_t *aout)
+{
+    aout_owner_t *owner = aout_owner (aout);
+    aout_input_t *input = owner->input;
+
+    aout_assert_locked (aout);
+
+    if (likely(!owner->need_restart))
+        return;
+    owner->need_restart = false;
+
+    /* Reinitializes the output */
+    aout_InputDelete (aout, owner->input);
+    aout_MixerDelete (owner->volume.mixer);
+    owner->volume.mixer = NULL;
+    aout_OutputDelete (aout);
+
+    if (aout_OutputNew (aout, &input->input))
+    {
+error:
+        input->b_error = true;
+        return; /* we are officially screwed */
+    }
+
+    owner->volume.mixer = aout_MixerNew (aout, owner->mixer_format.i_format);
+    if (owner->volume.mixer == NULL)
+    {
+        aout_OutputDelete (aout);
+        goto error;
+    }
+
+    if (aout_InputNew (aout, input, &input->request_vout))
+        assert (input->b_error);
+    else
+        assert (!input->b_error);
+}
+
 
 /*
  * Buffer management
@@ -201,8 +239,10 @@ int aout_DecPlay( audio_output_t * p_aout, aout_input_t * p_input,
         return -1;
     }
 
-    /* Input */
+    aout_CheckRestart( p_aout );
     aout_InputCheckAndRestart( p_aout, p_input );
+
+    /* Input */
     p_buffer = aout_InputPlay( p_aout, p_input, p_buffer, i_input_rate );
 
     if( p_buffer != NULL )
diff --git a/src/audio_output/intf.c b/src/audio_output/intf.c
index 5b91401..4c9e117 100644
--- a/src/audio_output/intf.c
+++ b/src/audio_output/intf.c
@@ -234,58 +234,18 @@ int aout_SetMute (vlc_object_t *obj, audio_volume_t *volp, bool mute)
  * Pipelines management
  */
 
-/*****************************************************************************
- * aout_Restart : re-open the output device and rebuild the input and output
- *                pipelines
- *****************************************************************************
- * This function is used whenever the parameters of the output plug-in are
- * changed (eg. selecting S/PDIF or PCM).
- *****************************************************************************/
-static int aout_Restart( audio_output_t * p_aout )
+/**
+ * Marks the audio output for restart, to update any parameter of the output
+ * plug-in (e.g. output device or channel mapping).
+ */
+static void aout_Restart (audio_output_t *aout)
 {
-    aout_input_t *p_input;
-    aout_owner_t *owner = aout_owner (p_aout);
-
-    aout_lock( p_aout );
-    p_input = owner->input;
-    if( p_input == NULL )
-    {
-        aout_unlock( p_aout );
-        msg_Err( p_aout, "no decoder thread" );
-        return -1;
-    }
-
-    /* Reinitializes the output */
-    aout_InputDelete( p_aout, p_input );
-    aout_MixerDelete (owner->volume.mixer);
-    owner->volume.mixer = NULL;
-    aout_OutputDelete( p_aout );
+    aout_owner_t *owner = aout_owner (aout);
 
-    /* FIXME: This function is notoriously dangerous/unsafe.
-     * By the way, if OutputNew or MixerNew fails, we are totally screwed. */
-    if ( aout_OutputNew( p_aout, &p_input->input ) == -1 )
-    {
-        /* Release all locks and report the error. */
-        aout_unlock( p_aout );
-        return -1;
-    }
-
-    owner->volume.mixer = aout_MixerNew (p_aout, owner->mixer_format.i_format);
-    if (owner->volume.mixer == NULL)
-    {
-        aout_OutputDelete( p_aout );
-        aout_unlock( p_aout );
-        return -1;
-    }
-
-    if( aout_InputNew( p_aout, p_input, &p_input->request_vout ) )
-    {
-#warning FIXME: deal with errors
-        aout_unlock( p_aout );
-        return -1;
-    }
-    aout_unlock( p_aout );
-    return 0;
+    aout_lock (aout);
+    if (owner->input != NULL)
+        owner->need_restart = true;
+    aout_unlock (aout);
 }
 
 /*****************************************************************************