[vlc-devel] [PATCH] RFC: clock/aout: audio preroll discussion

Thomas Guillem thomas at gllm.fr
Tue Jun 19 17:09:32 CEST 2018


Hello,

I think a discussion about audio preroll is necessary before advancing more on
the new clock (last WIP branch can be found here
https://code.videolan.org/jbk/vlc/tree/clock-core/8 and I recommend you to read
last WIP comment from TypX).

Currently, only the pulse audio module is able to start at a given time
(because the API allows to start a stream corked). Every other modules will
start right after the VLC Start callback.

The problem is that the first play often need to be played in the future. There
is often a 30/50ms pts delay (tested on current master). With the future output
clock, this delay should be arround the dejitter value of the vlc_clock_t (200
or 500ms). No problem for pulse audio, but every other modules will fail to
start at this given time.

We have some choices:

 1/ Simplify the date handling of aout modules: all modules behave the same
 way, and the core handle the delay (by playing silence when needed). So we
 move the delayed start functionally from PulseAudio to the core: this is what
 this patch is doing.

 2/ Modify the aout API to handle 2 type of aout modules: one like pulseaudio
 (that handle a play date), and all the others one.

I prefer the solution 1/ since it'll ease future debugging because all modules
will behave the same way.

If you prefer the solution 2/, we need to think about a proper aout API.

PS: This patch is not meant to be pushed like that.
---
 include/vlc_aout.h               |  6 +-
 modules/audio_output/alsa.c      | 16 +++---
 modules/audio_output/pulse.c     | 94 ++------------------------------
 src/audio_output/aout_internal.h |  1 +
 src/audio_output/dec.c           | 26 ++++++---
 5 files changed, 33 insertions(+), 110 deletions(-)

diff --git a/include/vlc_aout.h b/include/vlc_aout.h
index f03b26d4fb..7ca904b47c 100644
--- a/include/vlc_aout.h
+++ b/include/vlc_aout.h
@@ -185,16 +185,15 @@ struct audio_output
       * \note This callback cannot be called in stopped state.
       */
 
-    void (*play)(audio_output_t *, block_t *block, mtime_t date);
+    void (*play)(audio_output_t *, block_t *block);
     /**< Queues a block of samples for playback (mandatory, cannot be NULL).
       *
       * \param block block of audio samples
-      * \param date intended system time to render the first sample
       *
       * \note This callback cannot be called in stopped state.
       */
 
-    void (*pause)( audio_output_t *, bool pause, mtime_t date);
+    void (*pause)( audio_output_t *, bool pause);
     /**< Pauses or resumes playback (mandatory, cannot be NULL).
       *
       * This callback pauses or resumes audio playback as quickly as possible.
@@ -206,7 +205,6 @@ struct audio_output
       * fallback implementation of this callback.
       *
       * \param pause pause if true, resume from pause if false
-      * \param date timestamp when the pause or resume was requested
       *
       * \note This callback cannot be called in stopped state.
       */
diff --git a/modules/audio_output/alsa.c b/modules/audio_output/alsa.c
index b9319debba..03a07dbad8 100644
--- a/modules/audio_output/alsa.c
+++ b/modules/audio_output/alsa.c
@@ -291,9 +291,9 @@ out:
 #endif
 
 static int TimeGet (audio_output_t *aout, mtime_t *);
-static void Play(audio_output_t *, block_t *, mtime_t);
-static void Pause (audio_output_t *, bool, mtime_t);
-static void PauseDummy (audio_output_t *, bool, mtime_t);
+static void Play(audio_output_t *, block_t *);
+static void Pause (audio_output_t *, bool);
+static void PauseDummy (audio_output_t *, bool);
 static void Flush (audio_output_t *, bool);
 
 /** Initializes an ALSA playback stream */
@@ -647,7 +647,7 @@ static int TimeGet (audio_output_t *aout, mtime_t *restrict delay)
 /**
  * Queues one audio buffer to the hardware.
  */
-static void Play(audio_output_t *aout, block_t *block, mtime_t date)
+static void Play(audio_output_t *aout, block_t *block)
 {
     aout_sys_t *sys = aout->sys;
 
@@ -687,23 +687,22 @@ static void Play(audio_output_t *aout, block_t *block, mtime_t date)
         }
     }
     block_Release (block);
-    (void) date;
 }
 
 /**
  * Pauses/resumes the audio playback.
  */
-static void Pause (audio_output_t *aout, bool pause, mtime_t date)
+static void Pause (audio_output_t *aout, bool pause)
 {
     aout_sys_t *p_sys = aout->sys;
     snd_pcm_t *pcm = p_sys->pcm;
 
     int val = snd_pcm_pause (pcm, pause);
     if (unlikely(val))
-        PauseDummy (aout, pause, date);
+        PauseDummy (aout, pause);
 }
 
-static void PauseDummy (audio_output_t *aout, bool pause, mtime_t date)
+static void PauseDummy (audio_output_t *aout, bool pause)
 {
     aout_sys_t *p_sys = aout->sys;
     snd_pcm_t *pcm = p_sys->pcm;
@@ -713,7 +712,6 @@ static void PauseDummy (audio_output_t *aout, bool pause, mtime_t date)
         snd_pcm_drop (pcm);
     else
         snd_pcm_prepare (pcm);
-    (void) date;
 }
 
 /**
diff --git a/modules/audio_output/pulse.c b/modules/audio_output/pulse.c
index c9ff167211..4206e85728 100644
--- a/modules/audio_output/pulse.c
+++ b/modules/audio_output/pulse.c
@@ -68,8 +68,6 @@ typedef struct
     pa_threaded_mainloop *mainloop; /**< PulseAudio thread */
     pa_time_event *trigger; /**< Deferred stream trigger */
     pa_cvolume cvolume; /**< actual sink input volume */
-    mtime_t first_pts; /**< Play stream timestamp of buffer start */
-    mtime_t first_date; /**< Play system timestamp of buffer start */
 
     pa_volume_t volume_force; /**< Forced volume (stream must be NULL) */
     pa_stream_flags_t flags_force; /**< Forced flags (stream must be NULL) */
@@ -172,7 +170,7 @@ static void sink_event(pa_context *ctx, unsigned type, uint32_t idx,
 
 
 /*** Latency management and lip synchronization ***/
-static void stream_start_now(pa_stream *s, audio_output_t *aout)
+static void stream_start(pa_stream *s, audio_output_t *aout)
 {
     pa_operation *op;
 
@@ -201,70 +199,6 @@ static void stream_stop(pa_stream *s, audio_output_t *aout)
         pa_operation_unref(op);
 }
 
-static void stream_trigger_cb(pa_mainloop_api *api, pa_time_event *e,
-                              const struct timeval *tv, void *userdata)
-{
-    audio_output_t *aout = userdata;
-    aout_sys_t *sys = aout->sys;
-
-    assert (sys->trigger == e);
-
-    msg_Dbg(aout, "starting deferred");
-    vlc_pa_rttime_free(sys->mainloop, sys->trigger);
-    sys->trigger = NULL;
-    stream_start_now(sys->stream, aout);
-    (void) api; (void) e; (void) tv;
-}
-
-/**
- * Starts or resumes the playback stream.
- * Tries start playing back audio samples at the most accurate time
- * in order to minimize desync and resampling during early playback.
- * @note PulseAudio lock required.
- */
-static void stream_start(pa_stream *s, audio_output_t *aout, mtime_t date)
-{
-    aout_sys_t *sys = aout->sys;
-    mtime_t delta;
-
-    assert (sys->first_pts != VLC_TS_INVALID);
-
-    if (sys->trigger != NULL) {
-        vlc_pa_rttime_free(sys->mainloop, sys->trigger);
-        sys->trigger = NULL;
-    }
-
-    delta = vlc_pa_get_latency(aout, sys->context, s);
-    if (unlikely(delta == VLC_TS_INVALID)) {
-        msg_Dbg(aout, "cannot synchronize start");
-        delta = 0; /* screwed */
-    }
-
-    delta = (date - mdate()) - delta;
-    if (delta > 0) {
-        msg_Dbg(aout, "deferring start (%"PRId64" us)", delta);
-        delta += pa_rtclock_now();
-        sys->trigger = pa_context_rttime_new(sys->context, delta,
-                                             stream_trigger_cb, aout);
-    } else {
-        msg_Warn(aout, "starting late (%"PRId64" us)", delta);
-        stream_start_now(s, aout);
-    }
-}
-
-static void stream_latency_cb(pa_stream *s, void *userdata)
-{
-    audio_output_t *aout = userdata;
-    aout_sys_t *sys = aout->sys;
-
-    /* This callback is _never_ called while paused. */
-    if (sys->first_pts == VLC_TS_INVALID)
-        return; /* nothing to do if buffers are (still) empty */
-    if (pa_stream_is_corked(s) > 0)
-        stream_start(s, aout, sys->first_date);
-}
-
-
 /*** Stream helpers ***/
 static void stream_state_cb(pa_stream *s, void *userdata)
 {
@@ -323,7 +257,6 @@ static void stream_moved_cb(pa_stream *s, void *userdata)
 static void stream_overflow_cb(pa_stream *s, void *userdata)
 {
     audio_output_t *aout = userdata;
-    aout_sys_t *sys = aout->sys;
     pa_operation *op;
 
     msg_Err(aout, "overflow, flushing");
@@ -331,7 +264,6 @@ static void stream_overflow_cb(pa_stream *s, void *userdata)
     if (unlikely(op == NULL))
         return;
     pa_operation_unref(op);
-    sys->first_pts = VLC_TS_INVALID;
 }
 
 static void stream_started_cb(pa_stream *s, void *userdata)
@@ -486,7 +418,7 @@ static void *data_convert(block_t **pp)
 /**
  * Queue one audio frame to the playback stream
  */
-static void Play(audio_output_t *aout, block_t *block, mtime_t date)
+static void Play(audio_output_t *aout, block_t *block)
 {
     aout_sys_t *sys = aout->sys;
     pa_stream *s = sys->stream;
@@ -504,13 +436,9 @@ static void Play(audio_output_t *aout, block_t *block, mtime_t date)
      * will take place, and sooner or later a deadlock. */
     pa_threaded_mainloop_lock(sys->mainloop);
 
-    if (sys->first_pts == VLC_TS_INVALID) {
-        sys->first_pts = block->i_pts;
-        sys->first_date = date;
-    }
 
     if (pa_stream_is_corked(s) > 0)
-        stream_start(s, aout, date);
+        stream_start(s, aout);
 
 #if 0 /* Fault injector to test underrun recovery */
     static volatile unsigned u = 0;
@@ -531,24 +459,17 @@ static void Play(audio_output_t *aout, block_t *block, mtime_t date)
 /**
  * Cork or uncork the playback stream
  */
-static void Pause(audio_output_t *aout, bool paused, mtime_t date)
+static void Pause(audio_output_t *aout, bool paused)
 {
     aout_sys_t *sys = aout->sys;
     pa_stream *s = sys->stream;
 
     pa_threaded_mainloop_lock(sys->mainloop);
 
-    if (paused) {
-        pa_stream_set_latency_update_callback(s, NULL, NULL);
+    if (paused)
         stream_stop(s, aout);
-    } else {
-        pa_stream_set_latency_update_callback(s, stream_latency_cb, aout);
-        if (likely(sys->first_pts != VLC_TS_INVALID))
-            stream_start_now(s, aout);
-    }
 
     pa_threaded_mainloop_unlock(sys->mainloop);
-    (void) date;
 }
 
 /**
@@ -576,7 +497,6 @@ static void Flush(audio_output_t *aout, bool wait)
         op = pa_stream_flush(s, NULL, NULL);
     if (op != NULL)
         pa_operation_unref(op);
-    sys->first_pts = VLC_TS_INVALID;
     stream_stop(s, aout);
 
     pa_threaded_mainloop_unlock(sys->mainloop);
@@ -782,7 +702,6 @@ static int Start(audio_output_t *aout, audio_sample_format_t *restrict fmt)
 
     /* Stream parameters */
     pa_stream_flags_t flags = sys->flags_force
-                            | PA_STREAM_START_CORKED
                             | PA_STREAM_INTERPOLATE_TIMING
                             | PA_STREAM_NOT_MONOTONIC
                             | PA_STREAM_AUTO_TIMING_UPDATE
@@ -807,7 +726,6 @@ static int Start(audio_output_t *aout, audio_sample_format_t *restrict fmt)
 
     sys->trigger = NULL;
     pa_cvolume_init(&sys->cvolume);
-    sys->first_pts = VLC_TS_INVALID;
 
     pa_format_info *formatv = pa_format_info_new();
     formatv->encoding = encoding;
@@ -928,7 +846,6 @@ static int Start(audio_output_t *aout, audio_sample_format_t *restrict fmt)
     pa_stream_set_state_callback(s, stream_state_cb, sys->mainloop);
     pa_stream_set_buffer_attr_callback(s, stream_buffer_attr_cb, aout);
     pa_stream_set_event_callback(s, stream_event_cb, aout);
-    pa_stream_set_latency_update_callback(s, stream_latency_cb, aout);
     pa_stream_set_moved_callback(s, stream_moved_cb, aout);
     pa_stream_set_overflow_callback(s, stream_overflow_cb, aout);
     pa_stream_set_started_callback(s, stream_started_cb, aout);
@@ -985,7 +902,6 @@ static void Stop(audio_output_t *aout)
     pa_stream_set_state_callback(s, NULL, NULL);
     pa_stream_set_buffer_attr_callback(s, NULL, NULL);
     pa_stream_set_event_callback(s, NULL, NULL);
-    pa_stream_set_latency_update_callback(s, NULL, NULL);
     pa_stream_set_moved_callback(s, NULL, NULL);
     pa_stream_set_overflow_callback(s, NULL, NULL);
     pa_stream_set_started_callback(s, NULL, NULL);
diff --git a/src/audio_output/aout_internal.h b/src/audio_output/aout_internal.h
index 49e6e8e3b9..ce57fecdfb 100644
--- a/src/audio_output/aout_internal.h
+++ b/src/audio_output/aout_internal.h
@@ -70,6 +70,7 @@ typedef struct
 
     struct
     {
+        bool prebuf;
         mtime_t end; /**< Last seen PTS */
         float rate; /**< Play-out speed rate */
         mtime_t resamp_start_drift; /**< Resampler drift absolute value */
diff --git a/src/audio_output/dec.c b/src/audio_output/dec.c
index 4a0a42ff10..74b177b02d 100644
--- a/src/audio_output/dec.c
+++ b/src/audio_output/dec.c
@@ -100,6 +100,7 @@ error:
         return -1;
     }
 
+    owner->sync.prebuf = true;
     owner->sync.rate = 1.f;
     owner->sync.end = VLC_TS_INVALID;
     owner->sync.resamp_type = AOUT_RESAMPLING_NONE;
@@ -205,7 +206,7 @@ static void aout_StopResampling (audio_output_t *aout)
     aout_FiltersAdjustResampling (owner->filters, 0);
 }
 
-static void aout_DecSilence (audio_output_t *aout, mtime_t length, mtime_t pts)
+static void aout_DecSilence (audio_output_t *aout, mtime_t length)
 {
     aout_owner_t *owner = aout_owner (aout);
     const audio_sample_format_t *fmt = &owner->mixer_format;
@@ -219,10 +220,10 @@ static void aout_DecSilence (audio_output_t *aout, mtime_t length, mtime_t pts)
     msg_Dbg (aout, "inserting %zu zeroes", frames);
     memset (block->p_buffer, 0, block->i_buffer);
     block->i_nb_samples = frames;
-    block->i_pts = pts;
-    block->i_dts = pts;
+    block->i_pts = 0;
+    block->i_dts = 0;
     block->i_length = length;
-    aout->play(aout, block, pts);
+    aout->play(aout, block);
 }
 
 static void aout_DecSynchronize(audio_output_t *aout, mtime_t dec_pts)
@@ -286,7 +287,7 @@ static void aout_DecSynchronize(audio_output_t *aout, mtime_t dec_pts)
         if (!owner->sync.discontinuity)
             msg_Warn (aout, "playback way too early (%"PRId64"): "
                       "playing silence", drift);
-        aout_DecSilence (aout, -drift, dec_pts);
+        aout_DecSilence (aout, -drift);
 
         aout_StopResampling (aout);
         owner->sync.discontinuity = true;
@@ -377,13 +378,21 @@ int aout_DecPlay(audio_output_t *aout, block_t *block)
     /* Software volume */
     aout_volume_Amplify (owner->volume, block);
 
+    if (owner->sync.prebuf)
+    {
+        mtime_t delay = block->i_pts - mdate();
+        if (delay > 0)
+            aout_DecSilence(aout, delay);
+        owner->sync.prebuf = false;
+    }
+
     /* Drift correction */
     aout_DecSynchronize(aout, block->i_pts);
 
     /* Output */
     owner->sync.end = block->i_pts + block->i_length + 1;
     owner->sync.discontinuity = false;
-    aout->play(aout, block, block->i_pts);
+    aout->play(aout, block);
     atomic_fetch_add_explicit(&owner->buffers_played, 1, memory_order_relaxed);
     return ret;
 drop:
@@ -418,7 +427,7 @@ void aout_DecChangePause (audio_output_t *aout, bool paused, mtime_t date)
     }
 
     if (owner->mixer_format.i_format)
-        aout->pause(aout, paused, date);
+        aout->pause(aout, paused);
 }
 
 void aout_DecChangeRate(audio_output_t *aout, float rate)
@@ -439,11 +448,12 @@ void aout_DecFlush (audio_output_t *aout, bool wait)
         {
             block_t *block = aout_FiltersDrain (owner->filters);
             if (block)
-                aout->play(aout, block, block->i_pts);
+                aout->play(aout, block);
         }
         else
             aout_FiltersFlush (owner->filters);
 
         aout->flush(aout, wait);
+        owner->sync.prebuf = true;
     }
 }
-- 
2.17.1



More information about the vlc-devel mailing list