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

[Anssi Hannula]

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)) {
> +        av_log(NULL, AV_LOG_VERBOSE, "Timestamp inconsistency of %f seconds between playlists, assuming a wrapped counter.\n",
> +               FFABS(tsdiff) * av_q2d(tb_a));
> +        tsdiff = -tsdiff;
> +    }
> +
> +    if (tsdiff > 0) return 1;
> +    if (tsdiff < 0) return -1;
> +    return 0;
> +}
> +
>  static int hls_read_packet(AVFormatContext *s, AVPacket *pkt)
>  {
>      HLSContext *c = s->priv_data;
> @@ -1351,26 +1374,21 @@ start:
>                  reset_packet(&pls->pkt);
>              }
>          }
> -        /* Check if this stream still is on an earlier segment number, or
> -         * has the packet with the lowest dts */
> +        /* Check if this stream has the packet with the lowest dts */
>          if (pls->pkt.data) {
>              struct playlist *minpls = minplaylist < 0 ?
>                                       NULL : c->playlists[minplaylist];
> -            if (minplaylist < 0 || pls->cur_seq_no < minpls->cur_seq_no) {
> +            if (minplaylist < 0) {
>                  minplaylist = i;
> -            } else if (pls->cur_seq_no == minpls->cur_seq_no) {
> +            } else {
>                  int64_t dts     =    pls->pkt.dts;
>                  int64_t mindts  = minpls->pkt.dts;
>                  AVRational tb    = get_timebase(   pls,    pls->pkt.stream_index);
>                  AVRational mintb = get_timebase(minpls, minpls->pkt.stream_index);
>  
> -                if (dts == AV_NOPTS_VALUE) {
> +                if (dts == AV_NOPTS_VALUE ||
> +                    (mindts != AV_NOPTS_VALUE && compare_ts_with_wrapdetect(dts, tb, mindts, mintb) < 0))
>                      minplaylist = i;
> -                } else if (mindts != AV_NOPTS_VALUE) {
> -                    if (av_compare_ts(dts, tb,
> -                                      mindts, mintb) < 0)
> -                        minplaylist = i;
> -                }
>              }
>          }
>      }
> 


-- 
Anssi Hannula