Re: [FFmpeg-devel] [PATCH] avformat/mov: discard data streams with all zero sample_delta

["\"zhilizhao(赵志立)\"" <quinkblack@foxmail.com>]

> 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".

_______________________________________________
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".