Revert "document footgun behavior of codec munging and setCodecPreferences"
This reverts commit e7162754c0229d10c903525be3f900edb184dfb7.
Reason for revert: Breaks downstream project
Original change's description:
> document footgun behavior of codec munging and setCodecPreferences
>
> which drops the munged codec
>
> BUG=webrtc:396344370
>
> Change-Id: I4980c6980ad7df29d4d1451777a61f90dac73f74
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/389440
> Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
> Reviewed-by: Harald Alvestrand <hta@webrtc.org>
> Commit-Queue: Philipp Hancke <phancke@meta.com>
> Cr-Commit-Position: refs/heads/main@{#44555}
Bug: webrtc:396344370
No-Presubmit: true
No-Tree-Checks: true
No-Try: true
Change-Id: I86e7929217420bf4f1741910e628430b660d4418
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/390660
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Auto-Submit: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Evan Shrubsole <eshr@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#44560}
diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index a3e9750..95bd835 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -18,7 +18,6 @@
#include <stdint.h>
#include <algorithm>
-#include <cstddef>
#include <memory>
#include <optional>
#include <string>
@@ -4795,99 +4794,6 @@
}
TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
- MungeOfferCodecAndReOfferWorksWithSetCodecPreferencesIsAFootgun) {
- ASSERT_TRUE(CreatePeerConnectionWrappers());
- ConnectFakeSignaling();
- caller()->AddVideoTrack();
- auto munger = [](std::unique_ptr<SessionDescriptionInterface>& sdp) {
- auto video = GetFirstVideoContentDescription(sdp->description());
- auto codecs = video->codecs();
- std::optional<Codec> replacement_codec;
- for (auto&& codec : codecs) {
- if (codec.name == "AV1") {
- replacement_codec = codec;
- break;
- }
- }
- if (replacement_codec) {
- for (auto&& codec : codecs) {
- if (codec.name == "VP8") {
- RTC_LOG(LS_INFO) << "Remapping VP8 codec " << codec << " to AV1";
- codec.name = replacement_codec->name;
- codec.params = replacement_codec->params;
- break;
- }
- }
- video->set_codecs(codecs);
- } else {
- RTC_LOG(LS_INFO) << "Skipping munge, no AV1 codec found";
- }
- };
- caller()->SetGeneratedSdpMunger(munger);
- caller()->CreateAndSetAndSignalOffer();
- ASSERT_THAT(
- WaitUntil([&] { return SignalingStateStable(); }, ::testing::IsTrue()),
- IsRtcOk());
- caller()->SetGeneratedSdpMunger(nullptr);
-
- // Note currently negotiated codecs and count VP9 and AV1.
- auto codecs_after_munge = caller()
- ->pc()
- ->local_description()
- ->description()
- ->contents()[0]
- .media_description()
- ->codecs();
- size_t av1_munge = 0;
- size_t vp8_munge = 0;
- for (const auto& codec : codecs_after_munge) {
- if (codec.name == "AV1")
- av1_munge++;
- else if (codec.name == "VP8")
- vp8_munge++;
- }
- // We should have replaced VP8 with AV1.
- EXPECT_EQ(av1_munge, 2u);
- EXPECT_EQ(vp8_munge, 0u);
-
- // Call setCodecPreferences.
- std::vector<RtpCodecCapability> codecs =
- caller()
- ->pc_factory()
- ->GetRtpReceiverCapabilities(webrtc::MediaType::VIDEO)
- .codecs;
- auto transceivers = caller()->pc()->GetTransceivers();
- ASSERT_EQ(transceivers.size(), 1u);
- transceivers[0]->SetCodecPreferences(codecs);
-
- auto offer = caller()->CreateOfferAndWait();
- ASSERT_NE(offer, nullptr);
- // The offer should be acceptable.
- EXPECT_TRUE(caller()->SetLocalDescriptionAndSendSdpMessage(std::move(offer)));
-
- auto codecs_after_scp = caller()
- ->pc()
- ->local_description()
- ->description()
- ->contents()[0]
- .media_description()
- ->codecs();
- size_t av1_scp = 0;
- size_t vp8_scp = 0;
- for (const auto& codec : codecs_after_scp) {
- if (codec.name == "AV1")
- av1_scp++;
- else if (codec.name == "VP8")
- vp8_scp++;
- }
- // The SDP munging modification was reverted by sCP.
- // This is a footgun but please do not mix such munging with
- // setCodecPreferences, munging is on the way out.
- EXPECT_EQ(av1_scp, 1u);
- EXPECT_EQ(vp8_scp, 1u);
-}
-
-TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
SensibleRtxWithDuplicateCodecs) {
ASSERT_TRUE(CreatePeerConnectionWrappers());
caller()->AddVideoTrack();
@@ -4931,7 +4837,7 @@
// associated RTX codec.
std::unique_ptr<SessionDescriptionInterface> answer =
caller()->CreateAnswerForTest();
- ASSERT_NE(answer, nullptr);
+ ASSERT_THAT(answer, NotNull());
RTC_LOG(LS_ERROR) << "Answer is " << *answer;
ASSERT_THAT(answer->description()->contents().size(), Eq(1));
auto codecs =
@@ -4990,7 +4896,7 @@
caller()->AddVideoTrack();
auto offer2 = caller()->CreateOfferAndWait();
// Observe that packetization is raw on BOTH media sections.
- ASSERT_NE(offer2, nullptr);
+ ASSERT_THAT(offer2, NotNull());
EXPECT_EQ(offer2->description()->contents().size(), 2U);
for (const auto& content : offer2->description()->contents()) {
for (const auto& codec : content.media_description()->codecs()) {
