From 64d68c3984e80ab0e7ac56d9c745fc4498708cd0 Mon Sep 17 00:00:00 2001 From: Florent Castelli Date: Tue, 3 Sep 2024 09:14:37 +0000 Subject: [PATCH] Add WebRTC-MixedCodecSimulcast field trial Disable the checks ensuring we reject mixed-codec simulcast when the field trial is enabled. The feature is however not yet implemented. Bug: webrtc:362277533 Change-Id: Ib1601767c951d61aaa37a3d8767d0a81444d626c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/361404 Reviewed-by: Harald Alvestrand Commit-Queue: Florent Castelli Cr-Commit-Position: refs/heads/main@{#42942} --- experiments/field_trials.py | 3 ++ media/base/media_engine.cc | 12 +++--- ...er_connection_encodings_integrationtest.cc | 42 ++++++++++++++++++- pc/test/peer_connection_test_wrapper.cc | 20 ++++++++- pc/test/peer_connection_test_wrapper.h | 6 +++ 5 files changed, 76 insertions(+), 7 deletions(-) diff --git a/experiments/field_trials.py b/experiments/field_trials.py index fe3ca7d43b..ebcace03e2 100755 --- a/experiments/field_trials.py +++ b/experiments/field_trials.py @@ -101,6 +101,9 @@ ACTIVE_FIELD_TRIALS: FrozenSet[FieldTrial] = frozenset([ FieldTrial('WebRTC-LibvpxVp8Encoder-AndroidSpecificThreadingSettings', 42226191, date(2024, 9, 1)), + FieldTrial('WebRTC-MixedCodecSimulcast', + 362277533, + date(2025, 9, 1)), FieldTrial('WebRTC-Pacer-FastRetransmissions', 40235589, date(2024, 4, 1)), diff --git a/media/base/media_engine.cc b/media/base/media_engine.cc index 21ca8a260b..acb95d11c0 100644 --- a/media/base/media_engine.cc +++ b/media/base/media_engine.cc @@ -190,11 +190,13 @@ webrtc::RTCError CheckRtpParametersValues( "requested_resolution simultaniously."); } - if (i > 0 && rtp_parameters.encodings[i - 1].codec != - rtp_parameters.encodings[i].codec) { - LOG_AND_RETURN_ERROR(RTCErrorType::UNSUPPORTED_OPERATION, - "Attempted to use different codec values for " - "different encodings."); + if (!field_trials.IsEnabled("WebRTC-MixedCodecSimulcast")) { + if (i > 0 && rtp_parameters.encodings[i - 1].codec != + rtp_parameters.encodings[i].codec) { + LOG_AND_RETURN_ERROR(RTCErrorType::UNSUPPORTED_OPERATION, + "Attempted to use different codec values for " + "different encodings."); + } } } diff --git a/pc/peer_connection_encodings_integrationtest.cc b/pc/peer_connection_encodings_integrationtest.cc index 9b4bfba7fd..bce27fcfb5 100644 --- a/pc/peer_connection_encodings_integrationtest.cc +++ b/pc/peer_connection_encodings_integrationtest.cc @@ -23,6 +23,7 @@ #include "api/audio_codecs/builtin_audio_decoder_factory.h" #include "api/audio_codecs/builtin_audio_encoder_factory.h" #include "api/audio_options.h" +#include "api/field_trials_view.h" #include "api/jsep.h" #include "api/make_ref_counted.h" #include "api/media_stream_interface.h" @@ -51,6 +52,7 @@ #include "rtc_base/thread.h" #include "test/gmock.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" using ::testing::Eq; using ::testing::Optional; @@ -118,7 +120,8 @@ class PeerConnectionEncodingsIntegrationTest : public ::testing::Test { rtc::scoped_refptr CreatePc() { auto pc_wrapper = rtc::make_ref_counted( - "pc", &pss_, background_thread_.get(), background_thread_.get()); + "pc", &pss_, background_thread_.get(), background_thread_.get(), + field_trials_); pc_wrapper->CreatePc({}, CreateBuiltinAudioEncoderFactory(), CreateBuiltinAudioDecoderFactory()); return pc_wrapper; @@ -445,6 +448,7 @@ class PeerConnectionEncodingsIntegrationTest : public ::testing::Test { return true; } + test::ScopedKeyValueConfig field_trials_; rtc::PhysicalSocketServer pss_; std::unique_ptr background_thread_; }; @@ -2038,6 +2042,7 @@ TEST_F(PeerConnectionEncodingsIntegrationTest, std::optional vp9 = local_pc_wrapper->FindFirstSendCodecWithName(cricket::MEDIA_TYPE_VIDEO, "vp9"); + ASSERT_TRUE(vp9); RtpTransceiverInit init; init.direction = RtpTransceiverDirection::kSendOnly; @@ -2058,6 +2063,41 @@ TEST_F(PeerConnectionEncodingsIntegrationTest, RTCErrorType::UNSUPPORTED_OPERATION); } +TEST_F(PeerConnectionEncodingsIntegrationTest, + AddTransceiverAcceptsMixedCodecSimulcast) { + // Enable WIP mixed codec simulcast support + test::ScopedKeyValueConfig field_trials( + field_trials_, "WebRTC-MixedCodecSimulcast/Enabled/"); + rtc::scoped_refptr local_pc_wrapper = CreatePc(); + rtc::scoped_refptr remote_pc_wrapper = CreatePc(); + ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper); + + std::optional vp8 = + local_pc_wrapper->FindFirstSendCodecWithName(cricket::MEDIA_TYPE_VIDEO, + "vp8"); + ASSERT_TRUE(vp8); + std::optional vp9 = + local_pc_wrapper->FindFirstSendCodecWithName(cricket::MEDIA_TYPE_VIDEO, + "vp9"); + ASSERT_TRUE(vp9); + + RtpTransceiverInit init; + init.direction = RtpTransceiverDirection::kSendOnly; + RtpEncodingParameters encoding_parameters; + encoding_parameters.rid = "h"; + encoding_parameters.codec = vp8; + encoding_parameters.scale_resolution_down_by = 2; + init.send_encodings.push_back(encoding_parameters); + encoding_parameters.rid = "f"; + encoding_parameters.codec = vp9; + encoding_parameters.scale_resolution_down_by = 1; + init.send_encodings.push_back(encoding_parameters); + + auto transceiver_or_error = + local_pc_wrapper->pc()->AddTransceiver(cricket::MEDIA_TYPE_VIDEO, init); + ASSERT_TRUE(transceiver_or_error.ok()); +} + // Tests that use the standard path (specifying both `scalability_mode` and // `scale_resolution_down_by` or `requested_resolution`) should pass for all // codecs. diff --git a/pc/test/peer_connection_test_wrapper.cc b/pc/test/peer_connection_test_wrapper.cc index dc926c7de4..d77924fd4a 100644 --- a/pc/test/peer_connection_test_wrapper.cc +++ b/pc/test/peer_connection_test_wrapper.cc @@ -24,6 +24,7 @@ #include "api/audio/audio_processing.h" #include "api/create_peerconnection_factory.h" #include "api/environment/environment.h" +#include "api/field_trials_view.h" #include "api/media_types.h" #include "api/sequence_checker.h" #include "api/video_codecs/video_decoder_factory.h" @@ -50,6 +51,7 @@ #include "rtc_base/string_encode.h" #include "rtc_base/time_utils.h" #include "test/gtest.h" +#include "test/scoped_key_value_config.h" namespace { @@ -132,6 +134,21 @@ PeerConnectionTestWrapper::PeerConnectionTestWrapper( pc_thread_checker_.Detach(); } +PeerConnectionTestWrapper::PeerConnectionTestWrapper( + const std::string& name, + rtc::SocketServer* socket_server, + rtc::Thread* network_thread, + rtc::Thread* worker_thread, + webrtc::test::ScopedKeyValueConfig& field_trials) + : field_trials_(field_trials, ""), + name_(name), + socket_server_(socket_server), + network_thread_(network_thread), + worker_thread_(worker_thread), + pending_negotiation_(false) { + pc_thread_checker_.Detach(); +} + PeerConnectionTestWrapper::~PeerConnectionTestWrapper() { RTC_DCHECK_RUN_ON(&pc_thread_checker_); // To avoid flaky bot failures, make sure fake sources are stopped prior to @@ -173,7 +190,8 @@ bool PeerConnectionTestWrapper::CreatePc( webrtc::LibvpxVp9DecoderTemplateAdapter, webrtc::OpenH264DecoderTemplateAdapter, webrtc::Dav1dDecoderTemplateAdapter>>(), - nullptr /* audio_mixer */, nullptr /* audio_processing */); + nullptr /* audio_mixer */, nullptr /* audio_processing */, nullptr, + std::make_unique(field_trials_, "")); if (!peer_connection_factory_) { return false; } diff --git a/pc/test/peer_connection_test_wrapper.h b/pc/test/peer_connection_test_wrapper.h index 966bee0092..70337b8c45 100644 --- a/pc/test/peer_connection_test_wrapper.h +++ b/pc/test/peer_connection_test_wrapper.h @@ -19,6 +19,7 @@ #include "api/audio_codecs/audio_encoder_factory.h" #include "api/audio_options.h" #include "api/data_channel_interface.h" +#include "api/field_trials_view.h" #include "api/jsep.h" #include "api/media_stream_interface.h" #include "api/peer_connection_interface.h" @@ -48,6 +49,11 @@ class PeerConnectionTestWrapper rtc::SocketServer* socket_server, rtc::Thread* network_thread, rtc::Thread* worker_thread); + PeerConnectionTestWrapper(const std::string& name, + rtc::SocketServer* socket_server, + rtc::Thread* network_thread, + rtc::Thread* worker_thread, + webrtc::test::ScopedKeyValueConfig& field_trials); virtual ~PeerConnectionTestWrapper(); bool CreatePc(