Fix fullpel search limits (Ported from libaom commit fa59a65) Motion search is divided into a full-pixel search followed by a sub-pixel refinement step. These two steps have separate sets of limits for which motion vectors they will consider, set by the functions: fullpel: av1_set_mv_search_range() subpel: av1_set_subpel_mv_search_range() The subpel search limits are very simple - we allow ref_mv +/- 1023 pixels, with some appropriate clamping. For fullpel search, it appears that the intended logic was to allow any full-pixel MV which lies within the subpel limits. Put another way, the fullpel limits should be the same as the subpel limits, but pushed inwards to full-pixel values. The code in av1_set_mv_search_range almost does this, but the rounding isn't quite right. This sometimes causes the limits to be off by 1 pixel. That allows the fullpel search to select a motion vector which is outside of the subpel limits. That MV can then be carried through the subpixel search. This does not truly break anything, since the range of MV diffs that we can encode is larger than what we search. But it's definitely not intended, and in rare cases could trip an assertion in av1_refine_warped_mv, as seen in the linked bug report. By fixing the rounding, we can remove this discrepancy and prevent the assertion failure. It also allows one extra row/column of fullpel motion vectors which are inside the subpel limits, but which were not being searched. Also add assertions in many places to check that this issue does not reoccur. STATS_CHANGED Bug: aomedia:3384
diff --git a/av1/encoder/mcomp.c b/av1/encoder/mcomp.c index ff23048..479b0aa 100644 --- a/av1/encoder/mcomp.c +++ b/av1/encoder/mcomp.c
@@ -342,8 +342,6 @@ : 0; const int max_full_mv = av1_lower_mv_limit(MAX_FULL_PEL_VAL, prec_shift); - const int mv_low = av1_lower_mv_limit(GET_MV_RAWPEL(MV_LOW + 1), prec_shift); - const int mv_upp = av1_lower_mv_limit(GET_MV_RAWPEL(MV_UPP - 1), prec_shift); // Producing the reference mv value to the target precision FULLPEL_MV full_ref_mv = get_fullmv_from_mv(mv); @@ -351,18 +349,23 @@ GET_MV_SUBPEL(full_ref_mv.col) }; lower_mv_precision(&low_prec_mv, pb_mv_precision); - // generating min/max value based on differences - int col_min = GET_MV_RAWPEL(low_prec_mv.col) - max_full_mv + - (low_prec_mv.col & 7 ? 1 : 0); - int row_min = GET_MV_RAWPEL(low_prec_mv.row) - max_full_mv + - (low_prec_mv.row & 7 ? 1 : 0); - int col_max = GET_MV_RAWPEL(low_prec_mv.col) + max_full_mv; - int row_max = GET_MV_RAWPEL(low_prec_mv.row) + max_full_mv; + // Calculate the outermost full-pixel MVs which are inside the limits set by + // av1_set_subpel_mv_search_range(). + // + // The subpel limits are simply mv->col +/- 8*MAX_FULL_PEL_VAL, and similar + // for mv->row. We can then divide by 8 to find the fullpel MV limits. But + // we have to be careful about the rounding. We want these bounds to be + // at least as tight as the subpel limits, which means that we must round + // the minimum values up and the maximum values down when dividing. + int col_min = ((low_prec_mv.col + 7) >> 3) - max_full_mv; + int row_min = ((low_prec_mv.row + 7) >> 3) - max_full_mv; + int col_max = (low_prec_mv.col >> 3) + max_full_mv; + int row_max = (low_prec_mv.row >> 3) + max_full_mv; - col_min = AOMMAX(col_min, mv_low + (1 << prec_shift)); - row_min = AOMMAX(row_min, mv_low + (1 << prec_shift)); - col_max = AOMMIN(col_max, mv_upp - (1 << prec_shift)); - row_max = AOMMIN(row_max, mv_upp - (1 << prec_shift)); + col_min = AOMMAX(col_min, (MV_LOW >> 3) + (1 << prec_shift)); + row_min = AOMMAX(row_min, (MV_LOW >> 3) + (1 << prec_shift)); + col_max = AOMMIN(col_max, (MV_UPP >> 3) - (1 << prec_shift)); + row_max = AOMMIN(row_max, (MV_UPP >> 3) - (1 << prec_shift)); full_pel_lower_mv_precision_one_comp(&mv_limits->col_min, pb_mv_precision, 0); full_pel_lower_mv_precision_one_comp(&mv_limits->row_min, pb_mv_precision, 0); @@ -371,17 +374,15 @@ #else - int col_min = - GET_MV_RAWPEL(mv->col) - MAX_FULL_PEL_VAL + (mv->col & 7 ? 1 : 0); - int row_min = - GET_MV_RAWPEL(mv->row) - MAX_FULL_PEL_VAL + (mv->row & 7 ? 1 : 0); - int col_max = GET_MV_RAWPEL(mv->col) + MAX_FULL_PEL_VAL; - int row_max = GET_MV_RAWPEL(mv->row) + MAX_FULL_PEL_VAL; + int col_min = ((mv->col + 7) >> 3) - MAX_FULL_PEL_VAL; + int row_min = ((mv->row + 7) >> 3) - MAX_FULL_PEL_VAL; + int col_max = (mv->col >> 3) + MAX_FULL_PEL_VAL; + int row_max = (mv->row >> 3) + MAX_FULL_PEL_VAL; - col_min = AOMMAX(col_min, GET_MV_RAWPEL(MV_LOW) + 1); - row_min = AOMMAX(row_min, GET_MV_RAWPEL(MV_LOW) + 1); - col_max = AOMMIN(col_max, GET_MV_RAWPEL(MV_UPP) - 1); - row_max = AOMMIN(row_max, GET_MV_RAWPEL(MV_UPP) - 1); + col_min = AOMMAX(col_min, (MV_LOW >> 3) + 1); + row_min = AOMMAX(row_min, (MV_LOW >> 3) + 1); + col_max = AOMMIN(col_max, (MV_UPP >> 3) - 1); + row_max = AOMMIN(row_max, (MV_UPP >> 3) - 1); #endif // Get intersection of UMV window and valid MV window to reduce # of checks
diff --git a/av1/encoder/motion_search_facade.c b/av1/encoder/motion_search_facade.c index 9d334fb..5d9d4fd 100644 --- a/av1/encoder/motion_search_facade.c +++ b/av1/encoder/motion_search_facade.c
@@ -395,6 +395,7 @@ #endif cost_list); MV subpel_start_mv = get_mv_from_fullmv(&best_mv->as_fullmv); + assert(av1_is_subpelmv_in_range(&ms_params.mv_limits, subpel_start_mv)); #if CONFIG_C071_SUBBLK_WARPMV if ( #if CONFIG_FLEX_MVRES @@ -629,6 +630,7 @@ // ms_params.forced_stop = EIGHTH_PEL; MV start_mv1 = get_mv_from_fullmv(&curr_best_mv.as_fullmv); + assert(av1_is_subpelmv_in_range(&ms_params.mv_limits, start_mv1)); #if CONFIG_C071_SUBBLK_WARPMV if (pb_mv_precision >= MV_PRECISION_HALF_PEL) { start_mv1.col += sub_mv_offset.col; @@ -853,6 +855,7 @@ mask, mask_stride, id); ms_params.forced_stop = EIGHTH_PEL; MV start_mv = get_mv_from_fullmv(&best_mv.as_fullmv); + assert(av1_is_subpelmv_in_range(&ms_params.mv_limits, start_mv)); #if CONFIG_C071_SUBBLK_WARPMV if ( #if CONFIG_FLEX_MVRES @@ -1257,6 +1260,7 @@ mask, mask_stride, ref_idx); ms_params.forced_stop = EIGHTH_PEL; MV start_mv = get_mv_from_fullmv(&best_mv.as_fullmv); + assert(av1_is_subpelmv_in_range(&ms_params.mv_limits, start_mv)); #if CONFIG_C071_SUBBLK_WARPMV if ( #if CONFIG_FLEX_MVRES @@ -1567,6 +1571,7 @@ ms_params.forced_stop = cpi->sf.mv_sf.simple_motion_subpel_force_stop; MV subpel_start_mv = get_mv_from_fullmv(&best_mv.as_fullmv); + assert(av1_is_subpelmv_in_range(&ms_params.mv_limits, subpel_start_mv)); cpi->mv_search_params.find_fractional_mv_step( xd, cm, &ms_params, subpel_start_mv, &best_mv.as_mv, ¬_used, @@ -1714,6 +1719,7 @@ ms_params.forced_stop = cpi->sf.mv_sf.simple_motion_subpel_force_stop; MV subpel_start_mv = get_mv_from_fullmv(&best_mv.as_fullmv); + assert(av1_is_subpelmv_in_range(&ms_params.mv_limits, subpel_start_mv)); cpi->mv_search_params.find_fractional_mv_step( xd, cm, &ms_params, subpel_start_mv, &best_mv.as_mv, ¬_used,
diff --git a/av1/encoder/temporal_filter.c b/av1/encoder/temporal_filter.c index 913693d..f00c3e2 100644 --- a/av1/encoder/temporal_filter.c +++ b/av1/encoder/temporal_filter.c
@@ -192,6 +192,7 @@ ms_params.mv_cost_params.mv_cost_type = MV_COST_NONE; MV subpel_start_mv = get_mv_from_fullmv(&best_mv.as_fullmv); + assert(av1_is_subpelmv_in_range(&ms_params.mv_limits, subpel_start_mv)); #if CONFIG_FLEX_MVRES error = cpi->mv_search_params.find_fractional_mv_step( &mb->e_mbd, cm, &ms_params, subpel_start_mv, &best_mv.as_mv,
diff --git a/av1/encoder/tpl_model.c b/av1/encoder/tpl_model.c index e6ba250..f91ed5b 100644 --- a/av1/encoder/tpl_model.c +++ b/av1/encoder/tpl_model.c
@@ -196,6 +196,7 @@ ms_params.var_params.subpel_search_type = USE_2_TAPS; ms_params.mv_cost_params.mv_cost_type = MV_COST_NONE; MV subpel_start_mv = get_mv_from_fullmv(&best_mv->as_fullmv); + assert(av1_is_subpelmv_in_range(&ms_params.mv_limits, subpel_start_mv)); bestsme = cpi->mv_search_params.find_fractional_mv_step( xd, cm, &ms_params, subpel_start_mv, &best_mv->as_mv, &distortion, &sse, NULL);