PR #23808 opened by Alearner12 URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23808 Patch URL: https://code.ffmpeg.org/FFmpeg/FFmpeg/pulls/23808.patch
# avcodec/h264: avoid overflow when deriving frame rate ## Summary Widen `num_units_in_tick` before multiplying it by two when deriving the H.264 frame rate. Apply the same correction in the parser and decoder and add a focused parser regression test. ## Problem `SPS.num_units_in_tick` is a `uint32_t`. In both call sites, this expression: ```c sps->num_units_in_tick * 2 ``` is evaluated using 32-bit unsigned arithmetic before being passed to `av_reduce()`. Values above `UINT32_MAX / 2` therefore wrap before the wider function argument can preserve them. For example, an SPS containing: ```text num_units_in_tick = 0x80000000 time_scale = 0x01010101 ``` currently produces a frame rate of `1/0`. Widening before multiplication produces the correctly reduced value `1/255`. ## Fix Use: ```c 2 * (int64_t)sps->num_units_in_tick ``` in both the parser and decoder paths. ## Compatibility and performance There is no behavior change when `num_units_in_tick <= UINT32_MAX / 2`. Larger valid values now produce the intended rational instead of a wrapped numerator. The cast has no meaningful performance cost. ## Testing The new test constructs a minimal Annex-B SPS/PPS/IDR access unit containing the overflowing VUI timing value and verifies the public parser result. Sanitizer build command used: ```sh make -j2 libavcodec/tests/h264_parser ASAN_OPTIONS=detect_leaks=0 \ UBSAN_OPTIONS=print_stacktrace=1:halt_on_error=0 \ ./libavcodec/tests/h264_parser ``` The regression passes with ASan and UBSan enabled. The decoder-enabled build also successfully compiles `libavcodec/h264_slice.o`. >From bcee102ef4027b08752c65d1f01757356b217fab Mon Sep 17 00:00:00 2001 From: Alearner12 <[email protected]> Date: Wed, 15 Jul 2026 01:07:26 +0530 Subject: [PATCH] avcodec/h264: avoid overflow when deriving frame rate --- libavcodec/Makefile | 1 + libavcodec/h264_parser.c | 2 +- libavcodec/h264_slice.c | 2 +- libavcodec/tests/h264_parser.c | 193 +++++++++++++++++++++++++++++++++ 4 files changed, 196 insertions(+), 2 deletions(-) create mode 100644 libavcodec/tests/h264_parser.c diff --git a/libavcodec/Makefile b/libavcodec/Makefile index 5730463f3e..becf7d7c4e 100644 --- a/libavcodec/Makefile +++ b/libavcodec/Makefile @@ -1383,6 +1383,7 @@ TESTPROGS-$(CONFIG_DXV_ENCODER) += hashtable TESTPROGS-$(CONFIG_MJPEG_ENCODER) += mjpegenc_huffman TESTPROGS-$(CONFIG_MPEGVIDEO) += mpeg12framerate TESTPROGS-$(CONFIG_H264_METADATA_BSF) += h264_levels +TESTPROGS-$(CONFIG_H264_PARSER) += h264_parser TESTPROGS-$(CONFIG_HEVC_METADATA_BSF) += h265_levels TESTPROGS-$(CONFIG_RANGECODER) += rangecoder TESTPROGS-$(CONFIG_SNOW_ENCODER) += snowenc diff --git a/libavcodec/h264_parser.c b/libavcodec/h264_parser.c index 9d64fc603f..8c0a503143 100644 --- a/libavcodec/h264_parser.c +++ b/libavcodec/h264_parser.c @@ -568,7 +568,7 @@ static inline int parse_nal_units(AVCodecParserContext *s, if (p->sei.common.unregistered.x264_build < 44U) den *= 2; av_reduce(&avctx->framerate.den, &avctx->framerate.num, - sps->num_units_in_tick * 2, den, 1 << 30); + 2 * (int64_t)sps->num_units_in_tick, den, 1 << 30); } av_freep(&rbsp.rbsp_buffer); diff --git a/libavcodec/h264_slice.c b/libavcodec/h264_slice.c index 7e2c460293..e71fe87e85 100644 --- a/libavcodec/h264_slice.c +++ b/libavcodec/h264_slice.c @@ -981,7 +981,7 @@ static int h264_slice_header_init(H264Context *h) if (h->x264_build < 44U) den *= 2; av_reduce(&h->avctx->framerate.den, &h->avctx->framerate.num, - sps->num_units_in_tick * 2, den, 1 << 30); + 2 * (int64_t)sps->num_units_in_tick, den, 1 << 30); } ff_h264_free_tables(h); diff --git a/libavcodec/tests/h264_parser.c b/libavcodec/tests/h264_parser.c new file mode 100644 index 0000000000..6f71637c6d --- /dev/null +++ b/libavcodec/tests/h264_parser.c @@ -0,0 +1,193 @@ +/* + * H.264 parser regression tests + * + * 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 <stdint.h> +#include <stdio.h> +#include <string.h> + +#include "libavutil/error.h" +#include "libavutil/rational.h" + +#include "libavcodec/avcodec.h" +#include "libavcodec/put_bits.h" +#include "libavcodec/put_golomb.h" + +static int finish_rbsp(PutBitContext *pb) +{ + put_bits(pb, 1, 1); + align_put_bits(pb); + flush_put_bits(pb); + return put_bytes_output(pb); +} + +static int append_nal(uint8_t *dst, int dst_size, int offset, uint8_t header, + const uint8_t *rbsp, int rbsp_size) +{ + int zeros = 0; + + if (offset > dst_size - 4) + return AVERROR_BUFFER_TOO_SMALL; + dst[offset++] = 0; + dst[offset++] = 0; + dst[offset++] = 1; + dst[offset++] = header; + + for (int i = 0; i < rbsp_size; i++) { + if (zeros == 2 && rbsp[i] <= 3) { + if (offset >= dst_size) + return AVERROR_BUFFER_TOO_SMALL; + dst[offset++] = 3; + zeros = 0; + } + if (offset >= dst_size) + return AVERROR_BUFFER_TOO_SMALL; + dst[offset++] = rbsp[i]; + zeros = rbsp[i] ? 0 : zeros + 1; + } + + return offset; +} + +static int build_sps(uint8_t *dst, int dst_size, uint32_t num_units_in_tick, + uint32_t time_scale) +{ + PutBitContext pb; + + init_put_bits(&pb, dst, dst_size); + put_bits(&pb, 8, 66); /* profile_idc */ + put_bits(&pb, 8, 0); /* constraints and reserved bits */ + put_bits(&pb, 8, 10); /* level_idc */ + set_ue_golomb(&pb, 0); /* seq_parameter_set_id */ + set_ue_golomb(&pb, 0); /* log2_max_frame_num_minus4 */ + set_ue_golomb(&pb, 0); /* pic_order_cnt_type */ + set_ue_golomb(&pb, 0); /* log2_max_pic_order_cnt_lsb_minus4 */ + set_ue_golomb(&pb, 0); /* max_num_ref_frames */ + put_bits(&pb, 1, 0); /* gaps_in_frame_num_value_allowed_flag */ + set_ue_golomb(&pb, 0); /* pic_width_in_mbs_minus1 */ + set_ue_golomb(&pb, 0); /* pic_height_in_map_units_minus1 */ + put_bits(&pb, 1, 1); /* frame_mbs_only_flag */ + put_bits(&pb, 1, 1); /* direct_8x8_inference_flag */ + put_bits(&pb, 1, 0); /* frame_cropping_flag */ + put_bits(&pb, 1, 1); /* vui_parameters_present_flag */ + + put_bits(&pb, 1, 0); /* aspect_ratio_info_present_flag */ + put_bits(&pb, 1, 0); /* overscan_info_present_flag */ + put_bits(&pb, 1, 0); /* video_signal_type_present_flag */ + put_bits(&pb, 1, 0); /* chroma_loc_info_present_flag */ + put_bits(&pb, 1, 1); /* timing_info_present_flag */ + put_bits32(&pb, num_units_in_tick); + put_bits32(&pb, time_scale); + put_bits(&pb, 1, 1); /* fixed_frame_rate_flag */ + put_bits(&pb, 1, 0); /* nal_hrd_parameters_present_flag */ + put_bits(&pb, 1, 0); /* vcl_hrd_parameters_present_flag */ + put_bits(&pb, 1, 0); /* pic_struct_present_flag */ + put_bits(&pb, 1, 0); /* bitstream_restriction_flag */ + + return finish_rbsp(&pb); +} + +static int build_pps(uint8_t *dst, int dst_size) +{ + PutBitContext pb; + + init_put_bits(&pb, dst, dst_size); + set_ue_golomb(&pb, 0); /* pic_parameter_set_id */ + set_ue_golomb(&pb, 0); /* seq_parameter_set_id */ + put_bits(&pb, 1, 0); /* entropy_coding_mode_flag */ + put_bits(&pb, 1, 0); /* bottom_field_pic_order_in_frame_present_flag */ + set_ue_golomb(&pb, 0); /* num_slice_groups_minus1 */ + set_ue_golomb(&pb, 0); /* num_ref_idx_l0_default_active_minus1 */ + set_ue_golomb(&pb, 0); /* num_ref_idx_l1_default_active_minus1 */ + put_bits(&pb, 1, 0); /* weighted_pred_flag */ + put_bits(&pb, 2, 0); /* weighted_bipred_idc */ + set_se_golomb(&pb, 0); /* pic_init_qp_minus26 */ + set_se_golomb(&pb, 0); /* pic_init_qs_minus26 */ + set_se_golomb(&pb, 0); /* chroma_qp_index_offset */ + put_bits(&pb, 1, 0); /* deblocking_filter_control_present_flag */ + put_bits(&pb, 1, 0); /* constrained_intra_pred_flag */ + put_bits(&pb, 1, 0); /* redundant_pic_cnt_present_flag */ + + return finish_rbsp(&pb); +} + +static int build_idr(uint8_t *dst, int dst_size) +{ + PutBitContext pb; + + init_put_bits(&pb, dst, dst_size); + set_ue_golomb(&pb, 0); /* first_mb_in_slice */ + set_ue_golomb(&pb, 2); /* I slice */ + set_ue_golomb(&pb, 0); /* pic_parameter_set_id */ + put_bits(&pb, 4, 0); /* frame_num */ + set_ue_golomb(&pb, 0); /* idr_pic_id */ + put_bits(&pb, 4, 0); /* pic_order_cnt_lsb */ + + return finish_rbsp(&pb); +} + +int main(void) +{ + static const uint32_t num_units_in_tick = UINT32_C(0x80000000); + static const uint32_t time_scale = UINT32_C(0x01010101); + uint8_t stream[512] = { 0 }; + uint8_t rbsp[128] = { 0 }; + AVCodecParserContext *parser = NULL; + AVCodecContext *avctx = NULL; + uint8_t *out; + int out_size, offset = 0, size, used; + int expected_num, expected_den; + int ret = 1; + + size = build_sps(rbsp, sizeof(rbsp), num_units_in_tick, time_scale); + offset = append_nal(stream, sizeof(stream), offset, 0x67, rbsp, size); + if (offset < 0) + return 1; + + memset(rbsp, 0, sizeof(rbsp)); + size = build_pps(rbsp, sizeof(rbsp)); + offset = append_nal(stream, sizeof(stream), offset, 0x68, rbsp, size); + if (offset < 0) + return 1; + + memset(rbsp, 0, sizeof(rbsp)); + size = build_idr(rbsp, sizeof(rbsp)); + offset = append_nal(stream, sizeof(stream), offset, 0x65, rbsp, size); + if (offset < 0) + return 1; + + parser = av_parser_init(AV_CODEC_ID_H264); + avctx = avcodec_alloc_context3(NULL); + if (!parser || !avctx) + goto end; + avctx->codec_id = AV_CODEC_ID_H264; + parser->flags |= PARSER_FLAG_COMPLETE_FRAMES; + + used = av_parser_parse2(parser, avctx, &out, &out_size, + stream, offset, AV_NOPTS_VALUE, AV_NOPTS_VALUE, 0); + if (used != offset || out_size != offset) + goto end; + + av_reduce(&expected_den, &expected_num, + 2 * (int64_t)num_units_in_tick, time_scale, 1 << 30); + if (avctx->framerate.num != expected_num || + avctx->framerate.den != expected_den) { + fprintf(stderr, "unexpected framerate %d/%d, expected %d/%d\n", + avctx->framerate.num, avctx->framerate.den, + expected_num, expected_den); + goto end; + } + + ret = 0; +end: + av_parser_close(parser); + avcodec_free_context(&avctx); + return ret; +} -- 2.52.0 _______________________________________________ ffmpeg-devel mailing list -- [email protected] To unsubscribe send an email to [email protected]
