From c60622ebacb432cb12008b4df027ed12451da0c1 Mon Sep 17 00:00:00 2001 From: Wan-Teh Chang Date: Thu, 23 May 2024 14:38:50 -0700 Subject: [PATCH] Fix a UBSan error in vp9_rc_update_framerate() Fix a UBSan error in the calculation of rc->min_frame_bandwidth in vp9_rc_update_framerate(). A follow-up to https://chromium-review.googlesource.com/c/webm/libvpx/+/4944271. Similar to the libaom CL https://aomedia-review.googlesource.com/c/aom/+/190462. Bug: aomedia:3509 Change-Id: I36168a6d00cd81e60ae19a7d74c21f2e6c2f0caf (cherry picked from commit 1f65facb63c8227156b5a276e506bb44a7c10608) --- test/encode_api_test.cc | 72 ++++++++++++++++++++++++++++++++++++++ vp9/encoder/vp9_ratectrl.c | 9 ++--- 2 files changed, 77 insertions(+), 4 deletions(-) diff --git a/test/encode_api_test.cc b/test/encode_api_test.cc index af7ce4b9241..ea36ee5d71d 100644 --- a/test/encode_api_test.cc +++ b/test/encode_api_test.cc @@ -1510,6 +1510,78 @@ TEST(EncodeAPI, VP9GlobalHeaders) { } } +TEST(EncodeAPI, AomediaIssue3509VbrMinSection2Percent) { + // Initialize libvpx encoder. + vpx_codec_iface_t *const iface = vpx_codec_vp9_cx(); + vpx_codec_ctx_t enc; + vpx_codec_enc_cfg_t cfg; + + ASSERT_EQ(vpx_codec_enc_config_default(iface, &cfg, 0), VPX_CODEC_OK); + + cfg.g_w = 1920; + cfg.g_h = 1080; + cfg.g_lag_in_frames = 0; + cfg.rc_target_bitrate = 1000000; + // Set this to more than 1 percent to cause a signed integer overflow in the + // multiplication rc->avg_frame_bandwidth * oxcf->rc_cfg.vbrmin_section in + // vp9_rc_update_framerate() if the multiplication is done in the `int` type. + cfg.rc_2pass_vbr_minsection_pct = 2; + + ASSERT_EQ(vpx_codec_enc_init(&enc, iface, &cfg, 0), VPX_CODEC_OK); + + // Create input image. + vpx_image_t *const image = + CreateImage(VPX_BITS_8, VPX_IMG_FMT_I420, cfg.g_w, cfg.g_h); + ASSERT_NE(image, nullptr); + + // Encode frame. + // `duration` can go as high as 300, but the UBSan error is gone if + // `duration` is 301 or higher. + ASSERT_EQ( + vpx_codec_encode(&enc, image, 0, /*duration=*/300, 0, VPX_DL_REALTIME), + VPX_CODEC_OK); + + // Free resources. + vpx_img_free(image); + ASSERT_EQ(vpx_codec_destroy(&enc), VPX_CODEC_OK); +} + +TEST(EncodeAPI, AomediaIssue3509VbrMinSection101Percent) { + // Initialize libvpx encoder. + vpx_codec_iface_t *const iface = vpx_codec_vp9_cx(); + vpx_codec_ctx_t enc; + vpx_codec_enc_cfg_t cfg; + + ASSERT_EQ(vpx_codec_enc_config_default(iface, &cfg, 0), VPX_CODEC_OK); + + cfg.g_w = 1920; + cfg.g_h = 1080; + cfg.g_lag_in_frames = 0; + cfg.rc_target_bitrate = 1000000; + // Set this to more than 100 percent to cause an error when vbr_min_bits is + // cast to `int` in vp9_rc_update_framerate() if vbr_min_bits is not clamped + // to INT_MAX. + cfg.rc_2pass_vbr_minsection_pct = 101; + + ASSERT_EQ(vpx_codec_enc_init(&enc, iface, &cfg, 0), VPX_CODEC_OK); + + // Create input image. + vpx_image_t *const image = + CreateImage(VPX_BITS_8, VPX_IMG_FMT_I420, cfg.g_w, cfg.g_h); + ASSERT_NE(image, nullptr); + + // Encode frame. + // `duration` can go as high as 300, but the UBSan error is gone if + // `duration` is 301 or higher. + ASSERT_EQ( + vpx_codec_encode(&enc, image, 0, /*duration=*/300, 0, VPX_DL_REALTIME), + VPX_CODEC_OK); + + // Free resources. + vpx_img_free(image); + ASSERT_EQ(vpx_codec_destroy(&enc), VPX_CODEC_OK); +} + #endif // CONFIG_VP9_ENCODER } // namespace diff --git a/vp9/encoder/vp9_ratectrl.c b/vp9/encoder/vp9_ratectrl.c index 6452e349df2..44f52f96be4 100644 --- a/vp9/encoder/vp9_ratectrl.c +++ b/vp9/encoder/vp9_ratectrl.c @@ -2651,11 +2651,12 @@ void vp9_rc_update_framerate(VP9_COMP *cpi) { rc->avg_frame_bandwidth = (int)VPXMIN(oxcf->target_bandwidth / cpi->framerate, INT_MAX); - rc->min_frame_bandwidth = - (int)(rc->avg_frame_bandwidth * oxcf->two_pass_vbrmin_section / 100); - rc->min_frame_bandwidth = - VPXMAX(rc->min_frame_bandwidth, FRAME_OVERHEAD_BITS); + int64_t vbr_min_bits = + (int64_t)rc->avg_frame_bandwidth * oxcf->two_pass_vbrmin_section / 100; + vbr_min_bits = VPXMIN(vbr_min_bits, INT_MAX); + + rc->min_frame_bandwidth = VPXMAX((int)vbr_min_bits, FRAME_OVERHEAD_BITS); // A maximum bitrate for a frame is defined. // However this limit is extended if a very high rate is given on the command