PR #24347 opened by paulocsanz URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24347 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/24347.patch
Switch palette, presentation, object, RLE and the outer segment loop from bytestream_get_* to GetByteContext so each read is limited to the declared remaining size. fate-pgssub-bounds covers a 2x2 display set (contiguous and split object), a blank presentation, and truncated packets. make fate-pgssub-bounds >From 107352a6e045af477d1849023374279cbfe303fa Mon Sep 17 00:00:00 2001 From: Paulo Cabral Sanz <[email protected]> Date: Wed, 19 Aug 2026 15:03:18 -0300 Subject: [PATCH] avcodec/pgssubdec: use bytestream2 for segment parsing Switch palette, presentation, object, RLE and the outer segment loop from unchecked bytestream_get_* to GetByteContext. Short segments now stop at the declared remaining size. Add fate-pgssub-bounds: a 2x2 display set (contiguous and split object), a blank presentation, and a few truncated packets. Signed-off-by: Paulo Cabral Sanz <[email protected]> --- libavcodec/Makefile | 1 + libavcodec/pgssubdec.c | 183 +++++++++++++++++++++----------------- libavcodec/tests/pgssub.c | 175 ++++++++++++++++++++++++++++++++++++ tests/fate/libavcodec.mak | 5 ++ 4 files changed, 284 insertions(+), 80 deletions(-) create mode 100644 libavcodec/tests/pgssub.c diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 3cdfa4f383..cf9780cb11 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -1381,6 +1381,7 @@ TESTPROGS = avcodec \ jpeg2000dwt \ mathops \ +TESTPROGS-$(CONFIG_PGSSUB_DECODER) += pgssub TESTPROGS-$(CONFIG_APV_DECODER) += apv TESTPROGS-$(CONFIG_AV1_VAAPI_ENCODER) += av1_levels TESTPROGS-$(CONFIG_CABAC) += cabac diff --git a/libavcodec/pgssubdec.c b/libavcodec/pgssubdec.c index 3a4b60f419..af3de58dcc 100644 --- a/libavcodec/pgssubdec.c +++ b/libavcodec/pgssubdec.c @@ -162,10 +162,10 @@ static av_cold int close_decoder(AVCodecContext *avctx) static int decode_rle(AVCodecContext *avctx, AVSubtitleRect *rect, const uint8_t *buf, unsigned int buf_size) { - const uint8_t *rle_bitmap_end; + GetByteContext gb; int pixel_count, line_count; - rle_bitmap_end = buf + buf_size; + bytestream2_init(&gb, buf, buf_size); rect->data[0] = av_malloc_array(rect->w, rect->h); @@ -175,19 +175,30 @@ static int decode_rle(AVCodecContext *avctx, AVSubtitleRect *rect, pixel_count = 0; line_count = 0; - while (buf < rle_bitmap_end && line_count < rect->h) { + while (bytestream2_get_bytes_left(&gb) > 0 && line_count < rect->h) { uint8_t flags, color; int run; - color = bytestream_get_byte(&buf); + color = bytestream2_get_byte(&gb); run = 1; if (color == 0x00) { - flags = bytestream_get_byte(&buf); + if (bytestream2_get_bytes_left(&gb) < 1) + break; + flags = bytestream2_get_byte(&gb); run = flags & 0x3f; - if (flags & 0x40) - run = (run << 8) + bytestream_get_byte(&buf); - color = flags & 0x80 ? bytestream_get_byte(&buf) : 0; + if (flags & 0x40) { + if (bytestream2_get_bytes_left(&gb) < 1) + break; + run = (run << 8) + bytestream2_get_byte(&gb); + } + if (flags & 0x80) { + if (bytestream2_get_bytes_left(&gb) < 1) + break; + color = bytestream2_get_byte(&gb); + } else { + color = 0; + } } if (run > 0 && pixel_count + run <= rect->w * rect->h) { @@ -234,16 +245,16 @@ static int parse_object_segment(AVCodecContext *avctx, { PGSSubContext *ctx = avctx->priv_data; PGSSubObject *object; - + GetByteContext gb; uint8_t sequence_desc; unsigned int rle_bitmap_len, width, height; - int id; + int id, remain; if (buf_size <= 4) return AVERROR_INVALIDDATA; - buf_size -= 4; - id = bytestream_get_be16(&buf); + bytestream2_init(&gb, buf, buf_size); + id = bytestream2_get_be16(&gb); object = find_object(id, &ctx->objects); if (!object) { if (ctx->objects.count >= MAX_EPOCH_OBJECTS) { @@ -255,40 +266,42 @@ static int parse_object_segment(AVCodecContext *avctx, } /* skip object version number */ - buf += 1; + bytestream2_skip(&gb, 1); /* Read the Sequence Description to determine if start of RLE data or appended to previous RLE */ - sequence_desc = bytestream_get_byte(&buf); + sequence_desc = bytestream2_get_byte(&gb); if (!(sequence_desc & 0x80)) { + int copy = bytestream2_get_bytes_left(&gb); + /* Additional RLE data */ - if (buf_size > object->rle_remaining_len) + if (!object->rle || copy > object->rle_remaining_len) return AVERROR_INVALIDDATA; - memcpy(object->rle + object->rle_data_len, buf, buf_size); - object->rle_data_len += buf_size; - object->rle_remaining_len -= buf_size; + bytestream2_get_buffer(&gb, object->rle + object->rle_data_len, copy); + object->rle_data_len += copy; + object->rle_remaining_len -= copy; return 0; } - if (buf_size <= 7) + if (bytestream2_get_bytes_left(&gb) <= 7) return AVERROR_INVALIDDATA; - buf_size -= 7; /* Decode rle bitmap length, stored size includes width/height data */ - rle_bitmap_len = bytestream_get_be24(&buf) - 2*2; - - if (buf_size > rle_bitmap_len) { - av_log(avctx, AV_LOG_ERROR, - "Buffer dimension %d larger than the expected RLE data %d\n", - buf_size, rle_bitmap_len); - return AVERROR_INVALIDDATA; - } + rle_bitmap_len = bytestream2_get_be24(&gb) - 2 * 2; /* Get bitmap dimensions from data */ - width = bytestream_get_be16(&buf); - height = bytestream_get_be16(&buf); + width = bytestream2_get_be16(&gb); + height = bytestream2_get_be16(&gb); + + remain = bytestream2_get_bytes_left(&gb); + if (remain > rle_bitmap_len) { + av_log(avctx, AV_LOG_ERROR, + "Buffer dimension %d larger than the expected RLE data %d\n", + remain, rle_bitmap_len); + return AVERROR_INVALIDDATA; + } /* Make sure the bitmap is not too large */ if (avctx->width < width || avctx->height < height || !width || !height) { @@ -307,9 +320,9 @@ static int parse_object_segment(AVCodecContext *avctx, return AVERROR(ENOMEM); } - memcpy(object->rle, buf, buf_size); - object->rle_data_len = buf_size; - object->rle_remaining_len = rle_bitmap_len - buf_size; + bytestream2_get_buffer(&gb, object->rle, remain); + object->rle_data_len = remain; + object->rle_remaining_len = rle_bitmap_len - remain; return 0; } @@ -330,14 +343,18 @@ static int parse_palette_segment(AVCodecContext *avctx, PGSSubContext *ctx = avctx->priv_data; PGSSubPalette *palette; - const uint8_t *buf_end = buf + buf_size; + GetByteContext gb; const uint8_t *cm = ff_crop_tab + MAX_NEG_CROP; int color_id; int y, cb, cr, alpha; int r, g, b, r_add, g_add, b_add; int id; - id = bytestream_get_byte(&buf); + if (buf_size < 2) + return AVERROR_INVALIDDATA; + bytestream2_init(&gb, buf, buf_size); + + id = bytestream2_get_byte(&gb); palette = find_palette(id, &ctx->palettes); if (!palette) { if (ctx->palettes.count >= MAX_EPOCH_PALETTES) { @@ -349,14 +366,14 @@ static int parse_palette_segment(AVCodecContext *avctx, } /* Skip palette version */ - buf += 1; + bytestream2_skip(&gb, 1); - while (buf < buf_end) { - color_id = bytestream_get_byte(&buf); - y = bytestream_get_byte(&buf); - cr = bytestream_get_byte(&buf); - cb = bytestream_get_byte(&buf); - alpha = bytestream_get_byte(&buf); + while (bytestream2_get_bytes_left(&gb) >= 5) { + color_id = bytestream2_get_byte(&gb); + y = bytestream2_get_byte(&gb); + cr = bytestream2_get_byte(&gb); + cb = bytestream2_get_byte(&gb); + alpha = bytestream2_get_byte(&gb); /* Default to BT.709 colorspace. In case of <= 576 height use BT.601 */ if (avctx->height <= 0 || avctx->height > 576) { @@ -391,12 +408,16 @@ static int parse_presentation_segment(AVCodecContext *avctx, int64_t pts) { PGSSubContext *ctx = avctx->priv_data; - int i, state, ret; - const uint8_t *buf_end = buf + buf_size; + GetByteContext gb; + int i, state, ret, w, h; - // Video descriptor - int w = bytestream_get_be16(&buf); - int h = bytestream_get_be16(&buf); + if (buf_size < 11) + return AVERROR_INVALIDDATA; + bytestream2_init(&gb, buf, buf_size); + + /* Video descriptor */ + w = bytestream2_get_be16(&gb); + h = bytestream2_get_be16(&gb); ctx->presentation.pts = pts; @@ -407,10 +428,10 @@ static int parse_presentation_segment(AVCodecContext *avctx, return ret; /* Skip 1 bytes of unknown, frame rate */ - buf++; + bytestream2_skip(&gb, 1); - // Composition descriptor - ctx->presentation.id_number = bytestream_get_be16(&buf); + /* Composition descriptor */ + ctx->presentation.id_number = bytestream2_get_be16(&gb); /* * state is a 2 bit field that defines pgs epoch boundaries * 00 - Normal, previously defined objects and palettes are still valid @@ -420,7 +441,7 @@ static int parse_presentation_segment(AVCodecContext *avctx, * * reserved 6 bits discarded */ - state = bytestream_get_byte(&buf) >> 6; + state = bytestream2_get_byte(&gb) >> 6; if (state != 0) { flush_cache(avctx); } @@ -428,9 +449,9 @@ static int parse_presentation_segment(AVCodecContext *avctx, /* * skip palette_update_flag (0x80), */ - buf += 1; - ctx->presentation.palette_id = bytestream_get_byte(&buf); - ctx->presentation.object_count = bytestream_get_byte(&buf); + bytestream2_skip(&gb, 1); + ctx->presentation.palette_id = bytestream2_get_byte(&gb); + ctx->presentation.object_count = bytestream2_get_byte(&gb); if (ctx->presentation.object_count > MAX_OBJECT_REFS) { av_log(avctx, AV_LOG_ERROR, "Invalid number of presentation objects %d\n", @@ -446,25 +467,27 @@ static int parse_presentation_segment(AVCodecContext *avctx, { PGSSubObjectRef *const object = &ctx->presentation.objects[i]; - if (buf_end - buf < 8) { + if (bytestream2_get_bytes_left(&gb) < 8) { av_log(avctx, AV_LOG_ERROR, "Insufficient space for object\n"); ctx->presentation.object_count = i; return AVERROR_INVALIDDATA; } - object->id = bytestream_get_be16(&buf); - object->window_id = bytestream_get_byte(&buf); - object->composition_flag = bytestream_get_byte(&buf); + object->id = bytestream2_get_be16(&gb); + object->window_id = bytestream2_get_byte(&gb); + object->composition_flag = bytestream2_get_byte(&gb); - object->x = bytestream_get_be16(&buf); - object->y = bytestream_get_be16(&buf); + object->x = bytestream2_get_be16(&gb); + object->y = bytestream2_get_be16(&gb); - // If cropping + /* Cropping */ if (object->composition_flag & 0x80) { - object->crop_x = bytestream_get_be16(&buf); - object->crop_y = bytestream_get_be16(&buf); - object->crop_w = bytestream_get_be16(&buf); - object->crop_h = bytestream_get_be16(&buf); + if (bytestream2_get_bytes_left(&gb) < 8) + return AVERROR_INVALIDDATA; + object->crop_x = bytestream2_get_be16(&gb); + object->crop_y = bytestream2_get_be16(&gb); + object->crop_w = bytestream2_get_be16(&gb); + object->crop_h = bytestream2_get_be16(&gb); } ff_dlog(avctx, "Subtitle Placement x=%d, y=%d\n", @@ -596,11 +619,9 @@ static int decode(AVCodecContext *avctx, AVSubtitle *sub, { const uint8_t *buf = avpkt->data; int buf_size = avpkt->size; - - const uint8_t *buf_end; - uint8_t segment_type; - int segment_length; - int i, ret; + GetByteContext gb; + uint8_t segment_type; + int segment_length, i, ret; ff_dlog(avctx, "PGS sub packet:\n"); @@ -619,28 +640,30 @@ static int decode(AVCodecContext *avctx, AVSubtitle *sub, if (buf_size < 3) return -1; - buf_end = buf + buf_size; + bytestream2_init(&gb, buf, buf_size); /* Step through buffer to identify segments */ - while (buf < buf_end) { - segment_type = bytestream_get_byte(&buf); - segment_length = bytestream_get_be16(&buf); + while (bytestream2_get_bytes_left(&gb) >= 3) { + segment_type = bytestream2_get_byte(&gb); + segment_length = bytestream2_get_be16(&gb); ff_dlog(avctx, "Segment Length %d, Segment Type %x\n", segment_length, segment_type); - if (segment_type != DISPLAY_SEGMENT && segment_length > buf_end - buf) + if (segment_type != DISPLAY_SEGMENT && + segment_length > bytestream2_get_bytes_left(&gb)) break; ret = 0; switch (segment_type) { case PALETTE_SEGMENT: - ret = parse_palette_segment(avctx, buf, segment_length); + ret = parse_palette_segment(avctx, gb.buffer, segment_length); break; case OBJECT_SEGMENT: - ret = parse_object_segment(avctx, buf, segment_length); + ret = parse_object_segment(avctx, gb.buffer, segment_length); break; case PRESENTATION_SEGMENT: - ret = parse_presentation_segment(avctx, buf, segment_length, sub->pts); + ret = parse_presentation_segment(avctx, gb.buffer, segment_length, + sub->pts); break; case WINDOW_SEGMENT: /* @@ -658,7 +681,7 @@ static int decode(AVCodecContext *avctx, AVSubtitle *sub, ret = AVERROR_INVALIDDATA; break; } - ret = display_end_segment(avctx, sub, buf, segment_length); + ret = display_end_segment(avctx, sub, gb.buffer, segment_length); if (ret >= 0) *got_sub_ptr = ret; break; @@ -672,7 +695,7 @@ static int decode(AVCodecContext *avctx, AVSubtitle *sub, avctx->err_recognition & AV_EF_EXPLODE)) return ret; - buf += segment_length; + bytestream2_skip(&gb, segment_length); } return buf_size; diff --git a/libavcodec/tests/pgssub.c b/libavcodec/tests/pgssub.c new file mode 100644 index 0000000000..aba0ffce6c --- /dev/null +++ b/libavcodec/tests/pgssub.c @@ -0,0 +1,175 @@ +/* + * 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. + */ + +#include <stdio.h> +#include <string.h> + +#include "libavcodec/avcodec.h" +#include "libavutil/crc.h" +#include "libavutil/mem.h" + +/* 2x2 bitmap, palette index 5, RLE = two pixels + newline, twice. */ +static const uint8_t pkt_full[] = { + 0x16, 0x00, 0x13, + 0x00, 0x02, 0x00, 0x02, 0x00, + 0x00, 0x01, 0x80, 0x00, 0x01, 0x01, + 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x14, 0x00, 0x07, + 0x01, 0x00, 0x05, 16, 128, 128, 255, + 0x15, 0x00, 0x13, + 0x00, 0x01, 0x00, 0x80, + 0x00, 0x00, 0x0c, + 0x00, 0x02, 0x00, 0x02, + 0x05, 0x05, 0x00, 0x00, 0x05, 0x05, 0x00, 0x00, + 0x80, 0x00, 0x00, +}; + +/* Same picture, object RLE split across two segments. */ +static const uint8_t pkt_split[] = { + 0x16, 0x00, 0x13, + 0x00, 0x02, 0x00, 0x02, 0x00, + 0x00, 0x01, 0x80, 0x00, 0x01, 0x01, + 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00, + 0x14, 0x00, 0x07, + 0x01, 0x00, 0x05, 16, 128, 128, 255, + 0x15, 0x00, 0x0f, + 0x00, 0x01, 0x00, 0x80, + 0x00, 0x00, 0x0c, + 0x00, 0x02, 0x00, 0x02, + 0x05, 0x05, 0x00, 0x00, + 0x15, 0x00, 0x08, + 0x00, 0x01, 0x00, 0x00, + 0x05, 0x05, 0x00, 0x00, + 0x80, 0x00, 0x00, +}; + +static const uint8_t pkt_blank[] = { + 0x16, 0x00, 0x0b, + 0x02, 0x80, 0x01, 0xe0, 0x00, + 0x00, 0x01, 0x80, 0x00, 0x00, 0x00, + 0x80, 0x00, 0x00, +}; + +static const uint8_t pkt_short_pres[] = { 0x16, 0x00, 0x02, 0x01, 0xe0 }; +static const uint8_t pkt_short_pal[] = { + 0x14, 0x00, 0x08, 0x01, 0x00, 1, 2, 3, 4, 5, 6, +}; +static const uint8_t pkt_short_crop[] = { + 0x16, 0x00, 0x13, + 0x01, 0xe0, 0x01, 0xe0, 0x00, + 0x00, 0x01, 0x00, 0x00, 0x00, 0x01, + 0x00, 0x01, 0x00, 0x80, 0x00, 0x00, 0x00, 0x00, +}; +static const uint8_t pkt_tiny[] = { 0x16, 0x00 }; + +#define EXPECTED_CRC 0x3743da26u +#define EXPECTED_PAL 0xff000000u + +static int decode_one(AVCodecContext *ctx, const uint8_t *data, int size, + AVSubtitle *sub, int *got) +{ + AVPacket *pkt = av_packet_alloc(); + int ret; + + if (!pkt) + return AVERROR(ENOMEM); + pkt->data = (uint8_t *)data; + pkt->size = size; + memset(sub, 0, sizeof(*sub)); + ret = avcodec_decode_subtitle2(ctx, sub, got, pkt); + av_packet_free(&pkt); + return ret; +} + +static int check_bitmap(const char *name, AVSubtitle *sub) +{ + AVSubtitleRect *r; + uint32_t crc, pal = 0; + + if (sub->num_rects != 1 || !sub->rects || !sub->rects[0]) { + fprintf(stderr, "%s: expected 1 rect, got %d\n", name, sub->num_rects); + return -1; + } + r = sub->rects[0]; + if (r->w != 2 || r->h != 2 || !r->data[0] || !r->data[1]) { + fprintf(stderr, "%s: bad geometry %dx%d\n", name, r->w, r->h); + return -1; + } + memcpy(&pal, r->data[1] + 5 * 4, 4); + crc = av_crc(av_crc_get_table(AV_CRC_32_IEEE), 0, r->data[0], 4); + if (crc != EXPECTED_CRC || pal != EXPECTED_PAL) { + fprintf(stderr, "%s: crc=%08x pal=%08x\n", name, crc, pal); + return -1; + } + return 0; +} + +int main(void) +{ + const AVCodec *codec = avcodec_find_decoder(AV_CODEC_ID_HDMV_PGS_SUBTITLE); + AVCodecContext *ctx; + AVSubtitle sub; + int got, ret, failed = 0; + + if (!codec) + return 1; + ctx = avcodec_alloc_context3(codec); + if (!ctx || avcodec_open2(ctx, codec, NULL) < 0) { + avcodec_free_context(&ctx); + return 1; + } + + ret = decode_one(ctx, pkt_full, sizeof(pkt_full), &sub, &got); + if (ret != (int)sizeof(pkt_full) || !got || check_bitmap("full", &sub) < 0) + failed = 1; + avsubtitle_free(&sub); + + ret = decode_one(ctx, pkt_split, sizeof(pkt_split), &sub, &got); + if (ret < 0 || !got || check_bitmap("split", &sub) < 0) + failed = 1; + avsubtitle_free(&sub); + + ret = decode_one(ctx, pkt_blank, sizeof(pkt_blank), &sub, &got); + if (ret < 0 || !got || sub.num_rects != 0) { + fprintf(stderr, "blank: ret=%d got=%d rects=%d\n", ret, got, sub.num_rects); + failed = 1; + } + avsubtitle_free(&sub); + + ret = decode_one(ctx, pkt_short_pres, sizeof(pkt_short_pres), &sub, &got); + if (got) { + fprintf(stderr, "short presentation produced a subtitle\n"); + failed = 1; + } + avsubtitle_free(&sub); + + ret = decode_one(ctx, pkt_short_pal, sizeof(pkt_short_pal), &sub, &got); + if (got) { + fprintf(stderr, "short palette produced a subtitle\n"); + failed = 1; + } + avsubtitle_free(&sub); + + ret = decode_one(ctx, pkt_short_crop, sizeof(pkt_short_crop), &sub, &got); + if (got) { + fprintf(stderr, "short crop produced a subtitle\n"); + failed = 1; + } + avsubtitle_free(&sub); + + ret = decode_one(ctx, pkt_tiny, sizeof(pkt_tiny), &sub, &got); + if (got) { + fprintf(stderr, "truncated header produced a subtitle\n"); + failed = 1; + } + avsubtitle_free(&sub); + + avcodec_free_context(&ctx); + return failed; +} diff --git a/tests/fate/libavcodec.mak b/tests/fate/libavcodec.mak index e2d616e307..d7d0700b5e 100644 --- a/tests/fate/libavcodec.mak +++ b/tests/fate/libavcodec.mak @@ -8,6 +8,11 @@ fate-apv-entropy: libavcodec/tests/apv$(EXESUF) fate-apv-entropy: CMD = run libavcodec/tests/apv$(EXESUF) fate-apv-entropy: REF = /dev/null +FATE_LIBAVCODEC-$(CONFIG_PGSSUB_DECODER) += fate-pgssub-bounds +fate-pgssub-bounds: libavcodec/tests/pgssub$(EXESUF) +fate-pgssub-bounds: CMD = run libavcodec/tests/pgssub$(EXESUF) +fate-pgssub-bounds: CMP = null + FATE_LIBAVCODEC-yes += fate-avpacket fate-avpacket: libavcodec/tests/avpacket$(EXESUF) fate-avpacket: CMD = run libavcodec/tests/avpacket$(EXESUF) -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
