Re: [FFmpeg-devel] [PATCH] avformat/mov: The iloc test is not redundant

[Anton Khirnov <anton@khirnov.net>]

Quoting Michael Niedermayer (2023-10-19 20:53:05)
> On Thu, Oct 19, 2023 at 07:42:30PM +0200, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2023-10-19 18:33:13)
> > > On Thu, Oct 19, 2023 at 01:10:18PM +0200, Anton Khirnov wrote:
> > > > Quoting Michael Niedermayer (2023-10-15 02:13:23)
> > > > > Fixes: Assertion failure
> > > > > Fixes: 62866/clusterfuzz-testcase-minimized-ffmpeg_dem_MOV_fuzzer-5282997370486784
> > > > > 
> > > > > Found-by: continuous fuzzing process https://github.com/google/oss-fuzz/tree/master/projects/ffmpeg
> > > > > Signed-off-by: Michael Niedermayer <michael@niedermayer.cc>
> > > > > ---
> > > > 
> > > > The commit message is useless.
> > > 
> > > This comment is also not that usefull
> > > What would you like to see in the commit message ?
> > > 
> > > The 2 checks are not redundant. Should the message detail how
> > > the assertion failure occured ?
> > 
> > At least two people previously thought that the condition is redundant,
> > so it seems clear to me that an explanation is in order.
> > 
> > I actually find it quite baffling that this is not obvious to you. Do
> > you really think that "Fixes: Assertion failure" is sufficient
> > explanation for anyone reading this patch?
> 
> let me ask this from the other direction (and i should probably have done
> so sooner)
> 
> why would this be redundant ?
> 
> the failed check checks the number of streams, why should a random atom
> not occur after x streams for thf irst time ?
> what code was supposed to prevent this ?

The intent seems to be that for is_still_picture_avif=1 there should
only be one stream, created in avif_add_stream(), called after
mov_read_iloc(). Since avif_add_stream() will fail if any streams
already exist, or when mov_read_iloc() has not been called (since
avif_info_size will be 0), I'd say the correct thing to do is
fail at the very top of mov_read_trak() when is_still_picture_avif=1, so
no streams can be created for such files outside of avif_add_stream().
This should fix the assertion failure.

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