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 linesize
Thanks
[...]
--
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
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".
