Commit 703b9a7181 for aom
commit 703b9a7181be3dc4eb59d16ed97f2d55525b8dc2
Author: Marco Paniconi <marpan@google.com>
Date: Fri Apr 10 12:08:57 2026 -0700
Fix to OOB heap reads in rc_scene_detection
Remove the variables svc->mi_cols/rows_full_resoln
and directly set the num_mi_cols/rows in the
scene detection. This avoids issues with dynamic
layers changes and frame dropping, where the
svc->mi_cols/rows_full_resoln were not updated
properly.
Also fix a memory leak due to dynamic change in
number of layers with cyclic_refresh, triggered
by the unittests added for this issue. Fix is to make
sure the cyclic_refresh->map is cleared when the
number of layers change.
The unittest comes from the poc in the issue below.
Bug: 499606109
Change-Id: Ibec15766720e123f516028f5346eed48ae35194e
diff --git a/av1/av1_cx_iface.c b/av1/av1_cx_iface.c
index f9adad4099..e726da605d 100644
--- a/av1/av1_cx_iface.c
+++ b/av1/av1_cx_iface.c
@@ -4069,6 +4069,7 @@ static aom_codec_err_t ctrl_set_svc_params(aom_codec_alg_priv_t *ctx,
ppi->number_spatial_layers * ppi->number_temporal_layers - 1;
ctx->next_frame_flags |= AOM_EFLAG_FORCE_KF;
av1_set_svc_seq_params(ppi);
+ av1_free_svc_cyclic_refresh(cpi);
// Check for valid values for the spatial/temporal_layer_id here, since
// there has been a dynamic change in the number_spatial/temporal_layers,
// and if the ctrl_set_layer_id is not used after this call, the
diff --git a/av1/encoder/ratectrl.c b/av1/encoder/ratectrl.c
index 646219e6fb..c4c0e92d3d 100644
--- a/av1/encoder/ratectrl.c
+++ b/av1/encoder/ratectrl.c
@@ -3318,8 +3318,8 @@ void av1_rc_scene_detection_onepass_rt(AV1_COMP *cpi,
int num_mi_cols = cm->mi_params.mi_cols;
int num_mi_rows = cm->mi_params.mi_rows;
if (cpi->svc.number_spatial_layers > 1) {
- num_mi_cols = cpi->svc.mi_cols_full_resoln;
- num_mi_rows = cpi->svc.mi_rows_full_resoln;
+ num_mi_cols = size_in_mi(src_width);
+ num_mi_rows = size_in_mi(src_height);
}
int num_zero_temp_sad = 0;
uint32_t min_thresh =
diff --git a/av1/encoder/svc_layercontext.c b/av1/encoder/svc_layercontext.c
index 7ac5d5a9a7..5d8de75642 100644
--- a/av1/encoder/svc_layercontext.c
+++ b/av1/encoder/svc_layercontext.c
@@ -66,7 +66,8 @@ void av1_init_layer_context(AV1_COMP *const cpi) {
// Initialize the cyclic refresh parameters. If spatial layers are used
// (i.e., ss_number_layers > 1), these need to be updated per spatial
// layer. Cyclic refresh is only applied on base temporal layer.
- if (svc->number_spatial_layers > 1 && tl == 0) {
+ if (svc->number_spatial_layers > 1 && tl == 0 &&
+ cpi->oxcf.q_cfg.aq_mode == CYCLIC_REFRESH_AQ) {
lc->sb_index = 0;
lc->actual_num_seg1_blocks = 0;
lc->actual_num_seg2_blocks = 0;
@@ -89,6 +90,7 @@ void av1_init_layer_context(AV1_COMP *const cpi) {
bool av1_alloc_layer_context(AV1_COMP *cpi, int num_layers) {
SVC *const svc = &cpi->svc;
if (svc->layer_context == NULL || svc->num_allocated_layers < num_layers) {
+ av1_free_svc_cyclic_refresh(cpi);
assert(num_layers > 1);
aom_free(svc->layer_context);
svc->num_allocated_layers = 0;
@@ -153,6 +155,7 @@ void av1_update_layer_context_change_config(AV1_COMP *const cpi,
// or number of spatial layers has changed.
// Cyclic refresh is only applied on base temporal layer.
if (svc->number_spatial_layers > 1 && tl == 0 &&
+ cpi->oxcf.q_cfg.aq_mode == CYCLIC_REFRESH_AQ &&
(lc->map == NULL ||
svc->prev_number_spatial_layers != svc->number_spatial_layers)) {
lc->sb_index = 0;
@@ -385,10 +388,10 @@ int av1_svc_primary_ref_frame(const AV1_COMP *const cpi) {
void av1_free_svc_cyclic_refresh(AV1_COMP *const cpi) {
SVC *const svc = &cpi->svc;
- for (int sl = 0; sl < svc->number_spatial_layers; ++sl) {
- for (int tl = 0; tl < svc->number_temporal_layers; ++tl) {
- int layer = LAYER_IDS_TO_IDX(sl, tl, svc->number_temporal_layers);
- LAYER_CONTEXT *const lc = &svc->layer_context[layer];
+ if (svc->layer_context == NULL) return;
+ for (int i = 0; i < svc->num_allocated_layers; ++i) {
+ LAYER_CONTEXT *const lc = &svc->layer_context[i];
+ if (lc->map) {
aom_free(lc->map);
lc->map = NULL;
}
@@ -455,10 +458,6 @@ void av1_one_pass_cbr_svc_start_layer(AV1_COMP *const cpi) {
cm->height = height;
alloc_mb_mode_info_buffers(cpi);
av1_update_frame_size(cpi);
- if (svc->spatial_layer_id == svc->number_spatial_layers - 1) {
- svc->mi_cols_full_resoln = cm->mi_params.mi_cols;
- svc->mi_rows_full_resoln = cm->mi_params.mi_rows;
- }
}
enum {
diff --git a/av1/encoder/svc_layercontext.h b/av1/encoder/svc_layercontext.h
index fdf17480ea..ff052f25e7 100644
--- a/av1/encoder/svc_layercontext.h
+++ b/av1/encoder/svc_layercontext.h
@@ -109,8 +109,6 @@ typedef struct SVC {
int num_encoded_top_layer;
int first_layer_denoise;
YV12_BUFFER_CONFIG source_last_TL0;
- int mi_cols_full_resoln;
- int mi_rows_full_resoln;
/*!\endcond */
/*!
diff --git a/av1/ratectrl_rtc.cc b/av1/ratectrl_rtc.cc
index 9d176e582a..363521eabc 100644
--- a/av1/ratectrl_rtc.cc
+++ b/av1/ratectrl_rtc.cc
@@ -204,6 +204,9 @@ bool AV1RateControlRTC::UpdateRateControl(
rc_cfg.ts_number_layers > AOM_MAX_TS_LAYERS) {
return false;
}
+ if (cpi_->svc.number_spatial_layers != rc_cfg.ss_number_layers ||
+ cpi_->svc.number_temporal_layers != rc_cfg.ts_number_layers)
+ av1_free_svc_cyclic_refresh(cpi_);
const int num_layers = rc_cfg.ss_number_layers * rc_cfg.ts_number_layers;
if (num_layers > 1 && !av1_alloc_layer_context(cpi_, num_layers)) {
return false;
diff --git a/test/encode_api_test.cc b/test/encode_api_test.cc
index 10f0f39e29..5402644c78 100644
--- a/test/encode_api_test.cc
+++ b/test/encode_api_test.cc
@@ -46,6 +46,42 @@ static void *Memset16(void *dest, int val, size_t length) {
return dest;
}
+static void FillImage(aom_image_t *img, uint8_t val) {
+ for (int p = 0; p < 3; ++p) {
+ uint8_t *buf = img->planes[p];
+ const int h = p == 0 ? (int)img->d_h : (int)((img->d_h + 1) / 2);
+ const int w = p == 0 ? (int)img->d_w : (int)((img->d_w + 1) / 2);
+ for (int r = 0; r < h; ++r) {
+ memset(buf, val, w);
+ buf += img->stride[p];
+ }
+ }
+}
+
+static void SetSvc(aom_codec_ctx_t *codec, int num_sl, const int *num,
+ const int *den, const int *bitrates) {
+ aom_svc_params_t sp = {};
+ sp.number_spatial_layers = num_sl;
+ sp.number_temporal_layers = 1;
+ sp.framerate_factor[0] = 1;
+ for (int i = 0; i < num_sl; ++i) {
+ sp.scaling_factor_num[i] = num[i];
+ sp.scaling_factor_den[i] = den[i];
+ sp.max_quantizers[i] = 56;
+ sp.min_quantizers[i] = 2;
+ sp.layer_target_bitrate[i] = bitrates[i];
+ }
+ ASSERT_EQ(aom_codec_control(codec, AV1E_SET_SVC_PARAMS, &sp), AOM_CODEC_OK);
+}
+
+static void EncodeOne(aom_codec_ctx_t *codec, aom_image_t *img,
+ aom_codec_pts_t pts) {
+ ASSERT_EQ(aom_codec_encode(codec, img, pts, 1, 0), AOM_CODEC_OK);
+ aom_codec_iter_t iter = NULL;
+ while (aom_codec_get_cx_data(codec, &iter) != NULL) {
+ }
+}
+
TEST(EncodeAPI, InvalidParams) {
uint8_t buf[1] = { 0 };
aom_image_t img;
@@ -2001,4 +2037,207 @@ TEST(EncodeAPI, SizeAlignOverflow) {
ASSERT_EQ(aom_codec_destroy(&enc), AOM_CODEC_OK);
}
+TEST(EncodeAPI, DynamicSvcAq0Issue499606109) {
+ aom_codec_iface_t *iface = aom_codec_av1_cx();
+ aom_codec_enc_cfg_t cfg;
+ ASSERT_EQ(aom_codec_enc_config_default(iface, &cfg, AOM_USAGE_REALTIME),
+ AOM_CODEC_OK);
+
+ // Init at 1920x1080.
+ cfg.g_w = 1920;
+ cfg.g_h = 1080;
+ cfg.g_timebase.num = 1;
+ cfg.g_timebase.den = 30;
+ cfg.rc_end_usage = AOM_CBR;
+ cfg.rc_target_bitrate = 2000;
+ cfg.g_lag_in_frames = 0;
+ cfg.g_error_resilient = 1;
+ cfg.g_threads = 1;
+ cfg.kf_max_dist = 3000;
+ cfg.kf_min_dist = 3000;
+ cfg.rc_dropframe_thresh = 30;
+
+ aom_codec_ctx_t codec;
+ ASSERT_EQ(aom_codec_enc_init(&codec, iface, &cfg, 0), AOM_CODEC_OK);
+ ASSERT_EQ(aom_codec_control(&codec, AOME_SET_CPUUSED, 10), AOM_CODEC_OK);
+ ASSERT_EQ(
+ aom_codec_control(&codec, AV1E_SET_MAX_CONSEC_FRAME_DROP_MS_CBR, 34),
+ AOM_CODEC_OK);
+
+ // Phase 0: shrink to 640x360, 2 SLs. Frame 0 allocates
+ // source_last_TL0 at 640x360 (small buffer).
+ cfg.g_w = 640;
+ cfg.g_h = 360;
+ cfg.rc_target_bitrate = 500;
+ ASSERT_EQ(aom_codec_enc_config_set(&codec, &cfg), AOM_CODEC_OK);
+ {
+ const int num[] = { 1, 1 }, den[] = { 1, 1 }, br[] = { 200, 300 };
+ SetSvc(&codec, 2, num, den, br);
+ }
+ aom_codec_pts_t pts = 0;
+ aom_image_t *raw = aom_img_alloc(nullptr, AOM_IMG_FMT_I420, 640, 360, 1);
+ ASSERT_NE(raw, nullptr);
+ for (int f = 0; f < 3; ++f) {
+ for (int sl = 0; sl < 2; ++sl) {
+ aom_svc_layer_id_t lid = { sl, 0 };
+ ASSERT_EQ(aom_codec_control(&codec, AV1E_SET_SVC_LAYER_ID, &lid),
+ AOM_CODEC_OK);
+ FillImage(raw, static_cast<uint8_t>(128 + f));
+ EncodeOne(&codec, raw, pts++);
+ }
+ }
+ aom_img_free(raw);
+
+ // Phase 1: grow to 1920x1080, 3 SLs. SL2 encoding sets
+ // mi_cols_full_resoln = 480, mi_rows_full_resoln = 270.
+ cfg.g_w = 1920;
+ cfg.g_h = 1080;
+ cfg.rc_target_bitrate = 2000;
+ ASSERT_EQ(aom_codec_enc_config_set(&codec, &cfg), AOM_CODEC_OK);
+ {
+ const int num[] = { 1, 2, 1 }, den[] = { 3, 3, 1 },
+ br[] = { 200, 500, 1300 };
+ SetSvc(&codec, 3, num, den, br);
+ }
+ raw = aom_img_alloc(nullptr, AOM_IMG_FMT_I420, 1920, 1080, 1);
+ ASSERT_NE(raw, nullptr);
+ for (int f = 0; f < 3; ++f) {
+ for (int sl = 0; sl < 3; ++sl) {
+ aom_svc_layer_id_t lid = { sl, 0 };
+ ASSERT_EQ(aom_codec_control(&codec, AV1E_SET_SVC_LAYER_ID, &lid),
+ AOM_CODEC_OK);
+ FillImage(raw, static_cast<uint8_t>(64 + f));
+ EncodeOne(&codec, raw, pts++);
+ }
+ }
+ aom_img_free(raw);
+
+ // Phase 2: back to 640x360, 2 SLs. Encode SL0 only with mi_cols
+ // stays stale. Very low SL0 bitrate forces a frame drop.
+ // After the drop, next SL0 gets last_source = source_last_TL0
+ // (640x360 small buffer). SAD loop overruns it with OOB. */
+ cfg.g_w = 640;
+ cfg.g_h = 360;
+ cfg.rc_target_bitrate = 500;
+ ASSERT_EQ(aom_codec_enc_config_set(&codec, &cfg), AOM_CODEC_OK);
+ {
+ const int num[] = { 1, 1 }, den[] = { 1, 1 }, br[] = { 1, 499 };
+ SetSvc(&codec, 2, num, den, br);
+ }
+ raw = aom_img_alloc(nullptr, AOM_IMG_FMT_I420, 640, 360, 1);
+ ASSERT_NE(raw, nullptr);
+ for (int f = 0; f < 20; ++f) {
+ aom_svc_layer_id_t lid = { 0, 0 };
+ ASSERT_EQ(aom_codec_control(&codec, AV1E_SET_SVC_LAYER_ID, &lid),
+ AOM_CODEC_OK);
+ FillImage(raw, static_cast<uint8_t>((f % 2 == 0) ? 16 : 235));
+ EncodeOne(&codec, raw, pts++);
+ }
+
+ aom_img_free(raw);
+ ASSERT_EQ(aom_codec_destroy(&codec), AOM_CODEC_OK);
+}
+
+TEST(EncodeAPI, DynamicSvcAq3Issue499606109) {
+ aom_codec_iface_t *iface = aom_codec_av1_cx();
+ aom_codec_enc_cfg_t cfg;
+ ASSERT_EQ(aom_codec_enc_config_default(iface, &cfg, AOM_USAGE_REALTIME),
+ AOM_CODEC_OK);
+
+ // Init at 1920x1080.
+ cfg.g_w = 1920;
+ cfg.g_h = 1080;
+ cfg.g_timebase.num = 1;
+ cfg.g_timebase.den = 30;
+ cfg.rc_end_usage = AOM_CBR;
+ cfg.rc_target_bitrate = 2000;
+ cfg.g_lag_in_frames = 0;
+ cfg.g_error_resilient = 1;
+ cfg.g_threads = 1;
+ cfg.kf_max_dist = 3000;
+ cfg.kf_min_dist = 3000;
+ cfg.rc_dropframe_thresh = 30;
+
+ aom_codec_ctx_t codec;
+ ASSERT_EQ(aom_codec_enc_init(&codec, iface, &cfg, 0), AOM_CODEC_OK);
+ ASSERT_EQ(aom_codec_control(&codec, AOME_SET_CPUUSED, 10), AOM_CODEC_OK);
+ ASSERT_EQ(
+ aom_codec_control(&codec, AV1E_SET_MAX_CONSEC_FRAME_DROP_MS_CBR, 34),
+ AOM_CODEC_OK);
+ ASSERT_EQ(aom_codec_control(&codec, AV1E_SET_AQ_MODE, 3), AOM_CODEC_OK);
+
+ // Phase 0: shrink to 640x360, 2 SLs. Frame 0 allocates
+ // source_last_TL0 at 640x360 (small buffer).
+ cfg.g_w = 640;
+ cfg.g_h = 360;
+ cfg.rc_target_bitrate = 500;
+ ASSERT_EQ(aom_codec_enc_config_set(&codec, &cfg), AOM_CODEC_OK);
+ {
+ const int num[] = { 1, 1 }, den[] = { 1, 1 }, br[] = { 200, 300 };
+ SetSvc(&codec, 2, num, den, br);
+ }
+ aom_codec_pts_t pts = 0;
+ aom_image_t *raw = aom_img_alloc(nullptr, AOM_IMG_FMT_I420, 640, 360, 1);
+ ASSERT_NE(raw, nullptr);
+ for (int f = 0; f < 3; ++f) {
+ for (int sl = 0; sl < 2; ++sl) {
+ aom_svc_layer_id_t lid = { sl, 0 };
+ ASSERT_EQ(aom_codec_control(&codec, AV1E_SET_SVC_LAYER_ID, &lid),
+ AOM_CODEC_OK);
+ FillImage(raw, static_cast<uint8_t>(128 + f));
+ EncodeOne(&codec, raw, pts++);
+ }
+ }
+ aom_img_free(raw);
+
+ // Phase 1: grow to 1920x1080, 3 SLs. SL2 encoding sets
+ // mi_cols_full_resoln = 480, mi_rows_full_resoln = 270.
+ cfg.g_w = 1920;
+ cfg.g_h = 1080;
+ cfg.rc_target_bitrate = 2000;
+ ASSERT_EQ(aom_codec_enc_config_set(&codec, &cfg), AOM_CODEC_OK);
+ {
+ const int num[] = { 1, 2, 1 }, den[] = { 3, 3, 1 },
+ br[] = { 200, 500, 1300 };
+ SetSvc(&codec, 3, num, den, br);
+ }
+ raw = aom_img_alloc(nullptr, AOM_IMG_FMT_I420, 1920, 1080, 1);
+ ASSERT_NE(raw, nullptr);
+ for (int f = 0; f < 3; ++f) {
+ for (int sl = 0; sl < 3; ++sl) {
+ aom_svc_layer_id_t lid = { sl, 0 };
+ ASSERT_EQ(aom_codec_control(&codec, AV1E_SET_SVC_LAYER_ID, &lid),
+ AOM_CODEC_OK);
+ FillImage(raw, static_cast<uint8_t>(64 + f));
+ EncodeOne(&codec, raw, pts++);
+ }
+ }
+ aom_img_free(raw);
+
+ // Phase 2: back to 640x360, 2 SLs. Encode SL0 only with mi_cols
+ // stays stale. Very low SL0 bitrate forces a frame drop.
+ // After the drop, next SL0 gets last_source = source_last_TL0
+ // (640x360 small buffer). SAD loop overruns it with OOB. */
+ cfg.g_w = 640;
+ cfg.g_h = 360;
+ cfg.rc_target_bitrate = 500;
+ ASSERT_EQ(aom_codec_enc_config_set(&codec, &cfg), AOM_CODEC_OK);
+ {
+ const int num[] = { 1, 1 }, den[] = { 1, 1 }, br[] = { 1, 499 };
+ SetSvc(&codec, 2, num, den, br);
+ }
+ raw = aom_img_alloc(nullptr, AOM_IMG_FMT_I420, 640, 360, 1);
+ ASSERT_NE(raw, nullptr);
+ for (int f = 0; f < 20; ++f) {
+ aom_svc_layer_id_t lid = { 0, 0 };
+ ASSERT_EQ(aom_codec_control(&codec, AV1E_SET_SVC_LAYER_ID, &lid),
+ AOM_CODEC_OK);
+ FillImage(raw, static_cast<uint8_t>((f % 2 == 0) ? 16 : 235));
+ EncodeOne(&codec, raw, pts++);
+ }
+
+ aom_img_free(raw);
+ ASSERT_EQ(aom_codec_destroy(&codec), AOM_CODEC_OK);
+}
+
} // namespace