On Tue, Mar 30, 2021 at 10:56:13AM +0200, Michael Niedermayer wrote:
> On Tue, Feb 16, 2021 at 09:24:15PM +0100, Anton Khirnov wrote:
> > Current code is very confused and confusing. It uses two different
> > reference frames - "previous" and "last" - when only one is really
> > necessary. It also confuses the two, leading to incorrect output with
> > APNG_DISPOSE_OP_PREVIOUS mode.
> >
> > Fixes #9017.
> > ---
> > libavcodec/pngdec.c | 93 ++++++++++++++++++++-------------------------
> > 1 file changed, 42 insertions(+), 51 deletions(-)
>
>
> [...]
>
> > @@ -1088,23 +1084,23 @@ static int handle_p_frame_apng(AVCodecContext
> > *avctx, PNGDecContext *s,
> > if (!buffer)
> > return AVERROR(ENOMEM);
> >
> > + ff_thread_await_progress(&s->last_picture, INT_MAX, 0);
> >
> > - // Do the disposal operation specified by the last frame on the frame
> > - if (s->last_dispose_op != APNG_DISPOSE_OP_PREVIOUS) {
> > - ff_thread_await_progress(&s->last_picture, INT_MAX, 0);
> > - memcpy(buffer, s->last_picture.f->data[0], s->image_linesize *
> > s->height);
> > -
> > - if (s->last_dispose_op == APNG_DISPOSE_OP_BACKGROUND)
> > - for (y = s->last_y_offset; y < s->last_y_offset + s->last_h;
> > ++y)
> > - memset(buffer + s->image_linesize * y + s->bpp *
> > s->last_x_offset, 0, s->bpp * s->last_w);
> > + // need to reset a rectangle to background:
> > + // create a new writable copy
> > + if (s->last_dispose_op == APNG_DISPOSE_OP_BACKGROUND) {
> > + int ret = av_frame_make_writable(s->last_picture.f);
> > + if (ret < 0)
> > + return ret;
> >
> > - memcpy(s->previous_picture.f->data[0], buffer, s->image_linesize *
> > s->height);
> > - ff_thread_report_progress(&s->previous_picture, INT_MAX, 0);
> > - } else {
> > - ff_thread_await_progress(&s->previous_picture, INT_MAX, 0);
> > - memcpy(buffer, s->previous_picture.f->data[0], s->image_linesize *
> > s->height);
> > + for (y = s->last_y_offset; y < s->last_y_offset + s->last_h; y++) {
> > + memset(s->last_picture.f->data[0] + s->image_linesize * y +
> > + s->bpp * s->last_x_offset, 0, s->bpp * s->last_w);
> > + }
> > }
> >
> > + memcpy(buffer, s->last_picture.f->data[0], s->image_linesize *
> > s->height);
> > +
>
> This results in out of array reads
>
> av_frame_make_writable() decreases linesize [0] but the memcpy() now
> av_memdup()
> assumes all frames have the same linesizeCCing anton, to make sure this is not missed in its old thread A fix to this also should be backported to release/4.4 Thanks [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB The bravest are surely those who have the clearest vision of what is before them, glory and danger alike, and yet notwithstanding go out to meet it. -- Thucydides
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
