Fix two UBSan errors in av1_rc_update_framerate()
Fix UBSan errors in the calculations of rc->avg_frame_bandwidth and
rc->min_frame_bandwidth in av1_rc_update_framerate().
Bug: aomedia:3509
Change-Id: I3e3d560444b12b4911bc2317ae32f0e3cad8a505
(cherry picked from commit eac59789e01f137d94dcb39a03eaf33db2870dec)
diff --git a/av1/encoder/ratectrl.c b/av1/encoder/ratectrl.c
index 62a4149..a10ca8a 100644
--- a/av1/encoder/ratectrl.c
+++ b/av1/encoder/ratectrl.c
@@ -2550,16 +2550,17 @@
void av1_rc_update_framerate(AV1_COMP *cpi, int width, int height) {
const AV1EncoderConfig *const oxcf = &cpi->oxcf;
RATE_CONTROL *const rc = &cpi->rc;
- int64_t vbr_max_bits;
const int MBs = av1_get_MBs(width, height);
- rc->avg_frame_bandwidth =
- (int)round(oxcf->rc_cfg.target_bandwidth / cpi->framerate);
- rc->min_frame_bandwidth =
- (int)(rc->avg_frame_bandwidth * oxcf->rc_cfg.vbrmin_section / 100);
+ const double avg_frame_bandwidth =
+ round(oxcf->rc_cfg.target_bandwidth / cpi->framerate);
+ rc->avg_frame_bandwidth = (int)AOMMIN(avg_frame_bandwidth, INT_MAX);
- rc->min_frame_bandwidth =
- AOMMAX(rc->min_frame_bandwidth, FRAME_OVERHEAD_BITS);
+ int64_t vbr_min_bits =
+ (int64_t)rc->avg_frame_bandwidth * oxcf->rc_cfg.vbrmin_section / 100;
+ vbr_min_bits = AOMMIN(vbr_min_bits, INT_MAX);
+
+ rc->min_frame_bandwidth = AOMMAX((int)vbr_min_bits, FRAME_OVERHEAD_BITS);
// A maximum bitrate for a frame is defined.
// The baseline for this aligns with HW implementations that
@@ -2568,9 +2569,9 @@
// a very high rate is given on the command line or the the rate cannnot
// be acheived because of a user specificed max q (e.g. when the user
// specifies lossless encode.
- vbr_max_bits =
- ((int64_t)rc->avg_frame_bandwidth * oxcf->rc_cfg.vbrmax_section) / 100;
- vbr_max_bits = (vbr_max_bits < INT_MAX) ? vbr_max_bits : INT_MAX;
+ int64_t vbr_max_bits =
+ (int64_t)rc->avg_frame_bandwidth * oxcf->rc_cfg.vbrmax_section / 100;
+ vbr_max_bits = AOMMIN(vbr_max_bits, INT_MAX);
rc->max_frame_bandwidth =
AOMMAX(AOMMAX((MBs * MAX_MB_RATE), MAXRATE_1080P), (int)vbr_max_bits);
diff --git a/test/encode_api_test.cc b/test/encode_api_test.cc
index 27bcbc1..379d8d6 100644
--- a/test/encode_api_test.cc
+++ b/test/encode_api_test.cc
@@ -776,6 +776,76 @@
aom_codec_destroy(&enc);
}
+TEST(EncodeAPI, AomediaIssue3509VbrMinSection2Percent) {
+ // Initialize libaom encoder.
+ aom_codec_iface_t *const iface = aom_codec_av1_cx();
+ aom_codec_ctx_t enc;
+ aom_codec_enc_cfg_t cfg;
+
+ ASSERT_EQ(aom_codec_enc_config_default(iface, &cfg, AOM_USAGE_REALTIME),
+ AOM_CODEC_OK);
+
+ cfg.g_w = 1920;
+ cfg.g_h = 1080;
+ 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
+ // av1_rc_update_framerate() if the multiplication is done in the `int` type.
+ cfg.rc_2pass_vbr_minsection_pct = 2;
+
+ ASSERT_EQ(aom_codec_enc_init(&enc, iface, &cfg, 0), AOM_CODEC_OK);
+
+ // Create input image.
+ aom_image_t *const image =
+ CreateGrayImage(AOM_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(aom_codec_encode(&enc, image, 0, /*duration=*/300, 0),
+ AOM_CODEC_OK);
+
+ // Free resources.
+ aom_img_free(image);
+ ASSERT_EQ(aom_codec_destroy(&enc), AOM_CODEC_OK);
+}
+
+TEST(EncodeAPI, AomediaIssue3509VbrMinSection101Percent) {
+ // Initialize libaom encoder.
+ aom_codec_iface_t *const iface = aom_codec_av1_cx();
+ aom_codec_ctx_t enc;
+ aom_codec_enc_cfg_t cfg;
+
+ ASSERT_EQ(aom_codec_enc_config_default(iface, &cfg, AOM_USAGE_REALTIME),
+ AOM_CODEC_OK);
+
+ cfg.g_w = 1920;
+ cfg.g_h = 1080;
+ 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 av1_rc_update_framerate() if vbr_min_bits is not clamped
+ // to INT_MAX.
+ cfg.rc_2pass_vbr_minsection_pct = 101;
+
+ ASSERT_EQ(aom_codec_enc_init(&enc, iface, &cfg, 0), AOM_CODEC_OK);
+
+ // Create input image.
+ aom_image_t *const image =
+ CreateGrayImage(AOM_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(aom_codec_encode(&enc, image, 0, /*duration=*/300, 0),
+ AOM_CODEC_OK);
+
+ // Free resources.
+ aom_img_free(image);
+ ASSERT_EQ(aom_codec_destroy(&enc), AOM_CODEC_OK);
+}
+
class EncodeAPIParameterized
: public testing::TestWithParam<std::tuple<
/*usage=*/unsigned int, /*speed=*/int, /*aq_mode=*/unsigned int>> {};
