[FFmpeg-devel] [PATCH] avcodec/snow: ensure current_picture is writable before modifying its data

[Michael Niedermayer]

On Fri, May 29, 2020 at 07:00:12PM -0300, James Almer wrote:
> On 5/29/2020 5:37 PM, Michael Niedermayer wrote:
> > On Fri, May 29, 2020 at 02:20:37PM -0300, James Almer wrote:
> > > current_picture was not writable here because a reference existed in
> > > at least avctx->coded_frame, and potentially elsewhere if the caller
> > > created new ones from it.
> > 
> > Please elaborate when and how the encoder internal buffer becomes
> > read only
> > i simply took your code and replaced
> > 
> > -        ret = av_frame_make_writable(s->current_picture);
> > -        if (ret < 0)
> > -            return ret;
> > +        ret = av_frame_is_writable(s->current_picture);
> > +        if (ret <= 0)
> > +            return -1;
> > 
> > and fate is fully happy which tests both the encoder and the filters
> > using it
> > also if this is for coded_frame then shouldnt it be under FF_API_CODED_FRAME?
> > iam clearly missing something here
> 
> You need to also move the av_frame_unref(avctx->coded_frame) line back to
> where it was before my patch. I moved it before this check to ensure at
> least the reference stored there is freed before current_picture is written
> to, 

i quite intentionally did not move that, my question was just about
"why av_frame_make_writable" after all the other changes


> but there of course could still exist other references created by the
> user, and that's what this make_writable() call is for.

ok, understand this. I guess my gut feeling was that creating references
to coded_frame was not allowed. but i guess there is nothing that forbids
it so teh API allowes it, and the av_frame_make_writable is ok
 
 
> 
> And since this specific chunk is not strictly coded_frame related, it
> doesn't need to be under that guard.

but coded_frame is the only current way by which a user can create a
reference, unless iam missing something
Am i guessing correctly that you intend to add another way to export
the frame or is there something iam missing ?
because without some other method this make_writable doesnt make sense
without coded_frame and should be removed in case coded_frame is removed

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Its not that you shouldnt use gotos but rather that you should write
readable code and code with gotos often but not always is less readable
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200530/04099094/attachment.sig>