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}
diff --git a/media/base/media_engine.cc b/media/base/media_engine.cc
index 0ce26ff..eb74466 100644
--- a/media/base/media_engine.cc
+++ b/media/base/media_engine.cc
@@ -96,7 +96,8 @@
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;
diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc
index 560fb4c..ac0042e 100644
--- a/media/engine/webrtc_video_engine.cc
+++ b/media/engine/webrtc_video_engine.cc
@@ -1432,7 +1432,8 @@
// 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
diff --git a/pc/peer_connection_encodings_integrationtest.cc b/pc/peer_connection_encodings_integrationtest.cc
index f2fb6d8..1b6eeaf 100644
--- 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 @@
"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
diff --git a/pc/test/peer_connection_test_wrapper.cc b/pc/test/peer_connection_test_wrapper.cc
index 1774f1b..e458794 100644
--- a/pc/test/peer_connection_test_wrapper.cc
+++ b/pc/test/peer_connection_test_wrapper.cc
@@ -172,6 +172,8 @@
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 @@
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(
+ 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,
diff --git a/pc/test/peer_connection_test_wrapper.h b/pc/test/peer_connection_test_wrapper.h
index 0a3a689..5633ea8 100644
--- 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 @@
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 {
