From 3afa1b2ce8295e11d4f18b52050c01434772fb26 Mon Sep 17 00:00:00 2001 From: Jeremy Leconte Date: Wed, 28 Feb 2024 16:09:24 +0100 Subject: [PATCH] Add a SimulcastStream::GetScalabilityMode2 method that returns an optional. A call to GetScalabilityMode was added for logging purpose and causes an expectation failure for tests using 4 temporal layers. Plan is to remove the old GetScalabilityMode and keep only the one that returns an optional. Change-Id: I0e37a496bb621d9754d6572ef5838b58193aa183 Bug: b/327381318 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/341520 Reviewed-by: Henrik Andreassson Commit-Queue: Jeremy Leconte Cr-Commit-Position: refs/heads/main@{#41838} --- api/video_codecs/simulcast_stream.cc | 14 ++++++++++++ api/video_codecs/simulcast_stream.h | 3 +++ api/video_codecs/video_codec.cc | 10 +++++--- media/engine/simulcast_encoder_adapter.cc | 10 +++++--- video/video_stream_encoder.cc | 28 +++++++++++++---------- 5 files changed, 47 insertions(+), 18 deletions(-) diff --git a/api/video_codecs/simulcast_stream.cc b/api/video_codecs/simulcast_stream.cc index 312429ef9f..d4703b5e34 100644 --- a/api/video_codecs/simulcast_stream.cc +++ b/api/video_codecs/simulcast_stream.cc @@ -10,6 +10,7 @@ #include "api/video_codecs/simulcast_stream.h" +#include "absl/types/optional.h" #include "rtc_base/checks.h" namespace webrtc { @@ -34,4 +35,17 @@ ScalabilityMode SimulcastStream::GetScalabilityMode() const { return scalability_modes[numberOfTemporalLayers - 1]; } +// TODO(b/327381318): Rename to GetScalabilityMode. +absl::optional SimulcastStream::GetScalabilityMode2() const { + static const ScalabilityMode scalability_modes[3] = { + ScalabilityMode::kL1T1, + ScalabilityMode::kL1T2, + ScalabilityMode::kL1T3, + }; + if (numberOfTemporalLayers < 1 || numberOfTemporalLayers > 3) { + return absl::nullopt; + } + return scalability_modes[numberOfTemporalLayers - 1]; +} + } // namespace webrtc diff --git a/api/video_codecs/simulcast_stream.h b/api/video_codecs/simulcast_stream.h index 7c0dd5d786..57511854b7 100644 --- a/api/video_codecs/simulcast_stream.h +++ b/api/video_codecs/simulcast_stream.h @@ -11,6 +11,7 @@ #ifndef API_VIDEO_CODECS_SIMULCAST_STREAM_H_ #define API_VIDEO_CODECS_SIMULCAST_STREAM_H_ +#include "absl/types/optional.h" #include "api/video_codecs/scalability_mode.h" namespace webrtc { @@ -22,6 +23,8 @@ struct SimulcastStream { // setting to ScalabilityMode. unsigned char GetNumberOfTemporalLayers() const; ScalabilityMode GetScalabilityMode() const; + // TODO(b/327381318): Rename to GetScalabilityMode. + absl::optional GetScalabilityMode2() const; void SetNumberOfTemporalLayers(unsigned char n); int width = 0; diff --git a/api/video_codecs/video_codec.cc b/api/video_codecs/video_codec.cc index c071cb326d..9266b6d737 100644 --- a/api/video_codecs/video_codec.cc +++ b/api/video_codecs/video_codec.cc @@ -92,9 +92,13 @@ std::string VideoCodec::ToString() const { ss << ", Simulcast: {"; for (size_t i = 0; i < numberOfSimulcastStreams; ++i) { const SimulcastStream stream = simulcastStream[i]; - ss << "[" << stream.width << "x" << stream.height << " " - << ScalabilityModeToString(stream.GetScalabilityMode()) - << (stream.active ? ", active" : ", inactive") << "]"; + absl::optional scalability_mode = + stream.GetScalabilityMode2(); + if (scalability_mode.has_value()) { + ss << "[" << stream.width << "x" << stream.height << " " + << ScalabilityModeToString(*scalability_mode) + << (stream.active ? ", active" : ", inactive") << "]"; + } } ss << "}"; } diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc index e9de53d0cd..18435355aa 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -19,6 +19,7 @@ #include #include "absl/algorithm/container.h" +#include "absl/types/optional.h" #include "api/field_trials_view.h" #include "api/scoped_refptr.h" #include "api/transport/field_trial_based_config.h" @@ -808,7 +809,8 @@ webrtc::VideoCodec SimulcastEncoderAdapter::MakeStreamCodec( // By default, `scalability_mode` comes from SimulcastStream when // SimulcastEncoderAdapter is used. This allows multiple encodings of L1Tx, // but SimulcastStream currently does not support multiple spatial layers. - ScalabilityMode scalability_mode = stream_params.GetScalabilityMode(); + absl::optional scalability_mode = + stream_params.GetScalabilityMode2(); // To support the full set of scalability modes in the event that this is the // only active encoding, prefer VideoCodec::GetScalabilityMode() if all other // encodings are inactive. @@ -821,10 +823,12 @@ webrtc::VideoCodec SimulcastEncoderAdapter::MakeStreamCodec( } } if (only_active_stream) { - scalability_mode = codec.GetScalabilityMode().value(); + scalability_mode = codec.GetScalabilityMode(); } } - codec_params.SetScalabilityMode(scalability_mode); + if (scalability_mode.has_value()) { + codec_params.SetScalabilityMode(*scalability_mode); + } // Settings that are based on stream/resolution. if (is_lowest_quality_stream) { // Settings for lowest spatial resolutions. diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 73cd7826f9..beefbd2cb5 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -1158,18 +1158,22 @@ void VideoStreamEncoder::ReconfigureEncoder() { rtc::SimpleStringBuilder log_stream(log_stream_buf); log_stream << "ReconfigureEncoder: simulcast streams: "; for (size_t i = 0; i < codec.numberOfSimulcastStreams; ++i) { - log_stream << "{" << i << ": " << codec.simulcastStream[i].width << "x" - << codec.simulcastStream[i].height << " " - << ScalabilityModeToString( - codec.simulcastStream[i].GetScalabilityMode()) - << ", min_kbps: " << codec.simulcastStream[i].minBitrate - << ", target_kbps: " << codec.simulcastStream[i].targetBitrate - << ", max_kbps: " << codec.simulcastStream[i].maxBitrate - << ", max_fps: " << codec.simulcastStream[i].maxFramerate - << ", max_qp: " << codec.simulcastStream[i].qpMax << ", num_tl: " - << codec.simulcastStream[i].numberOfTemporalLayers - << ", active: " - << (codec.simulcastStream[i].active ? "true" : "false") << "}"; + absl::optional scalability_mode = + codec.simulcastStream[i].GetScalabilityMode2(); + if (scalability_mode) { + log_stream << "{" << i << ": " << codec.simulcastStream[i].width << "x" + << codec.simulcastStream[i].height << " " + << ScalabilityModeToString(*scalability_mode) + << ", min_kbps: " << codec.simulcastStream[i].minBitrate + << ", target_kbps: " << codec.simulcastStream[i].targetBitrate + << ", max_kbps: " << codec.simulcastStream[i].maxBitrate + << ", max_fps: " << codec.simulcastStream[i].maxFramerate + << ", max_qp: " << codec.simulcastStream[i].qpMax + << ", num_tl: " + << codec.simulcastStream[i].numberOfTemporalLayers + << ", active: " + << (codec.simulcastStream[i].active ? "true" : "false") << "}"; + } } if (encoder_config_.codec_type == kVideoCodecVP9 || encoder_config_.codec_type == kVideoCodecAV1) {