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 f9adad4..e726da6 100644 --- a/av1/av1_cx_iface.c +++ b/av1/av1_cx_iface.c
@@ -4069,6 +4069,7 @@ 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 646219e..c4c0e92 100644 --- a/av1/encoder/ratectrl.c +++ b/av1/encoder/ratectrl.c
@@ -3318,8 +3318,8 @@ 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 7ac5d5a..5d8de75 100644 --- a/av1/encoder/svc_layercontext.c +++ b/av1/encoder/svc_layercontext.c
@@ -66,7 +66,8 @@ // 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 @@ 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 @@ // 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 @@ 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 @@ 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 fdf1748..ff052f2 100644 --- a/av1/encoder/svc_layercontext.h +++ b/av1/encoder/svc_layercontext.h
@@ -109,8 +109,6 @@ 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 9d176e5..363521e 100644 --- a/av1/ratectrl_rtc.cc +++ b/av1/ratectrl_rtc.cc
@@ -204,6 +204,9 @@ 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 10f0f39..5402644 100644 --- a/test/encode_api_test.cc +++ b/test/encode_api_test.cc
@@ -46,6 +46,42 @@ 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 @@ 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