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>> {};