[FFmpeg-devel] Re: [PATCH 2/7] lavc/pngdec: perform APNG blending in-place
Anton Khirnov
anton at khirnov.net
Wed Apr 7 22:37:39 EEST 2021
Quoting Michael Niedermayer (2021-04-07 21:21:39)
> 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)
This set fixes a number of major issues in pngdec, so all non-test
patches except for the last one definitely should be in the release.
I've been waiting for more comments/busy with other things, but will
probably push it tomorrow.
Or feel free to do it first.
--
Anton Khirnov
More information about the ffmpeg-devel
mailing list