[vlc-devel] [RFC PATCH 12/13] FIXUP: fix preroll with !pulse aouts
Thomas Guillem
thomas at gllm.fr
Mon Jul 2 17:35:14 CEST 2018
On Wed, Jun 27, 2018, at 15:59, Rémi Denis-Courmont wrote:
> The patch description makes no sense to me, because it describes the
> existing state before the patch.
Indeed, the commit message is not that good. I'll try to improve it. In
the meantime, here is what I tried to fix:
>
> Besides, there are no problems with output not starting corked, as
> long as the output knows its latency immediately. Not all but several
> outputs can probably start corked (i.e. paused), but they might not
> need to.>
> The issue is PulseAudio has no latency estimate at start, so it needs
> to postpone start. This is more generic for the users: it can cope
> with fringe sink sorts. But for our purpose as developers, it is just
> an inconvenience of PulseAudio.>
> But really the assumption is that one of the following is true:
> 1) latency is known immediately (e.g. ALSA),
There is a chicken-egg situation with such modules (e.g ALSA):
Before this patch (and in the clock branch), the clock was updated after
the first time_get(), so it took only the audio delay into account, not
the dejitter value from the output clock. To take into account the
dejitter, you need to do (what this patch is doing):
- First, and if the delay is OK, convert the pts from now + delay:
vlc_tick_t system_pts = vlc_clock_ConvertToSystem(owner->sync.clock,
system_now + delay, block->i_pts);
- Compare this value with now: if it's positive, you need to silence the
aout (so, the dejitter value approximately)
vlc_tick_t silence_delay = system_pts - system_now;
if (silence_delay > 0)
aout_DecSilence (aout, silence_delay, block->i_pts);
- Get the new delay, and do go the original code path
(aout_RequestRetiming()):
if (aout->time_get(aout, &delay) == 0)
aout_RequestRetiming(aout, system_now + delay, block->i_pts);
This delay will take into account the audio device latency (if the aout
module permits it) and the dejitter value from the clock.
> 2) latency is signaled asynchronously when it becomes available (e.g.
> PulseAudio), or> 3) latency is irrelevant (e.g. file).
>
> And with that, I don't follow what this patch does, other than make
> the existing code more intricate.>
> Le 27 juin 2018 15:41:34 GMT+03:00, Thomas Guillem <thomas at gllm.fr>
> a écrit :>> The idea here is a play a silence corresponding to the difference
>> between the
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> first system pts and now. This will be used for aouts that can't
>> start corked,
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> so every aout modules except PulseAudio. To reach this new code
>> path, aout
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> modules need to return a valid value from the first pf_get_time
>> callback
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> (already the case of main modules).
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> ---
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> src/audio_output/aout_internal.h | 1 +
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> src/audio_output/dec.c | 54 ++++++++++++++++++++++----------
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> 2 files changed, 38 insertions(+), 17 deletions(-)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> diff --git a/src/audio_output/aout_internal.h
>> b/src/audio_output/aout_internal.h
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> index 55fab1a7e9..42d0f1ff35 100644
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> --- a/src/audio_output/aout_internal.h
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> +++ b/src/audio_output/aout_internal.h
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> @@ -72,6 +72,7 @@ typedef struct
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> struct
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> {
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + bool is_first;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> vlc_tick_t end; /**< Last seen PTS */
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> struct vlc_clock_t *clock;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> float rate; /**< Play-out speed rate */
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> diff --git a/src/audio_output/dec.c b/src/audio_output/dec.c
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> index 523682e619..8a2808c5a0 100644
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> --- a/src/audio_output/dec.c
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> +++ b/src/audio_output/dec.c
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> @@ -102,6 +102,7 @@ error:
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> return -1;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> }
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + owner->sync.is_first = true;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> owner->sync.rate = 1.f;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> owner->sync.end = VLC_TS_INVALID;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> owner->sync.resamp_type = AOUT_RESAMPLING_NONE;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> @@ -229,8 +230,8 @@ static void aout_DecSilence (audio_output_t
>> *aout, vlc_tick_t length, vlc_tick_t
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> vlc_clock_ConvertToSystem(owner->sync.clock,
>> vlc_tick_now(), pts));
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> }
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> -static void aout_DecSynchronize(audio_output_t *aout, vlc_tick_t
>> system_now,
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - vlc_tick_t dec_pts)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> +static vlc_tick_t aout_DecSynchronize(audio_output_t *aout,
>> vlc_tick_t system_now,
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + block_t *block)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> {
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> /**
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> * Depending on the drift between the actual and intended
>> playback times,
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> @@ -248,11 +249,39 @@ static void aout_DecSynchronize(audio_output_t
>> *aout, vlc_tick_t system_now,
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> * all samples in the buffer will have been played. Then:
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> * pts = vlc_tick_now() + delay
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> */
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + aout_owner_t *owner = aout_owner (aout);
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> vlc_tick_t delay;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - if (aout->time_get(aout, &delay) != 0)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - return; /* nothing can be done if timing is unknown */
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + if (aout->time_get(aout, &delay) == 0)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + {
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + if (owner->sync.is_first)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + {
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + vlc_tick_t system_pts =
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + vlc_clock_ConvertToSystem(owner->sync.clock,
>> system_now + delay,
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + block->i_pts);
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + vlc_tick_t silence_delay = system_pts - system_now;
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + if (silence_delay > 0)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + aout_DecSilence (aout, silence_delay, block->i_pts);
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + if (aout->time_get(aout, &delay) == 0)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + aout_RequestRetiming(aout, system_now + delay, block-
>> >i_pts);
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + }
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + else
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + aout_RequestRetiming(aout, system_now + delay, block-
>> >i_pts);
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + }
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> +
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + if (owner->sync.delay != 0)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + {
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + block->i_pts += owner->sync.delay;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + block->i_dts += owner->sync.delay;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + }
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - aout_RequestRetiming(aout, system_now, dec_pts - delay);
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + vlc_tick_t system_pts = vlc_clock_ConvertToSystem(owner-
>> >sync.clock, system_now,
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + block->i_pts);
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> +
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + owner->sync.end = block->i_pts + block->i_length + 1;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + owner->sync.discontinuity = false;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + owner->sync.is_first = false;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> +
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + return system_pts;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> }
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> void aout_RequestRetiming(audio_output_t *aout, vlc_tick_t
>> system_ts,
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> @@ -397,20 +426,10 @@ int aout_DecPlay(audio_output_t *aout, block_t
>> *block)
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> }
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> /* Drift correction */
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - vlc_tick_t system_now = vlc_tick_now();
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - aout_DecSynchronize(aout, system_now, block->i_pts);
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> -
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - if (owner->sync.delay != 0)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - {
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - block->i_pts += owner->sync.delay;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - block->i_dts += owner->sync.delay;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - }
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + vlc_tick_t system_pts = aout_DecSynchronize(aout,
>> vlc_tick_now(), block);
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> /* Output */
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - owner->sync.end = block->i_pts + block->i_length + 1;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - owner->sync.discontinuity = false;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - aout->play(aout, block, vlc_clock_ConvertToSystem(owner-
>> >sync.clock, system_now,
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> - block-
>> >i_pts));
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + aout->play(aout, block, system_pts);
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> atomic_fetch_add_explicit(&owner->buffers_played, 1,
>> memory_order_relaxed);
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> return ret;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> drop:
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> @@ -473,6 +492,7 @@ void aout_DecFlush (audio_output_t *aout,
>> bool wait)
>>>>
>>
>>
>>
>>
>>
>>
>>
>>
>> {
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> aout_owner_t *owner = aout_owner (aout);
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> + owner->sync.is_first = true;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> owner->sync.end = VLC_TS_INVALID;
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> if (owner->mixer_format.i_format)
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>> {
>>
>>
>>
>>
>>
>>
>>
>>
>>
>>
>
> --
> Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez
> excuser ma brièveté.> _________________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180702/e3d01461/attachment.html>
More information about the vlc-devel
mailing list