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

[Michael Niedermayer]

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

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 ?

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

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- 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/664a93b4/attachment.asc>