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

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

[Zhao Zhili]

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;
 }
-- 
2.35.3

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

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

[Gyan Doshi]

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

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

["zhilizhao(赵志立)"]

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

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

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

[Gyan Doshi]

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

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

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

["zhilizhao(赵志立)"]

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

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

[Gyan Doshi]

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

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

["zhilizhao(赵志立)"]

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

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

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

[Gyan Doshi]

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
_______________________________________________
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] [PATCH v2] avformat/mov: discard data streams with all zero sample_delta

[Zhao Zhili]

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>
---
v2: suggest how to override discard flag in log message.

 libavformat/mov.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 88669faa70..a3e4f05481 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3062,6 +3062,21 @@ 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, stream set to be discarded by default. Override "
+               "using AVStream->discard or -discard for ffmpeg\n",
+               st->index, st->id);
+        st->discard = AVDISCARD_ALL;
+    }
     sc->track_end = duration;
     return 0;
 }
-- 
2.35.3

_______________________________________________
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] [PATCH v3] avformat/mov: discard data streams with all zero sample_delta

[Zhao Zhili]

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>
---
v3: 'for ffmpeg' -> 'for ffmpeg command.'
v2: suggest how to override discard flag in log message.
 libavformat/mov.c | 15 +++++++++++++++
 1 file changed, 15 insertions(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 88669faa70..9a24e5effa 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -3062,6 +3062,21 @@ 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, stream set to be discarded by default. Override "
+               "using AVStream->discard or -discard for ffmpeg command.\n",
+               st->index, st->id);
+        st->discard = AVDISCARD_ALL;
+    }
     sc->track_end = duration;
     return 0;
 }
-- 
2.35.3

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

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

["zhilizhao(赵志立)"]

> On Jul 6, 2022, at 3:05 PM, Zhao Zhili <quinkblack@foxmail.com> 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>
> ---
> v3: 'for ffmpeg' -> 'for ffmpeg command.'
> v2: suggest how to override discard flag in log message.
> libavformat/mov.c | 15 +++++++++++++++
> 1 file changed, 15 insertions(+)
> 
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 88669faa70..9a24e5effa 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -3062,6 +3062,21 @@ 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, stream set to be discarded by default. Override "
> +               "using AVStream->discard or -discard for ffmpeg command.\n",
> +               st->index, st->id);
> +        st->discard = AVDISCARD_ALL;
> +    }
>     sc->track_end = duration;
>     return 0;
> }

Will apply tomorrow if no more comments.

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