diff --git a/experiments/field_trials.py b/experiments/field_trials.py index 6c935eaeeb..c748cd6ac4 100755 --- a/experiments/field_trials.py +++ b/experiments/field_trials.py @@ -131,6 +131,9 @@ ACTIVE_FIELD_TRIALS: FrozenSet[FieldTrial] = frozenset([ FieldTrial('WebRTC-SendPacketsOnWorkerThread', 'webrtc:14502', date(2024, 4, 1)), + FieldTrial('WebRTC-SetCodecPreferences-ReceiveOnlyFilterInsteadOfThrow', + 'webrtc:15396', + date(2024, 12, 1)), FieldTrial('WebRTC-SrtpRemoveReceiveStream', 'webrtc:15604', date(2024, 10, 1)), diff --git a/pc/peer_connection_media_unittest.cc b/pc/peer_connection_media_unittest.cc index acefd9ebec..e114e5d926 100644 --- a/pc/peer_connection_media_unittest.cc +++ b/pc/peer_connection_media_unittest.cc @@ -176,19 +176,19 @@ class PeerConnectionMediaBaseTest : public ::testing::Test { EnableFakeMedia(factory_dependencies, std::move(media_engine)); factory_dependencies.event_log_factory = std::make_unique(); - + factory_dependencies.trials = std::move(field_trials_); auto pc_factory = CreateModularPeerConnectionFactory(std::move(factory_dependencies)); auto fake_port_allocator = std::make_unique( rtc::Thread::Current(), - std::make_unique(vss_.get()), - &field_trials_); + std::make_unique(vss_.get()), nullptr); auto observer = std::make_unique(); auto modified_config = config; modified_config.sdp_semantics = sdp_semantics_; PeerConnectionDependencies pc_dependencies(observer.get()); pc_dependencies.allocator = std::move(fake_port_allocator); + auto result = pc_factory->CreatePeerConnectionOrError( modified_config, std::move(pc_dependencies)); if (!result.ok()) { @@ -253,7 +253,7 @@ class PeerConnectionMediaBaseTest : public ::testing::Test { return sdp_semantics_ == SdpSemantics::kUnifiedPlan; } - test::ScopedKeyValueConfig field_trials_; + std::unique_ptr field_trials_; std::unique_ptr vss_; rtc::AutoSocketServerThread main_; const SdpSemantics sdp_semantics_; @@ -1640,6 +1640,25 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan, EXPECT_TRUE(CompareCodecs(sender_audio_codecs, codecs)); } +TEST_F(PeerConnectionMediaTestUnifiedPlan, + SetCodecPreferencesAudioSendOnlyKillswitch) { + field_trials_ = std::make_unique( + "WebRTC-SetCodecPreferences-ReceiveOnlyFilterInsteadOfThrow/Disabled/"); + auto fake_engine = std::make_unique(); + auto send_codecs = fake_engine->voice().send_codecs(); + send_codecs.push_back(cricket::CreateAudioCodec(send_codecs.back().id + 1, + "send_only_codec", 0, 1)); + fake_engine->SetAudioSendCodecs(send_codecs); + + auto caller = CreatePeerConnectionWithAudio(std::move(fake_engine)); + + auto transceiver = caller->pc()->GetTransceivers().front(); + auto send_capabilities = caller->pc_factory()->GetRtpSenderCapabilities( + cricket::MediaType::MEDIA_TYPE_AUDIO); + + EXPECT_TRUE(transceiver->SetCodecPreferences(send_capabilities.codecs).ok()); +} + TEST_F(PeerConnectionMediaTestUnifiedPlan, SetCodecPreferencesVideoRejectsAudioCodec) { auto caller = CreatePeerConnectionWithVideo(); @@ -1734,6 +1753,25 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan, EXPECT_TRUE(CompareCodecs(sender_video_codecs, codecs)); } +TEST_F(PeerConnectionMediaTestUnifiedPlan, + SetCodecPreferencesVideoSendOnlyKillswitch) { + field_trials_ = std::make_unique( + "WebRTC-SetCodecPreferences-ReceiveOnlyFilterInsteadOfThrow/Disabled/"); + auto fake_engine = std::make_unique(); + auto send_codecs = fake_engine->voice().send_codecs(); + send_codecs.push_back( + cricket::CreateVideoCodec(send_codecs.back().id + 1, "send_only_codec")); + fake_engine->SetAudioSendCodecs(send_codecs); + + auto caller = CreatePeerConnectionWithAudio(std::move(fake_engine)); + + auto transceiver = caller->pc()->GetTransceivers().front(); + auto send_capabilities = caller->pc_factory()->GetRtpSenderCapabilities( + cricket::MediaType::MEDIA_TYPE_AUDIO); + + EXPECT_TRUE(transceiver->SetCodecPreferences(send_capabilities.codecs).ok()); +} + TEST_F(PeerConnectionMediaTestUnifiedPlan, SetCodecPreferencesVideoCodecDuplicatesRemoved) { auto caller = CreatePeerConnectionWithVideo(); diff --git a/pc/rtp_transceiver.cc b/pc/rtp_transceiver.cc index 55a0f7d584..e585667843 100644 --- a/pc/rtp_transceiver.cc +++ b/pc/rtp_transceiver.cc @@ -40,16 +40,17 @@ namespace webrtc { namespace { RTCError VerifyCodecPreferences( - const std::vector& codecs, - const std::vector& recv_codecs) { + const std::vector& unfiltered_codecs, + const std::vector& recv_codecs, + const FieldTrialsView& field_trials) { // If the intersection between codecs and - // RTCRtpSender.getCapabilities(kind).codecs or the intersection between - // codecs and RTCRtpReceiver.getCapabilities(kind).codecs contains only - // contains RTX, RED, FEC or CN codecs or is an empty set, throw + // RTCRtpReceiver.getCapabilities(kind).codecs only contains RTX, RED, FEC + // codecs or Comfort Noise codecs or is an empty set, throw // InvalidModificationError. // This ensures that we always have something to offer, regardless of // transceiver.direction. - + // TODO(fippo): clean up the filtering killswitch + std::vector codecs = unfiltered_codecs; if (!absl::c_any_of(codecs, [&recv_codecs](const RtpCodecCapability& codec) { return codec.IsMediaCodec() && absl::c_any_of(recv_codecs, @@ -71,10 +72,26 @@ RTCError VerifyCodecPreferences( return codec.MatchesRtpCodec(codec_preference); }); if (!is_recv_codec) { - LOG_AND_RETURN_ERROR( - RTCErrorType::INVALID_MODIFICATION, - std::string("Invalid codec preferences: invalid codec with name \"") + - codec_preference.name + "\"."); + if (!field_trials.IsDisabled( + "WebRTC-SetCodecPreferences-ReceiveOnlyFilterInsteadOfThrow")) { + LOG_AND_RETURN_ERROR( + RTCErrorType::INVALID_MODIFICATION, + std::string( + "Invalid codec preferences: invalid codec with name \"") + + codec_preference.name + "\"."); + } else { + // Killswitch behavior: filter out any codec not in receive codecs. + codecs.erase(std::remove_if( + codecs.begin(), codecs.end(), + [&recv_codecs](const RtpCodecCapability& codec) { + return codec.IsMediaCodec() && + !absl::c_any_of( + recv_codecs, + [&codec](const cricket::Codec& recv_codec) { + return recv_codec.MatchesRtpCodec(codec); + }); + })); + } } } @@ -670,7 +687,8 @@ RTCError RtpTransceiver::SetCodecPreferences( } else if (media_type_ == cricket::MEDIA_TYPE_VIDEO) { recv_codecs = media_engine()->video().recv_codecs(context()->use_rtx()); } - result = VerifyCodecPreferences(codecs, recv_codecs); + result = VerifyCodecPreferences(codecs, recv_codecs, + context()->env().field_trials()); if (result.ok()) { codec_preferences_ = codecs;