Refactor handle_inter_mode and separate single and compound mode searches by kslu-aom · Pull Request #5239 · AOMediaCodec/avm

kslu-aom · 2026-08-10T20:35:47Z

Reduce the search loop nesting by using a helper function for single and another for compound. The order of nested loops has been changed, so that a common search loop for MV precision is moved to higher level and remains in handle_inter_mode, while the other loops are moved to the helper functions. The double search loops for compound DRL and for BAWP are flattened as one single loop. Some pruning logic of skip_newmv are refactored using an inline function.

There is an ongoing effort to further unnest the loops inside handle_compound_inter_prediction.

Evaluation based on aba7683 with 33 frames

Speed 0 (1 point remaining for each set)

+---------+--------+--------+--------+--------+----------+----------+
| Summary |   Y    |   U    |   V    |  YUV   | Enc-time | Dec-time |
+---------+--------+--------+--------+--------+----------+----------+
| A1      | -0.05% | +0.00% | +0.28% | -0.03% | 99.4%    | 99.0%    |
| A2      | -0.03% | +0.04% | -0.20% | -0.03% | 100.4%   | 100.7%   |
+---------+--------+--------+--------+--------+----------+----------+

Speed 1

+---------+--------+--------+--------+--------+----------+----------+
| Summary |   Y    |   U    |   V    |  YUV   | Enc-time | Dec-time |
+---------+--------+--------+--------+--------+----------+----------+
| A1      | -0.01% | -0.02% | -0.10% | -0.02% | 100.8%   | 100.1%   |
| A2      | +0.01% | -0.20% | +0.02% | -0.00% |  99.2%   |  98.7%   |
+---------+--------+--------+--------+--------+----------+----------+

STATS_CHANGED

Combine ref_mv_id_1 and ref_mv_id_0 loops into a single flat_idx loop in handle_inter_mode. TAG=agy CONV=f3d7c114-6aba-4bcd-85bc-35cff99055d5

kslu-aom · 2026-08-10T20:42:52Z

Please review this large scale refactoring and let me know. I can do another iteration based on the comments and suggestions, and mark it as ready once I have rebased it with new test results.

yeqing-wu · 2026-08-11T15:45:41Z

Thanks Lester for helping refactor the code. I’ll review it today. With so many internal and external code reviews, I still prefer that we review the code carefully to identify issues as early as possible.

yunqingwang1 · 2026-08-11T18:56:57Z

@kslu-aom Thanks for the refactoring. Regarding "The double search loops for compound DRL and for BAWP are flattened as one single loop", separating single and compound mode search is needed to achieve this loop flattening?

kslu-aom · 2026-08-11T20:24:21Z

@kslu-aom Thanks for the refactoring. Regarding "The double search loops for compound DRL and for BAWP are flattened as one single loop", separating single and compound mode search is needed to achieve this loop flattening?

No, it is a separate cleanup with the intention to reduce the nesting.

yunqingwang1 · 2026-08-11T21:51:35Z

Looks good - thanks.

yeqing-wu

Thanks Lester. Hare are my comments.

yeqing-wu · 2026-08-11T23:24:43Z

-            for (i = 0; i < is_comp_pred + 1; ++i) {
-              bawp_off_mv[i].as_int = cur_mv[i].as_int;
-            }
+  set_precision_set(cm, xd, mbmi, bsize, 0);


We do not need ref_mv_idx inside set_precision_set(), thus please help clean it up so that it can avoid one input parameter

yeqing-wu · 2026-08-11T23:27:57Z

+    mbmi->motion_mode = SIMPLE_TRANSLATION;
+    mbmi->ref_mv_idx[1] = ref_mv_idx[1];
+    mbmi->ref_mv_idx[0] = ref_mv_idx[0];
+    set_mv_precision(mbmi, mbmi->max_mv_precision);


With the code in 5248 (mbmi->pb_mv_precision = pb_mv_precision;), I think set_mv_precision() call cite is redundant

Please check other places since inside set_mv_precision(), it only has one line code to assign the mv precision, which is similar as mbmi->pb_mv_precision = pb_mv_precision;

void set_mv_precision(MB_MODE_INFO *mbmi, MvSubpelPrecision precision) { mbmi->pb_mv_precision = precision; }

I have removed a few of these.

yeqing-wu · 2026-08-11T23:28:05Z

+    mbmi->ref_mv_idx[1] = 0;
+    mbmi->ref_mv_idx[0] = ref_mv_idx0;
+    int ref_mv_idx_type = av2_ref_mv_idx_type(mbmi, ref_mv_idx);
+    set_mv_precision(mbmi, mbmi->max_mv_precision);


With the code in 5248 (mbmi->pb_mv_precision = pb_mv_precision;), I think set_mv_precision() call cite is redundant

yeqing-wu · 2026-08-11T23:31:48Z

+    int_mv bawp_off_mv[2];
+    int64_t bawp_off_newmv_ret_val = 0;
+    int bawp_off_rate_mv = 0;
+    for (i = 0; i < is_comp_pred + 1; ++i) {


is_comp_pred is hardcoded to 0, thus I think we do not need this for loop. Meanwhile, as the function name indicates it is for single, I think we also do not need to is_comp_pred inside this function.

const int is_comp_pred = 0;

yeqing-wu · 2026-08-11T23:32:05Z

+      if (mbmi->bawp_flag[0] >= 1) {
+        bawp_off_mv[1].as_int = -1;
+        assert(is_comp_pred == 0);
+        for (i = 0; i < is_comp_pred + 1; ++i) {


same comment

yeqing-wu · 2026-08-11T23:32:28Z

+          end_timing(cpi, handle_newmv_time);
+#endif
+          bawp_off_rate_mv = rate_mv;
+          for (i = 0; i < is_comp_pred + 1; ++i) {


same comment

yeqing-wu · 2026-08-11T23:34:58Z

+    int prune_modes_based_on_tpl, int ref_match_found_in_above_nb,
+    int ref_match_found_in_left_nb,
+    PruneInfoFromTpl *inter_cost_info_from_tpl, int jmvd_scaling_factor_num,
+    PREDICTION_MODE best_ref_mode) {


In the future, we need to think about how to refactor the code to avoid enormous parameter lists. Not for this PR.

Will try to clean up some parameters in the next iteration

yeqing-wu · 2026-08-11T23:37:00Z

+  av2_zero(cwp_search_mask);
+
+  const int total_ref_mv_idx = ref_set[1] * ref_set[0];
+  for (int flat_idx = 0; flat_idx < total_ref_mv_idx; ++flat_idx) {


5209–5390 (single) vs 5422–5609 (compound), has lots of duplicate near-identical code. We can refactor it later, but add a TODO so that we would not forget this

Sounds good. I have added a TODO

yeqing-wu · 2026-08-11T23:40:25Z

+      bawp_off_mv[i].as_int = cur_mv[i].as_int;
+    }
+
+    int bawp_eanbled = cm->features.enable_bawp &&


typo, should be 'bawp_enabled'. Even it is from the existing code

yeqing-wu · 2026-08-11T23:41:55Z

+
+    int rate_mv = 0;
+    int64_t newmv_ret_val = 0;
+    int i;


Loose int i; C-style declaration, Move to for (int i = 0; ...) at each use

yeqing-wu

Thanks Lester. LGTM

kslu-aom added 2 commits August 7, 2026 17:21

Combine ref_mv_idx loops in handle_inter_mode into a single flat loop

140f941

Combine ref_mv_id_1 and ref_mv_id_0 loops into a single flat_idx loop in handle_inter_mode. TAG=agy CONV=f3d7c114-6aba-4bcd-85bc-35cff99055d5

Clean up the refactoring

b12a0d7

aom-bot added the needs-cla label Aug 10, 2026

kslu-aom changed the base branch from main to av2-enc August 10, 2026 20:38

aom-bot added cla-signed and removed needs-cla labels Aug 10, 2026

kslu-aom requested review from leolzhao, urvangjoshi, yeqing-wu and yunqingwang1 August 10, 2026 20:39

kslu-aom requested a review from jingninghan August 10, 2026 20:45

jingninghan requested a review from jianle973 August 10, 2026 20:59

himrefKc: more minor cleanups

4ef4577

Fix CI errors and add comments

8c0b607

yeqing-wu reviewed Aug 11, 2026

View reviewed changes

kslu-aom added 2 commits August 11, 2026 18:10

Address issues and cleanups in the comments

546cef6

Add a TODO for future cleanups

3c2b1d8

yeqing-wu approved these changes Aug 12, 2026

View reviewed changes

Uh oh!

Conversation

kslu-aom commented Aug 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

kslu-aom commented Aug 10, 2026

Uh oh!

yeqing-wu commented Aug 11, 2026

Uh oh!

yunqingwang1 commented Aug 11, 2026

Uh oh!

kslu-aom commented Aug 11, 2026

Uh oh!

yunqingwang1 commented Aug 11, 2026

Uh oh!

yeqing-wu left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

yeqing-wu left a comment

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

kslu-aom commented Aug 10, 2026 •

edited

Loading