| Message ID | 20221211115440.9942-1-cus@passwd.hu |
|---|---|
| State | New |
| Headers | show |
| Series | None | expand |
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
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.
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
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;
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(-)