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

diff --git a/test/vp9_ext_ratectrl_test.cc b/test/vp9_ext_ratectrl_test.cc
index e3e7afb..1289e2d 100644 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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 (file)
--- 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