[vlc-devel] [PATCH 08/17] es_out: use the input_clock as a master source of vlc_clock

[Thomas Guillem]

Hello,

Thanks for your review, and sorry for the answer delay (I was on holidays).

On Mon, Feb 22, 2021, at 12:04, Rémi Denis-Courmont wrote:
> Le perjantaina 19. helmikuuta 2021, 10.38.09 EET Thomas Guillem a écrit :
> > Thanks for this review, you raise an important point. Let me enumerate what
> > my patch set is changing and what could be discussed.
> > 
> >  - 1/ We add back the input (PCR) as clock master, disabled by default. This
> > feature is likely to be used by advanced users.
> > 
> >  - 2/ We now use the input clock to calculate the pts_delay for the Output.
> > When converting the first PTS, the master ES will use the input clock to
> > convert its PTS. Once the master ES is started, (the first
> > vlc_clock_Update() is called), it won't use the input clock anymore.
> > 
> > The pts_delay calculated by this new way is now the same as what we were
> > doing in VLC 3.0 (you can easily see it by comparing the starting differed
> > logs in pulse audio). When we added the clock, we tried to emulate this
> > calculation via vlc_clock_main_SetFirstPcr() and
> > vlc_clock_main_SetInputDejitter() but the results are not exactly the same.
> > 
> > - 3/ If there is no master, the input clock is used as master (like in VLC
> > 3.0).
> > 
> > 
> > I think the points 1/ and 2/ are valid. What do you think?
> 
> I don't see how that makes any sense, for the simpel reason already outlined 
> that, in most cases, there is NO input clock.


What do you mean by NO input clock here?

If I understand correctly, using the input_clock as master is not advised in most cases but can be useful with PACED content (reminder: the input_clock is directly feed by the demux module via the SET_PCR control).

Do we agree on that? If so, I can modify my patch set to:
 - Let the input_clock as master if the user asked it or if the demux is paced.
 - Otherwise, fallback to Audio master or monotonic clock (like before).

By the way, the main reason of this patchset is to fix drift when using the monotonic clock of the audio one when playing PACED content.

Best regards,

> You seem to confuse input clock with ES output pacing which meant for flow 
> control and buffering, not for clock. If anything, not confusing clock and 
> buffer was the main point of the whole clock rework.
> 
> -- 
> Rémi Denis-Courmont
> http://www.remlab.net/
> 
> 
> 
> _______________________________________________
> vlc-devel mailing list
> To unsubscribe or modify your subscription options:
> https://mailman.videolan.org/listinfo/vlc-devel