[vlc-devel] [RFC PATCH] pulse: hack: wait for drain completion

[Rémi Denis-Courmont]

On March 20, 2017 4:54:53 PM GMT+02:00, Thomas Guillem <thomas at gllm.fr> wrote:
>So, I guess the TODO was here for a good reason. stream_drain_cb is
>never
>called after a call to pa_stream_drain(). We could be notified when
>pulse is
>drained by listening to the underflow callback but that's not the good
>way.
>
>I would prefer to find why pulseaudio doesn't send the drain cb. I
>looked at
>some other players, they all seems to listen to the drain callback
>without any
>issues.
>
>I debugged a little on pulseaudio side, the drain is never sent, see
>https://github.com/pulseaudio/pulseaudio/blob/master/src/pulsecore/protocol-native.c#L1475
>pa_sink_input_safe_to_remove(s->sink_input) is always false since the
>render_memblockq is never empty.
>
>I'm still wondering if the problem comes from VLC or PulseAudio.
>
>Any hints are welcome.
>
>---
>
> modules/audio_output/pulse.c | 32 +++++++++++++++++++++++++++-----
> 1 file changed, 27 insertions(+), 5 deletions(-)
>
>diff --git a/modules/audio_output/pulse.c
>b/modules/audio_output/pulse.c
>index 38a0d6624e..4b2db4b8d4 100644
>--- a/modules/audio_output/pulse.c
>+++ b/modules/audio_output/pulse.c
>@@ -75,6 +75,8 @@ struct aout_sys_t
>     char *sink_force; /**< Forced sink name (stream must be NULL) */
> 
>     struct sink *sinks; /**< Locally-cached list of sinks */
>+
>+    bool underflow;
> };
> 
> static void VolumeReport(audio_output_t *aout)
>@@ -352,8 +354,11 @@ static void stream_suspended_cb(pa_stream *s, void
>*userdata)
> static void stream_underflow_cb(pa_stream *s, void *userdata)
> {
>     audio_output_t *aout = userdata;
>+    aout_sys_t *sys = aout->sys;
> 
>     msg_Dbg(aout, "underflow");
>+    sys->underflow = true;
>+    pa_threaded_mainloop_signal(aout->sys->mainloop, 0);
>     (void) s;
> }
> 
>@@ -503,6 +508,8 @@ static void Play(audio_output_t *aout, block_t
>*block)
>      * will take place, and sooner or later a deadlock. */
>     pa_threaded_mainloop_lock(sys->mainloop);
> 
>+    sys->underflow = false;
>+
>     if (sys->first_pts == VLC_TS_INVALID)
>         sys->first_pts = block->i_pts;
> 
>@@ -548,6 +555,12 @@ static void Pause(audio_output_t *aout, bool
>paused, mtime_t date)
>     (void) date;
> }
> 
>+static void stream_drain_cb(pa_stream *s, int success, void *userdata)
>+{
>+    (void) s; (void) success;
>+    pa_threaded_mainloop_signal(userdata, 0);
>+}
>+
> /**
>  * Flush or drain the playback stream
>  */
>@@ -558,14 +571,23 @@ static void Flush(audio_output_t *aout, bool
>wait)
>     pa_operation *op;
> 
>     pa_threaded_mainloop_lock(sys->mainloop);
>-
>     if (wait)
>-        op = pa_stream_drain(s, NULL, NULL);
>-        /* TODO: wait for drain completion*/
>+    {
>+        op = pa_stream_drain(s, stream_drain_cb, sys->mainloop);
>+        if (op != NULL)
>+        {
>+            while (pa_operation_get_state(op) == PA_OPERATION_RUNNING
>+                && !sys->underflow)
>+                pa_threaded_mainloop_wait(sys->mainloop);
>+            pa_operation_unref(op);
>+        }
>+    }
>     else
>+    {
>         op = pa_stream_flush(s, NULL, NULL);
>-    if (op != NULL)
>-        pa_operation_unref(op);
>+        if (op != NULL)
>+            pa_operation_unref(op);
>+    }
>     sys->first_pts = VLC_TS_INVALID;
>     stream_stop(s, aout);
> 
>-- 
>2.11.0
>
>_______________________________________________
>vlc-devel mailing list
>To unsubscribe or modify your subscription options:
>https://mailman.videolan.org/listinfo/vlc-devel

There are no warranties that the underflow callback relates to draining. It just means that there is space for new data. This patch is a bad idea IMO.
-- 
Rémi Denis-Courmont