| Message ID | tencent_B8C355B41C692E014D6BA27DD2AD223A3D0A@qq.com |
|---|---|
| State | New |
| Headers | show |
| Series | [FFmpeg-devel] avformat/mov: discard data streams with all zero sample_delta | expand |
| Context | Check | Description |
|---|---|---|
| yinshiyou/make_loongarch64 | success | Make finished |
| yinshiyou/make_fate_loongarch64 | success | Make fate finished |
| andriy/make_x86 | success | Make finished |
| andriy/make_fate_x86 | success | Make fate finished |
| andriy/make_armv7_RPi4 | success | Make finished |
| andriy/make_fate_armv7_RPi4 | success | Make fate finished |
On 2022-07-05 01:20 pm, Zhao Zhili wrote: > From: Zhao Zhili <zhilizhao@tencent.com> > > Streams with all zero sample_delta in 'stts' have all zero dts. > They have higher chance be chose by mov_find_next_sample(), which > leads to seek again and again. > > For example, GoPro created a 'GoPro SOS' stream: > Stream #0:4[0x5](eng): Data: none (fdsc / 0x63736466), 13 kb/s (default) > Metadata: > creation_time : 2022-06-21T08:49:19.000000Z > handler_name : GoPro SOS > > With 'ffprobe -show_frames http://example.com/gopro.mp4', ffprobe > blocks until all samples in 'GoPro SOS' stream are consumed first. > > Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> > --- > libavformat/mov.c | 14 ++++++++++++++ > 1 file changed, 14 insertions(+) > > diff --git a/libavformat/mov.c b/libavformat/mov.c > index 88669faa70..2a4eb79f27 100644 > --- a/libavformat/mov.c > +++ b/libavformat/mov.c > @@ -3062,6 +3062,20 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) > st->nb_frames= total_sample_count; > if (duration) > st->duration= FFMIN(st->duration, duration); > + > + // All samples have zero duration. They have higher chance be chose by > + // mov_find_next_sample, which leads to seek again and again. > + // > + // It's AVERROR_INVALIDDATA actually, but such files exist in the wild. > + // So only mark data stream as discarded for safety. > + if (!duration && sc->stts_count && > + st->codecpar->codec_type == AVMEDIA_TYPE_DATA) { > + av_log(c->fc, AV_LOG_WARNING, > + "All samples in data stream index:id [%d:%d] have zero duration, " > + "discard the stream\n", > + st->index, st->id); > + st->discard = AVDISCARD_ALL; > + } > sc->track_end = duration; > return 0; > } So this will allow audio and video streams to be demuxed, but not data? That distinction seems arbitrary. Print a warning and assign a duration to each sample. Either 1 or if not zero/Inf, st->duration/st->nb_frames. Regards, Gyan
> On Jul 5, 2022, at 8:07 PM, Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > On 2022-07-05 01:20 pm, Zhao Zhili wrote: >> From: Zhao Zhili <zhilizhao@tencent.com> >> >> Streams with all zero sample_delta in 'stts' have all zero dts. >> They have higher chance be chose by mov_find_next_sample(), which >> leads to seek again and again. >> >> For example, GoPro created a 'GoPro SOS' stream: >> Stream #0:4[0x5](eng): Data: none (fdsc / 0x63736466), 13 kb/s (default) >> Metadata: >> creation_time : 2022-06-21T08:49:19.000000Z >> handler_name : GoPro SOS >> >> With 'ffprobe -show_frames http://example.com/gopro.mp4', ffprobe >> blocks until all samples in 'GoPro SOS' stream are consumed first. >> >> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> >> --- >> libavformat/mov.c | 14 ++++++++++++++ >> 1 file changed, 14 insertions(+) >> >> diff --git a/libavformat/mov.c b/libavformat/mov.c >> index 88669faa70..2a4eb79f27 100644 >> --- a/libavformat/mov.c >> +++ b/libavformat/mov.c >> @@ -3062,6 +3062,20 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) >> st->nb_frames= total_sample_count; >> if (duration) >> st->duration= FFMIN(st->duration, duration); >> + >> + // All samples have zero duration. They have higher chance be chose by >> + // mov_find_next_sample, which leads to seek again and again. >> + // >> + // It's AVERROR_INVALIDDATA actually, but such files exist in the wild. >> + // So only mark data stream as discarded for safety. >> + if (!duration && sc->stts_count && >> + st->codecpar->codec_type == AVMEDIA_TYPE_DATA) { >> + av_log(c->fc, AV_LOG_WARNING, >> + "All samples in data stream index:id [%d:%d] have zero duration, " >> + "discard the stream\n", >> + st->index, st->id); >> + st->discard = AVDISCARD_ALL; >> + } >> sc->track_end = duration; >> return 0; >> } > > So this will allow audio and video streams to be demuxed, but not data? That distinction seems arbitrary. Disable audio/video streams may create regression. It’s unlikely for random and broken data stream. > > Print a warning and assign a duration to each sample. Either 1 or if not zero/Inf, st->duration/st->nb_frames. Set sample_duration to 1 doesn’t work. Dts still far behind other streams. Set sample_duration st->duration/st->nb_frames works for me, but I prefer current strategy for the following reasons: 1. AVDISCARD_ALL is more close to AVERROR_INVALIDDATA by giving up instead of trying correction and hope it works, which may not, e.g., st->duration is broken, or bad interleave even though we fixed sample_duration. 2. libavformat users can enable the stream and get the original dts/duration, if they want to. > > Regards, > Gyan > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 2022-07-05 07:05 pm, "zhilizhao(赵志立)" wrote: > >> On Jul 5, 2022, at 8:07 PM, Gyan Doshi <ffmpeg@gyani.pro> wrote: >> >> >> >> On 2022-07-05 01:20 pm, Zhao Zhili wrote: >>> From: Zhao Zhili <zhilizhao@tencent.com> >>> >>> Streams with all zero sample_delta in 'stts' have all zero dts. >>> They have higher chance be chose by mov_find_next_sample(), which >>> leads to seek again and again. >>> >>> For example, GoPro created a 'GoPro SOS' stream: >>> Stream #0:4[0x5](eng): Data: none (fdsc / 0x63736466), 13 kb/s (default) >>> Metadata: >>> creation_time : 2022-06-21T08:49:19.000000Z >>> handler_name : GoPro SOS >>> >>> With 'ffprobe -show_frames http://example.com/gopro.mp4', ffprobe >>> blocks until all samples in 'GoPro SOS' stream are consumed first. >>> >>> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> >>> --- >>> libavformat/mov.c | 14 ++++++++++++++ >>> 1 file changed, 14 insertions(+) >>> >>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>> index 88669faa70..2a4eb79f27 100644 >>> --- a/libavformat/mov.c >>> +++ b/libavformat/mov.c >>> @@ -3062,6 +3062,20 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) >>> st->nb_frames= total_sample_count; >>> if (duration) >>> st->duration= FFMIN(st->duration, duration); >>> + >>> + // All samples have zero duration. They have higher chance be chose by >>> + // mov_find_next_sample, which leads to seek again and again. >>> + // >>> + // It's AVERROR_INVALIDDATA actually, but such files exist in the wild. >>> + // So only mark data stream as discarded for safety. >>> + if (!duration && sc->stts_count && >>> + st->codecpar->codec_type == AVMEDIA_TYPE_DATA) { >>> + av_log(c->fc, AV_LOG_WARNING, >>> + "All samples in data stream index:id [%d:%d] have zero duration, " >>> + "discard the stream\n", >>> + st->index, st->id); >>> + st->discard = AVDISCARD_ALL; >>> + } >>> sc->track_end = duration; >>> return 0; >>> } >> So this will allow audio and video streams to be demuxed, but not data? That distinction seems arbitrary. > Disable audio/video streams may create regression. It’s unlikely for random > and broken data stream. > >> Print a warning and assign a duration to each sample. Either 1 or if not zero/Inf, st->duration/st->nb_frames. > Set sample_duration to 1 doesn’t work. Dts still far behind other streams. > > Set sample_duration st->duration/st->nb_frames works for me, but I prefer > current strategy for the following reasons: > > 1. AVDISCARD_ALL is more close to AVERROR_INVALIDDATA by giving up instead > of trying correction and hope it works, which may not, e.g., st->duration > is broken, or bad interleave even though we fixed sample_duration. It's not about hoping that it works. It's about not preventing the user from acquiring the stream payload. Can you test if setting -discard:d none -i INPUT allows reading the stream with your patch? Regards, Gyan
> On Jul 5, 2022, at 10:33 PM, Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > On 2022-07-05 07:05 pm, "zhilizhao(赵志立)" wrote: >> >>> On Jul 5, 2022, at 8:07 PM, Gyan Doshi <ffmpeg@gyani.pro> wrote: >>> >>> >>> >>> On 2022-07-05 01:20 pm, Zhao Zhili wrote: >>>> From: Zhao Zhili <zhilizhao@tencent.com> >>>> >>>> Streams with all zero sample_delta in 'stts' have all zero dts. >>>> They have higher chance be chose by mov_find_next_sample(), which >>>> leads to seek again and again. >>>> >>>> For example, GoPro created a 'GoPro SOS' stream: >>>> Stream #0:4[0x5](eng): Data: none (fdsc / 0x63736466), 13 kb/s (default) >>>> Metadata: >>>> creation_time : 2022-06-21T08:49:19.000000Z >>>> handler_name : GoPro SOS >>>> >>>> With 'ffprobe -show_frames http://example.com/gopro.mp4', ffprobe >>>> blocks until all samples in 'GoPro SOS' stream are consumed first. >>>> >>>> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> >>>> --- >>>> libavformat/mov.c | 14 ++++++++++++++ >>>> 1 file changed, 14 insertions(+) >>>> >>>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>>> index 88669faa70..2a4eb79f27 100644 >>>> --- a/libavformat/mov.c >>>> +++ b/libavformat/mov.c >>>> @@ -3062,6 +3062,20 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) >>>> st->nb_frames= total_sample_count; >>>> if (duration) >>>> st->duration= FFMIN(st->duration, duration); >>>> + >>>> + // All samples have zero duration. They have higher chance be chose by >>>> + // mov_find_next_sample, which leads to seek again and again. >>>> + // >>>> + // It's AVERROR_INVALIDDATA actually, but such files exist in the wild. >>>> + // So only mark data stream as discarded for safety. >>>> + if (!duration && sc->stts_count && >>>> + st->codecpar->codec_type == AVMEDIA_TYPE_DATA) { >>>> + av_log(c->fc, AV_LOG_WARNING, >>>> + "All samples in data stream index:id [%d:%d] have zero duration, " >>>> + "discard the stream\n", >>>> + st->index, st->id); >>>> + st->discard = AVDISCARD_ALL; >>>> + } >>>> sc->track_end = duration; >>>> return 0; >>>> } >>> So this will allow audio and video streams to be demuxed, but not data? That distinction seems arbitrary. >> Disable audio/video streams may create regression. It’s unlikely for random >> and broken data stream. >> >>> Print a warning and assign a duration to each sample. Either 1 or if not zero/Inf, st->duration/st->nb_frames. >> Set sample_duration to 1 doesn’t work. Dts still far behind other streams. >> >> Set sample_duration st->duration/st->nb_frames works for me, but I prefer >> current strategy for the following reasons: >> >> 1. AVDISCARD_ALL is more close to AVERROR_INVALIDDATA by giving up instead >> of trying correction and hope it works, which may not, e.g., st->duration >> is broken, or bad interleave even though we fixed sample_duration. > > It's not about hoping that it works. It's about not preventing the user from acquiring the stream payload. > > Can you test if setting -discard:d none -i INPUT allows reading the stream with your patch? Yes it does allow reading the stream. ’stts’ box is parsed during avformat_find_stream_info(), AVStream->discard flag can be modified after that. The patch has no effect if user changed AVStream->discard flag. > > Regards, > Gyan > > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 2022-07-06 08:23 am, "zhilizhao(赵志立)" wrote: > >> On Jul 5, 2022, at 10:33 PM, Gyan Doshi <ffmpeg@gyani.pro> wrote: >> >> >> >> On 2022-07-05 07:05 pm, "zhilizhao(赵志立)" wrote: >>>> On Jul 5, 2022, at 8:07 PM, Gyan Doshi <ffmpeg@gyani.pro> wrote: >>>> >>>> >>>> >>>> On 2022-07-05 01:20 pm, Zhao Zhili wrote: >>>>> From: Zhao Zhili <zhilizhao@tencent.com> >>>>> >>>>> Streams with all zero sample_delta in 'stts' have all zero dts. >>>>> They have higher chance be chose by mov_find_next_sample(), which >>>>> leads to seek again and again. >>>>> >>>>> For example, GoPro created a 'GoPro SOS' stream: >>>>> Stream #0:4[0x5](eng): Data: none (fdsc / 0x63736466), 13 kb/s (default) >>>>> Metadata: >>>>> creation_time : 2022-06-21T08:49:19.000000Z >>>>> handler_name : GoPro SOS >>>>> >>>>> With 'ffprobe -show_frames http://example.com/gopro.mp4', ffprobe >>>>> blocks until all samples in 'GoPro SOS' stream are consumed first. >>>>> >>>>> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> >>>>> --- >>>>> libavformat/mov.c | 14 ++++++++++++++ >>>>> 1 file changed, 14 insertions(+) >>>>> >>>>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>>>> index 88669faa70..2a4eb79f27 100644 >>>>> --- a/libavformat/mov.c >>>>> +++ b/libavformat/mov.c >>>>> @@ -3062,6 +3062,20 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) >>>>> st->nb_frames= total_sample_count; >>>>> if (duration) >>>>> st->duration= FFMIN(st->duration, duration); >>>>> + >>>>> + // All samples have zero duration. They have higher chance be chose by >>>>> + // mov_find_next_sample, which leads to seek again and again. >>>>> + // >>>>> + // It's AVERROR_INVALIDDATA actually, but such files exist in the wild. >>>>> + // So only mark data stream as discarded for safety. >>>>> + if (!duration && sc->stts_count && >>>>> + st->codecpar->codec_type == AVMEDIA_TYPE_DATA) { >>>>> + av_log(c->fc, AV_LOG_WARNING, >>>>> + "All samples in data stream index:id [%d:%d] have zero duration, " >>>>> + "discard the stream\n", >>>>> + st->index, st->id); >>>>> + st->discard = AVDISCARD_ALL; >>>>> + } >>>>> sc->track_end = duration; >>>>> return 0; >>>>> } >>>> So this will allow audio and video streams to be demuxed, but not data? That distinction seems arbitrary. >>> Disable audio/video streams may create regression. It’s unlikely for random >>> and broken data stream. >>> >>>> Print a warning and assign a duration to each sample. Either 1 or if not zero/Inf, st->duration/st->nb_frames. >>> Set sample_duration to 1 doesn’t work. Dts still far behind other streams. >>> >>> Set sample_duration st->duration/st->nb_frames works for me, but I prefer >>> current strategy for the following reasons: >>> >>> 1. AVDISCARD_ALL is more close to AVERROR_INVALIDDATA by giving up instead >>> of trying correction and hope it works, which may not, e.g., st->duration >>> is broken, or bad interleave even though we fixed sample_duration. >> It's not about hoping that it works. It's about not preventing the user from acquiring the stream payload. >> >> Can you test if setting -discard:d none -i INPUT allows reading the stream with your patch? > Yes it does allow reading the stream. ’stts’ box is parsed during > avformat_find_stream_info(), AVStream->discard flag can be modified > after that. The patch has no effect if user changed AVStream->discard > flag. What's the duration of the demuxed stream? Regards, Gyan
> On Jul 6, 2022, at 12:09 PM, Gyan Doshi <ffmpeg@gyani.pro> wrote: > > > > On 2022-07-06 08:23 am, "zhilizhao(赵志立)" wrote: >> >>> On Jul 5, 2022, at 10:33 PM, Gyan Doshi <ffmpeg@gyani.pro> wrote: >>> >>> >>> >>> On 2022-07-05 07:05 pm, "zhilizhao(赵志立)" wrote: >>>>> On Jul 5, 2022, at 8:07 PM, Gyan Doshi <ffmpeg@gyani.pro> wrote: >>>>> >>>>> >>>>> >>>>> On 2022-07-05 01:20 pm, Zhao Zhili wrote: >>>>>> From: Zhao Zhili <zhilizhao@tencent.com> >>>>>> >>>>>> Streams with all zero sample_delta in 'stts' have all zero dts. >>>>>> They have higher chance be chose by mov_find_next_sample(), which >>>>>> leads to seek again and again. >>>>>> >>>>>> For example, GoPro created a 'GoPro SOS' stream: >>>>>> Stream #0:4[0x5](eng): Data: none (fdsc / 0x63736466), 13 kb/s (default) >>>>>> Metadata: >>>>>> creation_time : 2022-06-21T08:49:19.000000Z >>>>>> handler_name : GoPro SOS >>>>>> >>>>>> With 'ffprobe -show_frames http://example.com/gopro.mp4', ffprobe >>>>>> blocks until all samples in 'GoPro SOS' stream are consumed first. >>>>>> >>>>>> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> >>>>>> --- >>>>>> libavformat/mov.c | 14 ++++++++++++++ >>>>>> 1 file changed, 14 insertions(+) >>>>>> >>>>>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>>>>> index 88669faa70..2a4eb79f27 100644 >>>>>> --- a/libavformat/mov.c >>>>>> +++ b/libavformat/mov.c >>>>>> @@ -3062,6 +3062,20 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) >>>>>> st->nb_frames= total_sample_count; >>>>>> if (duration) >>>>>> st->duration= FFMIN(st->duration, duration); >>>>>> + >>>>>> + // All samples have zero duration. They have higher chance be chose by >>>>>> + // mov_find_next_sample, which leads to seek again and again. >>>>>> + // >>>>>> + // It's AVERROR_INVALIDDATA actually, but such files exist in the wild. >>>>>> + // So only mark data stream as discarded for safety. >>>>>> + if (!duration && sc->stts_count && >>>>>> + st->codecpar->codec_type == AVMEDIA_TYPE_DATA) { >>>>>> + av_log(c->fc, AV_LOG_WARNING, >>>>>> + "All samples in data stream index:id [%d:%d] have zero duration, " >>>>>> + "discard the stream\n", >>>>>> + st->index, st->id); >>>>>> + st->discard = AVDISCARD_ALL; >>>>>> + } >>>>>> sc->track_end = duration; >>>>>> return 0; >>>>>> } >>>>> So this will allow audio and video streams to be demuxed, but not data? That distinction seems arbitrary. >>>> Disable audio/video streams may create regression. It’s unlikely for random >>>> and broken data stream. >>>> >>>>> Print a warning and assign a duration to each sample. Either 1 or if not zero/Inf, st->duration/st->nb_frames. >>>> Set sample_duration to 1 doesn’t work. Dts still far behind other streams. >>>> >>>> Set sample_duration st->duration/st->nb_frames works for me, but I prefer >>>> current strategy for the following reasons: >>>> >>>> 1. AVDISCARD_ALL is more close to AVERROR_INVALIDDATA by giving up instead >>>> of trying correction and hope it works, which may not, e.g., st->duration >>>> is broken, or bad interleave even though we fixed sample_duration. >>> It's not about hoping that it works. It's about not preventing the user from acquiring the stream payload. >>> >>> Can you test if setting -discard:d none -i INPUT allows reading the stream with your patch? >> Yes it does allow reading the stream. ’stts’ box is parsed during >> avformat_find_stream_info(), AVStream->discard flag can be modified >> after that. The patch has no effect if user changed AVStream->discard >> flag. > > What's the duration of the demuxed stream? The demuxed data track has correct duration since there is a check ``` if (duration) st->duration= FFMIN(st->duration, duration); ``` st->duration comes from ‘mdhd’ and not overwrite in this case. Every packet has zero as timestamp, as expected: ./ffmpeg -debug_ts -discard:d none -i ~/tmp/gopro.mp4 -map 0:4 -c copy -copy_unknown -f data /tmp/test demuxer -> ist_index:4 type:data next_dts:0 next_dts_time:0 next_pts:0 next_pts_time:0 pkt_pts:0 pkt_pts_time:0 pkt_dts:0 pkt_dts_time:0 duration:0 duration_time:0 off:0 off_time:0 demuxer+ffmpeg -> ist_index:4 type:data pkt_pts:0 pkt_pts_time:0 pkt_dts:0 pkt_dts_time:0 duration:0 duration_time:0 off:0 off_time:0 muxer <- type:data pkt_pts:0 pkt_pts_time:0 pkt_dts:0 pkt_dts_time:0 duration:0 duration_time:0 size:16 > > Regards, > Gyan > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > https://ffmpeg.org/mailman/listinfo/ffmpeg-devel > > To unsubscribe, visit link above, or email > ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
On 2022-07-06 10:54 am, "zhilizhao(赵志立)" wrote: > >> On Jul 6, 2022, at 12:09 PM, Gyan Doshi <ffmpeg@gyani.pro> wrote: >> >> >> >> On 2022-07-06 08:23 am, "zhilizhao(赵志立)" wrote: >>>> On Jul 5, 2022, at 10:33 PM, Gyan Doshi <ffmpeg@gyani.pro> wrote: >>>> >>>> >>>> >>>> On 2022-07-05 07:05 pm, "zhilizhao(赵志立)" wrote: >>>>>> On Jul 5, 2022, at 8:07 PM, Gyan Doshi <ffmpeg@gyani.pro> wrote: >>>>>> >>>>>> >>>>>> >>>>>> On 2022-07-05 01:20 pm, Zhao Zhili wrote: >>>>>>> From: Zhao Zhili <zhilizhao@tencent.com> >>>>>>> >>>>>>> Streams with all zero sample_delta in 'stts' have all zero dts. >>>>>>> They have higher chance be chose by mov_find_next_sample(), which >>>>>>> leads to seek again and again. >>>>>>> >>>>>>> For example, GoPro created a 'GoPro SOS' stream: >>>>>>> Stream #0:4[0x5](eng): Data: none (fdsc / 0x63736466), 13 kb/s (default) >>>>>>> Metadata: >>>>>>> creation_time : 2022-06-21T08:49:19.000000Z >>>>>>> handler_name : GoPro SOS >>>>>>> >>>>>>> With 'ffprobe -show_frames http://example.com/gopro.mp4', ffprobe >>>>>>> blocks until all samples in 'GoPro SOS' stream are consumed first. >>>>>>> >>>>>>> Signed-off-by: Zhao Zhili <zhilizhao@tencent.com> >>>>>>> --- >>>>>>> libavformat/mov.c | 14 ++++++++++++++ >>>>>>> 1 file changed, 14 insertions(+) >>>>>>> >>>>>>> diff --git a/libavformat/mov.c b/libavformat/mov.c >>>>>>> index 88669faa70..2a4eb79f27 100644 >>>>>>> --- a/libavformat/mov.c >>>>>>> +++ b/libavformat/mov.c >>>>>>> @@ -3062,6 +3062,20 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) >>>>>>> st->nb_frames= total_sample_count; >>>>>>> if (duration) >>>>>>> st->duration= FFMIN(st->duration, duration); >>>>>>> + >>>>>>> + // All samples have zero duration. They have higher chance be chose by >>>>>>> + // mov_find_next_sample, which leads to seek again and again. >>>>>>> + // >>>>>>> + // It's AVERROR_INVALIDDATA actually, but such files exist in the wild. >>>>>>> + // So only mark data stream as discarded for safety. >>>>>>> + if (!duration && sc->stts_count && >>>>>>> + st->codecpar->codec_type == AVMEDIA_TYPE_DATA) { >>>>>>> + av_log(c->fc, AV_LOG_WARNING, >>>>>>> + "All samples in data stream index:id [%d:%d] have zero duration, " >>>>>>> + "discard the stream\n", >>>>>>> + st->index, st->id); >>>>>>> + st->discard = AVDISCARD_ALL; >>>>>>> + } >>>>>>> sc->track_end = duration; >>>>>>> return 0; >>>>>>> } >>>>>> So this will allow audio and video streams to be demuxed, but not data? That distinction seems arbitrary. >>>>> Disable audio/video streams may create regression. It’s unlikely for random >>>>> and broken data stream. >>>>> >>>>>> Print a warning and assign a duration to each sample. Either 1 or if not zero/Inf, st->duration/st->nb_frames. >>>>> Set sample_duration to 1 doesn’t work. Dts still far behind other streams. >>>>> >>>>> Set sample_duration st->duration/st->nb_frames works for me, but I prefer >>>>> current strategy for the following reasons: >>>>> >>>>> 1. AVDISCARD_ALL is more close to AVERROR_INVALIDDATA by giving up instead >>>>> of trying correction and hope it works, which may not, e.g., st->duration >>>>> is broken, or bad interleave even though we fixed sample_duration. >>>> It's not about hoping that it works. It's about not preventing the user from acquiring the stream payload. >>>> >>>> Can you test if setting -discard:d none -i INPUT allows reading the stream with your patch? >>> Yes it does allow reading the stream. ’stts’ box is parsed during >>> avformat_find_stream_info(), AVStream->discard flag can be modified >>> after that. The patch has no effect if user changed AVStream->discard >>> flag. >> What's the duration of the demuxed stream? > The demuxed data track has correct duration since there is a check > ``` > if (duration) > st->duration= FFMIN(st->duration, duration); > ``` > st->duration comes from ‘mdhd’ and not overwrite in this case. > > Every packet has zero as timestamp, as expected: > > ./ffmpeg -debug_ts -discard:d none -i ~/tmp/gopro.mp4 -map 0:4 -c copy -copy_unknown -f data /tmp/test > > demuxer -> ist_index:4 type:data next_dts:0 next_dts_time:0 next_pts:0 next_pts_time:0 pkt_pts:0 pkt_pts_time:0 pkt_dts:0 pkt_dts_time:0 duration:0 duration_time:0 off:0 off_time:0 > demuxer+ffmpeg -> ist_index:4 type:data pkt_pts:0 pkt_pts_time:0 pkt_dts:0 pkt_dts_time:0 duration:0 duration_time:0 off:0 off_time:0 > muxer <- type:data pkt_pts:0 pkt_pts_time:0 pkt_dts:0 pkt_dts_time:0 duration:0 duration_time:0 size:16 Ok, change the log from "discard the stream" to "stream set to be discarded. Override using -discard or AVStream->discard" Regards, Gyan
diff --git a/libavformat/mov.c b/libavformat/mov.c index 88669faa70..2a4eb79f27 100644 --- a/libavformat/mov.c +++ b/libavformat/mov.c @@ -3062,6 +3062,20 @@ static int mov_read_stts(MOVContext *c, AVIOContext *pb, MOVAtom atom) st->nb_frames= total_sample_count; if (duration) st->duration= FFMIN(st->duration, duration); + + // All samples have zero duration. They have higher chance be chose by + // mov_find_next_sample, which leads to seek again and again. + // + // It's AVERROR_INVALIDDATA actually, but such files exist in the wild. + // So only mark data stream as discarded for safety. + if (!duration && sc->stts_count && + st->codecpar->codec_type == AVMEDIA_TYPE_DATA) { + av_log(c->fc, AV_LOG_WARNING, + "All samples in data stream index:id [%d:%d] have zero duration, " + "discard the stream\n", + st->index, st->id); + st->discard = AVDISCARD_ALL; + } sc->track_end = duration; return 0; }