From 2b781bf9080ab7070b67f8ce8429ce7191e200ef Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Niels=20M=C3=B6ller?= Date: Thu, 16 Jul 2020 14:19:41 +0200 Subject: [PATCH] Deprecate write-only member CodecInfo::is_hardware_accelerated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This member of the CodecInfo struct was set in several places, but not used for anything. To aid deletion, this cl defines a default implementation of VideoEncoderFactory::QueryVideoEncoder. The next step is to delete almost all downstream implementations of that method, since the only classes that have to implement it are the few factories that produce "internal source" encoders, e.g., for Chromium remoting. Bug: None Change-Id: I1f0dbf0d302933004ebdc779460cb2cb3a894e02 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/179520 Reviewed-by: Kári Helgason Reviewed-by: Sami Kalliomäki Reviewed-by: Sebastian Jansson Commit-Queue: Niels Moller Cr-Commit-Position: refs/heads/master@{#31844} --- api/test/video/function_video_encoder_factory.h | 8 -------- api/video_codecs/builtin_video_encoder_factory.cc | 2 -- api/video_codecs/video_encoder_factory.h | 12 ++++++++---- media/engine/fake_video_codec_factory.cc | 5 ----- media/engine/fake_video_codec_factory.h | 2 -- media/engine/fake_webrtc_video_engine.cc | 1 - media/engine/internal_encoder_factory.cc | 8 -------- media/engine/internal_encoder_factory.h | 2 -- media/engine/multiplex_codec_factory.cc | 8 -------- media/engine/multiplex_codec_factory.h | 1 - .../engine/simulcast_encoder_adapter_unittest.cc | 7 ------- media/engine/webrtc_video_engine_unittest.cc | 1 - .../src/jni/video_encoder_factory_wrapper.cc | 15 --------------- .../src/jni/video_encoder_factory_wrapper.h | 2 -- sdk/objc/native/src/objc_video_encoder_factory.h | 1 - sdk/objc/native/src/objc_video_encoder_factory.mm | 13 ------------- test/peer_scenario/peer_scenario_client.cc | 2 -- test/video_encoder_proxy_factory.h | 4 ---- 18 files changed, 8 insertions(+), 86 deletions(-) diff --git a/api/test/video/function_video_encoder_factory.h b/api/test/video/function_video_encoder_factory.h index 40a187acf2..a452eee7c4 100644 --- a/api/test/video/function_video_encoder_factory.h +++ b/api/test/video/function_video_encoder_factory.h @@ -43,14 +43,6 @@ class FunctionVideoEncoderFactory final : public VideoEncoderFactory { return {}; } - CodecInfo QueryVideoEncoder( - const SdpVideoFormat& /* format */) const override { - CodecInfo codec_info; - codec_info.is_hardware_accelerated = false; - codec_info.has_internal_source = false; - return codec_info; - } - std::unique_ptr CreateVideoEncoder( const SdpVideoFormat& format) override { return create_(format); diff --git a/api/video_codecs/builtin_video_encoder_factory.cc b/api/video_codecs/builtin_video_encoder_factory.cc index 6888daae48..2f722a4a5c 100644 --- a/api/video_codecs/builtin_video_encoder_factory.cc +++ b/api/video_codecs/builtin_video_encoder_factory.cc @@ -50,8 +50,6 @@ class BuiltinVideoEncoderFactory : public VideoEncoderFactory { RTC_DCHECK(IsFormatSupported( internal_encoder_factory_->GetSupportedFormats(), format)); VideoEncoderFactory::CodecInfo info; - info.has_internal_source = false; - info.is_hardware_accelerated = false; return info; } diff --git a/api/video_codecs/video_encoder_factory.h b/api/video_codecs/video_encoder_factory.h index c396090ea6..0a3c1aee66 100644 --- a/api/video_codecs/video_encoder_factory.h +++ b/api/video_codecs/video_encoder_factory.h @@ -28,8 +28,7 @@ class VideoEncoderFactory { public: // TODO(magjed): Try to get rid of this struct. struct CodecInfo { - // |is_hardware_accelerated| is true if the encoders created by this factory - // of the given codec will use hardware support. + // TODO(nisse): Unused in webrtc, delete as soon as downstream use is fixed. bool is_hardware_accelerated = false; // |has_internal_source| is true if encoders created by this factory of the // given codec will use internal camera sources, meaning that they don't @@ -73,8 +72,13 @@ class VideoEncoderFactory { // Returns information about how this format will be encoded. The specified // format must be one of the supported formats by this factory. - // TODO(magjed): Try to get rid of this method. - virtual CodecInfo QueryVideoEncoder(const SdpVideoFormat& format) const = 0; + + // TODO(magjed): Try to get rid of this method. Since is_hardware_accelerated + // is unused, only factories producing internal source encoders (in itself a + // deprecated feature) needs to override this method. + virtual CodecInfo QueryVideoEncoder(const SdpVideoFormat& format) const { + return CodecInfo(); + } // Creates a VideoEncoder for the specified format. virtual std::unique_ptr CreateVideoEncoder( diff --git a/media/engine/fake_video_codec_factory.cc b/media/engine/fake_video_codec_factory.cc index ba7513be24..63a1d5096d 100644 --- a/media/engine/fake_video_codec_factory.cc +++ b/media/engine/fake_video_codec_factory.cc @@ -44,11 +44,6 @@ std::vector FakeVideoEncoderFactory::GetSupportedFormats() 1, SdpVideoFormat(kFakeCodecFactoryCodecName)); } -VideoEncoderFactory::CodecInfo FakeVideoEncoderFactory::QueryVideoEncoder( - const SdpVideoFormat& format) const { - return VideoEncoderFactory::CodecInfo{false, false}; -} - std::unique_ptr FakeVideoEncoderFactory::CreateVideoEncoder( const SdpVideoFormat& format) { return std::make_unique(Clock::GetRealTimeClock()); diff --git a/media/engine/fake_video_codec_factory.h b/media/engine/fake_video_codec_factory.h index 029a695337..4a99120467 100644 --- a/media/engine/fake_video_codec_factory.h +++ b/media/engine/fake_video_codec_factory.h @@ -30,8 +30,6 @@ class RTC_EXPORT FakeVideoEncoderFactory : public VideoEncoderFactory { // VideoEncoderFactory implementation std::vector GetSupportedFormats() const override; - VideoEncoderFactory::CodecInfo QueryVideoEncoder( - const SdpVideoFormat& format) const override; std::unique_ptr CreateVideoEncoder( const SdpVideoFormat& format) override; }; diff --git a/media/engine/fake_webrtc_video_engine.cc b/media/engine/fake_webrtc_video_engine.cc index d7675228cf..c1fc2e6fa0 100644 --- a/media/engine/fake_webrtc_video_engine.cc +++ b/media/engine/fake_webrtc_video_engine.cc @@ -244,7 +244,6 @@ FakeWebRtcVideoEncoderFactory::QueryVideoEncoder( const webrtc::SdpVideoFormat& format) const { webrtc::VideoEncoderFactory::CodecInfo info; info.has_internal_source = encoders_have_internal_sources_; - info.is_hardware_accelerated = true; return info; } diff --git a/media/engine/internal_encoder_factory.cc b/media/engine/internal_encoder_factory.cc index aabb810283..738516eafc 100644 --- a/media/engine/internal_encoder_factory.cc +++ b/media/engine/internal_encoder_factory.cc @@ -41,14 +41,6 @@ std::vector InternalEncoderFactory::GetSupportedFormats() return SupportedFormats(); } -VideoEncoderFactory::CodecInfo InternalEncoderFactory::QueryVideoEncoder( - const SdpVideoFormat& format) const { - CodecInfo info; - info.is_hardware_accelerated = false; - info.has_internal_source = false; - return info; -} - std::unique_ptr InternalEncoderFactory::CreateVideoEncoder( const SdpVideoFormat& format) { if (absl::EqualsIgnoreCase(format.name, cricket::kVp8CodecName)) diff --git a/media/engine/internal_encoder_factory.h b/media/engine/internal_encoder_factory.h index c15d1790f3..3f43e461a7 100644 --- a/media/engine/internal_encoder_factory.h +++ b/media/engine/internal_encoder_factory.h @@ -26,8 +26,6 @@ class RTC_EXPORT InternalEncoderFactory : public VideoEncoderFactory { static std::vector SupportedFormats(); std::vector GetSupportedFormats() const override; - CodecInfo QueryVideoEncoder(const SdpVideoFormat& format) const override; - std::unique_ptr CreateVideoEncoder( const SdpVideoFormat& format) override; }; diff --git a/media/engine/multiplex_codec_factory.cc b/media/engine/multiplex_codec_factory.cc index 5a220d528a..fb296811db 100644 --- a/media/engine/multiplex_codec_factory.cc +++ b/media/engine/multiplex_codec_factory.cc @@ -57,14 +57,6 @@ std::vector MultiplexEncoderFactory::GetSupportedFormats() return formats; } -VideoEncoderFactory::CodecInfo MultiplexEncoderFactory::QueryVideoEncoder( - const SdpVideoFormat& format) const { - if (!IsMultiplexCodec(cricket::VideoCodec(format))) - return factory_->QueryVideoEncoder(format); - return factory_->QueryVideoEncoder( - SdpVideoFormat(kMultiplexAssociatedCodecName)); -} - std::unique_ptr MultiplexEncoderFactory::CreateVideoEncoder( const SdpVideoFormat& format) { if (!IsMultiplexCodec(cricket::VideoCodec(format))) diff --git a/media/engine/multiplex_codec_factory.h b/media/engine/multiplex_codec_factory.h index b8a15e21f6..ea57149a77 100644 --- a/media/engine/multiplex_codec_factory.h +++ b/media/engine/multiplex_codec_factory.h @@ -49,7 +49,6 @@ class RTC_EXPORT MultiplexEncoderFactory : public VideoEncoderFactory { bool supports_augmenting_data = false); std::vector GetSupportedFormats() const override; - CodecInfo QueryVideoEncoder(const SdpVideoFormat& format) const override; std::unique_ptr CreateVideoEncoder( const SdpVideoFormat& format) override; diff --git a/media/engine/simulcast_encoder_adapter_unittest.cc b/media/engine/simulcast_encoder_adapter_unittest.cc index 9a23ca437a..703d283b59 100644 --- a/media/engine/simulcast_encoder_adapter_unittest.cc +++ b/media/engine/simulcast_encoder_adapter_unittest.cc @@ -167,8 +167,6 @@ class MockVideoEncoderFactory : public VideoEncoderFactory { std::unique_ptr CreateVideoEncoder( const SdpVideoFormat& format) override; - CodecInfo QueryVideoEncoder(const SdpVideoFormat& format) const override; - const std::vector& encoders() const; void SetEncoderNames(const std::vector& encoder_names); void set_init_encode_return_value(int32_t value); @@ -357,11 +355,6 @@ void MockVideoEncoderFactory::DestroyVideoEncoder(VideoEncoder* encoder) { } } -VideoEncoderFactory::CodecInfo MockVideoEncoderFactory::QueryVideoEncoder( - const SdpVideoFormat& format) const { - return CodecInfo(); -} - const std::vector& MockVideoEncoderFactory::encoders() const { return encoders_; diff --git a/media/engine/webrtc_video_engine_unittest.cc b/media/engine/webrtc_video_engine_unittest.cc index eae83938d4..8b45c6582f 100644 --- a/media/engine/webrtc_video_engine_unittest.cc +++ b/media/engine/webrtc_video_engine_unittest.cc @@ -1129,7 +1129,6 @@ TEST(WebRtcVideoEngineNewVideoCodecFactoryTest, Vp8) { // Mock encoder creation. |engine| take ownership of the encoder. webrtc::VideoEncoderFactory::CodecInfo codec_info; - codec_info.is_hardware_accelerated = false; codec_info.has_internal_source = false; const webrtc::SdpVideoFormat format("VP8"); EXPECT_CALL(*encoder_factory, QueryVideoEncoder(format)) diff --git a/sdk/android/src/jni/video_encoder_factory_wrapper.cc b/sdk/android/src/jni/video_encoder_factory_wrapper.cc index d6a6cfaf2d..8ab4191db2 100644 --- a/sdk/android/src/jni/video_encoder_factory_wrapper.cc +++ b/sdk/android/src/jni/video_encoder_factory_wrapper.cc @@ -101,21 +101,6 @@ std::vector VideoEncoderFactoryWrapper::GetImplementations() return implementations_; } -VideoEncoderFactory::CodecInfo VideoEncoderFactoryWrapper::QueryVideoEncoder( - const SdpVideoFormat& format) const { - JNIEnv* jni = AttachCurrentThreadIfNeeded(); - ScopedJavaLocalRef j_codec_info = - SdpVideoFormatToVideoCodecInfo(jni, format); - ScopedJavaLocalRef encoder = Java_VideoEncoderFactory_createEncoder( - jni, encoder_factory_, j_codec_info); - - CodecInfo codec_info; - // Check if this is a wrapped native software encoder implementation. - codec_info.is_hardware_accelerated = IsHardwareVideoEncoder(jni, encoder); - codec_info.has_internal_source = false; - return codec_info; -} - std::unique_ptr VideoEncoderFactoryWrapper::GetEncoderSelector() const { JNIEnv* jni = AttachCurrentThreadIfNeeded(); diff --git a/sdk/android/src/jni/video_encoder_factory_wrapper.h b/sdk/android/src/jni/video_encoder_factory_wrapper.h index 799ae0f2bc..2be6b1b33f 100644 --- a/sdk/android/src/jni/video_encoder_factory_wrapper.h +++ b/sdk/android/src/jni/video_encoder_factory_wrapper.h @@ -37,8 +37,6 @@ class VideoEncoderFactoryWrapper : public VideoEncoderFactory { std::vector GetImplementations() const override; - CodecInfo QueryVideoEncoder(const SdpVideoFormat& format) const override; - std::unique_ptr GetEncoderSelector() const override; private: diff --git a/sdk/objc/native/src/objc_video_encoder_factory.h b/sdk/objc/native/src/objc_video_encoder_factory.h index 7e474c976a..38db5e6ae7 100644 --- a/sdk/objc/native/src/objc_video_encoder_factory.h +++ b/sdk/objc/native/src/objc_video_encoder_factory.h @@ -33,7 +33,6 @@ class ObjCVideoEncoderFactory : public VideoEncoderFactory { std::vector GetImplementations() const override; std::unique_ptr CreateVideoEncoder( const SdpVideoFormat& format) override; - CodecInfo QueryVideoEncoder(const SdpVideoFormat& format) const override; std::unique_ptr GetEncoderSelector() const override; private: diff --git a/sdk/objc/native/src/objc_video_encoder_factory.mm b/sdk/objc/native/src/objc_video_encoder_factory.mm index b54945f49e..78434c56ae 100644 --- a/sdk/objc/native/src/objc_video_encoder_factory.mm +++ b/sdk/objc/native/src/objc_video_encoder_factory.mm @@ -174,19 +174,6 @@ std::vector ObjCVideoEncoderFactory::GetImplementations() const return GetSupportedFormats(); } -VideoEncoderFactory::CodecInfo ObjCVideoEncoderFactory::QueryVideoEncoder( - const SdpVideoFormat &format) const { - // TODO(andersc): This is a hack until we figure out how this should be done properly. - NSString *formatName = [NSString stringForStdString:format.name]; - NSSet *wrappedSoftwareFormats = - [NSSet setWithObjects:kRTCVideoCodecVp8Name, kRTCVideoCodecVp9Name, nil]; - - CodecInfo codec_info; - codec_info.is_hardware_accelerated = ![wrappedSoftwareFormats containsObject:formatName]; - codec_info.has_internal_source = false; - return codec_info; -} - std::unique_ptr ObjCVideoEncoderFactory::CreateVideoEncoder( const SdpVideoFormat &format) { RTC_OBJC_TYPE(RTCVideoCodecInfo) *info = diff --git a/test/peer_scenario/peer_scenario_client.cc b/test/peer_scenario/peer_scenario_client.cc index 1ae53ee86b..465ee45adb 100644 --- a/test/peer_scenario/peer_scenario_client.cc +++ b/test/peer_scenario/peer_scenario_client.cc @@ -125,8 +125,6 @@ class FakeVideoEncoderFactory : public VideoEncoderFactory { CodecInfo QueryVideoEncoder(const SdpVideoFormat& format) const override { RTC_CHECK_EQ(format.name, "VP8"); CodecInfo info; - info.has_internal_source = false; - info.is_hardware_accelerated = false; return info; } std::unique_ptr CreateVideoEncoder( diff --git a/test/video_encoder_proxy_factory.h b/test/video_encoder_proxy_factory.h index 70e2c8aaf2..7c412bacfa 100644 --- a/test/video_encoder_proxy_factory.h +++ b/test/video_encoder_proxy_factory.h @@ -38,7 +38,6 @@ class VideoEncoderProxyFactory final : public VideoEncoderFactory { encoder_selector_(encoder_selector), num_simultaneous_encoder_instances_(0), max_num_simultaneous_encoder_instances_(0) { - codec_info_.is_hardware_accelerated = false; codec_info_.has_internal_source = false; } @@ -70,9 +69,6 @@ class VideoEncoderProxyFactory final : public VideoEncoderFactory { return nullptr; } - void SetIsHardwareAccelerated(bool is_hardware_accelerated) { - codec_info_.is_hardware_accelerated = is_hardware_accelerated; - } void SetHasInternalSource(bool has_internal_source) { codec_info_.has_internal_source = has_internal_source; }