From 5ad1daeed92b65b5b136cb5a1df549521783fbfb Mon Sep 17 00:00:00 2001 From: Qiu Jianlin Date: Wed, 20 Nov 2024 23:01:36 +0800 Subject: [PATCH] setParameters should not throw when only level mismatch. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit According to latest requirement, when the level reported by RtpSender.getCapabilities() for H.265 is different from that was negotiated, we should not throw when setParameters() is called with level-id set to that reported by RtpSender.getCapabilities(). Underlingly negotiated codec level should remain unchanged. Bug: chromium:41480904 Change-Id: I28bbdb5f0a0ab0d98315f56c80004601afc91a63 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/368781 Reviewed-by: Harald Alvestrand Reviewed-by: Henrik Boström Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/main@{#43434} --- media/base/codec_comparators.cc | 27 +++ media/base/codec_comparators.h | 3 + media/engine/webrtc_video_engine.cc | 6 +- media/engine/webrtc_video_engine_unittest.cc | 209 +++++++++++++++++++ 4 files changed, 243 insertions(+), 2 deletions(-) diff --git a/media/base/codec_comparators.cc b/media/base/codec_comparators.cc index 38f133ac1c..49525f48f4 100644 --- a/media/base/codec_comparators.cc +++ b/media/base/codec_comparators.cc @@ -377,4 +377,31 @@ bool IsSameRtpCodec(const Codec& codec, const RtpCodec& rtp_codec) { InsertDefaultParams(rtp_codec2.name, rtp_codec2.parameters); } +bool IsSameRtpCodecIgnoringLevel(const Codec& codec, + const RtpCodec& rtp_codec) { + RtpCodecParameters rtp_codec2 = codec.ToCodecParameters(); + + if (!absl::EqualsIgnoreCase(rtp_codec.name, rtp_codec2.name) || + rtp_codec.kind != rtp_codec2.kind || + rtp_codec.num_channels != rtp_codec2.num_channels || + rtp_codec.clock_rate != rtp_codec2.clock_rate) { + return false; + } + + CodecParameterMap params1 = + InsertDefaultParams(rtp_codec.name, rtp_codec.parameters); + 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); + } +#endif + + return params1 == params2; +} + } // namespace webrtc diff --git a/media/base/codec_comparators.h b/media/base/codec_comparators.h index 75ba006ab1..a797dc8375 100644 --- a/media/base/codec_comparators.h +++ b/media/base/codec_comparators.h @@ -43,6 +43,9 @@ std::optional FindMatchingCodec( // Unspecified parameters are treated as default. bool IsSameRtpCodec(const cricket::Codec& codec, const RtpCodec& rtp_codec); +// Similar to `IsSameRtpCodec` but ignoring the level related parameter. +bool IsSameRtpCodecIgnoringLevel(const cricket::Codec& codec, + const RtpCodec& rtp_codec); } // namespace webrtc #endif // MEDIA_BASE_CODEC_COMPARATORS_H_ diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 69b71fff7e..5f9c46a076 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -1435,10 +1435,12 @@ webrtc::RTCError WebRtcVideoSendChannel::SetRtpSendParameters( !IsSameRtpCodec(send_codec_->codec, *parameters.encodings[0].codec)) { RTC_LOG(LS_VERBOSE) << "Trying to change codec to " << parameters.encodings[0].codec->name; + // Ignore level when matching negotiated codecs against the requested + // codec. auto matched_codec = absl::c_find_if(negotiated_codecs_, [&](auto negotiated_codec) { - return IsSameRtpCodec(negotiated_codec.codec, - *parameters.encodings[0].codec); + return IsSameRtpCodecIgnoringLevel(negotiated_codec.codec, + *parameters.encodings[0].codec); }); if (matched_codec == negotiated_codecs_.end()) { return webrtc::InvokeSetParametersCallback( diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index 222ae0ad89..91249da4bd 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -8554,6 +8554,215 @@ TEST_F( EXPECT_TRUE(send_channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); } + +TEST_F(WebRtcVideoChannelTest, + SetRtpParametersForH265ShouldSucceedIgnoreLowerLevelId) { + encoder_factory_->AddSupportedVideoCodec( + webrtc::SdpVideoFormat("H265", + {{"profile-id", "1"}, + {"tier-flag", "0"}, + {"level-id", "156"}, + {"tx-mode", "SRST"}}, + {ScalabilityMode::kL1T1})); + cricket::VideoSenderParameters send_parameters; + send_parameters.codecs.push_back(GetEngineCodec("H265")); + for (auto& codec : send_parameters.codecs) { + if (absl::EqualsIgnoreCase(codec.name, "H265")) { + codec.params["level-id"] = "156"; + } + } + + EXPECT_TRUE(send_channel_->SetSenderParameters(send_parameters)); + FakeVideoSendStream* stream = AddSendStream(); + ASSERT_TRUE(stream); + + webrtc::test::FrameForwarder frame_forwarder; + VideoOptions options; + EXPECT_TRUE( + send_channel_->SetVideoSend(last_ssrc_, &options, &frame_forwarder)); + send_channel_->SetSend(true); + frame_forwarder.IncomingCapturedFrame(frame_source_.GetFrame()); + + webrtc::RtpParameters parameters = + send_channel_->GetRtpSendParameters(last_ssrc_); + + webrtc::RtpCodecParameters matched_codec; + for (const auto& codec : parameters.codecs) { + if (absl::EqualsIgnoreCase(codec.name, "H265")) { + EXPECT_EQ(codec.parameters.at("level-id"), "156"); + matched_codec = codec; + } + } + + FakeVideoSendStream* send_stream = fake_call_->GetVideoSendStreams().front(); + ASSERT_TRUE(send_stream); + webrtc::VideoEncoderConfig encoder_config = + send_stream->GetEncoderConfig().Copy(); + EXPECT_EQ(encoder_config.video_format.parameters.at("level-id"), "156"); + + // Set the level-id parameter to lower than the negotiated codec level-id. + EXPECT_EQ(1u, parameters.encodings.size()); + matched_codec.parameters["level-id"] = "120"; + parameters.encodings[0].codec = matched_codec; + + EXPECT_TRUE(send_channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); + webrtc::RtpParameters parameters2 = + send_channel_->GetRtpSendParameters(last_ssrc_); + + for (const auto& codec : parameters2.codecs) { + if (absl::EqualsIgnoreCase(codec.name, "H265")) { + EXPECT_EQ(codec.parameters.at("level-id"), "156"); + } + } + + FakeVideoSendStream* send_stream2 = fake_call_->GetVideoSendStreams().front(); + ASSERT_TRUE(send_stream2); + webrtc::VideoEncoderConfig encoder_config2 = + send_stream2->GetEncoderConfig().Copy(); + EXPECT_EQ(encoder_config2.video_format.parameters.at("level-id"), "156"); + + EXPECT_TRUE(send_channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); +} + +TEST_F(WebRtcVideoChannelTest, + SetRtpParametersForH265WithSameLevelIdShouldSucceed) { + encoder_factory_->AddSupportedVideoCodec( + webrtc::SdpVideoFormat("H265", + {{"profile-id", "1"}, + {"tier-flag", "0"}, + {"level-id", "156"}, + {"tx-mode", "SRST"}}, + {ScalabilityMode::kL1T1})); + cricket::VideoSenderParameters send_parameters; + send_parameters.codecs.push_back(GetEngineCodec("H265")); + for (auto& codec : send_parameters.codecs) { + if (absl::EqualsIgnoreCase(codec.name, "H265")) { + codec.params["level-id"] = "156"; + } + } + + EXPECT_TRUE(send_channel_->SetSenderParameters(send_parameters)); + FakeVideoSendStream* stream = AddSendStream(); + ASSERT_TRUE(stream); + + webrtc::test::FrameForwarder frame_forwarder; + VideoOptions options; + EXPECT_TRUE( + send_channel_->SetVideoSend(last_ssrc_, &options, &frame_forwarder)); + send_channel_->SetSend(true); + frame_forwarder.IncomingCapturedFrame(frame_source_.GetFrame()); + + webrtc::RtpParameters parameters = + send_channel_->GetRtpSendParameters(last_ssrc_); + + webrtc::RtpCodecParameters matched_codec; + for (const auto& codec : parameters.codecs) { + if (absl::EqualsIgnoreCase(codec.name, "H265")) { + EXPECT_EQ(codec.parameters.at("level-id"), "156"); + matched_codec = codec; + } + } + + FakeVideoSendStream* send_stream = fake_call_->GetVideoSendStreams().front(); + ASSERT_TRUE(send_stream); + webrtc::VideoEncoderConfig encoder_config = + send_stream->GetEncoderConfig().Copy(); + EXPECT_EQ(encoder_config.video_format.parameters.at("level-id"), "156"); + + // Set the level-id parameter to the same as the negotiated codec level-id. + EXPECT_EQ(1u, parameters.encodings.size()); + matched_codec.parameters["level-id"] = "156"; + parameters.encodings[0].codec = matched_codec; + + EXPECT_TRUE(send_channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); + + webrtc::RtpParameters parameters2 = + send_channel_->GetRtpSendParameters(last_ssrc_); + + for (const auto& codec : parameters2.codecs) { + if (absl::EqualsIgnoreCase(codec.name, "H265")) { + EXPECT_EQ(codec.parameters.at("level-id"), "156"); + matched_codec = codec; + } + } + + FakeVideoSendStream* send_stream2 = fake_call_->GetVideoSendStreams().front(); + ASSERT_TRUE(send_stream2); + webrtc::VideoEncoderConfig encoder_config2 = + send_stream2->GetEncoderConfig().Copy(); + EXPECT_EQ(encoder_config2.video_format.parameters.at("level-id"), "156"); + + EXPECT_TRUE(send_channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); +} + +TEST_F(WebRtcVideoChannelTest, + SetRtpParametersForH265ShouldSucceedIgnoreHigherLevelId) { + encoder_factory_->AddSupportedVideoCodec( + webrtc::SdpVideoFormat("H265", + {{"profile-id", "1"}, + {"tier-flag", "0"}, + {"level-id", "156"}, + {"tx-mode", "SRST"}}, + {ScalabilityMode::kL1T1})); + cricket::VideoSenderParameters send_parameters; + send_parameters.codecs.push_back(GetEngineCodec("H265")); + for (auto& codec : send_parameters.codecs) { + if (absl::EqualsIgnoreCase(codec.name, "H265")) { + codec.params["level-id"] = "156"; + } + } + + EXPECT_TRUE(send_channel_->SetSenderParameters(send_parameters)); + FakeVideoSendStream* stream = AddSendStream(); + ASSERT_TRUE(stream); + + webrtc::test::FrameForwarder frame_forwarder; + VideoOptions options; + EXPECT_TRUE( + send_channel_->SetVideoSend(last_ssrc_, &options, &frame_forwarder)); + send_channel_->SetSend(true); + frame_forwarder.IncomingCapturedFrame(frame_source_.GetFrame()); + + webrtc::RtpParameters parameters = + send_channel_->GetRtpSendParameters(last_ssrc_); + + webrtc::RtpCodecParameters matched_codec; + for (const auto& codec : parameters.codecs) { + if (absl::EqualsIgnoreCase(codec.name, "H265")) { + EXPECT_EQ(codec.parameters.at("level-id"), "156"); + matched_codec = codec; + } + } + + FakeVideoSendStream* send_stream = fake_call_->GetVideoSendStreams().front(); + ASSERT_TRUE(send_stream); + webrtc::VideoEncoderConfig encoder_config = + send_stream->GetEncoderConfig().Copy(); + EXPECT_EQ(encoder_config.video_format.parameters.at("level-id"), "156"); + + // Set the level-id parameter to higher than the negotiated codec level-id. + EXPECT_EQ(1u, parameters.encodings.size()); + matched_codec.parameters["level-id"] = "180"; + parameters.encodings[0].codec = matched_codec; + + EXPECT_TRUE(send_channel_->SetRtpSendParameters(last_ssrc_, parameters).ok()); + + webrtc::RtpParameters parameters2 = + send_channel_->GetRtpSendParameters(last_ssrc_); + + for (const auto& codec : parameters2.codecs) { + if (absl::EqualsIgnoreCase(codec.name, "H265")) { + EXPECT_EQ(codec.parameters.at("level-id"), "156"); + } + } + FakeVideoSendStream* send_stream2 = fake_call_->GetVideoSendStreams().front(); + ASSERT_TRUE(send_stream2); + webrtc::VideoEncoderConfig encoder_config2 = + send_stream2->GetEncoderConfig().Copy(); + EXPECT_EQ(encoder_config2.video_format.parameters.at("level-id"), "156"); + + EXPECT_TRUE(send_channel_->SetVideoSend(last_ssrc_, nullptr, nullptr)); +} #endif TEST_F(WebRtcVideoChannelTest,