From: Michael Froman Date: Wed, 24 Sep 2025 16:16:38 -0500 Subject: Bug 1990526 - Cherry-pick upstream libwebrtc commit d27b4ef208 r?ng Upstream commit: https://webrtc.googlesource.com/src/+/d27b4ef20820c2aeef358b4a465a8b514517b9f3 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-on: https://webrtc-review.googlesource.com/c/src/+/396640 > Reviewed-by: Per Kjellander > Commit-Queue: Harald Alvestrand > Cr-Commit-Position: refs/heads/main@{#44951} Bug: webrtc:383078466 Change-Id: Ie6db6d995021a8699c1c7aa91334ea0ea0e37c1f Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/397820 Reviewed-by: Per Kjellander Reviewed-by: Harald Alvestrand Commit-Queue: Mirko Bonadei Bot-Commit: rubber-stamper@appspot.gserviceaccount.com Owners-Override: Mirko Bonadei Reviewed-by: Christoffer Dewerin 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. // TODO: https://issues.webrtc.org/360058654 - reenable after cleanup // 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 -#include #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 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 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 report = pair->NewGetStats();