[FFmpeg-devel] avformat/hls: tag as AVFMT_TS_DISCONT

Submitted by Aman Gupta on May 16, 2018, 5:17 p.m.

Details

Message ID 20180516171758.16844-1-ffmpeg@tmm1.net
State New
Headers show

Commit Message

Aman Gupta May 16, 2018, 5:17 p.m.
From: Aman Gupta <aman@tmm1.net>

HLS streams can contain discontinuities. Mark the format as such.

This triggers various discontinuity fixes in lavf/utils.c and fftools

Signed-off-by: Aman Gupta <aman@tmm1.net>
---
 libavformat/hls.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

wm4 May 16, 2018, 6:14 p.m.
On Wed, 16 May 2018 10:17:58 -0700
Aman Gupta <ffmpeg@tmm1.net> wrote:

> From: Aman Gupta <aman@tmm1.net>
> 
> HLS streams can contain discontinuities. Mark the format as such.
> 
> This triggers various discontinuity fixes in lavf/utils.c and fftools
> 
> Signed-off-by: Aman Gupta <aman@tmm1.net>
> ---
>  libavformat/hls.c | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/libavformat/hls.c b/libavformat/hls.c
> index 4ee4be769d..3199b0ac8d 100644
> --- a/libavformat/hls.c
> +++ b/libavformat/hls.c
> @@ -2277,7 +2277,7 @@ AVInputFormat ff_hls_demuxer = {
>      .long_name      = NULL_IF_CONFIG_SMALL("Apple HTTP Live Streaming"),
>      .priv_class     = &hls_class,
>      .priv_data_size = sizeof(HLSContext),
> -    .flags          = AVFMT_NOGENSEARCH,
> +    .flags          = AVFMT_NOGENSEARCH | AVFMT_TS_DISCONT,
>      .read_probe     = hls_probe,
>      .read_header    = hls_read_header,
>      .read_packet    = hls_read_packet,

I think I'm against this. HLS streams do not typically contain
timestamp resets (even if they could). Otherwise you might as well add
this flag to the Matroska demuxer. Besides, it would break some of my
code, which uses this flag as a heuristic to detect mpeg-ts style
non-container formats.
Aman Gupta May 16, 2018, 6:45 p.m.
On Wed, May 16, 2018 at 11:14 AM, wm4 <nfxjfg@googlemail.com> wrote:

> On Wed, 16 May 2018 10:17:58 -0700
> Aman Gupta <ffmpeg@tmm1.net> wrote:
>
> > From: Aman Gupta <aman@tmm1.net>
> >
> > HLS streams can contain discontinuities. Mark the format as such.
> >
> > This triggers various discontinuity fixes in lavf/utils.c and fftools
> >
> > Signed-off-by: Aman Gupta <aman@tmm1.net>
> > ---
> >  libavformat/hls.c | 2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> >
> > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > index 4ee4be769d..3199b0ac8d 100644
> > --- a/libavformat/hls.c
> > +++ b/libavformat/hls.c
> > @@ -2277,7 +2277,7 @@ AVInputFormat ff_hls_demuxer = {
> >      .long_name      = NULL_IF_CONFIG_SMALL("Apple HTTP Live Streaming"),
> >      .priv_class     = &hls_class,
> >      .priv_data_size = sizeof(HLSContext),
> > -    .flags          = AVFMT_NOGENSEARCH,
> > +    .flags          = AVFMT_NOGENSEARCH | AVFMT_TS_DISCONT,
> >      .read_probe     = hls_probe,
> >      .read_header    = hls_read_header,
> >      .read_packet    = hls_read_packet,
>
> I think I'm against this. HLS streams do not typically contain
> timestamp resets (even if they could). Otherwise you might as well add
>

mpegts timestamps wrap every 26 hours, so any live continuous HLS broadcast
is guaranteed to have a timestamp reset.
Users have reported seeing these resets on various streams (
http://ffmpeg.org/pipermail/ffmpeg-devel/2018-March/226706.html), plus
the HLS spec itself defines a EXT-X-DISCONTINUITY tag to signal
discontinuities. So I don't think it's fair to say that discontinuities
don't typically occur and can be ignored.


> this flag to the Matroska demuxer. Besides, it would break some of my
> code, which uses this flag as a heuristic to detect mpeg-ts style
> non-container formats.
>

I'm working on a patchset with a new -remove_ts_discont option which will
remove discontinuities and normalize timestamps. This
option would only kick in for formats marked as AVFMT_TS_DISCONT, so I
would like to see any formats where timestamp
discontinuities are possible to be marked as such.

Maybe we can add another AVFMT flag that can be used as your heuristic?

Aman




> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
wm4 May 16, 2018, 7:02 p.m.
On Wed, 16 May 2018 11:45:58 -0700
Aman Gupta <ffmpeg@tmm1.net> wrote:

> On Wed, May 16, 2018 at 11:14 AM, wm4 <nfxjfg@googlemail.com> wrote:
> 
> > On Wed, 16 May 2018 10:17:58 -0700
> > Aman Gupta <ffmpeg@tmm1.net> wrote:
> >  
> > > From: Aman Gupta <aman@tmm1.net>
> > >
> > > HLS streams can contain discontinuities. Mark the format as such.
> > >
> > > This triggers various discontinuity fixes in lavf/utils.c and fftools
> > >
> > > Signed-off-by: Aman Gupta <aman@tmm1.net>
> > > ---
> > >  libavformat/hls.c | 2 +-
> > >  1 file changed, 1 insertion(+), 1 deletion(-)
> > >
> > > diff --git a/libavformat/hls.c b/libavformat/hls.c
> > > index 4ee4be769d..3199b0ac8d 100644
> > > --- a/libavformat/hls.c
> > > +++ b/libavformat/hls.c
> > > @@ -2277,7 +2277,7 @@ AVInputFormat ff_hls_demuxer = {
> > >      .long_name      = NULL_IF_CONFIG_SMALL("Apple HTTP Live Streaming"),
> > >      .priv_class     = &hls_class,
> > >      .priv_data_size = sizeof(HLSContext),
> > > -    .flags          = AVFMT_NOGENSEARCH,
> > > +    .flags          = AVFMT_NOGENSEARCH | AVFMT_TS_DISCONT,
> > >      .read_probe     = hls_probe,
> > >      .read_header    = hls_read_header,
> > >      .read_packet    = hls_read_packet,  
> >
> > I think I'm against this. HLS streams do not typically contain
> > timestamp resets (even if they could). Otherwise you might as well add
> >  
> 
> mpegts timestamps wrap every 26 hours, so any live continuous HLS broadcast
> is guaranteed to have a timestamp reset.
> Users have reported seeing these resets on various streams (
> http://ffmpeg.org/pipermail/ffmpeg-devel/2018-March/226706.html), plus
> the HLS spec itself defines a EXT-X-DISCONTINUITY tag to signal
> discontinuities. So I don't think it's fair to say that discontinuities
> don't typically occur and can be ignored.

Wouldn't it be better to introduce a discontinuity notification then?
Then adding the discont flag would be redundant and could be avoided.
It looks like the HLS tag would map cleanly to this.

> 
> > this flag to the Matroska demuxer. Besides, it would break some of my
> > code, which uses this flag as a heuristic to detect mpeg-ts style
> > non-container formats.
> >  
> 
> I'm working on a patchset with a new -remove_ts_discont option which will
> remove discontinuities and normalize timestamps. This
> option would only kick in for formats marked as AVFMT_TS_DISCONT, so I
> would like to see any formats where timestamp
> discontinuities are possible to be marked as such.

My suggestion would probably not block this.

> Maybe we can add another AVFMT flag that can be used as your heuristic?

Well, my heuristic is probably messed up and nonsense anyway. I'd just
find it nice if this isn't enabled to avoid currently released
versions, but I guess my argument is weak.
Timo Rothenpieler May 17, 2018, 10:02 a.m.
> I think I'm against this. HLS streams do not typically contain
> timestamp resets (even if they could). Otherwise you might as well add
> this flag to the Matroska demuxer. Besides, it would break some of my
> code, which uses this flag as a heuristic to detect mpeg-ts style
> non-container formats.

They do, a lot. For example sites insert ads in them which change pretty
much everything about the stream while they are running.

Patch hide | download patch | download mbox

diff --git a/libavformat/hls.c b/libavformat/hls.c
index 4ee4be769d..3199b0ac8d 100644
--- a/libavformat/hls.c
+++ b/libavformat/hls.c
@@ -2277,7 +2277,7 @@  AVInputFormat ff_hls_demuxer = {
     .long_name      = NULL_IF_CONFIG_SMALL("Apple HTTP Live Streaming"),
     .priv_class     = &hls_class,
     .priv_data_size = sizeof(HLSContext),
-    .flags          = AVFMT_NOGENSEARCH,
+    .flags          = AVFMT_NOGENSEARCH | AVFMT_TS_DISCONT,
     .read_probe     = hls_probe,
     .read_header    = hls_read_header,
     .read_packet    = hls_read_packet,