[FFmpeg-devel] [PATCH 2/7] lavc/pngdec: perform APNG blending in-place

[Michael Niedermayer]

On Sat, Apr 03, 2021 at 09:48:18AM +0200, Anton Khirnov wrote:
> Quoting James Almer (2021-04-02 16:54:47)
> > On 4/2/2021 11:40 AM, Anton Khirnov wrote:
> > > Saves an allocation+free and two frame copies per each frame.
> > > ---
> > >   libavcodec/pngdec.c | 51 +++++++++++++++++++++++++--------------------
> > >   1 file changed, 28 insertions(+), 23 deletions(-)
> > > 
> > > diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c
> > > index 63c22063d9..095e4e86c2 100644
> > > --- a/libavcodec/pngdec.c
> > > +++ b/libavcodec/pngdec.c
> > > @@ -1068,8 +1068,12 @@ static void handle_p_frame_png(PNGDecContext *s, AVFrame *p)
> > >   static int handle_p_frame_apng(AVCodecContext *avctx, PNGDecContext *s,
> > >                                  AVFrame *p)
> > >   {
> > > +    uint8_t       *dst        = p->data[0];
> > > +    ptrdiff_t      dst_stride = p->linesize[0];
> > > +    const uint8_t *src        = s->last_picture.f->data[0];
> > > +    ptrdiff_t      src_stride = s->last_picture.f->linesize[0];
> > > +
> > >       size_t x, y;
> > > -    uint8_t *buffer;
> > >   
> > >       if (s->blend_op == APNG_BLEND_OP_OVER &&
> > >           avctx->pix_fmt != AV_PIX_FMT_RGBA &&
> > > @@ -1089,26 +1093,32 @@ static int handle_p_frame_apng(AVCodecContext *avctx, PNGDecContext *s,
> > >           if (ret < 0)
> > >               return ret;
> > >   
> > > +        src        = s->last_picture.f->data[0];
> > > +        src_stride = s->last_picture.f->linesize[0];
> > 
> > Is calling av_frame_make_writable(s->last_picture.f) valid at all? 
> > Shouldn't the frame's buffer be freed with ff_thread_release_buffer()? 
> > Especially if a non threading safe get_buffer2() callback was used.
> > 
> > Considering you can't call ff_thread_get_buffer() at this point to get a 
> > new writable buffer to av_frame_copy() to, since this is long after 
> > ff_thread_finish_setup() was called, not sure what else could be done.
> 
> You're right, I guess making an unconditional copy local to this
> function is the only way to make it compatible with thread safe
> callbacks.
> 
> Will send a new patch.

Do you want this in the 4.4 release ?
iam asking as most other things are resolved ATM (might of course change
and more release blockers might appear)

thx

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

I know you won't believe me, but the highest form of Human Excellence is
to question oneself and others. -- Socrates
-------------- 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/20210407/dc7aa9fb/attachment.sig>