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

Anssi Hannula anssi.hannula at iki.fi
Sun Jan 5 05:12:44 CET 2014

05.01.2014 01:21, Michael Niedermayer kirjoitti:
> On Fri, Jan 03, 2014 at 04:21:39PM +0200, Anssi Hannula wrote:
>> 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 considerably longer segments and
>> therefore different numbering.
>> Since the code now exclusively syncs using timestamps that may wrap, add
>> some additional checking for that.
>> According to the HLS spec all the timestamps should be in 33-bit MPEG
>> format and synced together.
>> After this commit (and the preceding commits) HLS WebVTT subtitles
>> should work properly (ticket #2833).
>> v2: cleaner wrap detection with av_compare_mod()
>> Signed-off-by: Anssi Hannula <anssi.hannula at iki.fi>
>> ---
>> I still skipped pts_wrap_bits scaling since we are not supposed to
>> need it as per comment.
>>  libavformat/hls.c | 30 ++++++++++++++++++------------
>>  1 file changed, 18 insertions(+), 12 deletions(-)
>> diff --git a/libavformat/hls.c b/libavformat/hls.c
>> index 16e5799..e299cb3 100644
>> --- a/libavformat/hls.c
>> +++ b/libavformat/hls.c
>> @@ -1235,6 +1235,17 @@ static int reopen_subtitle_playlist(HLSContext *c, struct playlist *pls)
>>      return ret;
>>  }
>> +static int compare_ts_with_wrapdetect(int64_t ts_a, AVStream *st_a,
>> +                                      int64_t ts_b, AVStream *st_b)
>> +{
>> +    int64_t scaled_ts_b = av_rescale_q(ts_b, st_b->time_base, st_a->time_base);
>> +
>> +    /* NOTE: We do not bother to scale pts_wrap_bits here. All the allowed
>> +     * formats specified in HLS specification have 33-bit MPEG timestamps
>> +     * (which means we should not even have to perform any rescaling...). */
>> +    return av_compare_mod(ts_a, scaled_ts_b, 1LL << (FFMIN(st_a->pts_wrap_bits, st_b->pts_wrap_bits)));
> i dont think this works in all cases
> assume theres a mpeg timebase of 90khz going from 0..2^33
> and one thats lets randomly pick based on 32khz
> if the 32khz one is scaled to the 90khz one and the 33bits are used
> then it could work. (assuming HLS wraps it when the rescaled to 90khz
> would wrap)
> But if its done the other way around
> 0..2^33 * 32000 / 90000
> then the wrap around is nowhere near a power of 2

Yes, that is not handled. That was what I meant with the comment but I
wasn't too clear. Will improve it :)

> and when one assumes that timebases different from 90khz can occur
> then i guess 2 could occur together and if they wrap at 33bits
> when rescaled to 90khz then they wont neccesarily wrap at a power of
> 2 either even if they both would have the same timebase


> btw, are there samples where non 90khz based timestamps wrap ?

I don't know of any samples that have non-90kHz timestamps, period.
It would be indirectly against the spec (since spec allows mpegts,
hls_id3_audio, webvtt with timestamp_map, all of which have 90kHz).

Do you think it is still worth it to add code to handle those two cases
you mentioned? I'm not really against it...

All the scaling code was previously needed because before hls_id3_audio
such streams were handled as raw with non-90kHz generated timestamps,
but they are probably not needed now.

I've kept the scaling code around just in case there are some weird
streams, as I don't actually have a too large amount of HLS samples
myself... Maybe a "request sample/link" warning could/should be added
for those cases.

Anssi Hannula

More information about the ffmpeg-devel mailing list