Enforce VideoEncoderConfig.num_temporal_layers >= 1.

This change clarifies the semantics of this field:
  unset: Depends on context.
  == 0: Invalid.
  == 1: No temporal layering.
  >= 2: Temporal layering.

We should try to remove the wrapping optional later.

Bug: webrtc:11297
Change-Id: Id765f2dc1d31a4ba3cd424978ac6054cd60152ea
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/166528
Commit-Queue: Rasmus Brandt <brandtr@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#30336}

api/video_codecs/video_encoder_config.cc

@@ -24,6 +24,7 @@ VideoStream::VideoStream()
       max_bitrate_bps(-1),
       scale_resolution_down_by(-1.),
       max_qp(-1),
+      num_temporal_layers(absl::nullopt),
       active(true) {}
 VideoStream::VideoStream(const VideoStream& other) = default;
 
@@ -39,7 +40,7 @@ std::string VideoStream::ToString() const {
   ss << ", target_bitrate_bps:" << target_bitrate_bps;
   ss << ", max_bitrate_bps:" << max_bitrate_bps;
   ss << ", max_qp: " << max_qp;
-  ss << ", num_temporal_layers: " << num_temporal_layers.value_or(0);
+  ss << ", num_temporal_layers: " << num_temporal_layers.value_or(1);
   ss << ", bitrate_priority: " << bitrate_priority.value_or(0);
   ss << ", active: " << active;
 

api/video_codecs/video_encoder_config.h

@@ -24,28 +24,47 @@
 
 namespace webrtc {
 
+// The |VideoStream| struct describes a simulcast layer, or "stream".
 struct VideoStream {
   VideoStream();
   ~VideoStream();
   VideoStream(const VideoStream& other);
   std::string ToString() const;
 
+  // Width in pixels.
   size_t width;
+
+  // Height in pixels.
   size_t height;
+
+  // Frame rate in fps.
   int max_framerate;
 
+  // Bitrate, in bps, for the stream.
   int min_bitrate_bps;
   int target_bitrate_bps;
   int max_bitrate_bps;
+
   // Scaling factor applied to the stream size.
   // |width| and |height| values are already scaled down.
   double scale_resolution_down_by;
+
+  // Maximum Quantization Parameter to use when encoding the stream.
   int max_qp;
 
+  // Determines the number of temporal layers that the stream should be
+  // encoded with. This value should be greater than zero.
+  // TODO(brandtr): This class is used both for configuring the encoder
+  // (meaning that this field _must_ be set), and for signaling the app-level
+  // encoder settings (meaning that the field _may_ be set). We should separate
+  // this and remove this optional instead.
   absl::optional<size_t> num_temporal_layers;
 
+  // The priority of this stream, to be used when allocating resources
+  // between multiple streams.
   absl::optional<double> bitrate_priority;
 
+  // If this stream is enabled by the user, or not.
   bool active;
 };
 

api/video_codecs/vp8_temporal_layers_factory.cc

@@ -35,6 +35,7 @@ std::unique_ptr<Vp8FrameBufferController> Vp8TemporalLayersFactory::Create(
   for (int i = 0; i < num_streams; ++i) {
     int num_temporal_layers =
         SimulcastUtility::NumberOfTemporalLayers(codec, i);
+    RTC_DCHECK_GE(num_temporal_layers, 1);
     if (SimulcastUtility::IsConferenceModeScreenshare(codec) && i == 0) {
       // Legacy screenshare layers supports max 2 layers.
       num_temporal_layers = std::max(2, num_temporal_layers);

media/engine/simulcast.cc

@@ -272,7 +272,7 @@ std::vector<webrtc::VideoStream> GetNormalSimulcastLayers(
     // TODO(pbos): Fill actual temporal-layer bitrate thresholds.
     layers[s].max_qp = max_qp;
     layers[s].num_temporal_layers =
-        temporal_layers_supported ? DefaultNumberOfTemporalLayers(s, false) : 0;
+        temporal_layers_supported ? DefaultNumberOfTemporalLayers(s, false) : 1;
     layers[s].max_bitrate_bps = FindSimulcastMaxBitrateBps(width, height);
     layers[s].target_bitrate_bps = FindSimulcastTargetBitrateBps(width, height);
     int num_temporal_layers = DefaultNumberOfTemporalLayers(s, false);
@@ -343,7 +343,7 @@ std::vector<webrtc::VideoStream> GetScreenshareLayers(
   layers[0].min_bitrate_bps = webrtc::kDefaultMinVideoBitrateBps;
   layers[0].target_bitrate_bps = kScreenshareDefaultTl0BitrateKbps * 1000;
   layers[0].max_bitrate_bps = kScreenshareDefaultTl1BitrateKbps * 1000;
-  layers[0].num_temporal_layers = temporal_layers_supported ? 2 : 0;
+  layers[0].num_temporal_layers = temporal_layers_supported ? 2 : 1;
 
   // With simulcast enabled, add another spatial layer. This one will have a
   // more normal layout, with the regular 3 temporal layer pattern and no fps
@@ -388,7 +388,7 @@ std::vector<webrtc::VideoStream> GetScreenshareLayers(
     layers[1].max_qp = max_qp;
     layers[1].max_framerate = kDefaultVideoMaxFramerate;
     layers[1].num_temporal_layers =
-        temporal_layers_supported ? DefaultNumberOfTemporalLayers(1, true) : 0;
+        temporal_layers_supported ? DefaultNumberOfTemporalLayers(1, true) : 1;
     layers[1].min_bitrate_bps = using_boosted_bitrate
                                     ? kScreenshareHighStreamMinBitrateBps
                                     : layers[0].target_bitrate_bps * 2;

video/video_stream_encoder.cc

@@ -471,6 +471,16 @@ void VideoStreamEncoder::ReconfigureEncoder() {
       encoder_config_.video_stream_factory->CreateEncoderStreams(
           last_frame_info_->width, last_frame_info_->height, encoder_config_);
 
+  // Check that the higher layers do not try to set number of temporal layers
+  // to less than 1.
+  // TODO(brandtr): Get rid of the wrapping optional as it serves no purpose
+  // at this layer.
+#if RTC_DCHECK_IS_ON
+  for (const auto& stream : streams) {
+    RTC_DCHECK_GE(stream.num_temporal_layers.value_or(1), 1);
+  }
+#endif
+
   // TODO(ilnik): If configured resolution is significantly less than provided,
   // e.g. because there are not enough SSRCs for all simulcast streams,
   // signal new resolutions via SinkWants to video source.