| Message ID | 20180516171758.16844-1-ffmpeg@tmm1.net |
|---|---|
| State | Accepted |
| Commit | d6ac6650b911f0957e69545d7fc25be6b7728705 |
| Headers | show |
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.
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 >
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.
> 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.
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,