Source code
Revision control
Copy as Markdown
Other Tools
From: Michael Froman <mjfroman@mac.com>
Date: Fri, 19 Sep 2025 14:44:43 -0500
Subject: Moz backout [M139 merge] Revert "Change SetLocalContent /
SetRemoteContent to update header extensions"
Upstream attempted to revert a commit in the release branch, but did
a poor job. Commit 1ab982fa34 results in changes after reverting. We'll
back this one out as a pre-stack commit and then cherry-pick the real
trunk version of the revert in d27b4ef208.
---
media/base/codec.h | 3 ++
pc/channel.cc | 13 ++++--
pc/channel_unittest.cc | 24 +++++++-----
pc/congestion_control_integrationtest.cc | 50 +++++++++++++++++++++++-
pc/peer_connection_integrationtest.cc | 14 +++++--
test/peer_scenario/tests/l4s_test.cc | 2 +-
6 files changed, 87 insertions(+), 19 deletions(-)
diff --git a/media/base/codec.h b/media/base/codec.h
index 694589dd14..e26306df20 100644
--- a/media/base/codec.h
+++ b/media/base/codec.h
@@ -192,6 +192,9 @@ struct RTC_EXPORT Codec {
sink.Append("video/");
}
absl::Format(&sink, "%s/%d/%d", c.name, c.clockrate, c.channels);
+ if (c.packetization) {
+ absl::Format(&sink, ",packetization=%s", *c.packetization);
+ }
for (auto param : c.params) {
sink.Append(";");
sink.Append(param.first);
diff --git a/pc/channel.cc b/pc/channel.cc
index bccd1e2480..2b548064a4 100644
--- a/pc/channel.cc
+++ b/pc/channel.cc
@@ -911,7 +911,6 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content,
}
}
}
-
last_recv_params_ = recv_params;
if (!UpdateLocalStreams_w(content->streams(), type, error_desc)) {
@@ -922,7 +921,7 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content,
set_local_content_direction(content->direction());
UpdateMediaSendRecvState_w();
- // Disabled because suggeting PTs takes thread jumps.
+ // Disabled because suggesting PTs takes thread jumps.
// RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(0);
@@ -1038,7 +1037,6 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content,
media_send_channel()->SetExtmapAllowMixed(content->extmap_allow_mixed());
VideoReceiverParameters recv_params = last_recv_params_;
-
MediaChannelParametersFromMediaDescription(
content, header_extensions,
RtpTransceiverDirectionHasRecv(content->direction()), &recv_params);
@@ -1113,7 +1111,12 @@ bool VideoChannel::SetLocalContent_w(const MediaContentDescription* content,
last_recv_params_ = recv_params;
+ // Also update send parameters if header extensions are changed.
+ needs_send_params_update |=
+ (last_send_params_.extensions != header_extensions &&
+ !send_params.codecs.empty());
if (needs_send_params_update) {
+ send_params.extensions = header_extensions;
if (!media_send_channel()->SetSenderParameters(send_params)) {
error_desc = StringFormat(
"Failed to set send parameters for m-section with mid='%s'.",
@@ -1224,7 +1227,11 @@ bool VideoChannel::SetRemoteContent_w(const MediaContentDescription* content,
media_send_channel()->SendCodecRtxTime());
last_send_params_ = send_params;
+ needs_recv_params_update |=
+ (recv_params.extensions != send_params.extensions &&
+ !recv_params.codecs.empty());
if (needs_recv_params_update) {
+ recv_params.extensions = send_params.extensions;
if (!media_receive_channel()->SetReceiverParameters(recv_params)) {
error_desc = StringFormat(
"Failed to set recv parameters for m-section with mid='%s'.",
diff --git a/pc/channel_unittest.cc b/pc/channel_unittest.cc
index e3488a98a9..4bb9573632 100644
--- a/pc/channel_unittest.cc
+++ b/pc/channel_unittest.cc
@@ -2385,18 +2385,22 @@ TEST_F(VideoChannelSingleThreadTest,
EXPECT_THAT(
media_receive_channel1_impl()->recv_codecs(),
- ElementsAre(AllOf(Field(&webrtc::Codec::id, 96),
- Field(&webrtc::Codec::packetization, "foo")),
- AllOf(Field(&webrtc::Codec::id, 98),
- Field(&webrtc::Codec::packetization, std::nullopt))));
+ ElementsAre(
+ AllOf(Field("id", &webrtc::Codec::id, 96),
+ Field("packetization", &webrtc::Codec::packetization, "foo")),
+ AllOf(Field("id", &webrtc::Codec::id, 98),
+ Field("packetization", &webrtc::Codec::packetization,
+ std::nullopt))));
EXPECT_THAT(
media_send_channel1_impl()->send_codecs(),
- ElementsAre(AllOf(Field(&webrtc::Codec::id, 96),
- Field(&webrtc::Codec::packetization, "foo")),
- AllOf(Field(&webrtc::Codec::id, 97),
- Field(&webrtc::Codec::packetization, "bar")),
- AllOf(Field(&webrtc::Codec::id, 99),
- Field(&webrtc::Codec::packetization, std::nullopt))));
+ ElementsAre(
+ AllOf(Field("id", &webrtc::Codec::id, 96),
+ Field("packetization", &webrtc::Codec::packetization, "foo")),
+ AllOf(Field("id", &webrtc::Codec::id, 97),
+ Field("packetization", &webrtc::Codec::packetization, "bar")),
+ AllOf(Field("id", &webrtc::Codec::id, 99),
+ Field("packetization", &webrtc::Codec::packetization,
+ std::nullopt))));
}
TEST_F(VideoChannelSingleThreadTest,
diff --git a/pc/congestion_control_integrationtest.cc b/pc/congestion_control_integrationtest.cc
index a0bb6a6997..da5612d99e 100644
--- a/pc/congestion_control_integrationtest.cc
+++ b/pc/congestion_control_integrationtest.cc
@@ -84,6 +84,53 @@ TEST_F(PeerConnectionCongestionControlTest, ReceiveOfferSetsCcfbFlag) {
EXPECT_THAT(answer_str, Not(HasSubstr("transport-cc")));
}
+TEST_F(PeerConnectionCongestionControlTest, NegotiatingCcfbRemovesTsn) {
+ SetFieldTrials("WebRTC-RFC8888CongestionControlFeedback/Enabled/");
+ ASSERT_TRUE(CreatePeerConnectionWrappers());
+ ConnectFakeSignalingForSdpOnly();
+ callee()->AddVideoTrack();
+ // Add transceivers to caller in order to accomodate reception
+ caller()->pc()->AddTransceiver(MediaType::VIDEO);
+ auto parameters = caller()->pc()->GetSenders()[0]->GetParameters();
+ caller()->CreateAndSetAndSignalOffer();
+ ASSERT_THAT(WaitUntil([&] { return SignalingStateStable(); }, IsTrue()),
+ IsRtcOk());
+
+ std::vector<RtpHeaderExtensionCapability> negotiated_header_extensions =
+ caller()->pc()->GetTransceivers()[0]->GetNegotiatedHeaderExtensions();
+ EXPECT_THAT(
+ negotiated_header_extensions,
+ Not(Contains(
+ AllOf(Field("uri", &RtpHeaderExtensionCapability::uri,
+ RtpExtension::kTransportSequenceNumberUri),
+ Not(Field("direction", &RtpHeaderExtensionCapability::direction,
+ RtpTransceiverDirection::kStopped))))))
+ << " in caller negotiated header extensions";
+
+ parameters = caller()->pc()->GetSenders()[0]->GetParameters();
+ EXPECT_THAT(parameters.header_extensions,
+ Not(Contains(Field("uri", &RtpExtension::uri,
+ RtpExtension::kTransportSequenceNumberUri))))
+ << " in caller sender parameters";
+ parameters = caller()->pc()->GetReceivers()[0]->GetParameters();
+ EXPECT_THAT(parameters.header_extensions,
+ Not(Contains(Field("uri", &RtpExtension::uri,
+ RtpExtension::kTransportSequenceNumberUri))))
+ << " in caller receiver parameters";
+
+ parameters = callee()->pc()->GetSenders()[0]->GetParameters();
+ EXPECT_THAT(parameters.header_extensions,
+ Not(Contains(Field("uri", &RtpExtension::uri,
+ RtpExtension::kTransportSequenceNumberUri))))
+ << " in callee sender parameters";
+
+ parameters = callee()->pc()->GetReceivers()[0]->GetParameters();
+ EXPECT_THAT(parameters.header_extensions,
+ Not(Contains(Field("uri", &RtpExtension::uri,
+ RtpExtension::kTransportSequenceNumberUri))))
+ << " in callee receiver parameters";
+}
+
TEST_F(PeerConnectionCongestionControlTest, CcfbGetsUsed) {
SetFieldTrials("WebRTC-RFC8888CongestionControlFeedback/Enabled/");
ASSERT_TRUE(CreatePeerConnectionWrappers());
@@ -133,8 +180,7 @@ TEST_F(PeerConnectionCongestionControlTest, TransportCcGetsUsed) {
EXPECT_THAT(pc_internal->FeedbackAccordingToRfc8888CountForTesting(), Eq(0));
}
-TEST_F(PeerConnectionCongestionControlTest,
- DISABLED_CcfbGetsUsedCalleeToCaller) {
+TEST_F(PeerConnectionCongestionControlTest, CcfbGetsUsedCalleeToCaller) {
SetFieldTrials("WebRTC-RFC8888CongestionControlFeedback/Enabled/");
ASSERT_TRUE(CreatePeerConnectionWrappers());
ConnectFakeSignaling();
diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index 3a2babda3e..4d4ec552d7 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -4631,8 +4631,9 @@ TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
PeerConnectionInterface::kStable);
}
+// TODO: issues.webrtc.org/425336456 - figure out correct behavior and reenable
TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
- OnlyOnePairWantsCorruptionScorePlumbing) {
+ DISABLED_OnlyOnePairWantsCorruptionScorePlumbing) {
// In order for corruption score to be logged, encryption of RTP header
// extensions must be allowed.
CryptoOptions crypto_options;
@@ -4656,12 +4657,19 @@ TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
ASSERT_THAT(
WaitUntil([&] { return SignalingStateStable(); }, ::testing::IsTrue()),
IsRtcOk());
+ std::vector<RtpHeaderExtensionCapability> negotiated_extensions =
+ caller()->pc()->GetTransceivers()[0]->GetNegotiatedHeaderExtensions();
+ ASSERT_THAT(negotiated_extensions,
+ Contains(Field("uri", &RtpHeaderExtensionCapability::uri,
+ RtpExtension::kCorruptionDetectionUri)));
ASSERT_THAT(WaitUntil([&] { return caller()->GetCorruptionScoreCount(); },
::testing::Gt(0), {.timeout = kMaxWaitForStats}),
- IsRtcOk());
+ IsRtcOk())
+ << "Waiting for caller corruption score count > 0";
ASSERT_THAT(WaitUntil([&] { return callee()->GetCorruptionScoreCount(); },
::testing::Eq(0), {.timeout = kMaxWaitForStats}),
- IsRtcOk());
+ IsRtcOk())
+ << "Waiting for callee corruption score count = 0";
for (const auto& pair : {caller(), callee()}) {
scoped_refptr<const RTCStatsReport> report = pair->NewGetStats();
diff --git a/test/peer_scenario/tests/l4s_test.cc b/test/peer_scenario/tests/l4s_test.cc
index f65a24fff9..378ea2e746 100644
--- a/test/peer_scenario/tests/l4s_test.cc
+++ b/test/peer_scenario/tests/l4s_test.cc
@@ -120,7 +120,7 @@ DataRate GetAvailableSendBitrate(
return DataRate::BitsPerSec(*stats[0]->available_outgoing_bitrate);
}
-TEST(L4STest, DISABLED_NegotiateAndUseCcfbIfEnabled) {
+TEST(L4STest, NegotiateAndUseCcfbIfEnabled) {
PeerScenario s(*test_info_);
PeerScenarioClient::Config config;