PR #25367 opened by Forgejo_Fairy URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25367 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25367.patch
Flushing a CD+G decoder after decoding a stream retains its palette, transparency table and scroll offsets. The callback also clears a potentially shared picture in place, changing frames still held by the caller. Release the decoder's frame reference and reset the initialization flag, scroll offsets and alpha table. The existing first-packet initialization then clears the picture and palette without modifying previously returned frames. Add a sample-free FATE API test covering the two reported sequences, both palette halves, retained frames, scroll offsets, repeated flushes and flushing before the first packet. Expected results reside in the FATE reference file. Validation on aarch64 Linux with GCC 13.3: `fate-api-cdgraphics-flush` fails with the original decoder and passes with this change; the existing `fate-cdgraphics` test also passes. Both reported cases reproduce on master and releases 5.1 through 9.0. Release 4.4 also retains palette and transparency state, although its older transparency implementation requires the extended test to expose the latter. This resets decoder state across flush; it does not add preroll for midstream CD+G seeks that depend on earlier commands. Fixes #25319. >From 837135f27b3e88310eab8445057adad8f1954622 Mon Sep 17 00:00:00 2001 From: Michael Niedermayer <[email protected]> Date: Thu, 22 Oct 2026 17:19:53 +0000 Subject: [PATCH] avcodec/cdgraphics: reset all decoder state on flush The flush callback preserves the palette after a frame has been returned and leaves transparency and scroll offsets from the previous stream. It also clears a potentially shared picture in place, modifying frames still held by the caller. Release the internal frame reference and reset the initialization flag, scroll offsets and alpha table. The existing first-packet initialization then clears the picture and palette without changing caller-owned frames. Add a sample-free FATE API test covering palette and transparency reset, both palette halves, retained frames, scroll offsets, repeated flushes and flushing before the first packet. Expected output lives in the FATE reference rather than in the test program. Fixes: #25319 Assisted-by: Fairy --- libavcodec/cdgraphics.c | 12 +- tests/api/Makefile | 1 + tests/api/api-cdgraphics-flush-test.c | 190 ++++++++++++++++++++++++++ tests/fate/api.mak | 4 + tests/ref/fate/api-cdgraphics-flush | 30 ++++ 5 files changed, 230 insertions(+), 7 deletions(-) create mode 100644 tests/api/api-cdgraphics-flush-test.c create mode 100644 tests/ref/fate/api-cdgraphics-flush diff --git a/libavcodec/cdgraphics.c b/libavcodec/cdgraphics.c index 1b5a37875bf..53538ae08b0 100644 --- a/libavcodec/cdgraphics.c +++ b/libavcodec/cdgraphics.c @@ -373,13 +373,11 @@ static av_cold void cdg_decode_flush(AVCodecContext *avctx) { CDGraphicsContext *cc = avctx->priv_data; - if (!cc->frame->data[0]) - return; - - for (int y = 0; y < avctx->height; y++) - memset(cc->frame->data[0] + y * cc->frame->linesize[0], 0, avctx->width); - if (!avctx->frame_num) - memset(cc->frame->data[1], 0, AVPALETTE_SIZE); + av_frame_unref(cc->frame); + cc->cleared = 0; + cc->hscroll = 0; + cc->vscroll = 0; + memset(cc->alpha, 0xFF, sizeof(cc->alpha)); } static av_cold int cdg_decode_end(AVCodecContext *avctx) diff --git a/tests/api/Makefile b/tests/api/Makefile index e51dc6c5837..daf6ae19d62 100644 --- a/tests/api/Makefile +++ b/tests/api/Makefile @@ -1,3 +1,4 @@ +APITESTPROGS-$(CONFIG_CDGRAPHICS_DECODER) += api-cdgraphics-flush APITESTPROGS-$(call ALLYES, LIBASTCENC_ENCODER LIBASTCENC_DECODER) += api-astc-profile APITESTPROGS-$(call ENCDEC, FLAC, FLAC) += api-flac APITESTPROGS-$(call DEMDEC, H264, H264) += api-h264 diff --git a/tests/api/api-cdgraphics-flush-test.c b/tests/api/api-cdgraphics-flush-test.c new file mode 100644 index 00000000000..8639715ed63 --- /dev/null +++ b/tests/api/api-cdgraphics-flush-test.c @@ -0,0 +1,190 @@ +/* + * This file is part of FFmpeg. + * + * FFmpeg is free software; you can redistribute it and/or + * modify it under the terms of the GNU Lesser General Public + * License as published by the Free Software Foundation; either + * version 2.1 of the License, or (at your option) any later version. + * + * FFmpeg is distributed in the hope that it will be useful, + * but WITHOUT ANY WARRANTY; without even the implied warranty of + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU + * Lesser General Public License for more details. + * + * You should have received a copy of the GNU Lesser General Public + * License along with FFmpeg; if not, write to the Free Software + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301 USA + */ + +#include <inttypes.h> +#include <stdio.h> +#include <string.h> + +#include "libavcodec/avcodec.h" + +#define PACKET_SIZE 24 + +enum { + MEMORY_PRESET = 1, + TILE_BLOCK = 6, + SCROLL_PRESET = 20, + TRANSPARENT_COL = 28, + LOAD_PAL_LO = 30, + LOAD_PAL_HIGH = 31, +}; + +typedef struct Command { + int instruction; + uint8_t data[16]; +} Command; + +static int decode(AVCodecContext *ctx, const Command *commands, int count, + AVFrame *frame) +{ + AVPacket *packet = av_packet_alloc(); + int ret = 0; + + if (!packet) + return AVERROR(ENOMEM); + for (int i = 0; i < count; i++) { + ret = av_new_packet(packet, PACKET_SIZE); + if (ret < 0) + break; + memset(packet->data, 0, PACKET_SIZE); + packet->data[0] = 9; + packet->data[1] = commands[i].instruction; + memcpy(packet->data + 4, commands[i].data, 16); + ret = avcodec_send_packet(ctx, packet); + av_packet_unref(packet); + if (ret < 0) + break; + av_frame_unref(frame); + ret = avcodec_receive_frame(ctx, frame); + if (ret < 0) + break; + } + av_packet_free(&packet); + return ret; +} + +static void compare(const char *name, const AVFrame *a, const AVFrame *b) +{ + int pixels_differ = 0; + int palette_differs = !!memcmp(a->data[1], b->data[1], AVPALETTE_SIZE); + + for (int y = 0; y < a->height; y++) + pixels_differ |= !!memcmp(a->data[0] + y * a->linesize[0], + b->data[0] + y * b->linesize[0], a->width); + printf("%s: pixels differ %d, palette differs %d\n", + name, pixels_differ, palette_differs); +} + +static void palette(const char *name, const AVFrame *frame) +{ + const uint32_t *pal = (const uint32_t *)frame->data[1]; + + printf("%s palette:", name); + for (int i = 0; i < 16; i++) + printf(" %08"PRIx32, pal[i]); + printf("\n"); +} + +static int run(const char *name, const Command *a, int na, + const Command *b, int nb) +{ + const AVCodec *codec = avcodec_find_decoder(AV_CODEC_ID_CDGRAPHICS); + AVCodecContext *reused = avcodec_alloc_context3(codec); + AVCodecContext *fresh = avcodec_alloc_context3(codec); + AVFrame *old = av_frame_alloc(), *snapshot = av_frame_alloc(); + AVFrame *r = av_frame_alloc(), *f = av_frame_alloc(); + int ret = AVERROR(ENOMEM); + + if (!reused || !fresh || !old || !snapshot || !r || !f) + goto end; + ret = avcodec_open2(reused, codec, NULL); + if (ret < 0) + goto end; + ret = avcodec_open2(fresh, codec, NULL); + if (ret < 0) + goto end; + + printf("%s\n", name); + /* Flushing before the first packet must also be safe. */ + avcodec_flush_buffers(reused); + ret = decode(reused, a, na, old); + if (ret < 0) + goto end; + ret = av_frame_ref(snapshot, old); + if (ret < 0) + goto end; + /* Keep an independent copy to detect changes to the caller's frame. */ + ret = av_frame_make_writable(snapshot); + if (ret < 0) + goto end; + avcodec_flush_buffers(reused); + avcodec_flush_buffers(reused); + compare("retained frame after flush", old, snapshot); + ret = decode(reused, b, nb, r); + if (ret < 0) + goto end; + ret = decode(fresh, b, nb, f); + if (ret < 0) + goto end; + compare("reused versus fresh", r, f); + compare("retained frame after decoding", old, snapshot); + palette("reused", r); + palette("fresh", f); +end: + av_frame_free(&old); + av_frame_free(&snapshot); + av_frame_free(&r); + av_frame_free(&f); + avcodec_free_context(&reused); + avcodec_free_context(&fresh); + return ret; +} + +int main(void) +{ + Command pal[2] = { { LOAD_PAL_LO }, { LOAD_PAL_HIGH } }; + Command preset = { MEMORY_PRESET, { 1 } }; + Command tile = { TILE_BLOCK, { 0, 2 } }; + Command scroll = { SCROLL_PRESET, { 0, 5, 11 } }; + Command a[4]; + int ret; + + for (int i = 0; i < 16; i++) { + int color = (i << 8) | ((15 - i) << 4) | 8; + pal[i / 8].data[2 * (i % 8)] = color >> 6; + pal[i / 8].data[2 * (i % 8) + 1] = color & 0x3f; + } + for (int i = 4; i < 16; i++) + tile.data[i] = 0x2a; + + a[0] = pal[0]; + a[1] = tile; + ret = run("palette reset", a, 2, &preset, 1); + if (ret < 0) + return 1; + + a[0].instruction = TRANSPARENT_COL; + memset(a[0].data, 0x3f, sizeof(a[0].data)); + a[1] = pal[0]; + a[2] = tile; + ret = run("transparency reset", a, 3, pal, 1); + if (ret < 0) + return 1; + + a[1] = pal[0]; + a[2] = pal[1]; + a[3] = tile; + ret = run("both palette halves reset", a, 4, &preset, 1); + if (ret < 0) + return 1; + ret = run("both transparency halves reset", a, 4, pal, 2); + if (ret < 0) + return 1; + + ret = run("scroll offsets reset", &scroll, 1, &tile, 1); + return ret < 0; +} diff --git a/tests/fate/api.mak b/tests/fate/api.mak index ca836a40e3d..f408898819f 100644 --- a/tests/fate/api.mak +++ b/tests/fate/api.mak @@ -1,3 +1,7 @@ +FATE_API_LIBAVCODEC-$(CONFIG_CDGRAPHICS_DECODER) += fate-api-cdgraphics-flush +fate-api-cdgraphics-flush: $(APITESTSDIR)/api-cdgraphics-flush-test$(EXESUF) +fate-api-cdgraphics-flush: CMD = run $(APITESTSDIR)/api-cdgraphics-flush-test$(EXESUF) + FATE_API_LIBAVCODEC-$(call ALLYES, LIBASTCENC_ENCODER LIBASTCENC_DECODER) += fate-api-astc-profile fate-api-astc-profile: $(APITESTSDIR)/api-astc-profile-test$(EXESUF) fate-api-astc-profile: CMD = run $(APITESTSDIR)/api-astc-profile-test$(EXESUF) diff --git a/tests/ref/fate/api-cdgraphics-flush b/tests/ref/fate/api-cdgraphics-flush new file mode 100644 index 00000000000..593a6a020b7 --- /dev/null +++ b/tests/ref/fate/api-cdgraphics-flush @@ -0,0 +1,30 @@ +palette reset +retained frame after flush: pixels differ 0, palette differs 0 +reused versus fresh: pixels differ 0, palette differs 0 +retained frame after decoding: pixels differ 0, palette differs 0 +reused palette: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 +fresh palette: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 +transparency reset +retained frame after flush: pixels differ 0, palette differs 0 +reused versus fresh: pixels differ 0, palette differs 0 +retained frame after decoding: pixels differ 0, palette differs 0 +reused palette: ff00ff88 ff11ee88 ff22dd88 ff33cc88 ff44bb88 ff55aa88 ff669988 ff778888 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 +fresh palette: ff00ff88 ff11ee88 ff22dd88 ff33cc88 ff44bb88 ff55aa88 ff669988 ff778888 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 +both palette halves reset +retained frame after flush: pixels differ 0, palette differs 0 +reused versus fresh: pixels differ 0, palette differs 0 +retained frame after decoding: pixels differ 0, palette differs 0 +reused palette: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 +fresh palette: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 +both transparency halves reset +retained frame after flush: pixels differ 0, palette differs 0 +reused versus fresh: pixels differ 0, palette differs 0 +retained frame after decoding: pixels differ 0, palette differs 0 +reused palette: ff00ff88 ff11ee88 ff22dd88 ff33cc88 ff44bb88 ff55aa88 ff669988 ff778888 ff887788 ff996688 ffaa5588 ffbb4488 ffcc3388 ffdd2288 ffee1188 ffff0088 +fresh palette: ff00ff88 ff11ee88 ff22dd88 ff33cc88 ff44bb88 ff55aa88 ff669988 ff778888 ff887788 ff996688 ffaa5588 ffbb4488 ffcc3388 ffdd2288 ffee1188 ffff0088 +scroll offsets reset +retained frame after flush: pixels differ 0, palette differs 0 +reused versus fresh: pixels differ 0, palette differs 0 +retained frame after decoding: pixels differ 0, palette differs 0 +reused palette: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 +fresh palette: 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 00000000 -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
