From cf2c8915f4e2c1330245ef31ba237318bc70dc3e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Wed, 18 May 2022 10:45:46 +0200 Subject: [PATCH] Delete H264EncoderSpecificSettings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Production code always use the default settings. Bug: webrtc:6883 Change-Id: I213fc6433bb1cd0a6623ad523fee2df1506588e1 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/261903 Reviewed-by: Erik Språng Commit-Queue: Niels Moller Reviewed-by: Mirko Bonadei Cr-Commit-Position: refs/heads/main@{#36926} --- api/video_codecs/video_encoder_config.cc | 18 +----------------- api/video_codecs/video_encoder_config.h | 10 ---------- media/engine/fake_webrtc_call.cc | 2 -- media/engine/webrtc_video_engine.cc | 5 +---- .../video_coding/video_codec_initializer.cc | 5 +++-- rtc_tools/rtp_generator/rtp_generator.cc | 4 +--- test/scenario/video_stream.cc | 9 ++++----- video/video_quality_test.cc | 5 +---- video/video_send_stream_tests.cc | 12 ++++-------- 9 files changed, 15 insertions(+), 55 deletions(-) diff --git a/api/video_codecs/video_encoder_config.cc b/api/video_codecs/video_encoder_config.cc index 0cc9ce51ae..fd4a68fc05 100644 --- a/api/video_codecs/video_encoder_config.cc +++ b/api/video_codecs/video_encoder_config.cc @@ -92,9 +92,7 @@ VideoEncoderConfig::VideoEncoderConfig(const VideoEncoderConfig&) = default; void VideoEncoderConfig::EncoderSpecificSettings::FillEncoderSpecificSettings( VideoCodec* codec) const { - if (codec->codecType == kVideoCodecH264) { - FillVideoCodecH264(codec->H264()); - } else if (codec->codecType == kVideoCodecVP8) { + if (codec->codecType == kVideoCodecVP8) { FillVideoCodecVp8(codec->VP8()); } else if (codec->codecType == kVideoCodecVP9) { FillVideoCodecVp9(codec->VP9()); @@ -104,11 +102,6 @@ void VideoEncoderConfig::EncoderSpecificSettings::FillEncoderSpecificSettings( } } -void VideoEncoderConfig::EncoderSpecificSettings::FillVideoCodecH264( - VideoCodecH264* h264_settings) const { - RTC_DCHECK_NOTREACHED(); -} - void VideoEncoderConfig::EncoderSpecificSettings::FillVideoCodecVp8( VideoCodecVP8* vp8_settings) const { RTC_DCHECK_NOTREACHED(); @@ -119,15 +112,6 @@ void VideoEncoderConfig::EncoderSpecificSettings::FillVideoCodecVp9( RTC_DCHECK_NOTREACHED(); } -VideoEncoderConfig::H264EncoderSpecificSettings::H264EncoderSpecificSettings( - const VideoCodecH264& specifics) - : specifics_(specifics) {} - -void VideoEncoderConfig::H264EncoderSpecificSettings::FillVideoCodecH264( - VideoCodecH264* h264_settings) const { - *h264_settings = specifics_; -} - VideoEncoderConfig::Vp8EncoderSpecificSettings::Vp8EncoderSpecificSettings( const VideoCodecVP8& specifics) : specifics_(specifics) {} diff --git a/api/video_codecs/video_encoder_config.h b/api/video_codecs/video_encoder_config.h index f560362422..4076208b56 100644 --- a/api/video_codecs/video_encoder_config.h +++ b/api/video_codecs/video_encoder_config.h @@ -86,22 +86,12 @@ class VideoEncoderConfig { virtual void FillVideoCodecVp8(VideoCodecVP8* vp8_settings) const; virtual void FillVideoCodecVp9(VideoCodecVP9* vp9_settings) const; - virtual void FillVideoCodecH264(VideoCodecH264* h264_settings) const; private: ~EncoderSpecificSettings() override {} friend class VideoEncoderConfig; }; - class H264EncoderSpecificSettings : public EncoderSpecificSettings { - public: - explicit H264EncoderSpecificSettings(const VideoCodecH264& specifics); - void FillVideoCodecH264(VideoCodecH264* h264_settings) const override; - - private: - VideoCodecH264 specifics_; - }; - class Vp8EncoderSpecificSettings : public EncoderSpecificSettings { public: explicit Vp8EncoderSpecificSettings(const VideoCodecVP8& specifics); diff --git a/media/engine/fake_webrtc_call.cc b/media/engine/fake_webrtc_call.cc index 9375103c31..8400ae4314 100644 --- a/media/engine/fake_webrtc_call.cc +++ b/media/engine/fake_webrtc_call.cc @@ -288,8 +288,6 @@ void FakeVideoSendStream::ReconfigureVideoEncoder( num_temporal_layers; } } else if (config_.rtp.payload_name == "H264") { - config.encoder_specific_settings->FillVideoCodecH264( - &codec_specific_settings_.h264); codec_specific_settings_.h264.numberOfTemporalLayers = num_temporal_layers; } else { diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index 8135a526fb..115b50826e 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -512,10 +512,7 @@ WebRtcVideoChannel::WebRtcVideoSendStream::ConfigureVideoEncoderSettings( } if (absl::EqualsIgnoreCase(codec.name, kH264CodecName)) { - webrtc::VideoCodecH264 h264_settings = - webrtc::VideoEncoder::GetDefaultH264Settings(); - return rtc::make_ref_counted< - webrtc::VideoEncoderConfig::H264EncoderSpecificSettings>(h264_settings); + return nullptr; } if (absl::EqualsIgnoreCase(codec.name, kVp8CodecName)) { webrtc::VideoCodecVP8 vp8_settings = diff --git a/modules/video_coding/video_codec_initializer.cc b/modules/video_coding/video_codec_initializer.cc index 99eb67cb9b..dc8bde2c2f 100644 --- a/modules/video_coding/video_codec_initializer.cc +++ b/modules/video_coding/video_codec_initializer.cc @@ -314,8 +314,9 @@ VideoCodec VideoCodecInitializer::VideoEncoderConfigToVideoCodec( } break; case kVideoCodecH264: { - if (!config.encoder_specific_settings) - *video_codec.H264() = VideoEncoder::GetDefaultH264Settings(); + RTC_CHECK(!config.encoder_specific_settings); + + *video_codec.H264() = VideoEncoder::GetDefaultH264Settings(); video_codec.H264()->numberOfTemporalLayers = static_cast( streams.back().num_temporal_layers.value_or( video_codec.H264()->numberOfTemporalLayers)); diff --git a/rtc_tools/rtp_generator/rtp_generator.cc b/rtc_tools/rtp_generator/rtp_generator.cc index c2fc1cff06..8a3aceed75 100644 --- a/rtc_tools/rtp_generator/rtp_generator.cc +++ b/rtc_tools/rtp_generator/rtp_generator.cc @@ -202,9 +202,7 @@ RtpGenerator::RtpGenerator(const RtpGeneratorOptions& options) rtc::make_ref_counted( settings); } else if (video_config.rtp.payload_name == cricket::kH264CodecName) { - VideoCodecH264 settings = VideoEncoder::GetDefaultH264Settings(); - encoder_config.encoder_specific_settings = rtc::make_ref_counted< - VideoEncoderConfig::H264EncoderSpecificSettings>(settings); + encoder_config.encoder_specific_settings = nullptr; } encoder_config.video_format.name = video_config.rtp.payload_name; encoder_config.min_transmit_bitrate_bps = 0; diff --git a/test/scenario/video_stream.cc b/test/scenario/video_stream.cc index d00200aee5..305c69d0e0 100644 --- a/test/scenario/video_stream.cc +++ b/test/scenario/video_stream.cc @@ -198,11 +198,10 @@ CreateH264SpecificSettings(VideoStreamConfig config) { RTC_DCHECK_EQ(config.encoder.layers.temporal, 1); RTC_DCHECK_EQ(config.encoder.layers.spatial, 1); - VideoCodecH264 h264_settings = VideoEncoder::GetDefaultH264Settings(); - h264_settings.keyFrameInterval = - config.encoder.key_frame_interval.value_or(0); - return rtc::make_ref_counted( - h264_settings); + // TODO(bugs.webrtc.org/6883): Set a key frame interval as a setting that + // isn't codec specific. + RTC_CHECK_EQ(0, config.encoder.key_frame_interval.value_or(0)); + return nullptr; } rtc::scoped_refptr diff --git a/video/video_quality_test.cc b/video/video_quality_test.cc index 25d50b076b..4501096379 100644 --- a/video/video_quality_test.cc +++ b/video/video_quality_test.cc @@ -905,10 +905,7 @@ void VideoQualityTest::SetupVideo(Transport* send_transport, rtc::make_ref_counted< VideoEncoderConfig::Vp9EncoderSpecificSettings>(vp9_settings); } else if (params_.video[video_idx].codec == "H264") { - VideoCodecH264 h264_settings = VideoEncoder::GetDefaultH264Settings(); - video_encoder_configs_[video_idx].encoder_specific_settings = - rtc::make_ref_counted< - VideoEncoderConfig::H264EncoderSpecificSettings>(h264_settings); + video_encoder_configs_[video_idx].encoder_specific_settings = nullptr; } } total_streams_used += num_video_substreams; diff --git a/video/video_send_stream_tests.cc b/video/video_send_stream_tests.cc index 02bff7d3d0..a64e18ce2e 100644 --- a/video/video_send_stream_tests.cc +++ b/video/video_send_stream_tests.cc @@ -2435,9 +2435,7 @@ class VideoCodecConfigObserver : public test::SendTest, }; template <> -void VideoCodecConfigObserver::InitCodecSpecifics() { - encoder_settings_ = VideoEncoder::GetDefaultH264Settings(); -} +void VideoCodecConfigObserver::InitCodecSpecifics() {} template <> void VideoCodecConfigObserver::VerifyCodecSpecifics( @@ -2454,18 +2452,16 @@ void VideoCodecConfigObserver::VerifyCodecSpecifics( // Set expected temporal layers as they should have been set when // reconfiguring the encoder and not match the set config. - VideoCodecH264 encoder_settings = encoder_settings_; + VideoCodecH264 encoder_settings = VideoEncoder::GetDefaultH264Settings(); encoder_settings.numberOfTemporalLayers = kVideoCodecConfigObserverNumberOfTemporalLayers; - EXPECT_EQ( - 0, memcmp(&config.H264(), &encoder_settings, sizeof(encoder_settings_))); + EXPECT_EQ(config.H264(), encoder_settings); } template <> rtc::scoped_refptr VideoCodecConfigObserver::GetEncoderSpecificSettings() const { - return rtc::make_ref_counted( - encoder_settings_); + return nullptr; } template <>