diff --git a/media/base/codec_comparators.cc b/media/base/codec_comparators.cc index bf26f17c63..a1edd9a97c 100644 --- a/media/base/codec_comparators.cc +++ b/media/base/codec_comparators.cc @@ -399,13 +399,14 @@ bool IsSameRtpCodecIgnoringLevel(const Codec& codec, 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 f1286de977..41e0e6a38e 100644 --- a/media/base/codec_comparators_unittest.cc +++ b/media/base/codec_comparators_unittest.cc @@ -240,6 +240,112 @@ INSTANTIATE_TEST_SUITE_P( 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 de66c11d2e..d095bbac5c 100644 --- a/pc/peer_connection_encodings_integrationtest.cc +++ b/pc/peer_connection_encodings_integrationtest.cc @@ -20,6 +20,7 @@ #include #include +#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::Key; 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 @@ TEST_F(PeerConnectionEncodingsIntegrationTest, 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 local_pc_wrapper = CreatePc(); + rtc::scoped_refptr 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 video_transceiver = + transceiver_or_error.MoveValue(); + + std::unique_ptr 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 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 p1 = + SetLocalDescription(local_pc_wrapper, offer.get()); + rtc::scoped_refptr p2 = + SetRemoteDescription(remote_pc_wrapper, offer.get()); + EXPECT_TRUE(Await({p1, p2})); + + // Create answer and apply it + std::unique_ptr 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 local_pc_wrapper = CreatePc(); diff --git a/pc/rtp_transceiver_unittest.cc b/pc/rtp_transceiver_unittest.cc index bc01313887..3145fd5850 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 @@ class RtpTransceiverFilteredCodecPreferencesTest 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 @@ class RtpTransceiverFilteredCodecPreferencesTest {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 @@ class RtpTransceiverFilteredCodecPreferencesTest .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 @@ TEST_F(RtpTransceiverFilteredCodecPreferencesTest, 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 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, } TEST_F(RtpTransceiverFilteredCodecPreferencesTest, - H265LevelIdHasToFromSenderOrReceiverCapabilities) { + H265LevelIdHasToBeFromSenderOrReceiverCapabilities) { ConfigureH265CodecCapabilities(); cricket::Codec cricket_codec = cricket::CreateVideoCodec(SdpVideoFormat( "H265",