[FFmpeg-devel] [PATCH 1/2] avformat/mov: do not emit zero sized packets

[FFmpeg-devel] [PATCH 1/2] avformat/mov: do not emit zero sized packets

[Marton Balint]

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mov.c | 4 ++++
 1 file changed, 4 insertions(+)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 29bd3103e3..935b2f8d9f 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -8772,6 +8772,10 @@ static int mov_read_packet(AVFormatContext *s, AVPacket *pkt)
             goto retry;
         }
 
+        /* Empty packet or corrupt index? */
+        if (!sample->size)
+            goto retry;
+
         if (st->codecpar->codec_id == AV_CODEC_ID_EIA_608 && sample->size > 8)
             ret = get_eia608_packet(sc->pb, pkt, sample->size);
         else
-- 
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 2/2] avformat/mov: re-allow zero sample sizes if that is not the default

[Marton Balint]

Patch 03d81a044ad587ea83567f75dc36bc3d64278199 disallowed zero sample sizes,
but there are some files in the wild which have zero sized samples (e.g.
no audio in some part of a live recording).

Fix this by only disallowing zero sized samples if the size is coming from the
default sample size and not from the trun box. This approach fixes the original
timeout issue from fuzzed files differently.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mov.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 935b2f8d9f..9d3a2ab830 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -5230,6 +5230,9 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     if (index_entry_pos > 0)
         prev_dts = sti->index_entries[index_entry_pos-1].timestamp;
 
+    if (entries && !frag->size && !(flags & MOV_TRUN_SAMPLE_SIZE))
+        return AVERROR_INVALIDDATA;
+
     for (i = 0; i < entries && !pb->eof_reached; i++) {
         unsigned sample_size = frag->size;
         int sample_flags = i ? frag->flags : first_sample_flags;
@@ -5293,8 +5296,6 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         distance++;
         if (av_sat_add64(dts, sample_duration) != dts + (uint64_t)sample_duration)
             return AVERROR_INVALIDDATA;
-        if (!sample_size)
-            return AVERROR_INVALIDDATA;
         dts += sample_duration;
         offset += sample_size;
         sc->data_size += sample_size;
-- 
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 v2 2/2] avformat/mov: re-allow zero sample sizes if that is not the default

[Marton Balint]

Patch 03d81a044ad587ea83567f75dc36bc3d64278199 disallowed zero sample sizes,
but there are some files in the wild which have zero sized samples (e.g.
no audio in some part of a live recording).

Fix this by simply ignoring a trun box with zero sized samples. This approach
fixes the original timeout issue from fuzzed files differently.

Signed-off-by: Marton Balint <cus@passwd.hu>
---
 libavformat/mov.c | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/libavformat/mov.c b/libavformat/mov.c
index 935b2f8d9f..63e0b614df 100644
--- a/libavformat/mov.c
+++ b/libavformat/mov.c
@@ -5121,6 +5121,11 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
     if (flags & MOV_TRUN_DATA_OFFSET)        data_offset        = avio_rb32(pb);
     if (flags & MOV_TRUN_FIRST_SAMPLE_FLAGS) first_sample_flags = avio_rb32(pb);
 
+    if (entries && !frag->size && !(flags & MOV_TRUN_SAMPLE_SIZE)) {
+        av_log(c->fc, AV_LOG_WARNING, "Ignoring trun box with zero sized samples\n");
+        entries = 0;
+    }
+
     frag_stream_info = get_current_frag_stream_info(&c->frag_index);
     if (frag_stream_info) {
         if (frag_stream_info->next_trun_dts != AV_NOPTS_VALUE) {
@@ -5293,8 +5298,6 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
         distance++;
         if (av_sat_add64(dts, sample_duration) != dts + (uint64_t)sample_duration)
             return AVERROR_INVALIDDATA;
-        if (!sample_size)
-            return AVERROR_INVALIDDATA;
         dts += sample_duration;
         offset += sample_size;
         sc->data_size += sample_size;
-- 
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 v2 2/2] avformat/mov: re-allow zero sample sizes if that is not the default

[Chris Ribble]

On Sun, Dec 11, 2022 at 5:55 AM Marton Balint <cus@passwd.hu> wrote:
>
> Patch 03d81a044ad587ea83567f75dc36bc3d64278199 disallowed zero sample sizes,
> but there are some files in the wild which have zero sized samples (e.g.
> no audio in some part of a live recording).
>
> Fix this by simply ignoring a trun box with zero sized samples. This approach
> fixes the original timeout issue from fuzzed files differently.
>
> Signed-off-by: Marton Balint <cus@passwd.hu>
> ---
>  libavformat/mov.c | 7 +++++--
>  1 file changed, 5 insertions(+), 2 deletions(-)
>
> diff --git a/libavformat/mov.c b/libavformat/mov.c
> index 935b2f8d9f..63e0b614df 100644
> --- a/libavformat/mov.c
> +++ b/libavformat/mov.c
> @@ -5121,6 +5121,11 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>      if (flags & MOV_TRUN_DATA_OFFSET)        data_offset        = avio_rb32(pb);
>      if (flags & MOV_TRUN_FIRST_SAMPLE_FLAGS) first_sample_flags = avio_rb32(pb);
>
> +    if (entries && !frag->size && !(flags & MOV_TRUN_SAMPLE_SIZE)) {
> +        av_log(c->fc, AV_LOG_WARNING, "Ignoring trun box with zero sized samples\n");
> +        entries = 0;
> +    }
> +
>      frag_stream_info = get_current_frag_stream_info(&c->frag_index);
>      if (frag_stream_info) {
>          if (frag_stream_info->next_trun_dts != AV_NOPTS_VALUE) {
> @@ -5293,8 +5298,6 @@ static int mov_read_trun(MOVContext *c, AVIOContext *pb, MOVAtom atom)
>          distance++;
>          if (av_sat_add64(dts, sample_duration) != dts + (uint64_t)sample_duration)
>              return AVERROR_INVALIDDATA;
> -        if (!sample_size)
> -            return AVERROR_INVALIDDATA;
>          dts += sample_duration;
>          offset += sample_size;
>          sc->data_size += sample_size;
> --
> 2.35.3

Marton,

This looks great and I've confirmed it fixes the issue. I see the
warning on STDERR about ignoring the trun box: "Ignoring trun box with
zero sized samples", which is great.

Thanks for doing this!

Assuming this is applied to master, would it also be reasonable to
apply it to the 5.1 branch?

-Chris
_______________________________________________
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 v2 2/2] avformat/mov: re-allow zero sample sizes if that is not the default

[Chris Ribble]

On Sun, Dec 11, 2022 at 2:34 PM Chris Ribble <chris.ribble@resi.io> wrote:
>
> This looks great and I've confirmed it fixes the issue. I see the
> warning on STDERR about ignoring the trun box: "Ignoring trun box with
> zero sized samples", which is great.
>

Actually, this introduces a new issue when I try to transcode the
problematic mp4 and then segment it with the DASH muxer.

It appears that ignoring the trun box while reading the input affects
the segment alignment of the audio with the video.The durations jitter
around quite a bit and some of them are very long. The same problem
seems to exist whether I transcode or copy the audio.
_______________________________________________
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 v2 2/2] avformat/mov: re-allow zero sample sizes if that is not the default

[Marton Balint]

On Mon, 12 Dec 2022, Chris Ribble wrote:

> On Sun, Dec 11, 2022 at 2:34 PM Chris Ribble <chris.ribble@resi.io> wrote:
>>
>> This looks great and I've confirmed it fixes the issue. I see the
>> warning on STDERR about ignoring the trun box: "Ignoring trun box with
>> zero sized samples", which is great.
>>
>
> Actually, this introduces a new issue when I try to transcode the
> problematic mp4 and then segment it with the DASH muxer.
>
> It appears that ignoring the trun box while reading the input affects
> the segment alignment of the audio with the video.The durations jitter
> around quite a bit and some of them are very long. The same problem
> seems to exist whether I transcode or copy the audio.

Well, remuxing sparse audio packets certainly can cause issues. For a full 
transcode, I did not expect that much difference, for the audio gaps you 
should be able to generate silence with -af aresample=async=1 or similar.

Overall, I am not sure what to do here:

1) Simply revert original patch, and make it work same as before, and
    accept that 0-sized packets can be generated by the MOV demuxer, or
    fuzzed files can cause practically infinite loops (not that unusual for
    multimedia formats)

2) Apply this patchset which makes the MOV demuxer ignore the 0-sized
    packets and fixes the original fuzzing issue, but causing some strange
    issues when remuxing or transcoding because the audio packets become
    sparse.

3) Dig deeper what goes wrong with remuxing/reencoding with sparse audio.
    Unfortunately I won't have time to do this.

Comments are welcome.

Thanks,
Marton

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