[vlc-devel] Re: vlc: svn commit r13220 (md)

[Gildas Bazin]

On Sunday 13 November 2005 16:35, Marian Durkovic wrote:
> > Where did you see that CR_MAX_GAP was in microseconds ?
> 
> There are other examples which already consider something > 2 seconds a 
"hole"
> For example in modules/access_output/udp.c:
> 
>         i_date = p_thread->i_caching + p_pk->i_dts;
>         if( i_date_last > 0 )
>         {
>             if( i_date - i_date_last > 2000000 )
>             {
>                 if( !i_dropped_packets )
>                     msg_Dbg( p_thread, "mmh, hole ("I64Fd" > 2s) -> drop",
>                              i_date - i_date_last );
> 

This has nothing to do with detecting PCR gaps.

This is just a sanity check against bad muxers which don't mux their 
elementary streams in monotonic time fashion. In this case the udp output 
drops the packet instead of delaying all the elementary streams.

If I remember well, that used to happen for instance when muxing subtitles. 
Subtitle packets used to be sent way too much in advance and that was 
screwing the udp output.

> > Yep, 22 seconds was exactly what was wanted.
> 
> Then there is a serious problem for VLC's streaming using any kind of RTP
> or RAW UDP. MPEG-TS requires PCRs every 100 msec, and streams received
> via livedotcom will have PCRs extracted from RTP timestamps of RTP 
packets.
> Leaving upto 22 second gaps undetected is very bad here... 
> Imagine what happens if e.g. livedotcom receives the first RTCP report 
with 
> 20 seconds ofset - do we really want to stop playback for 20 seconds and
> buffer all those data ???
> 

Don't misunderstand me, I'm not against tuning the value of CR_MAX_GAP.

All I'm saying is that the current value has already been tuned to have a 
good enough compromise between handling real clock gaps and handling broken 
streams (or special formats / containers) properly. So any modification to 
it must be discussed and heavily tested (also meaning it is out of question 
for 0.8.4).

I'm also arguing that the logic you implemented in your commit is broken.
CR_MAX_GAP doesn't depend on our internal clock and it needs to stay a 
constant in 90Khz units.

>
> > Assuming you'll always have less than 2 seconds between 2 received PCRs 
> > isn't sensible. It might be for well formed MPEGs but it won't be for 
other 
> > kind of streams.
> 
> OK, if this is the case, I'll revert the commit and create a ticket, since
> this is IMHO a serious problem. Anyway, could you perhaps post the typical 
> time diffs between PCRs for other kind of streams you're talking about? 
> 

I really don't have time to do that kind of testing.

The point here is that we've already been bitten by such clock changes in 
the past where streams which used to be playable would suddenly stop 
playing properly altogether.

When you did your change, you assumed that all containers and demuxers had a 
similar notion of PCRs as MPEG and the same PCR updates requirments. But I 
don't think you can assume that. What about low bitrate / very high delay 
audio streams for instance (e.g. real audio) ?

> > What you're doing wrong is, as I already stated, that you can't do input 
> > clock gap detection based on our internal clock.
> > The input clock gap detection is supposed to be based on the input clock 
and 
> > nothing else.
> 
> OK, I see, so instead of i_rate there should be INPUT_RATE_DEFAULT.
> 

No, this has nothing to do with INPUT_RATE_DEFAULT either. 
INPUT_RATE_DEFAULT is the default speed of our internal clock... which 
again has nothing to do with the input clock.

--
Gildas

-- 
This is the vlc-devel mailing-list, see http://www.videolan.org/vlc/
To unsubscribe, please read http://developers.videolan.org/lists.html