Reland "document footgun behavior of codec munging and setCodecPreferences"
This is a reland of commit e7162754c0229d10c903525be3f900edb184dfb7
using VP9 instead of AV1.
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
Change-Id: I078c22d110d4adbf10641b1b4ba180cfc9b83ddd
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/390544
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Philipp Hancke <phancke@meta.com>
Cr-Commit-Position: refs/heads/main@{#44597}
diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index 67d9652..b083a75 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -18,6 +18,7 @@
#include <stdint.h>
#include <algorithm>
+#include <cstddef>
#include <memory>
#include <optional>
#include <string>
@@ -4780,6 +4781,107 @@
}
TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
+ MungeOfferCodecAndReOfferWorksWithSetCodecPreferencesIsAFootgun) {
+ ASSERT_TRUE(CreatePeerConnectionWrappers());
+ ConnectFakeSignaling();
+ caller()->AddVideoTrack();
+ bool has_munged = false;
+ auto munger =
+ [&has_munged](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 == "VP9") {
+ replacement_codec = codec;
+ break;
+ }
+ }
+ if (replacement_codec) {
+ for (auto&& codec : codecs) {
+ if (codec.name == "VP8") {
+ RTC_LOG(LS_INFO) << "Remapping VP8 codec " << codec << " to VP9";
+ codec.name = replacement_codec->name;
+ codec.params = replacement_codec->params;
+ has_munged = true;
+ break;
+ }
+ }
+ video->set_codecs(codecs);
+ } else {
+ RTC_LOG(LS_INFO) << "Skipping munge, no VP9 codec found ";
+ }
+ };
+ caller()->SetGeneratedSdpMunger(munger);
+ caller()->CreateAndSetAndSignalOffer();
+ ASSERT_THAT(
+ WaitUntil([&] { return SignalingStateStable(); }, ::testing::IsTrue()),
+ IsRtcOk());
+ caller()->SetGeneratedSdpMunger(nullptr);
+
+ if (!has_munged) {
+ GTEST_SKIP() << "SDP munging did not replace codec, skipping.";
+ }
+
+ // Note currently negotiated codecs and count VP8 and VP9.
+ auto codecs_after_munge = caller()
+ ->pc()
+ ->local_description()
+ ->description()
+ ->contents()[0]
+ .media_description()
+ ->codecs();
+ size_t vp8_munge = 0;
+ size_t vp9_munge = 0;
+ for (const auto& codec : codecs_after_munge) {
+ if (codec.name == "VP8")
+ vp8_munge++;
+ else if (codec.name == "VP9")
+ vp9_munge++;
+ }
+ // We should have replaced VP8 with VP9.
+ EXPECT_EQ(vp8_munge, 0u);
+ EXPECT_GE(vp9_munge, 2u);
+
+ // 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 vp8_scp = 0;
+ size_t vp9_scp = 0;
+ for (const auto& codec : codecs_after_scp) {
+ if (codec.name == "VP8")
+ vp8_scp++;
+ else if (codec.name == "VP9")
+ vp9_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(vp8_scp, 1u);
+ EXPECT_GE(vp9_scp, 1u);
+ EXPECT_GT(vp9_munge, vp9_scp);
+}
+
+TEST_F(PeerConnectionIntegrationTestUnifiedPlan,
SensibleRtxWithDuplicateCodecs) {
ASSERT_TRUE(CreatePeerConnectionWrappers());
caller()->AddVideoTrack();
@@ -4823,7 +4925,7 @@
// associated RTX codec.
std::unique_ptr<SessionDescriptionInterface> answer =
caller()->CreateAnswerForTest();
- ASSERT_THAT(answer, NotNull());
+ ASSERT_NE(answer, nullptr);
RTC_LOG(LS_ERROR) << "Answer is " << *answer;
ASSERT_THAT(answer->description()->contents().size(), Eq(1));
auto codecs =
@@ -4882,7 +4984,7 @@
caller()->AddVideoTrack();
auto offer2 = caller()->CreateOfferAndWait();
// Observe that packetization is raw on BOTH media sections.
- ASSERT_THAT(offer2, NotNull());
+ ASSERT_NE(offer2, nullptr);
EXPECT_EQ(offer2->description()->contents().size(), 2U);
for (const auto& content : offer2->description()->contents()) {
for (const auto& codec : content.media_description()->codecs()) {
