From 7ceb49c7b82fcb7ffc337f113767da9ba4689249 Mon Sep 17 00:00:00 2001 From: philipel Date: Wed, 5 Apr 2023 12:48:15 +0200 Subject: [PATCH] Fuzzy match the SdpVideoFormat in VideoEncoderFactoryTemplate::CreateVideoEncoder. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:13573 Change-Id: I1223f5f989d5298d3a6f7f6d06fd752e650adfd9 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/300342 Reviewed-by: Danil Chapovalov Commit-Queue: Philip Eliasson Reviewed-by: Erik Språng Cr-Commit-Position: refs/heads/main@{#39766} --- .../video_encoder_factory_template_tests.cc | 25 ++++++++++--------- .../video_encoder_factory_template.h | 12 ++++++++- 2 files changed, 24 insertions(+), 13 deletions(-) diff --git a/api/video_codecs/test/video_encoder_factory_template_tests.cc b/api/video_codecs/test/video_encoder_factory_template_tests.cc index 4c3d0cd24e..91b02aa905 100644 --- a/api/video_codecs/test/video_encoder_factory_template_tests.cc +++ b/api/video_codecs/test/video_encoder_factory_template_tests.cc @@ -22,8 +22,9 @@ using ::testing::Each; using ::testing::Eq; using ::testing::Field; using ::testing::IsEmpty; -using ::testing::Ne; +using ::testing::IsNull; using ::testing::Not; +using ::testing::NotNull; using ::testing::UnorderedElementsAre; namespace webrtc { @@ -68,8 +69,8 @@ struct BarEncoderTemplateAdapter { TEST(VideoEncoderFactoryTemplate, OneTemplateAdapterCreateEncoder) { VideoEncoderFactoryTemplate factory; EXPECT_THAT(factory.GetSupportedFormats(), UnorderedElementsAre(kFooSdp)); - EXPECT_THAT(factory.CreateVideoEncoder(kFooSdp), Ne(nullptr)); - EXPECT_THAT(factory.CreateVideoEncoder(SdpVideoFormat("FooX")), Eq(nullptr)); + EXPECT_THAT(factory.CreateVideoEncoder(kFooSdp), NotNull()); + EXPECT_THAT(factory.CreateVideoEncoder(SdpVideoFormat("FooX")), IsNull()); } TEST(VideoEncoderFactoryTemplate, OneTemplateAdapterCodecSupport) { @@ -97,11 +98,11 @@ TEST(VideoEncoderFactoryTemplate, TwoTemplateAdaptersCreateEncoders) { factory; EXPECT_THAT(factory.GetSupportedFormats(), UnorderedElementsAre(kFooSdp, kBarLowSdp, kBarHighSdp)); - EXPECT_THAT(factory.CreateVideoEncoder(kFooSdp), Ne(nullptr)); - EXPECT_THAT(factory.CreateVideoEncoder(kBarLowSdp), Ne(nullptr)); - EXPECT_THAT(factory.CreateVideoEncoder(kBarHighSdp), Ne(nullptr)); - EXPECT_THAT(factory.CreateVideoEncoder(SdpVideoFormat("FooX")), Eq(nullptr)); - EXPECT_THAT(factory.CreateVideoEncoder(SdpVideoFormat("Bar")), Eq(nullptr)); + EXPECT_THAT(factory.CreateVideoEncoder(kFooSdp), NotNull()); + EXPECT_THAT(factory.CreateVideoEncoder(kBarLowSdp), NotNull()); + EXPECT_THAT(factory.CreateVideoEncoder(kBarHighSdp), NotNull()); + EXPECT_THAT(factory.CreateVideoEncoder(SdpVideoFormat("FooX")), IsNull()); + EXPECT_THAT(factory.CreateVideoEncoder(SdpVideoFormat("Bar")), NotNull()); } TEST(VideoEncoderFactoryTemplate, TwoTemplateAdaptersCodecSupport) { @@ -131,7 +132,7 @@ TEST(VideoEncoderFactoryTemplate, LibvpxVp8) { EXPECT_THAT(formats[0], Field(&SdpVideoFormat::name, "VP8")); EXPECT_THAT(formats[0], Field(&SdpVideoFormat::scalability_modes, Contains(ScalabilityMode::kL1T3))); - EXPECT_THAT(factory.CreateVideoEncoder(formats[0]), Ne(nullptr)); + EXPECT_THAT(factory.CreateVideoEncoder(formats[0]), NotNull()); } TEST(VideoEncoderFactoryTemplate, LibvpxVp9) { @@ -141,7 +142,7 @@ TEST(VideoEncoderFactoryTemplate, LibvpxVp9) { EXPECT_THAT(formats, Each(Field(&SdpVideoFormat::name, "VP9"))); EXPECT_THAT(formats, Each(Field(&SdpVideoFormat::scalability_modes, Contains(ScalabilityMode::kL3T3_KEY)))); - EXPECT_THAT(factory.CreateVideoEncoder(formats[0]), Ne(nullptr)); + EXPECT_THAT(factory.CreateVideoEncoder(formats[0]), NotNull()); } // TODO(bugs.webrtc.org/13573): When OpenH264 is no longer a conditional build @@ -154,7 +155,7 @@ TEST(VideoEncoderFactoryTemplate, OpenH264) { EXPECT_THAT(formats, Each(Field(&SdpVideoFormat::name, "H264"))); EXPECT_THAT(formats, Each(Field(&SdpVideoFormat::scalability_modes, Contains(ScalabilityMode::kL1T3)))); - EXPECT_THAT(factory.CreateVideoEncoder(formats[0]), Ne(nullptr)); + EXPECT_THAT(factory.CreateVideoEncoder(formats[0]), NotNull()); } #endif // defined(WEBRTC_USE_H264) @@ -165,7 +166,7 @@ TEST(VideoEncoderFactoryTemplate, LibaomAv1) { EXPECT_THAT(formats[0], Field(&SdpVideoFormat::name, "AV1")); EXPECT_THAT(formats[0], Field(&SdpVideoFormat::scalability_modes, Contains(ScalabilityMode::kL3T3_KEY))); - EXPECT_THAT(factory.CreateVideoEncoder(formats[0]), Ne(nullptr)); + EXPECT_THAT(factory.CreateVideoEncoder(formats[0]), NotNull()); } } // namespace diff --git a/api/video_codecs/video_encoder_factory_template.h b/api/video_codecs/video_encoder_factory_template.h index 643096dbbb..10212ac816 100644 --- a/api/video_codecs/video_encoder_factory_template.h +++ b/api/video_codecs/video_encoder_factory_template.h @@ -17,6 +17,7 @@ #include "absl/algorithm/container.h" #include "api/array_view.h" +#include "api/video_codecs/sdp_video_format.h" #include "api/video_codecs/video_encoder.h" #include "api/video_codecs/video_encoder_factory.h" #include "modules/video_coding/svc/scalability_mode_util.h" @@ -51,7 +52,16 @@ class VideoEncoderFactoryTemplate : public VideoEncoderFactory { std::unique_ptr CreateVideoEncoder( const SdpVideoFormat& format) override { - return CreateVideoEncoderInternal(format); + // We fuzzy match the specified format for both valid and not so valid + // reasons. The valid reason is that there are many standardized codec + // specific fmtp parameters that have not been implemented, and in those + // cases we should not fail to instantiate an encoder just because we don't + // recognize the parameter. The not so valid reason is that we have started + // adding parameters completely unrelated to the SDP to the SdpVideoFormat. + // TODO(bugs.webrtc.org/13868): Remove FuzzyMatchSdpVideoFormat + absl::optional matched = + FuzzyMatchSdpVideoFormat(GetSupportedFormats(), format); + return CreateVideoEncoderInternal(matched.value_or(format)); } CodecSupport QueryCodecSupport(