Fix setParameters() throwing when level-id does not match.

In order to align with this PR[1], setParameters() should not throw if the H265 level ID we're trying to send does not match what was negotiated. This was believed to be fixed by [2] but we were still throwing due to a check on a different layer (media_engine.cc). In order to reproduce the issue despite WebRTC lacking SW encoder/decoder for H265, peer_connection_encodings_integrationtest.cc gets a new test with real stack but fake encoder/decoder factory. This allows negotiating H265 and doing SetParameters() even though the codec is not processing any frames. - Basic test coverage is added for singlecast and simulcast H265. - Test coverage for the bug being fixed added. - In Chrome the equivalent WPTs exists for when real HW is available here[3]. Those tests PASS with this CL (currently FAIL). [1] https://github.com/w3c/webrtc-pc/pull/3023 [2] https://webrtc-review.googlesource.com/c/src/+/368781 [3] https://source.chromium.org/chromium/chromium/src/+/main:third_party/blink/web_tests/external/wpt/webrtc/protocol/h265-level-id.https.html Bug: chromium:381407888 Change-Id: I3619a124586b8b26d3695cfad8890cf40bd475db Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/374164 Reviewed-by: Harald Alvestrand <hta@webrtc.org> Commit-Queue: Henrik Boström <hbos@webrtc.org> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> Reviewed-by: Jianlin Qiu <jianlin.qiu@intel.com> Cr-Commit-Position: refs/heads/main@{#43759}
2025-01-17 12:06:53 +01:00 · 2025-01-17 12:06:53 +01:00 · 9f68535e68
commit 9f68535e68
parent b0038dd14a
5 changed files with 198 additions and 8 deletions
--- a/media/base/media_engine.cc
+++ b/media/base/media_engine.cc
@ -96,7 +96,8 @@ webrtc::RTCError CheckScalabilityModeValues(
    if (rtp_parameters.encodings[i].codec) {
      bool codecFound = false;
      for (const cricket::Codec& codec : send_codecs) {
-        if (IsSameRtpCodec(codec, *rtp_parameters.encodings[i].codec) &&
+        if (IsSameRtpCodecIgnoringLevel(codec,
+                                        *rtp_parameters.encodings[i].codec) &&
            SupportsMode(codec, rtp_parameters.encodings[i].scalability_mode)) {
          codecFound = true;
          send_codec = codec;
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@ -1432,7 +1432,8 @@ webrtc::RTCError WebRtcVideoSendChannel::SetRtpSendParameters(
    // the first layer.
    // TODO: https://issues.webrtc.org/362277533 - Support mixed-codec simulcast
    if (parameters.encodings[0].codec && send_codec_ &&
-        !IsSameRtpCodec(send_codec_->codec, *parameters.encodings[0].codec)) {
+        !IsSameRtpCodecIgnoringLevel(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
--- a/pc/peer_connection_encodings_integrationtest.cc
+++ b/pc/peer_connection_encodings_integrationtest.cc
@ -41,6 +41,7 @@
 #include "api/test/rtc_error_matchers.h"
 #include "api/units/data_rate.h"
 #include "api/units/time_delta.h"
+#include "media/engine/fake_webrtc_video_engine.h"
 #include "pc/sdp_utils.h"
 #include "pc/session_description.h"
 #include "pc/simulcast_description.h"
@ -2994,4 +2995,169 @@ INSTANTIATE_TEST_SUITE_P(StandardPath,
                                           "AV1"),
                         StringParamToString());

+#ifdef RTC_ENABLE_H265
+// These tests use fake encoders and decoders, allowing testing of codec
+// preferences, SDP negotiation and get/setParamaters(). But because the codecs
+// implementations are fake, these tests do not encode or decode any frames.
+class PeerConnectionEncodingsFakeCodecsIntegrationTest
+    : public PeerConnectionEncodingsIntegrationTest {
+ public:
+  scoped_refptr<PeerConnectionTestWrapper> CreatePcWithFakeH265(
+      std::unique_ptr<FieldTrialsView> field_trials = nullptr) {
+    std::unique_ptr<cricket::FakeWebRtcVideoEncoderFactory>
+        video_encoder_factory =
+            std::make_unique<cricket::FakeWebRtcVideoEncoderFactory>();
+    video_encoder_factory->AddSupportedVideoCodec(
+        SdpVideoFormat("H265",
+                       {{"profile-id", "1"},
+                        {"tier-flag", "0"},
+                        {"level-id", "156"},
+                        {"tx-mode", "SRST"}},
+                       {ScalabilityMode::kL1T1}));
+    std::unique_ptr<cricket::FakeWebRtcVideoDecoderFactory>
+        video_decoder_factory =
+            std::make_unique<cricket::FakeWebRtcVideoDecoderFactory>();
+    video_decoder_factory->AddSupportedVideoCodecType("H265");
+    auto pc_wrapper = make_ref_counted<PeerConnectionTestWrapper>(
+        "pc", &pss_, background_thread_.get(), background_thread_.get());
+    pc_wrapper->CreatePc(
+        {}, CreateBuiltinAudioEncoderFactory(),
+        CreateBuiltinAudioDecoderFactory(), std::move(video_encoder_factory),
+        std::move(video_decoder_factory), std::move(field_trials));
+    return pc_wrapper;
+  }
+};
+
+TEST_F(PeerConnectionEncodingsFakeCodecsIntegrationTest, H265Singlecast) {
+  rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper =
+      CreatePcWithFakeH265();
+  rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper =
+      CreatePcWithFakeH265();
+  ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper);
+
+  rtc::scoped_refptr<RtpTransceiverInterface> transceiver =
+      local_pc_wrapper->pc()
+          ->AddTransceiver(cricket::MEDIA_TYPE_VIDEO)
+          .MoveValue();
+  std::vector<RtpCodecCapability> preferred_codecs =
+      GetCapabilitiesAndRestrictToCodec(local_pc_wrapper, "H265");
+  transceiver->SetCodecPreferences(preferred_codecs);
+
+  Negotiate(local_pc_wrapper, remote_pc_wrapper);
+  local_pc_wrapper->WaitForConnection();
+  remote_pc_wrapper->WaitForConnection();
+
+  // Verify codec.
+  rtc::scoped_refptr<const RTCStatsReport> report = GetStats(local_pc_wrapper);
+  std::vector<const RTCOutboundRtpStreamStats*> outbound_rtps =
+      report->GetStatsOfType<RTCOutboundRtpStreamStats>();
+  ASSERT_THAT(outbound_rtps, SizeIs(1u));
+  EXPECT_THAT(GetCurrentCodecMimeType(report, *outbound_rtps[0]),
+              StrCaseEq("video/H265"));
+}
+
+TEST_F(PeerConnectionEncodingsFakeCodecsIntegrationTest, H265Simulcast) {
+  rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper =
+      CreatePcWithFakeH265();
+  rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper =
+      CreatePcWithFakeH265();
+  ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper);
+
+  std::vector<cricket::SimulcastLayer> layers =
+      CreateLayers({"q", "h", "f"}, /*active=*/true);
+
+  rtc::scoped_refptr<RtpTransceiverInterface> transceiver =
+      AddTransceiverWithSimulcastLayers(local_pc_wrapper, remote_pc_wrapper,
+                                        layers);
+  std::vector<RtpCodecCapability> preferred_codecs =
+      GetCapabilitiesAndRestrictToCodec(local_pc_wrapper, "H265");
+  transceiver->SetCodecPreferences(preferred_codecs);
+
+  NegotiateWithSimulcastTweaks(local_pc_wrapper, remote_pc_wrapper);
+  local_pc_wrapper->WaitForConnection();
+  remote_pc_wrapper->WaitForConnection();
+
+  // Wait until all outbound RTPs exist.
+  EXPECT_THAT(
+      GetStatsUntil(local_pc_wrapper, OutboundRtpStatsAre(UnorderedElementsAre(
+                                          AllOf(RidIs("q")), AllOf(RidIs("h")),
+                                          AllOf(RidIs("f"))))),
+      IsRtcOk());
+
+  // Verify codec.
+  rtc::scoped_refptr<const RTCStatsReport> report = GetStats(local_pc_wrapper);
+  std::vector<const RTCOutboundRtpStreamStats*> outbound_rtps =
+      report->GetStatsOfType<RTCOutboundRtpStreamStats>();
+  ASSERT_THAT(outbound_rtps, SizeIs(3u));
+  EXPECT_THAT(GetCurrentCodecMimeType(report, *outbound_rtps[0]),
+              StrCaseEq("video/H265"));
+  EXPECT_THAT(GetCurrentCodecMimeType(report, *outbound_rtps[1]),
+              StrCaseEq("video/H265"));
+  EXPECT_THAT(GetCurrentCodecMimeType(report, *outbound_rtps[2]),
+              StrCaseEq("video/H265"));
+}
+
+TEST_F(PeerConnectionEncodingsFakeCodecsIntegrationTest,
+       H265SetParametersIgnoresLevelId) {
+  rtc::scoped_refptr<PeerConnectionTestWrapper> local_pc_wrapper =
+      CreatePcWithFakeH265();
+  rtc::scoped_refptr<PeerConnectionTestWrapper> remote_pc_wrapper =
+      CreatePcWithFakeH265();
+  ExchangeIceCandidates(local_pc_wrapper, remote_pc_wrapper);
+
+  std::vector<cricket::SimulcastLayer> layers =
+      CreateLayers({"f"}, /*active=*/true);
+
+  rtc::scoped_refptr<RtpTransceiverInterface> transceiver =
+      AddTransceiverWithSimulcastLayers(local_pc_wrapper, remote_pc_wrapper,
+                                        layers);
+  std::vector<RtpCodecCapability> preferred_codecs =
+      GetCapabilitiesAndRestrictToCodec(local_pc_wrapper, "H265");
+  transceiver->SetCodecPreferences(preferred_codecs);
+  rtc::scoped_refptr<RtpSenderInterface> sender = transceiver->sender();
+
+  NegotiateWithSimulcastTweaks(local_pc_wrapper, remote_pc_wrapper);
+  local_pc_wrapper->WaitForConnection();
+  remote_pc_wrapper->WaitForConnection();
+
+  // This includes non-codecs like rtx, red and flexfec too so we need to find
+  // H265.
+  std::vector<RtpCodecCapability> sender_codecs =
+      local_pc_wrapper->pc_factory()
+          ->GetRtpSenderCapabilities(cricket::MEDIA_TYPE_VIDEO)
+          .codecs;
+  auto it = std::find_if(sender_codecs.begin(), sender_codecs.end(),
+                         [](const RtpCodecCapability codec_capability) {
+                           return codec_capability.name == "H265";
+                         });
+  ASSERT_NE(it, sender_codecs.end());
+  RtpCodecCapability& h265_codec = *it;
+
+  // SetParameters() without changing level-id.
+  EXPECT_EQ(h265_codec.parameters["level-id"], "156");
+  {
+    RtpParameters parameters = sender->GetParameters();
+    ASSERT_THAT(parameters.encodings, SizeIs(1));
+    parameters.encodings[0].codec = h265_codec;
+    ASSERT_THAT(sender->SetParameters(parameters), IsRtcOk());
+  }
+  // SetParameters() with a lower level-id.
+  h265_codec.parameters["level-id"] = "30";
+  {
+    RtpParameters parameters = sender->GetParameters();
+    ASSERT_THAT(parameters.encodings, SizeIs(1));
+    parameters.encodings[0].codec = h265_codec;
+    ASSERT_THAT(sender->SetParameters(parameters), IsRtcOk());
+  }
+  // SetParameters() with a higher level-id.
+  h265_codec.parameters["level-id"] = "180";
+  {
+    RtpParameters parameters = sender->GetParameters();
+    ASSERT_THAT(parameters.encodings, SizeIs(1));
+    parameters.encodings[0].codec = h265_codec;
+    ASSERT_THAT(sender->SetParameters(parameters), IsRtcOk());
+  }
+}
+#endif  // RTC_ENABLE_H265
+
 }  // namespace webrtc
--- a/pc/test/peer_connection_test_wrapper.cc
+++ b/pc/test/peer_connection_test_wrapper.cc
@ -172,6 +172,8 @@ bool PeerConnectionTestWrapper::CreatePc(
    const webrtc::PeerConnectionInterface::RTCConfiguration& config,
    rtc::scoped_refptr<webrtc::AudioEncoderFactory> audio_encoder_factory,
    rtc::scoped_refptr<webrtc::AudioDecoderFactory> audio_decoder_factory,
+    std::unique_ptr<webrtc::VideoEncoderFactory> video_encoder_factory,
+    std::unique_ptr<webrtc::VideoDecoderFactory> video_decoder_factory,
    std::unique_ptr<webrtc::FieldTrialsView> field_trials) {
  std::unique_ptr<cricket::PortAllocator> port_allocator(
      new cricket::FakePortAllocator(
@ -190,12 +192,7 @@ bool PeerConnectionTestWrapper::CreatePc(
      network_thread_, worker_thread_, rtc::Thread::Current(),
      rtc::scoped_refptr<webrtc::AudioDeviceModule>(fake_audio_capture_module_),
      audio_encoder_factory, audio_decoder_factory,
-      std::make_unique<FuzzyMatchedVideoEncoderFactory>(),
-      std::make_unique<webrtc::VideoDecoderFactoryTemplate<
-          webrtc::LibvpxVp8DecoderTemplateAdapter,
-          webrtc::LibvpxVp9DecoderTemplateAdapter,
-          webrtc::OpenH264DecoderTemplateAdapter,
-          webrtc::Dav1dDecoderTemplateAdapter>>(),
+      std::move(video_encoder_factory), std::move(video_decoder_factory),
      nullptr /* audio_mixer */, nullptr /* audio_processing */, nullptr,
      std::move(field_trials));
  if (!peer_connection_factory_) {
@ -217,6 +214,22 @@ bool PeerConnectionTestWrapper::CreatePc(
  }
 }

+bool PeerConnectionTestWrapper::CreatePc(
+    const webrtc::PeerConnectionInterface::RTCConfiguration& config,
+    rtc::scoped_refptr<webrtc::AudioEncoderFactory> audio_encoder_factory,
+    rtc::scoped_refptr<webrtc::AudioDecoderFactory> audio_decoder_factory,
+    std::unique_ptr<webrtc::FieldTrialsView> field_trials) {
+  return CreatePc(config, std::move(audio_encoder_factory),
+                  std::move(audio_decoder_factory),
+                  std::make_unique<FuzzyMatchedVideoEncoderFactory>(),
+                  std::make_unique<webrtc::VideoDecoderFactoryTemplate<
+                      webrtc::LibvpxVp8DecoderTemplateAdapter,
+                      webrtc::LibvpxVp9DecoderTemplateAdapter,
+                      webrtc::OpenH264DecoderTemplateAdapter,
+                      webrtc::Dav1dDecoderTemplateAdapter>>(),
+                  std::move(field_trials));
+}
+
 rtc::scoped_refptr<webrtc::DataChannelInterface>
 PeerConnectionTestWrapper::CreateDataChannel(
    const std::string& label,
--- a/pc/test/peer_connection_test_wrapper.h
+++ b/pc/test/peer_connection_test_wrapper.h
@ -29,6 +29,8 @@
 #include "api/scoped_refptr.h"
 #include "api/sequence_checker.h"
 #include "api/video/resolution.h"
+#include "api/video_codecs/video_decoder_factory.h"
+#include "api/video_codecs/video_encoder_factory.h"
 #include "pc/test/fake_audio_capture_module.h"
 #include "pc/test/fake_periodic_video_source.h"
 #include "pc/test/fake_periodic_video_track_source.h"
@ -55,6 +57,13 @@ class PeerConnectionTestWrapper
      rtc::scoped_refptr<webrtc::AudioEncoderFactory> audio_encoder_factory,
      rtc::scoped_refptr<webrtc::AudioDecoderFactory> audio_decoder_factory,
      std::unique_ptr<webrtc::FieldTrialsView> field_trials = nullptr);
+  bool CreatePc(
+      const webrtc::PeerConnectionInterface::RTCConfiguration& config,
+      rtc::scoped_refptr<webrtc::AudioEncoderFactory> audio_encoder_factory,
+      rtc::scoped_refptr<webrtc::AudioDecoderFactory> audio_decoder_factory,
+      std::unique_ptr<webrtc::VideoEncoderFactory> video_encoder_factory,
+      std::unique_ptr<webrtc::VideoDecoderFactory> video_decoder_factory,
+      std::unique_ptr<webrtc::FieldTrialsView> field_trials = nullptr);

  rtc::scoped_refptr<webrtc::PeerConnectionFactoryInterface> pc_factory()
      const {