PR #24580 opened by Jun Zhao (mypopydev) URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24580 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24580.patch
APNG frames with less than 8 bits per sample were decoded packed at the frame's x_offset, which is a pixel and not a byte offset, shifting and truncating the frame. Unpack each row before moving it to its positionin the frame. The demuxer now also reports out of order fcTL/fdAT sequence numbers. >From ce8877ff2f8f65e2070ad18af254e759809faaae Mon Sep 17 00:00:00 2001 From: Jun Zhao <[email protected]> Date: Sat, 19 Sep 2026 20:29:52 +0800 Subject: [PATCH 1/2] avcodec/pngdec: unpack sub-8-bit APNG frames before applying the offset The offsets of an APNG frame are pixel offsets, but the rows of a frame using less than 8 bits per sample are decoded packed, so they cannot be placed at the frame offset: they end up shifted by (8 / bits_per_pixel - 1) * x_offset pixels, the pixels which no longer fit in the canvas are dropped, and the ones before the frame are taken from uninitialized memory. Decode such rows packed at the start of the frame row and unpack them one row at a time, moving every unpacked row to its position in the frame. Interlaced frames keep being decoded as before. See-Also: https://code.ffmpeg.org/FFmpeg/FFmpeg/issues/24553 Reported-by: Delgan <[email protected]> Signed-off-by: Jun Zhao <[email protected]> --- libavcodec/pngdec.c | 162 +++++++++++++++++++++++++++----------------- 1 file changed, 98 insertions(+), 64 deletions(-) diff --git a/libavcodec/pngdec.c b/libavcodec/pngdec.c index 3af36f47e1..f20cef76d9 100644 --- a/libavcodec/pngdec.c +++ b/libavcodec/pngdec.c @@ -126,6 +126,9 @@ typedef struct PNGDecContext { int row_size; /* decompressed row size */ int pass_row_size; /* decompress row size of the current pass */ int y; + /* sub-8-bit samples of an APNG frame are decoded packed at the start of + * the frame row and only moved to the frame position once unpacked */ + int subframe_packed; FFZStream zstream; AVBufferRef *exif_data; @@ -344,7 +347,8 @@ static void png_handle_row(PNGDecContext *s, uint8_t *dst, ptrdiff_t dst_stride) int got_line; if (!s->interlace_type) { - ptr = dst + dst_stride * (s->y + s->y_offset) + s->x_offset * s->bpp; + ptr = dst + dst_stride * (s->y + s->y_offset) + + (s->subframe_packed ? 0 : s->x_offset * s->bpp); if (s->y == 0) last_row = s->last_row; else @@ -917,6 +921,12 @@ static int decode_idat_chunk(AVCodecContext *avctx, PNGDecContext *s, s->bpp = (s->bits_per_pixel + 7) >> 3; s->row_size = (s->cur_w * s->bits_per_pixel + 7) >> 3; + /* Packed rows of sub-8-bit frames cannot be placed at the frame + * offset, as that one is a pixel and not a byte offset. */ + s->subframe_packed = CONFIG_APNG_DECODER && + avctx->codec_id == AV_CODEC_ID_APNG && + s->bits_per_pixel <= 4 && !s->interlace_type; + if ((s->bit_depth == 2 || s->bit_depth == 4 || s->bit_depth == 8) && s->color_type == PNG_COLOR_TYPE_RGB) { avctx->pix_fmt = AV_PIX_FMT_RGB24; @@ -1219,79 +1229,99 @@ static int decode_sbit_chunk(AVCodecContext *avctx, PNGDecContext *s, return 0; } -static void handle_small_bpp(PNGDecContext *s, AVFrame *p) +/* expand one row of packed sub-8-bit samples to one byte per sample, in place */ +static void unpack_small_row(PNGDecContext *s, uint8_t *pd, int width) { if (s->bits_per_pixel == 1 && s->color_type == PNG_COLOR_TYPE_PALETTE) { - int i, j, k; - uint8_t *pd = p->data[0]; - for (j = 0; j < s->height; j++) { - i = s->width / 8; - for (k = 7; k >= 1; k--) - if ((s->width&7) >= k) - pd[8*i + k - 1] = (pd[i]>>8-k) & 1; - for (i--; i >= 0; i--) { - pd[8*i + 7]= pd[i] & 1; - pd[8*i + 6]= (pd[i]>>1) & 1; - pd[8*i + 5]= (pd[i]>>2) & 1; - pd[8*i + 4]= (pd[i]>>3) & 1; - pd[8*i + 3]= (pd[i]>>4) & 1; - pd[8*i + 2]= (pd[i]>>5) & 1; - pd[8*i + 1]= (pd[i]>>6) & 1; - pd[8*i + 0]= pd[i]>>7; - } - pd += p->linesize[0]; + int i, k; + + i = width / 8; + for (k = 7; k >= 1; k--) + if ((width&7) >= k) + pd[8*i + k - 1] = (pd[i]>>8-k) & 1; + for (i--; i >= 0; i--) { + pd[8*i + 7]= pd[i] & 1; + pd[8*i + 6]= (pd[i]>>1) & 1; + pd[8*i + 5]= (pd[i]>>2) & 1; + pd[8*i + 4]= (pd[i]>>3) & 1; + pd[8*i + 3]= (pd[i]>>4) & 1; + pd[8*i + 2]= (pd[i]>>5) & 1; + pd[8*i + 1]= (pd[i]>>6) & 1; + pd[8*i + 0]= pd[i]>>7; } } else if (s->bits_per_pixel == 2) { - int i, j; - uint8_t *pd = p->data[0]; - for (j = 0; j < s->height; j++) { - i = s->width / 4; - if (s->color_type == PNG_COLOR_TYPE_PALETTE) { - if ((s->width&3) >= 3) pd[4*i + 2]= (pd[i] >> 2) & 3; - if ((s->width&3) >= 2) pd[4*i + 1]= (pd[i] >> 4) & 3; - if ((s->width&3) >= 1) pd[4*i + 0]= pd[i] >> 6; - for (i--; i >= 0; i--) { - pd[4*i + 3]= pd[i] & 3; - pd[4*i + 2]= (pd[i]>>2) & 3; - pd[4*i + 1]= (pd[i]>>4) & 3; - pd[4*i + 0]= pd[i]>>6; - } - } else { - if ((s->width&3) >= 3) pd[4*i + 2]= ((pd[i]>>2) & 3)*0x55; - if ((s->width&3) >= 2) pd[4*i + 1]= ((pd[i]>>4) & 3)*0x55; - if ((s->width&3) >= 1) pd[4*i + 0]= ( pd[i]>>6 )*0x55; - for (i--; i >= 0; i--) { - pd[4*i + 3]= ( pd[i] & 3)*0x55; - pd[4*i + 2]= ((pd[i]>>2) & 3)*0x55; - pd[4*i + 1]= ((pd[i]>>4) & 3)*0x55; - pd[4*i + 0]= ( pd[i]>>6 )*0x55; - } + int i; + + i = width / 4; + if (s->color_type == PNG_COLOR_TYPE_PALETTE) { + if ((width&3) >= 3) pd[4*i + 2]= (pd[i] >> 2) & 3; + if ((width&3) >= 2) pd[4*i + 1]= (pd[i] >> 4) & 3; + if ((width&3) >= 1) pd[4*i + 0]= pd[i] >> 6; + for (i--; i >= 0; i--) { + pd[4*i + 3]= pd[i] & 3; + pd[4*i + 2]= (pd[i]>>2) & 3; + pd[4*i + 1]= (pd[i]>>4) & 3; + pd[4*i + 0]= pd[i]>>6; + } + } else { + if ((width&3) >= 3) pd[4*i + 2]= ((pd[i]>>2) & 3)*0x55; + if ((width&3) >= 2) pd[4*i + 1]= ((pd[i]>>4) & 3)*0x55; + if ((width&3) >= 1) pd[4*i + 0]= ( pd[i]>>6 )*0x55; + for (i--; i >= 0; i--) { + pd[4*i + 3]= ( pd[i] & 3)*0x55; + pd[4*i + 2]= ((pd[i]>>2) & 3)*0x55; + pd[4*i + 1]= ((pd[i]>>4) & 3)*0x55; + pd[4*i + 0]= ( pd[i]>>6 )*0x55; } - pd += p->linesize[0]; } } else if (s->bits_per_pixel == 4) { - int i, j; - uint8_t *pd = p->data[0]; - for (j = 0; j < s->height; j++) { - i = s->width/2; - if (s->color_type == PNG_COLOR_TYPE_PALETTE) { - if (s->width&1) pd[2*i+0]= pd[i]>>4; - for (i--; i >= 0; i--) { - pd[2*i + 1] = pd[i] & 15; - pd[2*i + 0] = pd[i] >> 4; - } - } else { - if (s->width & 1) pd[2*i + 0]= (pd[i] >> 4) * 0x11; - for (i--; i >= 0; i--) { - pd[2*i + 1] = (pd[i] & 15) * 0x11; - pd[2*i + 0] = (pd[i] >> 4) * 0x11; - } + int i; + + i = width/2; + if (s->color_type == PNG_COLOR_TYPE_PALETTE) { + if (width&1) pd[2*i+0]= pd[i]>>4; + for (i--; i >= 0; i--) { + pd[2*i + 1] = pd[i] & 15; + pd[2*i + 0] = pd[i] >> 4; + } + } else { + if (width & 1) pd[2*i + 0]= (pd[i] >> 4) * 0x11; + for (i--; i >= 0; i--) { + pd[2*i + 1] = (pd[i] & 15) * 0x11; + pd[2*i + 0] = (pd[i] >> 4) * 0x11; } - pd += p->linesize[0]; } } } +static void handle_small_bpp(PNGDecContext *s, AVFrame *p) +{ + uint8_t *pd = p->data[0]; + + for (int j = 0; j < s->height; j++) { + unpack_small_row(s, pd, s->width); + pd += p->linesize[0]; + } +} + +/* The offsets of an APNG frame are pixel offsets, so they cannot be applied + * to the packed rows: those are decoded at the start of the frame row and + * moved to their position within the frame once unpacked. */ +static void handle_small_bpp_apng(PNGDecContext *s, AVFrame *p) +{ + /* one byte per sample once unpacked, that is one palette index or one + * gray sample per pixel */ + const int bpp = s->channels; + uint8_t *pd = p->data[0] + s->y_offset * p->linesize[0]; + + for (int j = 0; j < s->cur_h; j++) { + unpack_small_row(s, pd, s->cur_w); + if (s->x_offset) + memmove(pd + s->x_offset * bpp, pd, s->cur_w * bpp); + pd += p->linesize[0]; + } +} + static int decode_fctl_chunk(AVCodecContext *avctx, PNGDecContext *s, GetByteContext *gb) { @@ -1735,8 +1765,12 @@ exit_loop: goto fail; } - if (s->bits_per_pixel <= 4) - handle_small_bpp(s, p); + if (s->bits_per_pixel <= 4) { + if (CONFIG_APNG_DECODER && s->subframe_packed) + handle_small_bpp_apng(s, p); + else + handle_small_bpp(s, p); + } if (s->exif_data) { // we swap because ff_decode_exif_attach_buffer adds to p->metadata -- 2.52.0 >From f61180c2df9e2657659405fed3f08ec22fa35a19 Mon Sep 17 00:00:00 2001 From: Jun Zhao <[email protected]> Date: Sat, 19 Sep 2026 20:30:14 +0800 Subject: [PATCH 2/2] avformat/apngdec: report out of order APNG chunks The fcTL and fdAT chunks share a single sequence number, which must start at 0 and increase by one for every chunk, with no gap and no duplicate. Report the chunks which do not follow it, resynchronizing on the value which was read so that the following chunks are not reported as well. Out of order chunks are only reported and not rejected, so that files written by encoders getting this wrong are still played. Signed-off-by: Jun Zhao <[email protected]> --- libavformat/apngdec.c | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/libavformat/apngdec.c b/libavformat/apngdec.c index 7d560bdddd..bf8cabef37 100644 --- a/libavformat/apngdec.c +++ b/libavformat/apngdec.c @@ -50,6 +50,10 @@ typedef struct APNGDemuxContext { int is_key_frame; + /* fcTL and fdAT chunks share a single sequence number, which must start + * at 0 and increase by one for every chunk, with no gap or duplicate */ + uint32_t expected_seq; + /* * loop options */ @@ -235,6 +239,26 @@ static int apng_read_header(AVFormatContext *s) } } +/* Out-of-order chunks are only reported: the sequence is resynchronized on the + * value actually read, so that every following chunk is not reported as well. */ +static void check_chunk_sequence(AVFormatContext *s, APNGDemuxContext *ctx, + const char *name, uint32_t sequence_number) +{ + if (sequence_number != ctx->expected_seq) + av_log(s, AV_LOG_WARNING, "Invalid %s sequence number %"PRIu32 + ", expected %"PRIu32"\n", name, sequence_number, ctx->expected_seq); + + ctx->expected_seq = sequence_number + 1; +} + +/* check the sequence number of an fdAT chunk: it is the first field of the + * chunk, which the caller has just appended at the end of the packet */ +static void check_fdat_chunk(AVFormatContext *s, APNGDemuxContext *ctx, + AVPacket *pkt, uint32_t len) +{ + check_chunk_sequence(s, ctx, "fdAT", AV_RB32(pkt->data + pkt->size - len - 4)); +} + static int decode_fctl_chunk(AVFormatContext *s, APNGDemuxContext *ctx) { uint32_t sequence_number, width, height, x_offset, y_offset; @@ -252,6 +276,8 @@ static int decode_fctl_chunk(AVFormatContext *s, APNGDemuxContext *ctx) blend_op = avio_r8(s->pb); avio_skip(s->pb, 4); /* crc */ + check_chunk_sequence(s, ctx, "fcTL", sequence_number); + /* default is hundredths of seconds */ if (!delay_den) delay_den = 100; @@ -360,6 +386,9 @@ static int apng_read_packet(AVFormatContext *s, AVPacket *pkt) (ret = av_append_packet(pb, pkt, size)) < 0) return ret; + if (tag == MKTAG('f', 'd', 'A', 'T')) + check_fdat_chunk(s, ctx, pkt, len); + if (ctx->num_play == 1 && (ret = ffio_ensure_seekback(pb, 8)) < 0) return ret; @@ -373,6 +402,10 @@ static int apng_read_packet(AVFormatContext *s, AVPacket *pkt) if ((ret = avio_seek(pb, -8, SEEK_CUR)) < 0 || (ret = av_append_packet(pb, pkt, len + 12)) < 0) return ret; + + if (tag == MKTAG('f', 'd', 'A', 'T')) + check_fdat_chunk(s, ctx, pkt, len); + if (ctx->num_play == 1 && (ret = ffio_ensure_seekback(pb, 8)) < 0) return ret; len = avio_rb32(pb); @@ -394,6 +427,7 @@ static int apng_read_packet(AVFormatContext *s, AVPacket *pkt) } if ((ret = avio_seek(pb, s->streams[0]->codecpar->extradata_size + 8, SEEK_SET)) < 0) return ret; + ctx->expected_seq = 0; return 0; default: avpriv_request_sample(s, "In-stream tag=%s (0x%08"PRIX32") len=%"PRIu32, -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
