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}
diff --git a/api/video_codecs/video_encoder_config.cc b/api/video_codecs/video_encoder_config.cc
index 2b1adc0..6efcbf2 100644
--- a/api/video_codecs/video_encoder_config.cc
+++ b/api/video_codecs/video_encoder_config.cc
@@ -24,6 +24,7 @@
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 @@
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;
diff --git a/api/video_codecs/video_encoder_config.h b/api/video_codecs/video_encoder_config.h
index 399b3cc..ef8db10 100644
--- a/api/video_codecs/video_encoder_config.h
+++ b/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;
};
diff --git a/api/video_codecs/vp8_temporal_layers_factory.cc b/api/video_codecs/vp8_temporal_layers_factory.cc
index 1de925d..193494d 100644
--- a/api/video_codecs/vp8_temporal_layers_factory.cc
+++ b/api/video_codecs/vp8_temporal_layers_factory.cc
@@ -35,6 +35,7 @@
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);