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}
This commit is contained in:
Philipp Hancke 2024-02-28 11:57:35 +01:00 committed by WebRTC LUCI CQ
parent 16ac10d9f7
commit a5cd6643f6
3 changed files with 74 additions and 15 deletions

View File

@ -131,6 +131,9 @@ ACTIVE_FIELD_TRIALS: FrozenSet[FieldTrial] = frozenset([
FieldTrial('WebRTC-SendPacketsOnWorkerThread', FieldTrial('WebRTC-SendPacketsOnWorkerThread',
'webrtc:14502', 'webrtc:14502',
date(2024, 4, 1)), date(2024, 4, 1)),
FieldTrial('WebRTC-SetCodecPreferences-ReceiveOnlyFilterInsteadOfThrow',
'webrtc:15396',
date(2024, 12, 1)),
FieldTrial('WebRTC-SrtpRemoveReceiveStream', FieldTrial('WebRTC-SrtpRemoveReceiveStream',
'webrtc:15604', 'webrtc:15604',
date(2024, 10, 1)), date(2024, 10, 1)),

View File

@ -176,19 +176,19 @@ class PeerConnectionMediaBaseTest : public ::testing::Test {
EnableFakeMedia(factory_dependencies, std::move(media_engine)); EnableFakeMedia(factory_dependencies, std::move(media_engine));
factory_dependencies.event_log_factory = factory_dependencies.event_log_factory =
std::make_unique<RtcEventLogFactory>(); std::make_unique<RtcEventLogFactory>();
factory_dependencies.trials = std::move(field_trials_);
auto pc_factory = auto pc_factory =
CreateModularPeerConnectionFactory(std::move(factory_dependencies)); CreateModularPeerConnectionFactory(std::move(factory_dependencies));
auto fake_port_allocator = std::make_unique<cricket::FakePortAllocator>( auto fake_port_allocator = std::make_unique<cricket::FakePortAllocator>(
rtc::Thread::Current(), rtc::Thread::Current(),
std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get()), std::make_unique<rtc::BasicPacketSocketFactory>(vss_.get()), nullptr);
&field_trials_);
auto observer = std::make_unique<MockPeerConnectionObserver>(); auto observer = std::make_unique<MockPeerConnectionObserver>();
auto modified_config = config; auto modified_config = config;
modified_config.sdp_semantics = sdp_semantics_; modified_config.sdp_semantics = sdp_semantics_;
PeerConnectionDependencies pc_dependencies(observer.get()); PeerConnectionDependencies pc_dependencies(observer.get());
pc_dependencies.allocator = std::move(fake_port_allocator); pc_dependencies.allocator = std::move(fake_port_allocator);
auto result = pc_factory->CreatePeerConnectionOrError( auto result = pc_factory->CreatePeerConnectionOrError(
modified_config, std::move(pc_dependencies)); modified_config, std::move(pc_dependencies));
if (!result.ok()) { if (!result.ok()) {
@ -253,7 +253,7 @@ class PeerConnectionMediaBaseTest : public ::testing::Test {
return sdp_semantics_ == SdpSemantics::kUnifiedPlan; return sdp_semantics_ == SdpSemantics::kUnifiedPlan;
} }
test::ScopedKeyValueConfig field_trials_; std::unique_ptr<test::ScopedKeyValueConfig> field_trials_;
std::unique_ptr<rtc::VirtualSocketServer> vss_; std::unique_ptr<rtc::VirtualSocketServer> vss_;
rtc::AutoSocketServerThread main_; rtc::AutoSocketServerThread main_;
const SdpSemantics sdp_semantics_; const SdpSemantics sdp_semantics_;
@ -1640,6 +1640,25 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan,
EXPECT_TRUE(CompareCodecs(sender_audio_codecs, codecs)); EXPECT_TRUE(CompareCodecs(sender_audio_codecs, codecs));
} }
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, TEST_F(PeerConnectionMediaTestUnifiedPlan,
SetCodecPreferencesVideoRejectsAudioCodec) { SetCodecPreferencesVideoRejectsAudioCodec) {
auto caller = CreatePeerConnectionWithVideo(); auto caller = CreatePeerConnectionWithVideo();
@ -1734,6 +1753,25 @@ TEST_F(PeerConnectionMediaTestUnifiedPlan,
EXPECT_TRUE(CompareCodecs(sender_video_codecs, codecs)); EXPECT_TRUE(CompareCodecs(sender_video_codecs, codecs));
} }
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, TEST_F(PeerConnectionMediaTestUnifiedPlan,
SetCodecPreferencesVideoCodecDuplicatesRemoved) { SetCodecPreferencesVideoCodecDuplicatesRemoved) {
auto caller = CreatePeerConnectionWithVideo(); auto caller = CreatePeerConnectionWithVideo();

View File

@ -40,16 +40,17 @@ namespace webrtc {
namespace { namespace {
RTCError VerifyCodecPreferences( RTCError VerifyCodecPreferences(
const std::vector<RtpCodecCapability>& codecs, const std::vector<RtpCodecCapability>& unfiltered_codecs,
const std::vector<cricket::Codec>& recv_codecs) { const std::vector<cricket::Codec>& recv_codecs,
const FieldTrialsView& field_trials) {
// If the intersection between codecs and // If the intersection between codecs and
// RTCRtpSender.getCapabilities(kind).codecs or the intersection between // RTCRtpReceiver.getCapabilities(kind).codecs only contains RTX, RED, FEC
// codecs and RTCRtpReceiver.getCapabilities(kind).codecs contains only // codecs or Comfort Noise codecs or is an empty set, throw
// contains RTX, RED, FEC or CN codecs or is an empty set, throw
// InvalidModificationError. // InvalidModificationError.
// This ensures that we always have something to offer, regardless of // This ensures that we always have something to offer, regardless of
// transceiver.direction. // 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) { if (!absl::c_any_of(codecs, [&recv_codecs](const RtpCodecCapability& codec) {
return codec.IsMediaCodec() && return codec.IsMediaCodec() &&
absl::c_any_of(recv_codecs, absl::c_any_of(recv_codecs,
@ -71,10 +72,26 @@ RTCError VerifyCodecPreferences(
return codec.MatchesRtpCodec(codec_preference); return codec.MatchesRtpCodec(codec_preference);
}); });
if (!is_recv_codec) { if (!is_recv_codec) {
if (!field_trials.IsDisabled(
"WebRTC-SetCodecPreferences-ReceiveOnlyFilterInsteadOfThrow")) {
LOG_AND_RETURN_ERROR( LOG_AND_RETURN_ERROR(
RTCErrorType::INVALID_MODIFICATION, RTCErrorType::INVALID_MODIFICATION,
std::string("Invalid codec preferences: invalid codec with name \"") + std::string(
"Invalid codec preferences: invalid codec with name \"") +
codec_preference.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) { } else if (media_type_ == cricket::MEDIA_TYPE_VIDEO) {
recv_codecs = media_engine()->video().recv_codecs(context()->use_rtx()); 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()) { if (result.ok()) {
codec_preferences_ = codecs; codec_preferences_ = codecs;