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

[Michael Niedermayer]

On Sun, Jan 05, 2014 at 06:12:44AM +0200, Anssi Hannula wrote:
> 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
> 
> Yep.
> 
> > 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...

does it require more than scaling both to 90khz ?


> 
> 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
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No human being will ever know the Truth, for even if they happen to say it
by chance, they would not even known they had done so. -- Xenophanes
-------------- 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/20140105/23daa4fa/attachment.asc>