Skip to content

Commit

Permalink
Fix a UBSan error in vp9_rc_update_framerate()
Browse files Browse the repository at this point in the history
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 1f65fac)
  • Loading branch information
wantehchang authored and jeromejj committed May 24, 2024
1 parent 18da856 commit c60622e
Show file tree
Hide file tree
Showing 2 changed files with 77 additions and 4 deletions.
72 changes: 72 additions & 0 deletions test/encode_api_test.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
9 changes: 5 additions & 4 deletions vp9/encoder/vp9_ratectrl.c
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit c60622e

Please sign in to comment.