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

[Anssi Hannula]

02.01.2014 12:09, Michael Niedermayer kirjoitti:
> 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

Yeah, this assumes the timebases are same (MPEG TS) or at least synced
to MPEG TS (i.e. will "wrap" at the same time even if actual wrap point
would be later), since the spec requirement is that timestamps are
synced in all playlists.

HLS spec also requires all files to be either MPEG TS, ID3 timestamped
MPEG audio elementary streams, or WebVTT files with MPEGTS timestamp
map, so really all we should encounter are MPEG TS timestamps.
(earlier versions of spec apparently also had MPEG PS as an option,
though, but it was removed in 2010)

> 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

General discontinuities have to be specifically marked:

   A Transport Stream or audio elementary stream segment MUST be the
   continuation of the encoded media at the end of the segment with the
   previous sequence number, where values in a continuous series, such
   as timestamps and Continuity Counters, continue uninterrupted -
   unless the media segment was the first ever to appear in the Playlist
   file or has an EXT-X-DISCONTINUITY tag applied to it.

We don't support discontinuity counters (EXT-X-DISCONTINUITY et al) at
the moment, though, but would not be hard to add (they are also
guaranteed to be synced across streams so we can compare them before
timestamps).

Earlier versions did not have EXT-X-DISCONTINUITY, though, but they
didn't have the "client must not assume anything about sequence numbers"
either so I guess for those we could just use the current code that
compares sequence numbers instead, since apparently it didn't do too badly.

-- 
Anssi Hannula


-- 
Anssi Hannula