From 4e7c56332de65eb191cef5644c010e309302a993 Mon Sep 17 00:00:00 2001 From: Cheng Chen Date: Tue, 31 May 2022 11:07:15 -0700 Subject: [PATCH] L2E: Return error when GOP model is not set - Return error instead of OK when GOP model is not set. - Update descriptions for a few variables. Change-Id: I213f6b7085c487507c3935e7ce615e807f4474cc --- test/vp9_ext_ratectrl_test.cc | 12 ++++---- vp9/encoder/vp9_ext_ratectrl.c | 43 ++++++++++++++-------------- vp9/encoder/vp9_firstpass.c | 6 ++-- vp9/encoder/vp9_ratectrl.h | 4 +-- vpx/vpx_ext_ratectrl.h | 64 +++++++++++++++++++++++++++++++++--------- 5 files changed, 82 insertions(+), 47 deletions(-) diff --git a/test/vp9_ext_ratectrl_test.cc b/test/vp9_ext_ratectrl_test.cc index e3e7afb..1289e2d 100644 --- a/test/vp9_ext_ratectrl_test.cc +++ b/test/vp9_ext_ratectrl_test.cc @@ -44,7 +44,7 @@ struct ToyRateCtrl { int magic_number; int coding_index; - int gop_id; + int gop_index; int frames_since_key; int show_index; }; @@ -73,7 +73,7 @@ vpx_rc_status_t rc_create_model_gop(void *priv, ToyRateCtrl *toy_rate_ctrl = new (std::nothrow) ToyRateCtrl; if (toy_rate_ctrl == nullptr) return VPX_RC_ERROR; toy_rate_ctrl->magic_number = kModelMagicNumber; - toy_rate_ctrl->gop_id = 0; + toy_rate_ctrl->gop_index = 0; toy_rate_ctrl->frames_since_key = 0; toy_rate_ctrl->show_index = 0; toy_rate_ctrl->coding_index = 0; @@ -198,13 +198,13 @@ vpx_rc_status_t rc_get_gop_decision(vpx_rc_model_t rate_ctrl_model, if (gop_info->is_key_frame) { EXPECT_EQ(gop_info->last_gop_use_alt_ref, 0); EXPECT_EQ(gop_info->frames_since_key, 0); - EXPECT_EQ(gop_info->gop_id, 0); - toy_rate_ctrl->gop_id = 0; + EXPECT_EQ(gop_info->gop_index, 0); + toy_rate_ctrl->gop_index = 0; toy_rate_ctrl->frames_since_key = 0; } else { EXPECT_EQ(gop_info->last_gop_use_alt_ref, 1); } - EXPECT_EQ(gop_info->gop_id, toy_rate_ctrl->gop_id); + EXPECT_EQ(gop_info->gop_index, toy_rate_ctrl->gop_index); EXPECT_EQ(gop_info->frames_since_key, toy_rate_ctrl->frames_since_key); EXPECT_EQ(gop_info->show_index, toy_rate_ctrl->show_index); EXPECT_EQ(gop_info->coding_index, toy_rate_ctrl->coding_index); @@ -217,7 +217,7 @@ vpx_rc_status_t rc_get_gop_decision(vpx_rc_model_t rate_ctrl_model, toy_rate_ctrl->show_index += gop_decision->gop_coding_frames - gop_decision->use_alt_ref; toy_rate_ctrl->coding_index += gop_decision->gop_coding_frames; - ++toy_rate_ctrl->gop_id; + ++toy_rate_ctrl->gop_index; return VPX_RC_OK; } diff --git a/vp9/encoder/vp9_ext_ratectrl.c b/vp9/encoder/vp9_ext_ratectrl.c index 48c9091..ba57b86 100644 --- a/vp9/encoder/vp9_ext_ratectrl.c +++ b/vp9/encoder/vp9_ext_ratectrl.c @@ -202,30 +202,29 @@ vpx_codec_err_t vp9_extrc_update_encodeframe_result( vpx_codec_err_t vp9_extrc_get_gop_decision( EXT_RATECTRL *ext_ratectrl, const vpx_rc_gop_info_t *const gop_info, vpx_rc_gop_decision_t *gop_decision) { - if (ext_ratectrl == NULL) { + vpx_rc_status_t rc_status; + if (ext_ratectrl == NULL || !ext_ratectrl->ready || + ext_ratectrl->funcs.rc_type != VPX_RC_GOP) { return VPX_CODEC_INVALID_PARAM; } - if (ext_ratectrl->ready && ext_ratectrl->funcs.rc_type == VPX_RC_GOP) { - vpx_rc_status_t rc_status; - rc_status = ext_ratectrl->funcs.get_gop_decision(ext_ratectrl->model, - gop_info, gop_decision); - if (gop_decision->use_alt_ref) { - const int arf_constraint = - gop_decision->gop_coding_frames >= gop_info->min_gf_interval && - gop_decision->gop_coding_frames < gop_info->lag_in_frames; - if (!arf_constraint || !gop_info->allow_alt_ref) return VPX_CODEC_ERROR; - } - // TODO(chengchen): Take min and max gf interval from the model - // and overwrite libvpx's decision so that we can get rid - // of one of the checks here. - if (gop_decision->gop_coding_frames > gop_info->frames_to_key || - gop_decision->gop_coding_frames - gop_decision->use_alt_ref > - gop_info->max_gf_interval) { - return VPX_CODEC_ERROR; - } - if (rc_status == VPX_RC_ERROR) { - return VPX_CODEC_ERROR; - } + rc_status = ext_ratectrl->funcs.get_gop_decision(ext_ratectrl->model, + gop_info, gop_decision); + if (gop_decision->use_alt_ref) { + const int arf_constraint = + gop_decision->gop_coding_frames >= gop_info->min_gf_interval && + gop_decision->gop_coding_frames < gop_info->lag_in_frames; + if (!arf_constraint || !gop_info->allow_alt_ref) return VPX_CODEC_ERROR; + } + // TODO(chengchen): Take min and max gf interval from the model + // and overwrite libvpx's decision so that we can get rid + // of one of the checks here. + if (gop_decision->gop_coding_frames > gop_info->frames_to_key || + gop_decision->gop_coding_frames - gop_decision->use_alt_ref > + gop_info->max_gf_interval) { + return VPX_CODEC_ERROR; + } + if (rc_status == VPX_RC_ERROR) { + return VPX_CODEC_ERROR; } return VPX_CODEC_OK; } diff --git a/vp9/encoder/vp9_firstpass.c b/vp9/encoder/vp9_firstpass.c index 6e1f797..2b3c174 100644 --- a/vp9/encoder/vp9_firstpass.c +++ b/vp9/encoder/vp9_firstpass.c @@ -2714,9 +2714,9 @@ static void define_gf_group(VP9_COMP *cpi, int gf_start_show_idx) { // frame in which case it will already have been done. if (is_key_frame == 0) { vp9_zero(twopass->gf_group); - ++rc->gop_id; + ++rc->gop_index; } else { - rc->gop_id = 0; + rc->gop_index = 0; } vpx_clear_system_state(); @@ -2772,7 +2772,7 @@ static void define_gf_group(VP9_COMP *cpi, int gf_start_show_idx) { gop_info.lag_in_frames = cpi->oxcf.lag_in_frames; gop_info.show_index = cm->current_video_frame; gop_info.coding_index = cm->current_frame_coding_index; - gop_info.gop_id = rc->gop_id; + gop_info.gop_index = rc->gop_index; codec_status = vp9_extrc_get_gop_decision(&cpi->ext_ratectrl, &gop_info, &gop_decision); diff --git a/vp9/encoder/vp9_ratectrl.h b/vp9/encoder/vp9_ratectrl.h index 42547d1..48a21bd 100644 --- a/vp9/encoder/vp9_ratectrl.h +++ b/vp9/encoder/vp9_ratectrl.h @@ -212,9 +212,9 @@ typedef struct { // VBR. int constrain_gf_key_freq_onepass_vbr; - // The id of the current GOP. Start from zero. + // The index of the current GOP. Start from zero. // When a key frame is inserted, it resets to zero. - int gop_id; + int gop_index; } RATE_CONTROL; struct VP9_COMP; diff --git a/vpx/vpx_ext_ratectrl.h b/vpx/vpx_ext_ratectrl.h index e2c475a..db9af44 100644 --- a/vpx/vpx_ext_ratectrl.h +++ b/vpx/vpx_ext_ratectrl.h @@ -25,7 +25,7 @@ extern "C" { * types, removing or reassigning enums, adding/removing/rearranging * fields to structures. */ -#define VPX_EXT_RATECTRL_ABI_VERSION (3) +#define VPX_EXT_RATECTRL_ABI_VERSION (4) /*!\brief The control type of the inference API. * In VPX_RC_QP mode, the external rate control model determines the @@ -33,7 +33,7 @@ extern "C" { * In VPX_RC_GOP mode, the external rate control model determines the * group of picture (GOP) of the video sequence. */ -typedef enum vpx_rc_type { VPX_RC_QP = 0, VPX_RC_GOP = 1 } vpx_rc_type_t; +typedef enum vpx_rc_type { VPX_RC_QP = 1, VPX_RC_GOP = 2 } vpx_rc_type_t; /*!\brief Abstract rate control model handler * @@ -270,18 +270,54 @@ typedef struct vpx_rc_config { * help make GOP decisions. */ typedef struct vpx_rc_gop_info { - int min_gf_interval; /**< mininum allowed gf interval */ - int max_gf_interval; /**< maximum allowed gf interval */ - int allow_alt_ref; /**< whether to allow the use of alt ref */ - int is_key_frame; /**< is the current frame a key frame */ - int last_gop_use_alt_ref; /**< does the last gop use alt ref or not */ - int frames_since_key; /**< current frame distance to the last keyframe */ - int frames_to_key; /**< current frame distance to the next keyframe */ - int lag_in_frames; /**< number of lookahead source frames */ - int show_index; /**< display index of this frame, starts from zero*/ - int coding_index; /**< coding index of this frame, starts from zero*/ - int gop_id; /**< the id of the current gop, starts from zero, resets to zero - when a keyframe is set*/ + /*! + * Minimum allowed gf interval, fixed for the whole clip. + */ + int min_gf_interval; + /*! + * Maximum allowed gf interval, fixed for the whole clip. + */ + int max_gf_interval; + /*! + * Whether to allow the use of alt ref, can be changed per gop. + */ + int allow_alt_ref; + /*! + * Is the current frame a key frame. + */ + int is_key_frame; + /*! + * Does the previous gop use alt ref or not. + */ + int last_gop_use_alt_ref; + /*! + * Current frame distance to the last keyframe, e.g., if Nth frame is a key, + * then the value of the N+1 th frame is 1. + */ + int frames_since_key; + /*! + * Current frame distance to the next keyframe, e.g. if Nth frame is a key, + * then the value of frame N - 1 is 1. + */ + int frames_to_key; + /*! + * Number of lookahead source frames. + */ + int lag_in_frames; + /*! + * Display index (temporal stamp) of this frame in the whole clip, + * starts from zero. + */ + int show_index; + /*! + * Coding index of this frame in the whole clip, starts from zero. + */ + int coding_index; + /*! + * The index of the current gop, starts from zero, resets to zero + * when a keyframe is set. + */ + int gop_index; } vpx_rc_gop_info_t; /*!\brief The decision made by the external rate control model to set the -- 2.7.4