Commit 8785cbc03f for aom
commit 8785cbc03fb201223189893e3bf63be3dceb211b
Author: Jerome Jiang <jianj@google.com>
Date: Wed Jan 7 16:17:37 2026 -0500
Ext RC: Only advance GOP on second define_gf_group
define_gf_group() is called twice in av1_get_second_pass_params()
with is_final_pass being 0 and 1 separately. This caused ext RC to
advance 2 GOPs which is wrong.
Change-Id: I19ccbe3b95541029e8d2a1199b9942d3b1470e00
diff --git a/av1/encoder/gop_structure.c b/av1/encoder/gop_structure.c
index a366d1f039..079b8ea2e1 100644
--- a/av1/encoder/gop_structure.c
+++ b/av1/encoder/gop_structure.c
@@ -873,7 +873,7 @@ static void construct_gop_structure_from_rc(
}
}
-void av1_gop_setup_structure(AV1_COMP *cpi) {
+void av1_gop_setup_structure(AV1_COMP *cpi, const int is_final_pass) {
RATE_CONTROL *const rc = &cpi->rc;
PRIMARY_RATE_CONTROL *const p_rc = &cpi->ppi->p_rc;
GF_GROUP *const gf_group = &cpi->ppi->gf_group;
@@ -882,9 +882,12 @@ void av1_gop_setup_structure(AV1_COMP *cpi) {
const int key_frame = rc->frames_since_key == 0;
FRAME_UPDATE_TYPE first_frame_update_type = ARF_UPDATE;
+ // define_gf_group() is called twice in av1_set_second_pass_params() with
+ // `is_final_pass` being 0 and 1 separately. But only one GOP can be advanced
+ // with the external RC. That is only done when `is_final_pass` is true.
if (cpi->ext_ratectrl.ready &&
(cpi->ext_ratectrl.funcs.rc_type & AOM_RC_GOP) != 0 &&
- cpi->ext_ratectrl.funcs.get_gop_decision != NULL) {
+ cpi->ext_ratectrl.funcs.get_gop_decision != NULL && is_final_pass) {
aom_rc_gop_decision_t gop_decision;
aom_codec_err_t codec_status =
av1_extrc_get_gop_decision(&cpi->ext_ratectrl, &gop_decision);
diff --git a/av1/encoder/gop_structure.h b/av1/encoder/gop_structure.h
index 92380abed8..14ef5bf734 100644
--- a/av1/encoder/gop_structure.h
+++ b/av1/encoder/gop_structure.h
@@ -36,11 +36,13 @@ struct EncodeFrameParams;
* the group. It does this primarily by updateing entries in
* cpi->twopass.gf_group.update_type[].
*
- * \param[in] cpi Top - level encoder instance structure
+ * \param[in] cpi Top - level encoder instance structure
+ * \param[in] is_final_pass Whether it is the second call to
+ * define_gf_group().
*
* \remark No return value but this function updates group data structures.
*/
-void av1_gop_setup_structure(struct AV1_COMP *cpi);
+void av1_gop_setup_structure(struct AV1_COMP *cpi, const int is_final_pass);
/*!\brief Check whether a frame in the GOP is a forward key frame
*
diff --git a/av1/encoder/pass2_strategy.c b/av1/encoder/pass2_strategy.c
index 202ebaeead..be22b63931 100644
--- a/av1/encoder/pass2_strategy.c
+++ b/av1/encoder/pass2_strategy.c
@@ -2179,10 +2179,12 @@ static void correct_frames_to_key(AV1_COMP *cpi) {
* case of one pass encoding where no lookahead stats are avialable.
*
* \param[in] cpi Top-level encoder structure
+ * \param[in] is_final_pass Whether it is the second call to
+ * define_gf_group().
*
* \remark Nothing is returned. Instead, cpi->ppi->gf_group is changed.
*/
-static void define_gf_group_pass0(AV1_COMP *cpi) {
+static void define_gf_group_pass0(AV1_COMP *cpi, const int is_final_pass) {
RATE_CONTROL *const rc = &cpi->rc;
PRIMARY_RATE_CONTROL *const p_rc = &cpi->ppi->p_rc;
GF_GROUP *const gf_group = &cpi->ppi->gf_group;
@@ -2220,7 +2222,7 @@ static void define_gf_group_pass0(AV1_COMP *cpi) {
gf_group->max_layer_depth_allowed = 0;
// Set up the structure of this Group-Of-Pictures (same as GF_GROUP)
- av1_gop_setup_structure(cpi);
+ av1_gop_setup_structure(cpi, is_final_pass);
// Allocate bits to each of the frames in the GF group.
// TODO(sarahparker) Extend this to work with pyramid structure.
@@ -2544,7 +2546,7 @@ static void define_gf_group(AV1_COMP *cpi, EncodeFrameParams *frame_params,
}
if (has_no_stats_stage(cpi)) {
- define_gf_group_pass0(cpi);
+ define_gf_group_pass0(cpi, is_final_pass);
return;
}
@@ -2645,7 +2647,7 @@ static void define_gf_group(AV1_COMP *cpi, EncodeFrameParams *frame_params,
update_gop_length(rc, p_rc, i, is_final_pass);
// Set up the structure of this Group-Of-Pictures (same as GF_GROUP)
- av1_gop_setup_structure(cpi);
+ av1_gop_setup_structure(cpi, is_final_pass);
set_gop_bits_boost(cpi, i, is_intra_only, is_final_pass, use_alt_ref,
alt_offset, start_pos, &gf_stats);
@@ -2721,7 +2723,7 @@ static int define_gf_group_pass3(AV1_COMP *cpi, EncodeFrameParams *frame_params,
update_gop_length(rc, p_rc, i, is_final_pass);
// Set up the structure of this Group-Of-Pictures (same as GF_GROUP)
- av1_gop_setup_structure(cpi);
+ av1_gop_setup_structure(cpi, is_final_pass);
set_gop_bits_boost(cpi, i, is_intra_only, is_final_pass, use_alt_ref, 0,
start_pos, &gf_stats);