Make IsSameRtpCodecIgnoringLevel work for any codec.
Prior to this CL, IsSameRtpCodecIgnoringLevel() only ignored level IDs
if the codec was H265, incorrectly considering, for example, different
levels of H264 Baseline as not equal.
- This CL fixes that problem by using IsSameCodecSpecific() which is
already used in other places, reducing the risk of different
comparisons using different comparison rules.
This also fixes https://crbug.com/webrtc/391340599 where
setParameters() would throw if unrecognized SDP FMTP parameters were
added to a codec as part of SDP negotiation via SDP munging.
This CL makes the following WPT tests pass:
- external/wpt/webrtc/protocol/h264-unidirectional-codec-offer.https.html
- fast/peerconnection/RTCRtpSender-setParameters.html
Bug: chromium:381407888, webrtc:391340599
Change-Id: I5991403b56c86ba97e670996c6687f6315dde304
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374043
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Commit-Queue: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#43797}
diff --git a/media/base/codec_comparators.cc b/media/base/codec_comparators.cc
index bf26f17..a1edd9a 100644
--- a/media/base/codec_comparators.cc
+++ b/media/base/codec_comparators.cc
@@ -399,13 +399,14 @@
CodecParameterMap params2 =
InsertDefaultParams(rtp_codec2.name, rtp_codec2.parameters);
- // Currently we only ignore H.265 level-id parameter.
-#ifdef RTC_ENABLE_H265
- if (absl::EqualsIgnoreCase(rtp_codec.name, cricket::kH265CodecName)) {
- params1.erase(cricket::kH265FmtpLevelId);
- params2.erase(cricket::kH265FmtpLevelId);
+ // Some video codecs are compatible with others (e.g. same profile but
+ // different level). This comparison looks at the relevant parameters,
+ // ignoring ones that are either irrelevant or unrecognized.
+ if (rtp_codec.kind == cricket::MediaType::MEDIA_TYPE_VIDEO &&
+ rtp_codec.IsMediaCodec()) {
+ return IsSameCodecSpecific(rtp_codec.name, params1, rtp_codec2.name,
+ params2);
}
-#endif
return params1 == params2;
}
diff --git a/media/base/codec_comparators_unittest.cc b/media/base/codec_comparators_unittest.cc
index f1286de..41e0e6a 100644
--- a/media/base/codec_comparators_unittest.cc
+++ b/media/base/codec_comparators_unittest.cc
@@ -240,6 +240,112 @@
return info.param.name;
});
+// For H264, the profile and level IDs are entangled into a single
+// "profile-level-id" attribute, so let's test many different versions.
+// See https://cconcolato.github.io/media-mime-support/ for inspiration.
+TEST(IsSameRtpCodecIgnoringLevelTest, IgnoresH264Levels) {
+ // AVC Baseline Level 3.1
+ Codec baseline_3_1 = CreateVideoCodec(
+ SdpVideoFormat("H264",
+ {{cricket::kH264FmtpLevelAsymmetryAllowed, "1"},
+ {cricket::kH264FmtpPacketizationMode, "1"},
+ {cricket::kH264FmtpProfileLevelId, "42001f"}},
+ {ScalabilityMode::kL1T1}));
+ // AVC Baseline Level 5.2
+ Codec baseline_5_2 = CreateVideoCodec(
+ SdpVideoFormat("H264",
+ {{cricket::kH264FmtpLevelAsymmetryAllowed, "1"},
+ {cricket::kH264FmtpPacketizationMode, "1"},
+ {cricket::kH264FmtpProfileLevelId, "420034"}},
+ {ScalabilityMode::kL1T1}));
+ // AVC High Level 3.1
+ Codec high_3_1 = CreateVideoCodec(
+ SdpVideoFormat("H264",
+ {{cricket::kH264FmtpLevelAsymmetryAllowed, "1"},
+ {cricket::kH264FmtpPacketizationMode, "1"},
+ {cricket::kH264FmtpProfileLevelId, "64001f"}},
+ {ScalabilityMode::kL1T1}));
+ // AVC High Level 5.2
+ Codec high_5_2 = CreateVideoCodec(
+ SdpVideoFormat("H264",
+ {{cricket::kH264FmtpLevelAsymmetryAllowed, "1"},
+ {cricket::kH264FmtpPacketizationMode, "1"},
+ {cricket::kH264FmtpProfileLevelId, "640034"}},
+ {ScalabilityMode::kL1T1}));
+ // AVC High 4:4:4 Predictive Level 3.1
+ Codec high_444_predictive_3_1 = CreateVideoCodec(
+ SdpVideoFormat("H264",
+ {{cricket::kH264FmtpLevelAsymmetryAllowed, "1"},
+ {cricket::kH264FmtpPacketizationMode, "1"},
+ {cricket::kH264FmtpProfileLevelId, "f4001f"}},
+ {ScalabilityMode::kL1T1}));
+
+ // AVC Baseline Level 5.2 is compatible with AVC Baseline Level 3.1.
+ EXPECT_TRUE(IsSameRtpCodecIgnoringLevel(baseline_5_2,
+ baseline_3_1.ToCodecParameters()));
+ // AVC High is NOT compatible with AVC Baseline.
+ EXPECT_FALSE(
+ IsSameRtpCodecIgnoringLevel(baseline_3_1, high_3_1.ToCodecParameters()));
+ EXPECT_FALSE(
+ IsSameRtpCodecIgnoringLevel(baseline_3_1, high_5_2.ToCodecParameters()));
+ EXPECT_FALSE(
+ IsSameRtpCodecIgnoringLevel(baseline_5_2, high_3_1.ToCodecParameters()));
+ EXPECT_FALSE(
+ IsSameRtpCodecIgnoringLevel(baseline_5_2, high_5_2.ToCodecParameters()));
+ // AVC High 5.2 is compatible with AVC High 3.1
+ EXPECT_TRUE(
+ IsSameRtpCodecIgnoringLevel(high_5_2, high_3_1.ToCodecParameters()));
+ // 4:4:4 Predictive is NOT compatible with either High or Baseline.
+ EXPECT_FALSE(IsSameRtpCodecIgnoringLevel(high_444_predictive_3_1,
+ high_3_1.ToCodecParameters()));
+ EXPECT_FALSE(IsSameRtpCodecIgnoringLevel(high_444_predictive_3_1,
+ high_5_2.ToCodecParameters()));
+ EXPECT_FALSE(IsSameRtpCodecIgnoringLevel(high_444_predictive_3_1,
+ baseline_3_1.ToCodecParameters()));
+ EXPECT_FALSE(IsSameRtpCodecIgnoringLevel(high_444_predictive_3_1,
+ baseline_3_1.ToCodecParameters()));
+}
+
+#ifdef RTC_ENABLE_H265
+// For H265, the "profile-id" and "level-id" are separate so test can be simple.
+// The level-id value for Level X.Y is calculated as (X * 10 + Y) * 3.
+// The lowest Level, 1.0, is thus (1 * 10 + 0) * 3 = 30.
+TEST(IsSameRtpCodecIgnoringLevelTest, IgnoresH265Levels) {
+ // Profile 1, Level 5.2
+ Codec profile_1_level_5_2 =
+ CreateVideoCodec(SdpVideoFormat("H265",
+ {{cricket::kH265FmtpProfileId, "1"},
+ {cricket::kH265FmtpTierFlag, "0"},
+ {cricket::kH265FmtpLevelId, "156"},
+ {cricket::kH265FmtpTxMode, "SRST"}},
+ {ScalabilityMode::kL1T1}));
+ // Profile 1, Level 6.0
+ Codec profile_1_level_6_0 =
+ CreateVideoCodec(SdpVideoFormat("H265",
+ {{cricket::kH265FmtpProfileId, "1"},
+ {cricket::kH265FmtpTierFlag, "0"},
+ {cricket::kH265FmtpLevelId, "180"},
+ {cricket::kH265FmtpTxMode, "SRST"}},
+ {ScalabilityMode::kL1T1}));
+ // Profile 2, Level 6.0
+ Codec profile_2_level_6_0 =
+ CreateVideoCodec(SdpVideoFormat("H265",
+ {{cricket::kH265FmtpProfileId, "2"},
+ {cricket::kH265FmtpTierFlag, "0"},
+ {cricket::kH265FmtpLevelId, "180"},
+ {cricket::kH265FmtpTxMode, "SRST"}},
+ {ScalabilityMode::kL1T1}));
+ // Profile 1 codecs are compatible with each other.
+ EXPECT_TRUE(IsSameRtpCodecIgnoringLevel(
+ profile_1_level_5_2, profile_1_level_6_0.ToCodecParameters()));
+ // Profile 2 codecs are NOT compatible with profile 1 codecs.
+ EXPECT_FALSE(IsSameRtpCodecIgnoringLevel(
+ profile_2_level_6_0, profile_1_level_5_2.ToCodecParameters()));
+ EXPECT_FALSE(IsSameRtpCodecIgnoringLevel(
+ profile_2_level_6_0, profile_1_level_6_0.ToCodecParameters()));
+}
+#endif // RTC_ENABLE_H265
+
TEST(CodecTest, TestCodecMatches) {
// Test a codec with a static payload type.
Codec c0 = cricket::CreateAudioCodec(34, "A", 44100, 1);
diff --git a/pc/peer_connection_encodings_integrationtest.cc b/pc/peer_connection_encodings_integrationtest.cc
index de66c11..d095bba 100644
--- a/pc/peer_connection_encodings_integrationtest.cc
+++ b/pc/peer_connection_encodings_integrationtest.cc
@@ -20,6 +20,7 @@
#include <utility>
#include <vector>
+#include "absl/algorithm/container.h"
#include "absl/strings/match.h"
#include "absl/strings/string_view.h"
#include "api/audio_codecs/builtin_audio_decoder_factory.h"
@@ -41,6 +42,7 @@
#include "api/test/rtc_error_matchers.h"
#include "api/units/data_rate.h"
#include "api/units/time_delta.h"
+#include "media/base/codec.h"
#include "media/engine/fake_webrtc_video_engine.h"
#include "pc/sdp_utils.h"
#include "pc/session_description.h"
@@ -59,6 +61,7 @@
using ::testing::AllOf;
using ::testing::AnyOf;
+using ::testing::Contains;
using ::testing::Each;
using ::testing::Eq;
using ::testing::Field;
@@ -70,6 +73,7 @@
using ::testing::Le;
using ::testing::Matcher;
using ::testing::Ne;
+using ::testing::NotNull;
using ::testing::Optional;
using ::testing::Pair;
using ::testing::Pointer;
@@ -1837,6 +1841,72 @@
EXPECT_EQ(error.type(), RTCErrorType::INVALID_MODIFICATION);
}
+// Test coverage for https://crbug.com/webrtc/391340599.
+// Some web apps add non-standard FMTP parameters to video codecs and because
+// they get successfully negotiated due to being ignored by SDP rules, they show
+// up in GetParameters().codecs. Using SetParameters() with such codecs should
+// still work.
+TEST_F(PeerConnectionEncodingsIntegrationTest,
+ SetParametersAcceptsMungedCodecFromGetParameters) {
+ rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc();
+ rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper = CreatePc();
+ ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper);
+
+ auto transceiver_or_error =
+ local_pc_wrapper->pc()->AddTransceiver(cricket::MEDIA_TYPE_VIDEO);
+ ASSERT_TRUE(transceiver_or_error.ok());
+ rtc::scoped_refptr<RtpTransceiverInterface> video_transceiver =
+ transceiver_or_error.MoveValue();
+
+ std::unique_ptr<SessionDescriptionInterface> offer =
+ CreateOffer(local_pc_wrapper);
+ // Munge a new parameter for VP8 in the offer.
+ auto* mcd = offer->description()->contents()[0].media_description();
+ ASSERT_THAT(mcd, NotNull());
+ std::vector<cricket::Codec> codecs = mcd->codecs();
+ ASSERT_THAT(codecs, Contains(Field(&cricket::Codec::name, "VP8")));
+ auto vp8_codec = absl::c_find_if(
+ codecs, [](const cricket::Codec& codec) { return codec.name == "VP8"; });
+ vp8_codec->params.emplace("non-standard-param", "true");
+ mcd->set_codecs(codecs);
+
+ rtc::scoped_refptr<MockSetSessionDescriptionObserver> p1 =
+ SetLocalDescription(local_pc_wrapper, offer.get());
+ rtc::scoped_refptr<MockSetSessionDescriptionObserver> p2 =
+ SetRemoteDescription(remote_pc_wrapper, offer.get());
+ EXPECT_TRUE(Await({p1, p2}));
+
+ // Create answer and apply it
+ std::unique_ptr<SessionDescriptionInterface> answer =
+ CreateAnswer(remote_pc_wrapper);
+ mcd = answer->description()->contents()[0].media_description();
+ ASSERT_THAT(mcd, NotNull());
+ codecs = mcd->codecs();
+ ASSERT_THAT(codecs, Contains(Field(&cricket::Codec::name, "VP8")));
+ vp8_codec = absl::c_find_if(
+ codecs, [](const cricket::Codec& codec) { return codec.name == "VP8"; });
+ vp8_codec->params.emplace("non-standard-param", "true");
+ mcd->set_codecs(codecs);
+ p1 = SetLocalDescription(remote_pc_wrapper, answer.get());
+ p2 = SetRemoteDescription(local_pc_wrapper, answer.get());
+ EXPECT_TRUE(Await({p1, p2}));
+
+ local_pc_wrapper->WaitForConnection();
+ remote_pc_wrapper->WaitForConnection();
+
+ RtpParameters parameters = video_transceiver->sender()->GetParameters();
+ auto it = absl::c_find_if(
+ parameters.codecs, [](const auto& codec) { return codec.name == "VP8"; });
+ ASSERT_NE(it, parameters.codecs.end());
+ RtpCodecParameters& vp8_codec_from_parameters = *it;
+ EXPECT_THAT(vp8_codec_from_parameters.parameters,
+ Contains(Pair("non-standard-param", "true")));
+ parameters.encodings[0].codec = vp8_codec_from_parameters;
+
+ EXPECT_THAT(video_transceiver->sender()->SetParameters(parameters),
+ IsRtcOk());
+}
+
TEST_F(PeerConnectionEncodingsIntegrationTest,
SetParametersRejectsNonNegotiatedCodecParameterVideo) {
rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper = CreatePc();
diff --git a/pc/rtp_transceiver_unittest.cc b/pc/rtp_transceiver_unittest.cc
index bc01313..3145fd5 100644
--- a/pc/rtp_transceiver_unittest.cc
+++ b/pc/rtp_transceiver_unittest.cc
@@ -21,6 +21,7 @@
#include "api/peer_connection_interface.h"
#include "api/rtp_parameters.h"
#include "api/test/rtc_error_matchers.h"
+#include "media/base/codec_comparators.h"
#include "media/base/fake_media_engine.h"
#include "pc/rtp_parameters_conversion.h"
#include "pc/test/enable_fake_media.h"
@@ -220,8 +221,8 @@
RtpCodecCapability rtx_codec;
};
- // For H264, the profile and level IDs are entangled and not ignored by
- // IsSameRtpCodecIgnoringLevel().
+ // For H264, the profile and level IDs are entangled. This function uses
+ // profile-level-id values that are not equal even when levels are ignored.
H264CodecCapabilities ConfigureH264CodecCapabilities() {
cricket::Codec cricket_sendrecv_codec = cricket::CreateVideoCodec(
SdpVideoFormat("H264",
@@ -247,7 +248,7 @@
{cricket_sendrecv_codec, cricket_sendonly_codec, cricket_rtx_codec});
media_engine()->SetVideoRecvCodecs(
{cricket_sendrecv_codec, cricket_recvonly_codec, cricket_rtx_codec});
- return {
+ H264CodecCapabilities capabilities = {
.cricket_sendrecv_codec = cricket_sendrecv_codec,
.sendrecv_codec = ToRtpCodecCapability(cricket_sendrecv_codec),
.cricket_sendonly_codec = cricket_sendonly_codec,
@@ -257,6 +258,13 @@
.cricket_rtx_codec = cricket_rtx_codec,
.rtx_codec = ToRtpCodecCapability(cricket_rtx_codec),
};
+ EXPECT_FALSE(IsSameRtpCodecIgnoringLevel(
+ capabilities.cricket_sendrecv_codec, capabilities.sendonly_codec));
+ EXPECT_FALSE(IsSameRtpCodecIgnoringLevel(
+ capabilities.cricket_sendrecv_codec, capabilities.recvonly_codec));
+ EXPECT_FALSE(IsSameRtpCodecIgnoringLevel(
+ capabilities.cricket_sendonly_codec, capabilities.recvonly_codec));
+ return capabilities;
}
#ifdef RTC_ENABLE_H265
@@ -412,6 +420,57 @@
ElementsAre(codecs.recvonly_codec, codecs.rtx_codec));
}
+TEST_F(RtpTransceiverFilteredCodecPreferencesTest,
+ H264LevelIdsIgnoredByFilter) {
+ // Baseline 3.1 and 5.2 are compatible when ignoring level IDs.
+ cricket::Codec baseline_3_1 = cricket::CreateVideoCodec(
+ SdpVideoFormat("H264",
+ {{"level-asymmetry-allowed", "1"},
+ {"packetization-mode", "1"},
+ {"profile-level-id", "42001f"}},
+ {ScalabilityMode::kL1T1}));
+ cricket::Codec baseline_5_2 = cricket::CreateVideoCodec(
+ SdpVideoFormat("H264",
+ {{"level-asymmetry-allowed", "1"},
+ {"packetization-mode", "1"},
+ {"profile-level-id", "420034"}},
+ {ScalabilityMode::kL1T1}));
+ // High is NOT compatible with baseline.
+ cricket::Codec high_3_1 = cricket::CreateVideoCodec(
+ SdpVideoFormat("H264",
+ {{"level-asymmetry-allowed", "1"},
+ {"packetization-mode", "1"},
+ {"profile-level-id", "64001f"}},
+ {ScalabilityMode::kL1T1}));
+ // Configure being able to both send and receive Baseline but using different
+ // level IDs in either direction, while the High profile is "truly" recvonly.
+ media_engine()->SetVideoSendCodecs({baseline_3_1});
+ media_engine()->SetVideoRecvCodecs({baseline_5_2, high_3_1});
+
+ // Prefer to "sendrecv" Baseline 5.2. Even though we can only send 3.1 this
+ // codec is not filtered out due to 5.2 and 3.1 being compatible when ignoring
+ // level IDs.
+ std::vector<RtpCodecCapability> codec_capabilities = {
+ ToRtpCodecCapability(baseline_5_2)};
+ EXPECT_THAT(transceiver_->SetCodecPreferences(codec_capabilities), IsRtcOk());
+ EXPECT_THAT(
+ transceiver_->SetDirectionWithError(RtpTransceiverDirection::kSendRecv),
+ IsRtcOk());
+ EXPECT_THAT(transceiver_->filtered_codec_preferences(),
+ ElementsAre(codec_capabilities[0]));
+ // Prefer to "sendrecv" High 3.1. This gets filtered out because we cannot
+ // send it (Baseline 3.1 is not compatible with it).
+ codec_capabilities = {ToRtpCodecCapability(high_3_1)};
+ EXPECT_THAT(transceiver_->SetCodecPreferences(codec_capabilities), IsRtcOk());
+ EXPECT_THAT(transceiver_->filtered_codec_preferences(), SizeIs(0));
+ // Change direction to "recvonly" to avoid High 3.1 being filtered out.
+ EXPECT_THAT(
+ transceiver_->SetDirectionWithError(RtpTransceiverDirection::kRecvOnly),
+ IsRtcOk());
+ EXPECT_THAT(transceiver_->filtered_codec_preferences(),
+ ElementsAre(codec_capabilities[0]));
+}
+
#ifdef RTC_ENABLE_H265
TEST_F(RtpTransceiverFilteredCodecPreferencesTest,
H265LevelIdIsIgnoredByFilter) {
@@ -438,7 +497,7 @@
}
TEST_F(RtpTransceiverFilteredCodecPreferencesTest,
- H265LevelIdHasToFromSenderOrReceiverCapabilities) {
+ H265LevelIdHasToBeFromSenderOrReceiverCapabilities) {
ConfigureH265CodecCapabilities();
cricket::Codec cricket_codec = cricket::CreateVideoCodec(SdpVideoFormat(
"H265",
