Source code
Revision control
Copy as Markdown
Other Tools
From: Michael Froman <mjfroman@mac.com>
Date: Wed, 24 Sep 2025 16:16:38 -0500
Revert "Change SetLocalContent / SetRemoteContent to update header extensions"
This reverts commit 9496cc0e1b8cc50185ded19522b68d7c94c17298.
Reason for revert: Investigating downstream issue.
Bug: webrtc:383078466
Original change's description:
> Change SetLocalContent / SetRemoteContent to update header extensions
>
> The functions were skipping over updating the sender and receiver when
> the list of header extensions was changed. Now it should be better.
>
> Bug: webrtc:383078466, b/425662432, b/426394283
> Change-Id: I1b93ed1ba4bbbf6c5b13861a6e21a7a7c82a5a66
> Reviewed-by: Per Kjellander <perkj@webrtc.org>
> Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> Cr-Commit-Position: refs/heads/main@{#44951}
Bug: webrtc:383078466
Change-Id: Ie6db6d995021a8699c1c7aa91334ea0ea0e37c1f
Reviewed-by: Per Kjellander <perkj@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Mirko Bonadei <mbonadei@webrtc.org>
Bot-Commit: rubber-stamper@appspot.gserviceaccount.com <rubber-stamper@appspot.gserviceaccount.com>
Owners-Override: Mirko Bonadei <mbonadei@webrtc.org>
Reviewed-by: Christoffer Dewerin <jansson@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#45012}
---
media/base/codec.h | 3 --
pc/channel.cc | 15 ++----
pc/channel_unittest.cc | 24 ++++------
pc/congestion_control_integrationtest.cc | 58 ++----------------------
pc/peer_connection_integrationtest.cc | 14 ++----
5 files changed, 20 insertions(+), 94 deletions(-)
diff --git a/media/base/codec.h b/media/base/codec.h
index e26306df20..694589dd14 100644
--- a/media/base/codec.h
+++ b/media/base/codec.h
@@ -192,9 +192,6 @@ 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 8b8d7a7ffb..1ab970f245 100644
--- a/pc/channel.cc
+++ b/pc/channel.cc
@@ -99,7 +99,7 @@ void MediaChannelParametersFromMediaDescription(
desc->type() == MediaType::VIDEO);
params->is_stream_active = is_stream_active;
params->codecs = desc->codecs();
- // TODO: bugs.webrtc.org/11513 - See if we really need
+ // TODO(bugs.webrtc.org/11513): See if we really need
// rtp_header_extensions_set() and remove it if we don't.
if (desc->rtp_header_extensions_set()) {
params->extensions = extensions;
@@ -915,6 +915,7 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content,
}
}
}
+
last_recv_params_ = recv_params;
if (!UpdateLocalStreams_w(content->streams(), type, error_desc)) {
@@ -925,7 +926,7 @@ bool VoiceChannel::SetLocalContent_w(const MediaContentDescription* content,
set_local_content_direction(content->direction());
UpdateMediaSendRecvState_w();
- // Disabled because suggesting PTs takes thread jumps.
+ // Disabled because suggeting PTs takes thread jumps.
// RTC_DCHECK_BLOCK_COUNT_NO_MORE_THAN(0);
@@ -1041,6 +1042,7 @@ 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);
@@ -1115,12 +1117,7 @@ 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'.",
@@ -1231,11 +1228,7 @@ 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 4bb9573632..e3488a98a9 100644
--- a/pc/channel_unittest.cc
+++ b/pc/channel_unittest.cc
@@ -2385,22 +2385,18 @@ TEST_F(VideoChannelSingleThreadTest,
EXPECT_THAT(
media_receive_channel1_impl()->recv_codecs(),
- 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))));
+ ElementsAre(AllOf(Field(&webrtc::Codec::id, 96),
+ Field(&webrtc::Codec::packetization, "foo")),
+ AllOf(Field(&webrtc::Codec::id, 98),
+ Field(&webrtc::Codec::packetization, std::nullopt))));
EXPECT_THAT(
media_send_channel1_impl()->send_codecs(),
- 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))));
+ 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))));
}
TEST_F(VideoChannelSingleThreadTest,
diff --git a/pc/congestion_control_integrationtest.cc b/pc/congestion_control_integrationtest.cc
index e5ba493d1a..d93a31924f 100644
--- a/pc/congestion_control_integrationtest.cc
+++ b/pc/congestion_control_integrationtest.cc
@@ -12,13 +12,9 @@
// are correctly negotiated in the SDP offer/answer.
#include <string>
-#include <vector>
#include "absl/strings/str_cat.h"
-#include "api/media_types.h"
#include "api/peer_connection_interface.h"
-#include "api/rtp_parameters.h"
-#include "api/rtp_transceiver_direction.h"
#include "api/test/rtc_error_matchers.h"
#include "pc/test/integration_test_helpers.h"
#include "test/gmock.h"
@@ -27,12 +23,11 @@
namespace webrtc {
-using ::testing::Eq;
-using ::testing::Field;
+using testing::Eq;
using ::testing::Gt;
-using ::testing::HasSubstr;
+using testing::HasSubstr;
using ::testing::IsTrue;
-using ::testing::Not;
+using testing::Not;
class PeerConnectionCongestionControlTest
: public PeerConnectionIntegrationBaseTest {
@@ -82,53 +77,6 @@ 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());
diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index 4d4ec552d7..3a2babda3e 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -4631,9 +4631,8 @@ TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
PeerConnectionInterface::kStable);
}
-// TODO: issues.webrtc.org/425336456 - figure out correct behavior and reenable
TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
- DISABLED_OnlyOnePairWantsCorruptionScorePlumbing) {
+ OnlyOnePairWantsCorruptionScorePlumbing) {
// In order for corruption score to be logged, encryption of RTP header
// extensions must be allowed.
CryptoOptions crypto_options;
@@ -4657,19 +4656,12 @@ 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())
- << "Waiting for caller corruption score count > 0";
+ IsRtcOk());
ASSERT_THAT(WaitUntil([&] { return callee()->GetCorruptionScoreCount(); },
::testing::Eq(0), {.timeout = kMaxWaitForStats}),
- IsRtcOk())
- << "Waiting for callee corruption score count = 0";
+ IsRtcOk());
for (const auto& pair : {caller(), callee()}) {
scoped_refptr<const RTCStatsReport> report = pair->NewGetStats();