From 73588223c17468f865abca6f2cf0f225b218ac5a Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Thu, 4 Apr 2024 14:56:21 +0200 Subject: [PATCH] Require webrtc::Environment to create H264 Encoder Bug: webrtc:15860 Change-Id: I76517ee3603847ff064f16fddc9423a3568818a7 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/345741 Commit-Queue: Danil Chapovalov Reviewed-by: Philip Eliasson Cr-Commit-Position: refs/heads/main@{#42004} --- modules/video_coding/BUILD.gn | 3 +- modules/video_coding/codecs/h264/h264.cc | 17 ----------- .../codecs/h264/h264_decoder_impl.cc | 1 + .../codecs/h264/h264_encoder_impl.cc | 21 ++------------ .../codecs/h264/h264_encoder_impl.h | 6 +--- .../codecs/h264/h264_encoder_impl_unittest.cc | 28 ++++++------------- .../codecs/h264/h264_simulcast_unittest.cc | 4 ++- .../video_coding/codecs/h264/include/h264.h | 13 +++------ 8 files changed, 22 insertions(+), 71 deletions(-) diff --git a/modules/video_coding/BUILD.gn b/modules/video_coding/BUILD.gn index ef63d2d8bc..e30ad6154d 100644 --- a/modules/video_coding/BUILD.gn +++ b/modules/video_coding/BUILD.gn @@ -481,6 +481,7 @@ rtc_library("webrtc_h264") { defines = [] deps = [ + ":codec_globals_headers", ":video_codec_interface", ":video_coding_utility", "../../api/environment", @@ -492,7 +493,6 @@ rtc_library("webrtc_h264") { "../../api/video_codecs:scalability_mode", "../../api/video_codecs:video_codecs_api", "../../common_video", - "../../media:codec", "../../media:media_constants", "../../media:rtc_media_base", "../../rtc_base:checks", @@ -500,7 +500,6 @@ rtc_library("webrtc_h264") { "../../rtc_base:logging", "../../rtc_base:timeutils", "../../rtc_base/system:rtc_export", - "../../system_wrappers:field_trial", "../../system_wrappers:metrics", "svc:scalability_structures", "svc:scalable_video_controller", diff --git a/modules/video_coding/codecs/h264/h264.cc b/modules/video_coding/codecs/h264/h264.cc index 16000f42ec..60142c88c1 100644 --- a/modules/video_coding/codecs/h264/h264.cc +++ b/modules/video_coding/codecs/h264/h264.cc @@ -145,23 +145,6 @@ absl::Nonnull> CreateH264Encoder( #endif } -std::unique_ptr H264Encoder::Create() { - return Create(cricket::CreateVideoCodec(cricket::kH264CodecName)); -} - -std::unique_ptr H264Encoder::Create( - const cricket::VideoCodec& codec) { - RTC_DCHECK(H264Encoder::IsSupported()); -#if defined(WEBRTC_USE_H264) - RTC_CHECK(g_rtc_use_h264); - RTC_LOG(LS_INFO) << "Creating H264EncoderImpl."; - return std::make_unique(codec); -#else - RTC_DCHECK_NOTREACHED(); - return nullptr; -#endif -} - bool H264Encoder::IsSupported() { return IsH264CodecSupported(); } diff --git a/modules/video_coding/codecs/h264/h264_decoder_impl.cc b/modules/video_coding/codecs/h264/h264_decoder_impl.cc index feceb37716..7e01d6a66e 100644 --- a/modules/video_coding/codecs/h264/h264_decoder_impl.cc +++ b/modules/video_coding/codecs/h264/h264_decoder_impl.cc @@ -31,6 +31,7 @@ extern "C" { #include "api/video/i420_buffer.h" #include "common_video/include/video_frame_buffer.h" #include "modules/video_coding/codecs/h264/h264_color_space.h" +#include "modules/video_coding/include/video_error_codes.h" #include "rtc_base/checks.h" #include "rtc_base/logging.h" #include "system_wrappers/include/metrics.h" diff --git a/modules/video_coding/codecs/h264/h264_encoder_impl.cc b/modules/video_coding/codecs/h264/h264_encoder_impl.cc index 65d60a94e7..5b21d6c5ee 100644 --- a/modules/video_coding/codecs/h264/h264_encoder_impl.cc +++ b/modules/video_coding/codecs/h264/h264_encoder_impl.cc @@ -25,6 +25,8 @@ #include "api/video/video_codec_constants.h" #include "api/video_codecs/scalability_mode.h" #include "common_video/libyuv/include/webrtc_libyuv.h" +#include "modules/video_coding/include/video_codec_interface.h" +#include "modules/video_coding/include/video_error_codes.h" #include "modules/video_coding/svc/create_scalability_structure.h" #include "modules/video_coding/utility/simulcast_rate_allocator.h" #include "modules/video_coding/utility/simulcast_utility.h" @@ -172,24 +174,7 @@ static void RtpFragmentize(EncodedImage* encoded_image, SFrameBSInfo* info) { H264EncoderImpl::H264EncoderImpl(const Environment& env, H264EncoderSettings settings) - : H264EncoderImpl(settings.packetization_mode) {} - -H264EncoderImpl::H264EncoderImpl(const cricket::VideoCodec& codec) - : H264EncoderImpl([&] { - std::string packetization_mode_string; - if (codec.GetParam(cricket::kH264FmtpPacketizationMode, - &packetization_mode_string) && - packetization_mode_string == "1") { - return H264PacketizationMode::NonInterleaved; - } else { - return H264PacketizationMode::SingleNalUnit; - } - }()) { - RTC_CHECK(absl::EqualsIgnoreCase(codec.name, cricket::kH264CodecName)); -} - -H264EncoderImpl::H264EncoderImpl(H264PacketizationMode packetization_mode) - : packetization_mode_(packetization_mode), + : packetization_mode_(settings.packetization_mode), max_payload_size_(0), number_of_cores_(0), encoded_image_callback_(nullptr), diff --git a/modules/video_coding/codecs/h264/h264_encoder_impl.h b/modules/video_coding/codecs/h264/h264_encoder_impl.h index e89f9f9e74..cfc28d7d4e 100644 --- a/modules/video_coding/codecs/h264/h264_encoder_impl.h +++ b/modules/video_coding/codecs/h264/h264_encoder_impl.h @@ -40,7 +40,7 @@ class ISVCEncoder; namespace webrtc { -class H264EncoderImpl : public H264Encoder { +class H264EncoderImpl : public VideoEncoder { public: struct LayerConfig { int simulcast_idx = 0; @@ -60,8 +60,6 @@ class H264EncoderImpl : public H264Encoder { H264EncoderImpl(const Environment& env, H264EncoderSettings settings); - // Deprecated, bugs.webrtc.org/15860 - explicit H264EncoderImpl(const cricket::VideoCodec& codec); ~H264EncoderImpl() override; // `settings.max_payload_size` is ignored. @@ -92,8 +90,6 @@ class H264EncoderImpl : public H264Encoder { } private: - explicit H264EncoderImpl(H264PacketizationMode packetization_mode); - SEncParamExt CreateEncoderParams(size_t i) const; webrtc::H264BitstreamParser h264_bitstream_parser_; diff --git a/modules/video_coding/codecs/h264/h264_encoder_impl_unittest.cc b/modules/video_coding/codecs/h264/h264_encoder_impl_unittest.cc index 3a139ab1c3..e714e277e8 100644 --- a/modules/video_coding/codecs/h264/h264_encoder_impl_unittest.cc +++ b/modules/video_coding/codecs/h264/h264_encoder_impl_unittest.cc @@ -11,7 +11,9 @@ #include "modules/video_coding/codecs/h264/h264_encoder_impl.h" +#include "api/environment/environment_factory.h" #include "api/video_codecs/video_encoder.h" +#include "modules/video_coding/include/video_error_codes.h" #include "test/gtest.h" namespace webrtc { @@ -39,7 +41,7 @@ void SetDefaultSettings(VideoCodec* codec_settings) { } TEST(H264EncoderImplTest, CanInitializeWithDefaultParameters) { - H264EncoderImpl encoder(cricket::CreateVideoCodec("H264")); + H264EncoderImpl encoder(CreateEnvironment(), {}); VideoCodec codec_settings; SetDefaultSettings(&codec_settings); EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, @@ -49,9 +51,9 @@ TEST(H264EncoderImplTest, CanInitializeWithDefaultParameters) { } TEST(H264EncoderImplTest, CanInitializeWithNonInterleavedModeExplicitly) { - cricket::VideoCodec codec = cricket::CreateVideoCodec("H264"); - codec.SetParam(cricket::kH264FmtpPacketizationMode, "1"); - H264EncoderImpl encoder(codec); + H264EncoderImpl encoder( + CreateEnvironment(), + {.packetization_mode = H264PacketizationMode::NonInterleaved}); VideoCodec codec_settings; SetDefaultSettings(&codec_settings); EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, @@ -61,21 +63,9 @@ TEST(H264EncoderImplTest, CanInitializeWithNonInterleavedModeExplicitly) { } TEST(H264EncoderImplTest, CanInitializeWithSingleNalUnitModeExplicitly) { - cricket::VideoCodec codec = cricket::CreateVideoCodec("H264"); - codec.SetParam(cricket::kH264FmtpPacketizationMode, "0"); - H264EncoderImpl encoder(codec); - VideoCodec codec_settings; - SetDefaultSettings(&codec_settings); - EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, - encoder.InitEncode(&codec_settings, kSettings)); - EXPECT_EQ(H264PacketizationMode::SingleNalUnit, - encoder.PacketizationModeForTesting()); -} - -TEST(H264EncoderImplTest, CanInitializeWithRemovedParameter) { - cricket::VideoCodec codec = cricket::CreateVideoCodec("H264"); - codec.RemoveParam(cricket::kH264FmtpPacketizationMode); - H264EncoderImpl encoder(codec); + H264EncoderImpl encoder( + CreateEnvironment(), + {.packetization_mode = H264PacketizationMode::SingleNalUnit}); VideoCodec codec_settings; SetDefaultSettings(&codec_settings); EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, diff --git a/modules/video_coding/codecs/h264/h264_simulcast_unittest.cc b/modules/video_coding/codecs/h264/h264_simulcast_unittest.cc index 65c1add89f..b9ea324ca2 100644 --- a/modules/video_coding/codecs/h264/h264_simulcast_unittest.cc +++ b/modules/video_coding/codecs/h264/h264_simulcast_unittest.cc @@ -24,7 +24,9 @@ namespace { std::unique_ptr CreateSpecificSimulcastTestFixture() { std::unique_ptr encoder_factory = std::make_unique( - []() { return H264Encoder::Create(); }); + [](const Environment& env, const SdpVideoFormat& format) { + return CreateH264Encoder(env); + }); std::unique_ptr decoder_factory = std::make_unique( []() { return H264Decoder::Create(); }); diff --git a/modules/video_coding/codecs/h264/include/h264.h b/modules/video_coding/codecs/h264/include/h264.h index 5b9907e2ec..6e4bc9f4da 100644 --- a/modules/video_coding/codecs/h264/include/h264.h +++ b/modules/video_coding/codecs/h264/include/h264.h @@ -20,15 +20,14 @@ #include "api/environment/environment.h" #include "api/video_codecs/h264_profile_level_id.h" #include "api/video_codecs/scalability_mode.h" +#include "api/video_codecs/sdp_video_format.h" +#include "api/video_codecs/video_decoder.h" #include "api/video_codecs/video_encoder.h" -#include "media/base/codec.h" -#include "modules/video_coding/include/video_codec_interface.h" +#include "modules/video_coding/codecs/h264/include/h264_globals.h" #include "rtc_base/system/rtc_export.h" namespace webrtc { -struct SdpVideoFormat; - // Creates an H264 SdpVideoFormat entry with specified paramters. RTC_EXPORT SdpVideoFormat CreateH264Format(H264Profile profile, @@ -52,15 +51,11 @@ std::vector SupportedH264Codecs( // only connections. std::vector SupportedH264DecoderCodecs(); -class RTC_EXPORT H264Encoder : public VideoEncoder { +class RTC_EXPORT H264Encoder { public: - static std::unique_ptr Create(const cricket::VideoCodec& codec); - static std::unique_ptr Create(); // If H.264 is supported (any implementation). static bool IsSupported(); static bool SupportsScalabilityMode(ScalabilityMode scalability_mode); - - ~H264Encoder() override {} }; struct H264EncoderSettings {