[FFmpeg-devel] [PATCH 1/8] avcodec/cinepak: Require 1 bit per 4x4 block as minimum input

Michael Niedermayer michael at niedermayer.cc
Sun Aug 18 20:04:48 EEST 2019


On Sun, Aug 18, 2019 at 02:49:39PM +0200, Tomas Härdin wrote:
> sön 2019-08-18 klockan 14:18 +0200 skrev Michael Niedermayer:
> > On Sun, Aug 18, 2019 at 01:40:01PM +0200, Tomas Härdin wrote:
> > > sön 2019-08-18 klockan 12:19 +0200 skrev Michael Niedermayer:
> > > > On Sun, Aug 18, 2019 at 12:00:45PM +0200, Paul B Mahol wrote:
> > > > > On Sun, Aug 18, 2019 at 11:44 AM Michael Niedermayer <michael at niedermayer.cc>
> > > > > > and yes i too wish there was a magic fix but i think most things that
> > > > > > look like magic fixes have a fatal flaw. But maybe iam missing something
> > > > > > in fact i hope that iam missing something and that there is a magic fix
> > > > > > 
> > > > > 
> > > > > Magic fix is enabling reference counted frames in fuzzer.
> > > > 
> > > > That is covered by the part below which you maybe did not read
> > > > 
> > > > > > PS: if you think of changing the API, i dont think its the API.
> > > > > > I mean every user application will read the frames it receives, so
> > > > > > even if inside the decoder we just return the same frame with 2 pixels
> > > > > > different the user doesnt know this and has to read the whole frame.
> > > > > > The problem is moved around but its still there.
> > > 
> > > Copying is still slower than not copying. Enabling refcounting fixes
> > > the timeout issue here, and will likely silence a whole bunch of false
> > > positives for this class of files.
> > 
> > it makes probably sense to enable ref counting but we should
> > immedeatly in the next or a previous commit make the fuzzer read the frames
> > from the  decoder. Thats what basically every user app would do.
> > Otherwise we would have a bunch of issues closed and then reopened
> > later.
> 
> Why should we care how much work the user does? We're fuzzing lavc
> here, not user programs. Certain use cases are decode-only (ffprobe
> -show_frames for example)

thats a valid point of view

The user though has few options if she gets many frames, she can just
continue processing or stop, she doesnt know how (in)valid the stream is

OTOH libavcodec knows the codec bitstream, and can check various things

so just moving the responsibility to the user app would move it where
its much harder to fix well
That is currently, we could export all kinds of metadata about the
amount of errors. Then a user app could decide what to do.
It would become duplicated code between user apps though...


> 
> > an alternative viewpoint to this would be to set the refcounting flag
> > from the input so the fuzzer itself has control over it and we test
> > both codepathes. This would improve coverage
> 
> Not a bad idea. But as this example shows, not refcounting has
> performance implications. The fuzzer should be more lenient with
> timeout in that case, imo.

this is possible, yes

Thanks

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

When you are offended at any man's fault, turn to yourself and study your
own failings. Then you will forget your anger. -- Epictetus
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20190818/bd2505fa/attachment.sig>


More information about the ffmpeg-devel mailing list