diff --git a/api/video_codecs/video_codec.h b/api/video_codecs/video_codec.h index 10bceda0d2..496cfb5e22 100644 --- a/api/video_codecs/video_codec.h +++ b/api/video_codecs/video_codec.h @@ -148,6 +148,15 @@ class RTC_EXPORT VideoCodec { bool active; unsigned int qpMax; + // The actual number of simulcast streams. This is <= 1 in singlecast (it can + // be 0 in old code paths), but it is also 1 in the {active,inactive,inactive} + // "single RTP simulcast" use case and the legacy kSVC use case. In all other + // cases this is the same as the number of encodings (which may include + // inactive encodings). In other words: + // - `numberOfSimulcastStreams <= 1` in singlecast and singlecast-like setups + // including legacy kSVC (encodings interpreted as spatial layers) or + // standard kSVC (1 active encoding). + // - `numberOfSimulcastStreams > 1` in simulcast of 2+ active encodings. unsigned char numberOfSimulcastStreams; SimulcastStream simulcastStream[kMaxSimulcastStreams]; SpatialLayer spatialLayers[kMaxSpatialLayers]; diff --git a/pc/BUILD.gn b/pc/BUILD.gn index a89e8a6908..2f671d2694 100644 --- a/pc/BUILD.gn +++ b/pc/BUILD.gn @@ -2435,6 +2435,7 @@ if (rtc_include_tests && !build_with_chromium) { "../api/transport:field_trial_based_config", "../api/transport:sctp_transport_factory_interface", "../api/transport/rtp:rtp_source", + "../api/units:data_rate", "../api/units:time_delta", "../api/units:timestamp", "../api/video:builtin_video_bitrate_allocator_factory", diff --git a/pc/peer_connection_encodings_integrationtest.cc b/pc/peer_connection_encodings_integrationtest.cc index 8e86724179..4ebcc0a17e 100644 --- a/pc/peer_connection_encodings_integrationtest.cc +++ b/pc/peer_connection_encodings_integrationtest.cc @@ -17,6 +17,7 @@ #include "api/audio_codecs/opus_audio_encoder_factory.h" #include "api/rtp_parameters.h" #include "api/stats/rtcstats_objects.h" +#include "api/units/data_rate.h" #include "api/video_codecs/video_decoder_factory_template.h" #include "api/video_codecs/video_decoder_factory_template_dav1d_adapter.h" #include "api/video_codecs/video_decoder_factory_template_libvpx_vp8_adapter.h" @@ -55,6 +56,12 @@ constexpr TimeDelta kDefaultTimeout = TimeDelta::Seconds(5); // using simulated time. constexpr TimeDelta kLongTimeoutForRampingUp = TimeDelta::Minutes(1); +// The max bitrate 1500 kbps may be subject to change in the future. What we're +// interested in here is that all code paths that result in L1T3 result in the +// same target bitrate which does not exceed this limit. +constexpr DataRate kVp9ExpectedMaxBitrateForL1T3 = + DataRate::KilobitsPerSec(1500); + struct StringParamToString { std::string operator()(const ::testing::TestParamInfo& info) { return info.param; @@ -792,6 +799,103 @@ TEST_F(PeerConnectionEncodingsIntegrationTest, EXPECT_EQ(*outbound_rtps[2]->bytes_sent, 0u); } +TEST_F(PeerConnectionEncodingsIntegrationTest, VP9_TargetBitrate_LegacyL1T3) { + rtc::scoped_refptr local_pc_wrapper = CreatePc(); + rtc::scoped_refptr remote_pc_wrapper = CreatePc(); + ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper); + + std::vector layers = + CreateLayers({"f", "h", "q"}, /*active=*/true); + rtc::scoped_refptr transceiver = + AddTransceiverWithSimulcastLayers(local_pc_wrapper, remote_pc_wrapper, + layers); + std::vector codecs = + GetCapabilitiesAndRestrictToCodec(local_pc_wrapper, "VP9"); + transceiver->SetCodecPreferences(codecs); + + // In legacy SVC, disabling the bottom two layers encodings is interpreted as + // disabling the bottom two spatial layers resulting in L1T3. + rtc::scoped_refptr sender = transceiver->sender(); + RtpParameters parameters = sender->GetParameters(); + parameters.encodings[0].active = false; + parameters.encodings[1].active = false; + parameters.encodings[2].active = true; + sender->SetParameters(parameters); + + NegotiateWithSimulcastTweaks(local_pc_wrapper, remote_pc_wrapper, layers); + local_pc_wrapper->WaitForConnection(); + remote_pc_wrapper->WaitForConnection(); + + // Wait until 720p L1T3 has ramped up to 720p. It may take additional time + // for the target bitrate to reach its maximum. + ASSERT_TRUE_WAIT(HasOutboundRtpWithRidAndScalabilityMode(local_pc_wrapper, + "f", "L1T3", 720), + kLongTimeoutForRampingUp.ms()); + + // The target bitrate typically reaches `kVp9ExpectedMaxBitrateForL1T3` + // in a short period of time. However to reduce risk of flakiness in bot + // environments, this test only fails if we we exceed the expected target. + rtc::Thread::Current()->SleepMs(1000); + rtc::scoped_refptr report = GetStats(local_pc_wrapper); + std::vector outbound_rtps = + report->GetStatsOfType(); + ASSERT_THAT(outbound_rtps, SizeIs(1)); + DataRate target_bitrate = + DataRate::BitsPerSec(*outbound_rtps[0]->target_bitrate); + EXPECT_LE(target_bitrate.kbps(), kVp9ExpectedMaxBitrateForL1T3.kbps()); +} + +// Test coverage for https://crbug.com/1455039. +TEST_F(PeerConnectionEncodingsIntegrationTest, VP9_TargetBitrate_StandardL1T3) { + rtc::scoped_refptr local_pc_wrapper = CreatePc(); + rtc::scoped_refptr remote_pc_wrapper = CreatePc(); + ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper); + + std::vector layers = + CreateLayers({"f", "h", "q"}, /*active=*/true); + rtc::scoped_refptr transceiver = + AddTransceiverWithSimulcastLayers(local_pc_wrapper, remote_pc_wrapper, + layers); + std::vector codecs = + GetCapabilitiesAndRestrictToCodec(local_pc_wrapper, "VP9"); + transceiver->SetCodecPreferences(codecs); + + // With standard APIs, L1T3 is explicitly specified and the encodings refers + // to the RTP streams, not the spatial layers. The end result should be + // equivalent to the legacy L1T3 case. + rtc::scoped_refptr sender = transceiver->sender(); + RtpParameters parameters = sender->GetParameters(); + parameters.encodings[0].active = true; + parameters.encodings[0].scale_resolution_down_by = 1.0; + parameters.encodings[0].scalability_mode = "L1T3"; + parameters.encodings[1].active = false; + parameters.encodings[2].active = false; + sender->SetParameters(parameters); + + NegotiateWithSimulcastTweaks(local_pc_wrapper, remote_pc_wrapper, layers); + local_pc_wrapper->WaitForConnection(); + remote_pc_wrapper->WaitForConnection(); + + // Wait until 720p L1T3 has ramped up to 720p. It may take additional time + // for the target bitrate to reach its maximum. + ASSERT_TRUE_WAIT(HasOutboundRtpWithRidAndScalabilityMode(local_pc_wrapper, + "f", "L1T3", 720), + kLongTimeoutForRampingUp.ms()); + + // The target bitrate typically reaches `kVp9ExpectedMaxBitrateForL1T3` + // in a short period of time. However to reduce risk of flakiness in bot + // environments, this test only fails if we we exceed the expected target. + rtc::Thread::Current()->SleepMs(1000); + rtc::scoped_refptr report = GetStats(local_pc_wrapper); + std::vector outbound_rtps = + report->GetStatsOfType(); + ASSERT_THAT(outbound_rtps, SizeIs(3)); + auto* outbound_rtp = FindOutboundRtpByRid(outbound_rtps, "f"); + ASSERT_TRUE(outbound_rtp); + DataRate target_bitrate = DataRate::BitsPerSec(*outbound_rtp->target_bitrate); + EXPECT_LE(target_bitrate.kbps(), kVp9ExpectedMaxBitrateForL1T3.kbps()); +} + // Tests that use the standard path (specifying both `scalability_mode` and // `scale_resolution_down_by`) should pass for all codecs. class PeerConnectionEncodingsIntegrationParameterizedTest diff --git a/video/adaptation/bitrate_constraint.cc b/video/adaptation/bitrate_constraint.cc index bc36723d48..2f92095b2b 100644 --- a/video/adaptation/bitrate_constraint.cc +++ b/video/adaptation/bitrate_constraint.cc @@ -58,7 +58,8 @@ bool BitrateConstraint::IsAdaptationUpAllowed( } if (VideoStreamEncoderResourceManager::IsSimulcastOrMultipleSpatialLayers( - encoder_settings_->encoder_config())) { + encoder_settings_->encoder_config(), + encoder_settings_->video_codec())) { // Resolution bitrate limits usage is restricted to singlecast. return true; } diff --git a/video/adaptation/bitrate_constraint_unittest.cc b/video/adaptation/bitrate_constraint_unittest.cc index f9cb87e3c1..8a416db1fa 100644 --- a/video/adaptation/bitrate_constraint_unittest.cc +++ b/video/adaptation/bitrate_constraint_unittest.cc @@ -47,7 +47,7 @@ void FillCodecConfig(VideoCodec* video_codec, bool svc) { size_t num_layers = params.size(); video_codec->codecType = kVideoCodecVP8; - video_codec->numberOfSimulcastStreams = num_layers; + video_codec->numberOfSimulcastStreams = svc ? 1 : num_layers; encoder_config->number_of_streams = svc ? 1 : num_layers; encoder_config->simulcast_layers.resize(num_layers); diff --git a/video/adaptation/video_stream_encoder_resource_manager.cc b/video/adaptation/video_stream_encoder_resource_manager.cc index 0dcbc01ab6..46db686703 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.cc +++ b/video/adaptation/video_stream_encoder_resource_manager.cc @@ -815,7 +815,8 @@ void VideoStreamEncoderResourceManager::OnQualityRampUp() { } bool VideoStreamEncoderResourceManager::IsSimulcastOrMultipleSpatialLayers( - const VideoEncoderConfig& encoder_config) { + const VideoEncoderConfig& encoder_config, + const VideoCodec& video_codec) { const std::vector& simulcast_layers = encoder_config.simulcast_layers; if (simulcast_layers.empty()) { @@ -824,7 +825,7 @@ bool VideoStreamEncoderResourceManager::IsSimulcastOrMultipleSpatialLayers( absl::optional num_spatial_layers; if (simulcast_layers[0].scalability_mode.has_value() && - encoder_config.number_of_streams == 1) { + video_codec.numberOfSimulcastStreams == 1) { num_spatial_layers = ScalabilityModeToNumSpatialLayers( *simulcast_layers[0].scalability_mode); } diff --git a/video/adaptation/video_stream_encoder_resource_manager.h b/video/adaptation/video_stream_encoder_resource_manager.h index e0de3f7d19..8925157bcf 100644 --- a/video/adaptation/video_stream_encoder_resource_manager.h +++ b/video/adaptation/video_stream_encoder_resource_manager.h @@ -153,7 +153,8 @@ class VideoStreamEncoderResourceManager void OnQualityRampUp() override; static bool IsSimulcastOrMultipleSpatialLayers( - const VideoEncoderConfig& encoder_config); + const VideoEncoderConfig& encoder_config, + const VideoCodec& video_codec); private: class InitialFrameDropper; diff --git a/video/config/video_encoder_config.h b/video/config/video_encoder_config.h index 5a79d58cbf..59c9a39f82 100644 --- a/video/config/video_encoder_config.h +++ b/video/config/video_encoder_config.h @@ -181,9 +181,15 @@ class VideoEncoderConfig { // down to lower layers for the video encoding. // `simulcast_layers` is also used for configuring non-simulcast (when there // is a single VideoStream). + // We have the same number of `simulcast_layers` as we have negotiated + // encodings, for example 3 are used in both simulcast and legacy kSVC. std::vector simulcast_layers; // Max number of encoded VideoStreams to produce. + // This is the same as the number of encodings negotiated (i.e. SSRCs), + // whether or not those encodings are `active`, except for when legacy kSVC + // is used. In this case we have three SSRCs but `number_of_streams` is + // changed to 1 to tell lower layers to limit the number of streams. size_t number_of_streams; // Legacy Google conference mode flag for simulcast screenshare diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 44fdbaa86b..c3d855c1c6 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -405,7 +405,7 @@ void ApplySpatialLayerBitrateLimits( return; } if (VideoStreamEncoderResourceManager::IsSimulcastOrMultipleSpatialLayers( - encoder_config) || + encoder_config, *codec) || encoder_config.simulcast_layers.size() <= 1) { // Resolution bitrate limits usage is restricted to singlecast. return;