Add killswitch for receive-only setCodecPreferences change
Adds a killswitch
WebRTC-SetCodecPreferences-ReceiveOnlyFilterInsteadOfThrow
to accompany the spec-change to throw when codec capabilities
are taken from the RtpSender instead of the RtpReceiver.
With the killswitch triggered, such codecs will be filtered.
BUG=webrtc:15396
Change-Id: I7d27111c72085eb7a7b2a1e66d0a08d12883ce17
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/341460
Reviewed-by: Florent Castelli <orphis@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@microsoft.com>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#41845}
diff --git a/experiments/field_trials.py b/experiments/field_trials.py
index 6c935ea..c748cd6 100755
--- a/experiments/field_trials.py
+++ b/experiments/field_trials.py
@@ -131,6 +131,9 @@
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 acefd9e..e114e5d 100644
--- a/pc/peer_connection_media_unittest.cc
+++ b/pc/peer_connection_media_unittest.cc
@@ -176,19 +176,19 @@
EnableFakeMedia(factory_dependencies, std::move(media_engine));
factory_dependencies.event_log_factory =
std::make_unique<RtcEventLogFactory>();
-
+ factory_dependencies.trials = std::move(field_trials_);
auto pc_factory =
CreateModularPeerConnectionFactory(std::move(factory_dependencies));
auto fake_port_allocator = std::make_unique<cricket::FakePortAllocator>(
rtc::Thread::Current(),
- std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get()),
- &field_trials_);
+ std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get()), nullptr);
auto observer = std::make_unique<MockPeerConnectionObserver>();
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 @@
return sdp_semantics_ == SdpSemantics::kUnifiedPlan;
}
- test::ScopedKeyValueConfig field_trials_;
+ std::unique_ptr<test::ScopedKeyValueConfig> field_trials_;
std::unique_ptr<rtc::VirtualSocketServer> vss_;
rtc::AutoSocketServerThread main_;
const SdpSemantics sdp_semantics_;
@@ -1641,6 +1641,25 @@
}
TEST_F(PeerConnectionMediaTestUnifiedPlan,
+ SetCodecPreferencesAudioSendOnlyKillswitch) {
+ field_trials_ = std::make_unique<test::ScopedKeyValueConfig>(
+ "WebRTC-SetCodecPreferences-ReceiveOnlyFilterInsteadOfThrow/Disabled/");
+ auto fake_engine = std::make_unique<FakeMediaEngine>();
+ 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();
@@ -1735,6 +1754,25 @@
}
TEST_F(PeerConnectionMediaTestUnifiedPlan,
+ SetCodecPreferencesVideoSendOnlyKillswitch) {
+ field_trials_ = std::make_unique<test::ScopedKeyValueConfig>(
+ "WebRTC-SetCodecPreferences-ReceiveOnlyFilterInsteadOfThrow/Disabled/");
+ auto fake_engine = std::make_unique<FakeMediaEngine>();
+ 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 55a0f7d..e585667 100644
--- a/pc/rtp_transceiver.cc
+++ b/pc/rtp_transceiver.cc
@@ -40,16 +40,17 @@
namespace {
RTCError VerifyCodecPreferences(
- const std::vector<RtpCodecCapability>& codecs,
- const std::vector<cricket::Codec>& recv_codecs) {
+ const std::vector<RtpCodecCapability>& unfiltered_codecs,
+ const std::vector<cricket::Codec>& 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<RtpCodecCapability> 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 @@
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 @@
} 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;
