Fix a bug where an uninitalized search_site is used BUG=aomedia:3348 Change-Id: I0a7c84b61f6a4588b1eda63fff259fa158ddb58d (cherry picked from commit 945edd6712eb8bc61f520a5dec03cc6ed489e01c)
diff --git a/av1/encoder/mcomp.c b/av1/encoder/mcomp.c index 4c2eb21..f8f8d8d 100644 --- a/av1/encoder/mcomp.c +++ b/av1/encoder/mcomp.c
@@ -94,11 +94,10 @@ void av1_make_default_fullpel_ms_params( FULLPEL_MOTION_SEARCH_PARAMS *ms_params, const struct AV1_COMP *cpi, - const MACROBLOCK *x, BLOCK_SIZE bsize, const MV *ref_mv, + MACROBLOCK *x, BLOCK_SIZE bsize, const MV *ref_mv, const search_site_config search_sites[NUM_DISTINCT_SEARCH_METHODS], int fine_search_interval) { const MV_SPEED_FEATURES *mv_sf = &cpi->sf.mv_sf; - const int sf_blk_search_method = mv_sf->use_bsize_dependent_search_method; // High level params ms_params->bsize = bsize; @@ -107,14 +106,25 @@ init_ms_buffers(&ms_params->ms_buffers, x); SEARCH_METHODS search_method = mv_sf->search_method; + const int sf_blk_search_method = mv_sf->use_bsize_dependent_search_method; const int min_dim = AOMMIN(block_size_wide[bsize], block_size_high[bsize]); - if (sf_blk_search_method >= 2) { - const int qband = x->qindex >> (QINDEX_BITS - 2); - if (min_dim >= 16 && x->content_state_sb.source_sad_nonrd <= kMedSad && - qband < 3) - search_method = get_faster_search_method(search_method); - } else if (sf_blk_search_method >= 1 && min_dim >= 32) { + const int qband = x->qindex >> (QINDEX_BITS - 2); + const bool use_faster_search_method = + (sf_blk_search_method == 1 && min_dim >= 32) || + (sf_blk_search_method >= 2 && min_dim >= 16 && + x->content_state_sb.source_sad_nonrd <= kMedSad && qband < 3); + + if (use_faster_search_method) { search_method = get_faster_search_method(search_method); + + // We might need to update the search site config since search_method + // is changed here. + const int ref_stride = ms_params->ms_buffers.ref->stride; + if (ref_stride != search_sites[search_method].stride) { + av1_refresh_search_site_config(x->search_site_cfg_buf, search_method, + ref_stride); + search_sites = x->search_site_cfg_buf; + } } av1_set_mv_search_method(ms_params, search_sites, search_method);
diff --git a/av1/encoder/mcomp.h b/av1/encoder/mcomp.h index 5aba8d9..1e8bbab 100644 --- a/av1/encoder/mcomp.h +++ b/av1/encoder/mcomp.h
@@ -144,7 +144,7 @@ void av1_make_default_fullpel_ms_params( FULLPEL_MOTION_SEARCH_PARAMS *ms_params, const struct AV1_COMP *cpi, - const MACROBLOCK *x, BLOCK_SIZE bsize, const MV *ref_mv, + MACROBLOCK *x, BLOCK_SIZE bsize, const MV *ref_mv, const search_site_config search_sites[NUM_DISTINCT_SEARCH_METHODS], int fine_search_interval); @@ -201,6 +201,17 @@ BIGDIA // VFAST_DIAMOND }; +// Reinitialize the search site config. +static AOM_INLINE void av1_refresh_search_site_config( + search_site_config *ss_cfg_buf, SEARCH_METHODS search_method, + const int ref_stride) { + const int level = + search_method == NSTEP_8PT || search_method == CLAMPED_DIAMOND; + search_method = search_method_lookup[search_method]; + av1_init_motion_compensation[search_method](&ss_cfg_buf[search_method], + ref_stride, level); +} + // Mv beyond the range do not produce new/different prediction block. static INLINE void av1_set_mv_search_method( FULLPEL_MOTION_SEARCH_PARAMS *ms_params,
diff --git a/av1/encoder/motion_search_facade.c b/av1/encoder/motion_search_facade.c index 2a2ad2e..81232ef 100644 --- a/av1/encoder/motion_search_facade.c +++ b/av1/encoder/motion_search_facade.c
@@ -221,9 +221,8 @@ // MotionVectorSearchParams::search_site_cfg. When this happens, we need to // readjust the stride. const SEARCH_METHODS search_method = cpi->sf.mv_sf.search_method; - const int ref_stride = xd->plane[0].pre[0].stride; - const search_site_config *src_search_site_cfg = av1_get_search_site_config( - x->search_site_cfg_buf, mv_search_params, search_method, ref_stride); + const search_site_config *src_search_site_cfg = + av1_get_search_site_config(cpi, x, search_method); // Further reduce the search range. if (search_range < INT_MAX) { @@ -597,10 +596,8 @@ // Make motion search params FULLPEL_MOTION_SEARCH_PARAMS full_ms_params; const SEARCH_METHODS search_method = cpi->sf.mv_sf.search_method; - const int ref_stride = xd->plane[0].pre[0].stride; - const search_site_config *src_search_sites = av1_get_search_site_config( - x->search_site_cfg_buf, &cpi->mv_search_params, search_method, - ref_stride); + const search_site_config *src_search_sites = + av1_get_search_site_config(cpi, x, search_method); av1_make_default_fullpel_ms_params(&full_ms_params, cpi, x, bsize, &ref_mv[id].as_mv, src_search_sites, /*fine_search_interval=*/0); @@ -748,10 +745,8 @@ // Make motion search params FULLPEL_MOTION_SEARCH_PARAMS full_ms_params; const SEARCH_METHODS search_method = cpi->sf.mv_sf.search_method; - const int ref_stride = xd->plane[0].pre[0].stride; const search_site_config *src_search_sites = - av1_get_search_site_config(x->search_site_cfg_buf, &cpi->mv_search_params, - search_method, ref_stride); + av1_get_search_site_config(cpi, x, search_method); av1_make_default_fullpel_ms_params(&full_ms_params, cpi, x, bsize, &ref_mv.as_mv, src_search_sites, /*fine_search_interval=*/0); @@ -970,10 +965,8 @@ const int fine_search_interval = use_fine_search_interval(cpi); FULLPEL_MOTION_SEARCH_PARAMS full_ms_params; const SEARCH_METHODS search_method = cpi->sf.mv_sf.search_method; - const int ref_stride = xd->plane[0].pre[0].stride; const search_site_config *src_search_sites = - av1_get_search_site_config(x->search_site_cfg_buf, &cpi->mv_search_params, - search_method, ref_stride); + av1_get_search_site_config(cpi, x, search_method); av1_make_default_fullpel_ms_params(&full_ms_params, cpi, x, bsize, &ref_mv, src_search_sites, fine_search_interval);
diff --git a/av1/encoder/motion_search_facade.h b/av1/encoder/motion_search_facade.h index bc69a65..4d76287 100644 --- a/av1/encoder/motion_search_facade.h +++ b/av1/encoder/motion_search_facade.h
@@ -71,9 +71,13 @@ unsigned int *sse, unsigned int *var); static AOM_INLINE const search_site_config *av1_get_search_site_config( - search_site_config *ss_cfg_buf, - const MotionVectorSearchParams *mv_search_params, - SEARCH_METHODS search_method, const int ref_stride) { + const AV1_COMP *cpi, MACROBLOCK *x, SEARCH_METHODS search_method) { + const int ref_stride = x->e_mbd.plane[0].pre[0].stride; + + // AV1_COMP::mv_search_params.search_site_config is a compressor level cache + // that's shared by multiple threads. In most cases where all frames have the + // same resolution, the cache contains the search site config that we need. + const MotionVectorSearchParams *mv_search_params = &cpi->mv_search_params; if (ref_stride == mv_search_params->search_site_cfg[SS_CFG_SRC]->stride) { return mv_search_params->search_site_cfg[SS_CFG_SRC]; } else if (ref_stride == @@ -81,15 +85,18 @@ return mv_search_params->search_site_cfg[SS_CFG_LOOKAHEAD]; } - if (ref_stride != ss_cfg_buf[search_method].stride) { - const int level = - search_method == NSTEP_8PT || search_method == CLAMPED_DIAMOND; - search_method = search_method_lookup[search_method]; - av1_init_motion_compensation[search_method](&ss_cfg_buf[search_method], - ref_stride, level); + // If the cache does not contain the correct stride, then we will need to rely + // on the thread level config MACROBLOCK::search_site_cfg_buf. If even the + // thread level config doesn't match, then we need to update it. + search_method = search_method_lookup[search_method]; + assert(search_method_lookup[search_method] == search_method && + "The search_method_lookup table should be idempotent."); + if (ref_stride != x->search_site_cfg_buf[search_method].stride) { + av1_refresh_search_site_config(x->search_site_cfg_buf, search_method, + ref_stride); } - return ss_cfg_buf; + return x->search_site_cfg_buf; } #ifdef __cplusplus
diff --git a/av1/encoder/nonrd_pickmode.c b/av1/encoder/nonrd_pickmode.c index 4e6b3a9..9c14bb1 100644 --- a/av1/encoder/nonrd_pickmode.c +++ b/av1/encoder/nonrd_pickmode.c
@@ -449,10 +449,8 @@ center_mv = tmp_mv->as_mv; const SEARCH_METHODS search_method = sf->mv_sf.search_method; - const MotionVectorSearchParams *mv_search_params = &cpi->mv_search_params; - const int ref_stride = xd->plane[0].pre[0].stride; - const search_site_config *src_search_sites = av1_get_search_site_config( - x->search_site_cfg_buf, mv_search_params, search_method, ref_stride); + const search_site_config *src_search_sites = + av1_get_search_site_config(cpi, x, search_method); FULLPEL_MOTION_SEARCH_PARAMS full_ms_params; av1_make_default_fullpel_ms_params(&full_ms_params, cpi, x, bsize, ¢er_mv, src_search_sites,
diff --git a/av1/encoder/temporal_filter.c b/av1/encoder/temporal_filter.c index db2f098..4845c5d 100644 --- a/av1/encoder/temporal_filter.c +++ b/av1/encoder/temporal_filter.c
@@ -109,9 +109,6 @@ // Parameters used for motion search. FULLPEL_MOTION_SEARCH_PARAMS full_ms_params; SUBPEL_MOTION_SEARCH_PARAMS ms_params; - const SEARCH_METHODS search_method = NSTEP; - const search_site_config *search_site_cfg = av1_get_search_site_config( - mb->search_site_cfg_buf, &cpi->mv_search_params, search_method, y_stride); const int step_param = av1_init_search_range( AOMMAX(frame_to_filter->y_crop_width, frame_to_filter->y_crop_height)); const SUBPEL_SEARCH_TYPE subpel_search_type = USE_8_TAPS; @@ -131,6 +128,11 @@ mb->plane[0].src.stride = y_stride; mbd->plane[0].pre[0].buf = ref_frame->y_buffer + y_offset; mbd->plane[0].pre[0].stride = y_stride; + + const SEARCH_METHODS search_method = NSTEP; + const search_site_config *search_site_cfg = + av1_get_search_site_config(cpi, mb, search_method); + // Unused intermediate results for motion search. unsigned int sse, error; int distortion;
diff --git a/test/avif_progressive_test.cc b/test/avif_progressive_test.cc index 457891c..cf94fd5 100644 --- a/test/avif_progressive_test.cc +++ b/test/avif_progressive_test.cc
@@ -103,9 +103,7 @@ // This test emulates how libavif calls libaom functions to encode a // progressive AVIF image in libavif's ProgressiveTest.DimensionChange test. -// TODO(https://crbug.com/aomedia/3348): Fix the assertion failure at -// av1/encoder/mcomp.c:1760 and enable this test. -TEST(AVIFProgressiveTest, DISABLED_DimensionChange) { +TEST(AVIFProgressiveTest, DimensionChange) { constexpr int kWidth = 256; constexpr int kHeight = 256; // Dummy buffer of neutral gray samples.