PR #25851 opened by romainhedouin URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25851 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/25851.patch
With non_modifying_colour_flag set, pixels of colour code 1 must leave the region unchanged. The readers did not store them, which is right, but they did not move the destination pointer either, so every later pixel of the same string was stored too far to the left. Advance the destination pointer for skipped pixels, for single pixels and for runs, and never past the end of the line. The returned pixel count is unchanged. | 4-bit string `1 2 3 4`, non_mod=1, line pre-filled with aa | line after decoding | |------------------------------------------------------------|--------------------------| | before | `02 03 04 aa aa aa aa aa` | | after | `aa 02 03 04 aa aa aa aa` | Testing: independent model (exhaustive short strings and fuzzing), real decoder on generated streams, ASan/UBSan, DVB FATE tests. Found during a code review sweep of ffmpeg using an LLM. <!-- If this PR requires new FATE test samples, attach them to the PR and list their target paths below (relative to the fate-suite root). Attached filenames must match the sample's filename: ```fate-samples # e.g. vorbis/new-sample.ogg ``` --> >From e48fdb93f567b00c2251254c8c363d6f20fdd47a Mon Sep 17 00:00:00 2001 From: romainhedouin <[email protected]> Date: Sun, 8 Nov 2026 00:38:38 +0100 Subject: [PATCH] avcodec/dvbsubdec: keep the position after a non-modifying colour pixel With non_modifying_colour_flag set, pixels of colour code 1 must leave the region unchanged. The readers did not store them, which is right, but they only advanced pixels_read and not the destination pointer, so every later pixel of the same string was stored too far to the left. Advance the destination pointer too, for single pixels and for runs, and never past the end of the line. The returned pixel count is unchanged. Found during a code review sweep of ffmpeg using an LLM. Signed-off-by: romainhedouin <[email protected]> --- libavcodec/dvbsubdec.c | 44 ++++++++++++++++++++++++++---------------- 1 file changed, 27 insertions(+), 17 deletions(-) diff --git a/libavcodec/dvbsubdec.c b/libavcodec/dvbsubdec.c index 2426da896d1..bfd8204a211 100644 --- a/libavcodec/dvbsubdec.c +++ b/libavcodec/dvbsubdec.c @@ -400,7 +400,8 @@ static int dvbsub_read_2bit_string(AVCodecContext *avctx, *destbuf++ = map_table[bits]; else *destbuf++ = bits; - } + } else + destbuf++; pixels_read++; } else { bits = get_bits1(&gb); @@ -408,9 +409,10 @@ static int dvbsub_read_2bit_string(AVCodecContext *avctx, run_length = get_bits(&gb, 3) + 3; bits = get_bits(&gb, 2); - if (non_mod == 1 && bits == 1) + if (non_mod == 1 && bits == 1) { + destbuf += FFMIN(run_length, dbuf_len - pixels_read); pixels_read += run_length; - else { + } else { if (map_table) bits = map_table[bits]; while (run_length-- > 0 && pixels_read < dbuf_len) { @@ -426,9 +428,10 @@ static int dvbsub_read_2bit_string(AVCodecContext *avctx, run_length = get_bits(&gb, 4) + 12; bits = get_bits(&gb, 2); - if (non_mod == 1 && bits == 1) + if (non_mod == 1 && bits == 1) { + destbuf += FFMIN(run_length, dbuf_len - pixels_read); pixels_read += run_length; - else { + } else { if (map_table) bits = map_table[bits]; while (run_length-- > 0 && pixels_read < dbuf_len) { @@ -440,9 +443,10 @@ static int dvbsub_read_2bit_string(AVCodecContext *avctx, run_length = get_bits(&gb, 8) + 29; bits = get_bits(&gb, 2); - if (non_mod == 1 && bits == 1) + if (non_mod == 1 && bits == 1) { + destbuf += FFMIN(run_length, dbuf_len - pixels_read); pixels_read += run_length; - else { + } else { if (map_table) bits = map_table[bits]; while (run_length-- > 0 && pixels_read < dbuf_len) { @@ -507,7 +511,8 @@ static int dvbsub_read_4bit_string(AVCodecContext *avctx, uint8_t *destbuf, int *destbuf++ = map_table[bits]; else *destbuf++ = bits; - } + } else + destbuf++; pixels_read++; } else { bits = get_bits1(&gb); @@ -536,9 +541,10 @@ static int dvbsub_read_4bit_string(AVCodecContext *avctx, uint8_t *destbuf, int run_length = get_bits(&gb, 2) + 4; bits = get_bits(&gb, 4); - if (non_mod == 1 && bits == 1) + if (non_mod == 1 && bits == 1) { + destbuf += FFMIN(run_length, dbuf_len - pixels_read); pixels_read += run_length; - else { + } else { if (map_table) bits = map_table[bits]; while (run_length-- > 0 && pixels_read < dbuf_len) { @@ -552,9 +558,10 @@ static int dvbsub_read_4bit_string(AVCodecContext *avctx, uint8_t *destbuf, int run_length = get_bits(&gb, 4) + 9; bits = get_bits(&gb, 4); - if (non_mod == 1 && bits == 1) + if (non_mod == 1 && bits == 1) { + destbuf += FFMIN(run_length, dbuf_len - pixels_read); pixels_read += run_length; - else { + } else { if (map_table) bits = map_table[bits]; while (run_length-- > 0 && pixels_read < dbuf_len) { @@ -566,9 +573,10 @@ static int dvbsub_read_4bit_string(AVCodecContext *avctx, uint8_t *destbuf, int run_length = get_bits(&gb, 8) + 25; bits = get_bits(&gb, 4); - if (non_mod == 1 && bits == 1) + if (non_mod == 1 && bits == 1) { + destbuf += FFMIN(run_length, dbuf_len - pixels_read); pixels_read += run_length; - else { + } else { if (map_table) bits = map_table[bits]; while (run_length-- > 0 && pixels_read < dbuf_len) { @@ -629,7 +637,8 @@ static int dvbsub_read_8bit_string(AVCodecContext *avctx, *destbuf++ = map_table[bits]; else *destbuf++ = bits; - } + } else + destbuf++; pixels_read++; } else { bits = bytestream2_get_byte(gb); @@ -644,9 +653,10 @@ static int dvbsub_read_8bit_string(AVCodecContext *avctx, } else { bits = bytestream2_get_byte(gb); } - if (non_mod == 1 && bits == 1) + if (non_mod == 1 && bits == 1) { + destbuf += FFMIN(run_length, dbuf_len - pixels_read); pixels_read += run_length; - else { + } else { if (map_table) bits = map_table[bits]; while (run_length-- > 0 && pixels_read < dbuf_len) { -- 2.54.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
