From 9bbd4d34e99de30bdf9fd5b5b2fba0b4e616597b Mon Sep 17 00:00:00 2001 From: Philipp Hancke Date: Thu, 6 Feb 2025 14:42:59 -0800 Subject: [PATCH] Pick H265 payload type from lower dynamic PT range MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit as the "upper range" is already filled and new codecs should prefer the lower dynamic PT range. drive-by: restore audio/red behavior too even though in practice it has not changed. BUG=chromium:391903235 Change-Id: Iefc78253bf0fe88567f9032059ead3c2557e36a1 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/376520 Reviewed-by: Henrik Boström Reviewed-by: Harald Alvestrand Commit-Queue: Philipp Hancke Cr-Commit-Position: refs/heads/main@{#43866} --- call/payload_type_picker.cc | 5 +++-- call/payload_type_picker_unittest.cc | 21 ++++++++++++++++++--- 2 files changed, 21 insertions(+), 5 deletions(-) diff --git a/call/payload_type_picker.cc b/call/payload_type_picker.cc index c7983ce172..5c80f5808b 100644 --- a/call/payload_type_picker.cc +++ b/call/payload_type_picker.cc @@ -54,10 +54,11 @@ struct MapTableEntry { bool CodecPrefersLowerRange(const cricket::Codec& codec) { // All audio codecs prefer upper range. if (codec.type == cricket::Codec::Type::kAudio) { - return false; + return absl::EqualsIgnoreCase(codec.name, cricket::kRedCodecName); } if (absl::EqualsIgnoreCase(codec.name, cricket::kFlexfecCodecName) || - absl::EqualsIgnoreCase(codec.name, cricket::kAv1CodecName)) { + absl::EqualsIgnoreCase(codec.name, cricket::kAv1CodecName) || + absl::EqualsIgnoreCase(codec.name, cricket::kH265CodecName)) { return true; } else if (absl::EqualsIgnoreCase(codec.name, cricket::kH264CodecName)) { std::string profile_level_id; diff --git a/call/payload_type_picker_unittest.cc b/call/payload_type_picker_unittest.cc index 5a3e39bb5b..a1f89e4e7f 100644 --- a/call/payload_type_picker_unittest.cc +++ b/call/payload_type_picker_unittest.cc @@ -21,6 +21,7 @@ namespace webrtc { using testing::Eq; using testing::Ge; +using testing::Le; using testing::Lt; using testing::Ne; @@ -189,6 +190,14 @@ TEST(PayloadTypePicker, AudioGetsHigherRange) { EXPECT_THAT(result, Ge(96)); } +TEST(PayloadTypePicker, AudioRedGetsLowerRange) { + PayloadTypePicker picker; + cricket::Codec an_audio_codec = + cricket::CreateAudioCodec(-1, "red", 48000, 2); + auto result = picker.SuggestMapping(an_audio_codec, nullptr).value(); + EXPECT_THAT(result, Le(63)); +} + TEST(PayloadTypePicker, VideoGetsTreatedSpecially) { PayloadTypePicker picker; cricket::Codec h264_constrained = cricket::CreateVideoCodec(SdpVideoFormat( @@ -203,17 +212,23 @@ TEST(PayloadTypePicker, VideoGetsTreatedSpecially) { {cricket::kVp9CodecName, {{cricket::kVP9ProfileId, "2"}}})); cricket::Codec vp9_profile_3 = cricket::CreateVideoCodec(SdpVideoFormat( {cricket::kVp9CodecName, {{cricket::kVP9ProfileId, "3"}}})); + cricket::Codec h265 = cricket::CreateVideoCodec(SdpVideoFormat( + cricket::kH265CodecName, {{cricket::kH265FmtpProfileId, "1"}, + {cricket::kH265FmtpTierFlag, "0"}, + {cricket::kH265FmtpLevelId, "93"}, + {cricket::kH265FmtpTxMode, "SRST"}})); // Valid for high range only. EXPECT_THAT(picker.SuggestMapping(h264_constrained, nullptr).value(), Ge(96)); EXPECT_THAT(picker.SuggestMapping(vp9_profile_2, nullptr).value(), Ge(96)); // Valid for lower range. - EXPECT_THAT(picker.SuggestMapping(h264_yuv444, nullptr).value(), Lt(63)); - EXPECT_THAT(picker.SuggestMapping(vp9_profile_3, nullptr).value(), Lt(63)); + EXPECT_THAT(picker.SuggestMapping(h264_yuv444, nullptr).value(), Le(63)); + EXPECT_THAT(picker.SuggestMapping(vp9_profile_3, nullptr).value(), Le(63)); + EXPECT_THAT(picker.SuggestMapping(h265, nullptr).value(), Le(63)); // RTX with a primary codec in the lower range is valid for lower range. cricket::Codec lower_range_rtx = cricket::CreateVideoRtxCodec(cricket::Codec::kIdNotSet, 63); - EXPECT_THAT(picker.SuggestMapping(lower_range_rtx, nullptr).value(), Lt(63)); + EXPECT_THAT(picker.SuggestMapping(lower_range_rtx, nullptr).value(), Le(63)); } TEST(PayloadTypePicker, ChoosingH264Profiles) {