Refactor handling of configuration overrides from Vp8FrameBufferContr…

…oller

Make Vp8FrameBufferController::UpdateConfiguration return a set
of desired overrides. These overrides are cumulative with
previously returned override sets.

Bug: webrtc:10382
Change-Id: I1aa9544ae0cf6c57115e80963b3bbcdc3101db5e
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/134649
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Elad Alon <eladalon@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#27835}

api/video_codecs/vp8_frame_buffer_controller.h

@@ -11,6 +11,7 @@
 #ifndef API_VIDEO_CODECS_VP8_FRAME_BUFFER_CONTROLLER_H_
 #define API_VIDEO_CODECS_VP8_FRAME_BUFFER_CONTROLLER_H_
 
+#include <array>
 #include <memory>
 #include <vector>
 
@@ -46,34 +47,48 @@ namespace webrtc {
 
 struct CodecSpecificInfo;
 
-// TODO(eladalon): This configuration is temporal-layers specific; refactor.
+// Each member represents an override of the VPX configuration if the optional
+// value is set.
 struct Vp8EncoderConfig {
-  static constexpr size_t kMaxPeriodicity = 16;
-  static constexpr size_t kMaxLayers = 5;
-
-  // Number of active temporal layers. Set to 0 if not used.
-  uint32_t ts_number_layers;
-  // Arrays of length |ts_number_layers|, indicating (cumulative) target bitrate
-  // and rate decimator (e.g. 4 if every 4th frame is in the given layer) for
-  // each active temporal layer, starting with temporal id 0.
-  uint32_t ts_target_bitrate[kMaxLayers];
-  uint32_t ts_rate_decimator[kMaxLayers];
-
-  // The periodicity of the temporal pattern. Set to 0 if not used.
-  uint32_t ts_periodicity;
-  // Array of length |ts_periodicity| indicating the sequence of temporal id's
-  // to assign to incoming frames.
-  uint32_t ts_layer_id[kMaxPeriodicity];
+  struct TemporalLayerConfig {
+    bool operator!=(const TemporalLayerConfig& other) const {
+      return ts_number_layers != other.ts_number_layers ||
+             ts_target_bitrate != other.ts_target_bitrate ||
+             ts_rate_decimator != other.ts_rate_decimator ||
+             ts_periodicity != other.ts_periodicity ||
+             ts_layer_id != other.ts_layer_id;
+    }
+
+    static constexpr size_t kMaxPeriodicity = 16;
+    static constexpr size_t kMaxLayers = 5;
+
+    // Number of active temporal layers. Set to 0 if not used.
+    uint32_t ts_number_layers;
+
+    // Arrays of length |ts_number_layers|, indicating (cumulative) target
+    // bitrate and rate decimator (e.g. 4 if every 4th frame is in the given
+    // layer) for each active temporal layer, starting with temporal id 0.
+    std::array<uint32_t, kMaxLayers> ts_target_bitrate;
+    std::array<uint32_t, kMaxLayers> ts_rate_decimator;
+
+    // The periodicity of the temporal pattern. Set to 0 if not used.
+    uint32_t ts_periodicity;
+
+    // Array of length |ts_periodicity| indicating the sequence of temporal id's
+    // to assign to incoming frames.
+    std::array<uint32_t, kMaxPeriodicity> ts_layer_id;
+  };
+
+  absl::optional<TemporalLayerConfig> temporal_layer_config;
 
   // Target bitrate, in bps.
-  uint32_t rc_target_bitrate;
+  absl::optional<uint32_t> rc_target_bitrate;
 
-  // Clamp QP to min/max. Use 0 to disable clamping.
-  uint32_t rc_min_quantizer;
-  uint32_t rc_max_quantizer;
+  // Clamp QP to max. Use 0 to disable clamping.
+  absl::optional<uint32_t> rc_max_quantizer;
 
-  // If has_value(), override error resilience to value().
-  absl::optional<uint32_t> error_resilient;
+  // Error resilience mode.
+  absl::optional<uint32_t> g_error_resilient;
 };
 
 // This interface defines a way of delegating the logic of buffer management.
@@ -83,6 +98,10 @@ class Vp8FrameBufferController {
  public:
   virtual ~Vp8FrameBufferController() = default;
 
+  // Set limits on QP.
+  // The limits are suggestion-only; the controller is allowed to exceed them.
+  virtual void SetQpLimits(size_t stream_index, int min_qp, int max_qp) = 0;
+
   // Number of streamed controlled by |this|.
   virtual size_t StreamCount() const = 0;
 
@@ -103,12 +122,13 @@ class Vp8FrameBufferController {
                               const std::vector<uint32_t>& bitrates_bps,
                               int framerate_fps) = 0;
 
-  // Called by the encoder before encoding a frame. |cfg| contains the current
-  // configuration. If the encoder wishes any part of that to be changed before
-  // the encode step, |cfg| should be changed and then return true. If false is
-  // returned, the encoder will proceed without updating the configuration.
-  virtual bool UpdateConfiguration(size_t stream_index,
-                                   Vp8EncoderConfig* cfg) = 0;
+  // Called by the encoder before encoding a frame. Returns a set of overrides
+  // the controller wishes to enact in the encoder's configuration.
+  // If a value is not overridden, previous overrides are still in effect.
+  // (It is therefore not possible to go from a specific override to
+  // no-override. Once the controller takes responsibility over a value, it
+  // must maintain responsibility for it.)
+  virtual Vp8EncoderConfig UpdateConfiguration(size_t stream_index) = 0;
 
   // Returns the recommended VP8 encode flags needed.
   // The timestamp may be used as both a time and a unique identifier, and so

api/video_codecs/vp8_temporal_layers.cc

@@ -28,6 +28,13 @@ Vp8TemporalLayers::Vp8TemporalLayers(
       }));
 }
 
+void Vp8TemporalLayers::SetQpLimits(size_t stream_index,
+                                    int min_qp,
+                                    int max_qp) {
+  RTC_DCHECK_LT(stream_index, controllers_.size());
+  return controllers_[stream_index]->SetQpLimits(0, min_qp, max_qp);
+}
+
 size_t Vp8TemporalLayers::StreamCount() const {
   return controllers_.size();
 }
@@ -47,10 +54,9 @@ void Vp8TemporalLayers::OnRatesUpdated(
                                                     framerate_fps);
 }
 
-bool Vp8TemporalLayers::UpdateConfiguration(size_t stream_index,
-                                            Vp8EncoderConfig* cfg) {
+Vp8EncoderConfig Vp8TemporalLayers::UpdateConfiguration(size_t stream_index) {
   RTC_DCHECK_LT(stream_index, controllers_.size());
-  return controllers_[stream_index]->UpdateConfiguration(0, cfg);
+  return controllers_[stream_index]->UpdateConfiguration(0);
 }
 
 Vp8FrameConfig Vp8TemporalLayers::NextFrameConfig(size_t stream_index,

api/video_codecs/vp8_temporal_layers.h

@@ -35,6 +35,8 @@ class Vp8TemporalLayers final : public Vp8FrameBufferController {
       std::vector<std::unique_ptr<Vp8FrameBufferController>>&& controllers);
   ~Vp8TemporalLayers() override = default;
 
+  void SetQpLimits(size_t stream_index, int min_qp, int max_qp) override;
+
   size_t StreamCount() const override;
 
   bool SupportsEncoderFrameDropping(size_t stream_index) const override;
@@ -43,7 +45,7 @@ class Vp8TemporalLayers final : public Vp8FrameBufferController {
                       const std::vector<uint32_t>& bitrates_bps,
                       int framerate_fps) override;
 
-  bool UpdateConfiguration(size_t stream_index, Vp8EncoderConfig* cfg) override;
+  Vp8EncoderConfig UpdateConfiguration(size_t stream_index) override;
 
   Vp8FrameConfig NextFrameConfig(size_t stream_index,
                                  uint32_t rtp_timestamp) override;

modules/video_coding/codecs/vp8/default_temporal_layers.cc

@@ -10,7 +10,6 @@
 #include "modules/video_coding/codecs/vp8/default_temporal_layers.h"
 
 #include <stdlib.h>
-#include <string.h>
 
 #include <algorithm>
 #include <array>
@@ -251,6 +250,13 @@ DefaultTemporalLayers::DefaultTemporalLayers(int number_of_temporal_layers)
 
 DefaultTemporalLayers::~DefaultTemporalLayers() = default;
 
+void DefaultTemporalLayers::SetQpLimits(size_t stream_index,
+                                        int min_qp,
+                                        int max_qp) {
+  RTC_DCHECK_LT(stream_index, StreamCount());
+  // Ignore.
+}
+
 size_t DefaultTemporalLayers::StreamCount() const {
   return 1;
 }
@@ -278,28 +284,34 @@ void DefaultTemporalLayers::OnRatesUpdated(
   }
 }
 
-bool DefaultTemporalLayers::UpdateConfiguration(size_t stream_index,
-                                                Vp8EncoderConfig* cfg) {
+Vp8EncoderConfig DefaultTemporalLayers::UpdateConfiguration(
+    size_t stream_index) {
   RTC_DCHECK_LT(stream_index, StreamCount());
 
+  Vp8EncoderConfig config;
+
   if (!new_bitrates_bps_) {
-    return false;
+    return config;
   }
 
+  config.temporal_layer_config.emplace();
+  Vp8EncoderConfig::TemporalLayerConfig& ts_config =
+      config.temporal_layer_config.value();
+
   for (size_t i = 0; i < num_layers_; ++i) {
-    cfg->ts_target_bitrate[i] = (*new_bitrates_bps_)[i] / 1000;
+    ts_config.ts_target_bitrate[i] = (*new_bitrates_bps_)[i] / 1000;
     // ..., 4, 2, 1
-    cfg->ts_rate_decimator[i] = 1 << (num_layers_ - i - 1);
+    ts_config.ts_rate_decimator[i] = 1 << (num_layers_ - i - 1);
   }
 
-  cfg->ts_number_layers = num_layers_;
-  cfg->ts_periodicity = temporal_ids_.size();
-  memcpy(cfg->ts_layer_id, &temporal_ids_[0],
-         sizeof(unsigned int) * temporal_ids_.size());
+  ts_config.ts_number_layers = num_layers_;
+  ts_config.ts_periodicity = temporal_ids_.size();
+  std::copy(temporal_ids_.begin(), temporal_ids_.end(),
+            ts_config.ts_layer_id.begin());
 
   new_bitrates_bps_.reset();
 
-  return true;
+  return config;
 }
 
 bool DefaultTemporalLayers::IsSyncFrame(const Vp8FrameConfig& config) const {

modules/video_coding/codecs/vp8/default_temporal_layers.h

@@ -34,6 +34,8 @@ class DefaultTemporalLayers final : public Vp8FrameBufferController {
   explicit DefaultTemporalLayers(int number_of_temporal_layers);
   ~DefaultTemporalLayers() override;
 
+  void SetQpLimits(size_t stream_index, int min_qp, int max_qp) override;
+
   size_t StreamCount() const override;
 
   bool SupportsEncoderFrameDropping(size_t stream_index) const override;
@@ -48,7 +50,7 @@ class DefaultTemporalLayers final : public Vp8FrameBufferController {
                       const std::vector<uint32_t>& bitrates_bps,
                       int framerate_fps) override;
 
-  bool UpdateConfiguration(size_t stream_index, Vp8EncoderConfig* cfg) override;
+  Vp8EncoderConfig UpdateConfiguration(size_t stream_index) override;
 
   void OnEncodeDone(size_t stream_index,
                     uint32_t rtp_timestamp,