[FFmpeg-devel] [PATCH 18/18] avformat/hls: do not use sequence numbers for packet ordering

[Michael Niedermayer]

On Thu, Jan 02, 2014 at 12:02:00AM +0200, Anssi Hannula wrote:
> 30.12.2013 13:14, Anssi Hannula kirjoitti:
> > As per spec 3.4.3 ("A client MUST NOT assume that segments with the same
> > sequence number in different Media Playlists contain matching content.")
> > we cannot use sequence numbers for packet ordering.
> > 
> > This can be seen e.g. in the subtitle streams of
> > bipbop_16x9_variant.m3u8 that have considerable longer segments and
> > therefore different numbering.
> > 
> > Since the only remaining "always-available" differentiator is timestamps
> > that may wrap, add some very rudimentary checks to try to detect such
> > situations in at least the most common cases (all timestamps MPEG TS as
> > per spec).
> > 
> > After this commit (and the preceding commits) HLS WebVTT subtitles
> > should work properly (ticket #2833).
> > 
> > Signed-off-by: Anssi Hannula <anssi.hannula at iki.fi>
> > ---
> > 
> > compare_ts_with_wrapdetect() is very hacky, but I could not see any
> > better ways that were implementable in a reasonable time (though
> > I'm not that familiar this timestamp stuff, maybe I missed something).
> > 
> > I'm also not 100% sure if comparing tsdiff is enough or if I should
> > call av_compare_ts() there at the end (which would duplicate the
> > rescaling a bit).
> > 
> > I did try the code with wrapping MPEG TS timestamps and it seemed
> > to do its job, though.
> 
> For non-live streams we could store the segment startpos AV_TIME_BASE
> timestamps in struct segment and then use them here for comparison
> before raw timestamps, to avoid relying on the wrapdetect hack in that case.
> 
> Not sure what can be done for live streams to avoid the hack altogether,
> though... maybe some heuristics involving checking for non-monotonic
> timestamps and/or prev/curr segment startpos...
> 
> 
> >  libavformat/hls.c | 38 ++++++++++++++++++++++++++++----------
> >  1 file changed, 28 insertions(+), 10 deletions(-)
> > 
> > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > index 07c3c80..f7889ef 100644
> > --- a/libavformat/hls.c
> > +++ b/libavformat/hls.c
> > @@ -1256,6 +1256,29 @@ static AVRational get_timebase(struct playlist *pls, int stream_index)
> >      return pls->ctx->streams[stream_index]->time_base;
> >  }
> >  
> > +static int compare_ts_with_wrapdetect(int64_t ts_a, AVRational tb_a,
> > +                                      int64_t ts_b, AVRational tb_b)
> > +{
> > +    /*
> > +     * Perform some rudimentary checking for timestamp discontinuities.
> > +     * This will not catch everything but should handle at least the
> > +     * spec-compliant case of all timestamps being MPEG TS timestamps...
> > +     * Commonly only a single playlist is played back at a time, anyway,
> > +     * so this code is not even reached.
> > +     */
> > +    int64_t scaled_ts_b = av_rescale_q(ts_b, tb_b, tb_a);
> > +    int64_t tsdiff = ts_a - scaled_ts_b;
> > +    if (FFABS(tsdiff) > (1LL << 31)) {

this looks very odd
if one would have 2 timebases and wraping timestamps then one would
expect they wrap at unrelated times. so one might wrap every 31 minutes
one wrap every 123 minutes. now if you consider x*31 + y*123 where
x,y are the number of wraps of each you can get any difference
by choosing appropriate wrap counts. So i dont think that wraping
timestamps and different timebases could work in general

also is it timestamp wraps or general discontinuities that are allowed
or timestamp resets to 0 ?
mpeg on its own allows arbitrary ones, i dont know/remember what hls
says about it

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is what and why we do it that matters, not just one of them.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20140102/0b42dca7/attachment.asc>