[vlc-devel] [RFC PATCH 12/13] FIXUP: fix preroll with !pulse aouts

Rémi Denis-Courmont remi at remlab.net
Mon Jul 2 18:02:39 CEST 2018


I don't perceive a catch-22 at all. If the dejitter delay is accounted by the clock, which it obviously should, then everything should work as things stand.

There is the corner case that the dejitter is too short to begin with, and audio is not master, but then I would argue that skipping ahead is the right thing. 

Le 2 juillet 2018 18:35:14 GMT+03:00, Thomas Guillem <thomas at gllm.fr> a écrit :
>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

-- 
Envoyé de mon appareil Android avec Courriel K-9 Mail. Veuillez excuser ma brièveté.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mailman.videolan.org/pipermail/vlc-devel/attachments/20180702/73537708/attachment.html>


More information about the vlc-devel mailing list