[vlc-devel] [PATCH v3 2/6] clock: add vlc_clock_main_SetInputDrift

[Alexandre Janniaux]

On Wed, Mar 17, 2021 at 10:55:39AM +0200, Rémi Denis-Courmont wrote:
> Le tiistaina 16. maaliskuuta 2021, 23.41.51 EET Alexandre Janniaux a écrit :
> > Hi,
> >
> > On Tue, Mar 16, 2021 at 11:18:58PM +0200, Rémi Denis-Courmont wrote:
> > > Le tiistaina 16. maaliskuuta 2021, 22.45.36 EET Alexandre Janniaux a écrit
> :
> > > > > - Sometimes playback never "catches up".
> > > >
> > > > What do you mean by «never catches up»?
> > >
> > > The same symptoms as Thomas claims to be happening now.
> > >
> > > > > - Especially as start/resume, doppler sound effect (resampling) due to
> > > > > abnormally quick succession of SetPCR calls.
> > > >
> > > > The «doppler sound effect» on 3.0 and the artifacts were
> > > > happening because of the way this clock was implemented, ie
> > > > it's only about how you resample and filter the result.
> > >
> > > No. It's about how the clock was measured. Resampling was working as
> > > intended when the master clock and the audio clock drift from one
> > > another.
> >
> > If you squeeze audio, it will get pitched up, so you need to
> > be sure there's silence in that place, or audio that has been
> > stretched to match the frequency used. Using the input PCR as
> > clock will guarantee that there are audio glitch someday, but
> > it's cannot generate high pitch sound. Clock is about time, not
> > sound.
>
> I completely disagree here as already explained.
>
> > > > It doesn't mean that using the input PCR is incorrect.
> > >
> > > Actually, it does mean exactly that.
> >
> > My interpretation is arbitrary too but is at least sustained
> > by the clock design document. Can you be more explicit about
> > why it means exactly that for you?
>
> I have no problem with the design document. The points are:
> 1) This patchset is not what the design document says. It is nowhere said to
> add the input to the audio master clock. Also what Denis pointed out.
> 2) The old actual *implementation* of the "input PCR clock" is well-known
> broken causing the issues I already mentioned. Implementation, not concept.

Thank you for finally clarifying this. That's exactly what I've
been trying to clarify in previous messages.

I 100% agree that the previous input PCR clock implementation
has flaws, what I did not understand was why you were against
its very concept.

So if I understand, what you expect is that the input PCR clock
is actually exposed as a clock setter to provide its own clock
parameters instead of using a separate drift value in the main
clock and using it with whatever other clock is there, just
like the audio is doing currently?

Regards,
--
Alexandre Janniaux
Videolabs