Change UpdateRateControl() to return bool

Change the VP9RateControlRtcConfig constructor to initialize
ss_number_layers (to 1).

Change UpdateRateControl() to return bool so that it can report failure
(due to invalid configuration).

Also change InitRateControl() to return bool to propagate the return
value of UpdateRateControl().

Note: This is a port of the libaom CL
https://aomedia-review.googlesource.com/c/aom/+/172042.

Change-Id: I90b60353b5f15692dba5d89e7b1a9c81bb2fdd89

diff --git a/test/vp8_ratectrl_rtc_test.cc b/test/vp8_ratectrl_rtc_test.cc
index 56c26a9..b76bcae 100644 (file)
--- a/test/vp8_ratectrl_rtc_test.cc
+++ b/test/vp8_ratectrl_rtc_test.cc
@@ -160,7 +160,7 @@ class Vp8RcInterfaceTest
     if (test_video_.width == 640 && target_bitrate_ == 1000) return;
     SetConfig();
     rc_api_ = libvpx::VP8RateControlRTC::Create(rc_cfg_);
-    rc_api_->UpdateRateControl(rc_cfg_);
+    ASSERT_TRUE(rc_api_->UpdateRateControl(rc_cfg_));
 
     ::libvpx_test::I420VideoSource video(test_video_.name, test_video_.width,
                                          test_video_.height, 30, 1, 0,
@@ -177,7 +177,7 @@ class Vp8RcInterfaceTest
     key_interval_ = 100;
     SetConfig();
     rc_api_ = libvpx::VP8RateControlRTC::Create(rc_cfg_);
-    rc_api_->UpdateRateControl(rc_cfg_);
+    ASSERT_TRUE(rc_api_->UpdateRateControl(rc_cfg_));
 
     ::libvpx_test::I420VideoSource video(test_video_.name, test_video_.width,
                                          test_video_.height, 30, 1, 0,
@@ -193,7 +193,7 @@ class Vp8RcInterfaceTest
     if (test_video_.width == 640 && target_bitrate_ == 1000) return;
     SetConfigTemporalLayers(2);
     rc_api_ = libvpx::VP8RateControlRTC::Create(rc_cfg_);
-    rc_api_->UpdateRateControl(rc_cfg_);
+    ASSERT_TRUE(rc_api_->UpdateRateControl(rc_cfg_));
 
     ::libvpx_test::I420VideoSource video(test_video_.name, test_video_.width,
                                          test_video_.height, 30, 1, 0,
@@ -209,7 +209,7 @@ class Vp8RcInterfaceTest
     if (test_video_.width == 640 && target_bitrate_ == 1000) return;
     SetConfigTemporalLayers(3);
     rc_api_ = libvpx::VP8RateControlRTC::Create(rc_cfg_);
-    rc_api_->UpdateRateControl(rc_cfg_);
+    ASSERT_TRUE(rc_api_->UpdateRateControl(rc_cfg_));
 
     ::libvpx_test::I420VideoSource video(test_video_.name, test_video_.width,
                                          test_video_.height, 30, 1, 0,

diff --git a/test/vp9_ratectrl_rtc_test.cc b/test/vp9_ratectrl_rtc_test.cc
index c6ab5b0..5abda12 100644 (file)
--- a/test/vp9_ratectrl_rtc_test.cc
+++ b/test/vp9_ratectrl_rtc_test.cc
@@ -212,7 +212,7 @@ class RcInterfaceSvcTest
         rc_cfg_.layer_target_bitrate[6] = 0;
         rc_cfg_.layer_target_bitrate[7] = 0;
         rc_cfg_.layer_target_bitrate[8] = 0;
-        rc_api_->UpdateRateControl(rc_cfg_);
+        ASSERT_TRUE(rc_api_->UpdateRateControl(rc_cfg_));
       } else if (video->frame() == 200) {
         // Go down to 1 spatial layer.
         // Update the encoder config.
@@ -226,7 +226,7 @@ class RcInterfaceSvcTest
         rc_cfg_.layer_target_bitrate[3] = 0;
         rc_cfg_.layer_target_bitrate[4] = 0;
         rc_cfg_.layer_target_bitrate[5] = 0;
-        rc_api_->UpdateRateControl(rc_cfg_);
+        ASSERT_TRUE(rc_api_->UpdateRateControl(rc_cfg_));
       } else if (0 && video->frame() == 280) {
         // TODO(marpan): Re-enable this going back up when issue is fixed.
         // Go back up to 3 spatial layers.
@@ -235,7 +235,7 @@ class RcInterfaceSvcTest
         encoder->Config(&cfg_);
         // Update the RC config.
         SetRCConfigSvc(3, 3);
-        rc_api_->UpdateRateControl(rc_cfg_);
+        ASSERT_TRUE(rc_api_->UpdateRateControl(rc_cfg_));
       }
     }
   }

diff --git a/vp8/vp8_ratectrl_rtc.cc b/vp8/vp8_ratectrl_rtc.cc
index c36cfea..65c5853 100644 (file)
--- a/vp8/vp8_ratectrl_rtc.cc
+++ b/vp8/vp8_ratectrl_rtc.cc
@@ -62,7 +62,7 @@ std::unique_ptr<VP8RateControlRTC> VP8RateControlRTC::Create(
   if (!rc_api->cpi_) return nullptr;
   vp8_zero(*rc_api->cpi_);
 
-  rc_api->InitRateControl(cfg);
+  if (!rc_api->InitRateControl(cfg)) return nullptr;
 
   return rc_api;
 }
@@ -74,7 +74,7 @@ VP8RateControlRTC::~VP8RateControlRTC() {
   }
 }
 
-void VP8RateControlRTC::InitRateControl(const VP8RateControlRtcConfig &rc_cfg) {
+bool VP8RateControlRTC::InitRateControl(const VP8RateControlRtcConfig &rc_cfg) {
   VP8_COMMON *cm = &cpi_->common;
   VP8_CONFIG *oxcf = &cpi_->oxcf;
   oxcf->end_usage = USAGE_STREAM_FROM_SERVER;
@@ -92,13 +92,19 @@ void VP8RateControlRTC::InitRateControl(const VP8RateControlRtcConfig &rc_cfg) {
   cpi_->kf_bitrate_adjustment = 0;
   cpi_->gf_overspend_bits = 0;
   cpi_->non_gf_bitrate_adjustment = 0;
-  UpdateRateControl(rc_cfg);
+  if (!UpdateRateControl(rc_cfg)) return false;
   cpi_->buffer_level = oxcf->starting_buffer_level;
   cpi_->bits_off_target = oxcf->starting_buffer_level;
+  return true;
 }
 
-void VP8RateControlRTC::UpdateRateControl(
+bool VP8RateControlRTC::UpdateRateControl(
     const VP8RateControlRtcConfig &rc_cfg) {
+  if (rc_cfg.ts_number_layers < 1 ||
+      rc_cfg.ts_number_layers > VPX_TS_MAX_LAYERS) {
+    return false;
+  }
+
   VP8_COMMON *cm = &cpi_->common;
   VP8_CONFIG *oxcf = &cpi_->oxcf;
   const unsigned int prev_number_of_layers = oxcf->number_of_layers;
@@ -199,6 +205,7 @@ void VP8RateControlRTC::UpdateRateControl(
 
   vp8_new_framerate(cpi_, cpi_->framerate);
   vpx_clear_system_state();
+  return true;
 }
 
 void VP8RateControlRTC::ComputeQP(const VP8FrameParamsQpRTC &frame_params) {

diff --git a/vp8/vp8_ratectrl_rtc.h b/vp8/vp8_ratectrl_rtc.h
index 0e81592..a8a886c 100644 (file)
--- a/vp8/vp8_ratectrl_rtc.h
+++ b/vp8/vp8_ratectrl_rtc.h
@@ -39,7 +39,7 @@ class VP8RateControlRTC {
       const VP8RateControlRtcConfig &cfg);
   ~VP8RateControlRTC();
 
-  void UpdateRateControl(const VP8RateControlRtcConfig &rc_cfg);
+  bool UpdateRateControl(const VP8RateControlRtcConfig &rc_cfg);
   // GetQP() needs to be called after ComputeQP() to get the latest QP
   int GetQP() const;
   // int GetLoopfilterLevel() const;
@@ -49,7 +49,7 @@ class VP8RateControlRTC {
 
  private:
   VP8RateControlRTC() {}
-  void InitRateControl(const VP8RateControlRtcConfig &cfg);
+  bool InitRateControl(const VP8RateControlRtcConfig &cfg);
   struct VP8_COMP *cpi_;
   int q_;
 };

diff --git a/vp9/ratectrl_rtc.cc b/vp9/ratectrl_rtc.cc
index cc12ea3..29033d4 100644 (file)
--- a/vp9/ratectrl_rtc.cc
+++ b/vp9/ratectrl_rtc.cc
@@ -25,22 +25,16 @@ std::unique_ptr<VP9RateControlRTC> VP9RateControlRTC::Create(
                                                 VP9RateControlRTC());
   if (!rc_api) return nullptr;
   rc_api->cpi_ = static_cast<VP9_COMP *>(vpx_memalign(32, sizeof(*cpi_)));
-  if (!rc_api->cpi_) {
-    rc_api.reset();
-    return nullptr;
-  }
+  if (!rc_api->cpi_) return nullptr;
   vp9_zero(*rc_api->cpi_);
 
-  rc_api->InitRateControl(cfg);
+  if (!rc_api->InitRateControl(cfg)) return nullptr;
   if (cfg.aq_mode) {
     VP9_COMP *const cpi = rc_api->cpi_;
     cpi->segmentation_map = static_cast<uint8_t *>(
         vpx_calloc(cpi->common.mi_rows * cpi->common.mi_cols,
                    sizeof(*cpi->segmentation_map)));
-    if (!cpi->segmentation_map) {
-      rc_api.reset();
-      return nullptr;
-    }
+    if (!cpi->segmentation_map) return nullptr;
     cpi->cyclic_refresh =
         vp9_cyclic_refresh_alloc(cpi->common.mi_rows, cpi->common.mi_cols);
     cpi->cyclic_refresh->content_mode = 0;
@@ -71,7 +65,7 @@ VP9RateControlRTC::~VP9RateControlRTC() {
   }
 }
 
-void VP9RateControlRTC::InitRateControl(const VP9RateControlRtcConfig &rc_cfg) {
+bool VP9RateControlRTC::InitRateControl(const VP9RateControlRtcConfig &rc_cfg) {
   VP9_COMMON *cm = &cpi_->common;
   VP9EncoderConfig *oxcf = &cpi_->oxcf;
   RATE_CONTROL *const rc = &cpi_->rc;
@@ -88,7 +82,7 @@ void VP9RateControlRTC::InitRateControl(const VP9RateControlRtcConfig &rc_cfg) {
   cm->current_video_frame = 0;
   rc->kf_boost = DEFAULT_KF_BOOST;
 
-  UpdateRateControl(rc_cfg);
+  if (!UpdateRateControl(rc_cfg)) return false;
   vp9_set_mb_mi(cm, cm->width, cm->height);
 
   cpi_->use_svc = (cpi_->svc.number_spatial_layers > 1 ||
@@ -102,10 +96,21 @@ void VP9RateControlRTC::InitRateControl(const VP9RateControlRtcConfig &rc_cfg) {
   vp9_rc_init(oxcf, 0, rc);
   rc->constrain_gf_key_freq_onepass_vbr = 0;
   cpi_->sf.use_nonrd_pick_mode = 1;
+  return true;
 }
 
-void VP9RateControlRTC::UpdateRateControl(
+bool VP9RateControlRTC::UpdateRateControl(
     const VP9RateControlRtcConfig &rc_cfg) {
+  // Since VPX_MAX_LAYERS (12) is less than the product of VPX_SS_MAX_LAYERS (5)
+  // and VPX_TS_MAX_LAYERS (5), check all three.
+  if (rc_cfg.ss_number_layers < 1 ||
+      rc_cfg.ss_number_layers > VPX_SS_MAX_LAYERS ||
+      rc_cfg.ts_number_layers < 1 ||
+      rc_cfg.ts_number_layers > VPX_TS_MAX_LAYERS ||
+      rc_cfg.ss_number_layers * rc_cfg.ts_number_layers > VPX_MAX_LAYERS) {
+    return false;
+  }
+
   VP9_COMMON *cm = &cpi_->common;
   VP9EncoderConfig *oxcf = &cpi_->oxcf;
   RATE_CONTROL *const rc = &cpi_->rc;
@@ -163,6 +168,7 @@ void VP9RateControlRTC::UpdateRateControl(
                                            (int)cpi_->oxcf.target_bandwidth);
   }
   vp9_check_reset_rc_flag(cpi_);
+  return true;
 }
 
 void VP9RateControlRTC::ComputeQP(const VP9FrameParamsQpRTC &frame_params) {

diff --git a/vp9/ratectrl_rtc.h b/vp9/ratectrl_rtc.h
index a82c776..7f3c900 100644 (file)
--- a/vp9/ratectrl_rtc.h
+++ b/vp9/ratectrl_rtc.h
@@ -30,6 +30,7 @@ namespace libvpx {
 struct VP9RateControlRtcConfig : public VpxRateControlRtcConfig {
  public:
   VP9RateControlRtcConfig() {
+    ss_number_layers = 1;
     vp9_zero(max_quantizers);
     vp9_zero(min_quantizers);
     vp9_zero(scaling_factor_den);
@@ -89,7 +90,7 @@ class VP9RateControlRTC {
       const VP9RateControlRtcConfig &cfg);
   ~VP9RateControlRTC();
 
-  void UpdateRateControl(const VP9RateControlRtcConfig &rc_cfg);
+  bool UpdateRateControl(const VP9RateControlRtcConfig &rc_cfg);
   // GetQP() needs to be called after ComputeQP() to get the latest QP
   int GetQP() const;
   int GetLoopfilterLevel() const;
@@ -101,7 +102,7 @@ class VP9RateControlRTC {
 
  private:
   VP9RateControlRTC() {}
-  void InitRateControl(const VP9RateControlRtcConfig &cfg);
+  bool InitRateControl(const VP9RateControlRtcConfig &cfg);
   struct VP9_COMP *cpi_;
 };