From e2fd86a79ca75281689398999c0120c7fb4ad80e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Wed, 24 Oct 2018 11:32:39 +0200 Subject: [PATCH] Move encoder metadata into EncoderInfo struct. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This deprecates the following methods in VideoEncoder: virtual ScalingSettings GetScalingSettings() const; virtual bool SupportsNativeHandle() const; virtual const char* ImplementationName() const; Though they are not marked RTC_DEPRECATED since we still want to call them from within the default GetEncoderInfo() until downstream projects have been updated. Furthmore, implementation name is changed from const char* to std:string, which prevents some lifetime issues with dynamic encoder names, and CodecSpecificInfo.codec_name is removed in favor of getting the implementation name via GetEncoderInfo(). This CL removes calls to these deprecated methods, follow-ups will also remove implementations of the methods and replace them with new GetEncoderInfo() substitutions. Bug: webrtc:9890 Change-Id: I6fd6e531480c0b952f53dbd5105e0b0adc3e3b0c Reviewed-on: https://webrtc-review.googlesource.com/c/106905 Reviewed-by: Stefan Holmer Reviewed-by: Rasmus Brandt Reviewed-by: Niels Moller Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/master@{#25351} --- api/video/video_stream_encoder_observer.h | 4 ++ ...oder_software_fallback_wrapper_unittest.cc | 38 ++++++++--------- api/video_codecs/video_encoder.cc | 19 +++++++++ api/video_codecs/video_encoder.h | 33 ++++++++++++--- ...video_encoder_software_fallback_wrapper.cc | 42 +++++++++---------- media/engine/simulcast_encoder_adapter.cc | 5 +-- .../simulcast_encoder_adapter_unittest.cc | 19 +++++---- .../vp8_encoder_simulcast_proxy_unittest.cc | 5 ++- .../test/videocodec_test_fixture_impl.cc | 2 +- .../codecs/vp8/libvpx_vp8_encoder.cc | 1 - .../codecs/vp8/test/vp8_impl_unittest.cc | 8 ++-- modules/video_coding/codecs/vp9/vp9_impl.cc | 1 - modules/video_coding/generic_encoder.cc | 2 +- .../include/video_codec_interface.h | 1 + sdk/android/src/jni/videoencoderwrapper.cc | 1 - test/fake_encoder.cc | 1 - test/fake_vp8_encoder.cc | 1 - video/send_statistics_proxy.cc | 29 ++++++++----- video/send_statistics_proxy.h | 12 ++++++ video/send_statistics_proxy_unittest.cc | 32 +++++++------- video/video_stream_encoder.cc | 15 ++++++- video/video_stream_encoder.h | 2 + 22 files changed, 171 insertions(+), 102 deletions(-) diff --git a/api/video/video_stream_encoder_observer.h b/api/video/video_stream_encoder_observer.h index b940efdf11..98b5cfcb2c 100644 --- a/api/video/video_stream_encoder_observer.h +++ b/api/video/video_stream_encoder_observer.h @@ -11,6 +11,7 @@ #ifndef API_VIDEO_VIDEO_STREAM_ENCODER_OBSERVER_H_ #define API_VIDEO_VIDEO_STREAM_ENCODER_OBSERVER_H_ +#include #include #include "absl/types/optional.h" @@ -68,6 +69,9 @@ class VideoStreamEncoderObserver : public CpuOveruseMetricsObserver { virtual void OnSendEncodedImage(const EncodedImage& encoded_image, const CodecSpecificInfo* codec_info) = 0; + virtual void OnEncoderImplementationChanged( + const std::string& implementation_name) = 0; + virtual void OnFrameDropped(DropReason reason) = 0; // Used to indicate change in content type, which may require a change in diff --git a/api/video_codecs/test/video_encoder_software_fallback_wrapper_unittest.cc b/api/video_codecs/test/video_encoder_software_fallback_wrapper_unittest.cc index 91d65ae1d5..fceea8b5d5 100644 --- a/api/video_codecs/test/video_encoder_software_fallback_wrapper_unittest.cc +++ b/api/video_codecs/test/video_encoder_software_fallback_wrapper_unittest.cc @@ -61,9 +61,7 @@ class VideoEncoderSoftwareFallbackWrapperTest : public ::testing::Test { ++encode_count_; if (encode_complete_callback_ && encode_return_code_ == WEBRTC_VIDEO_CODEC_OK) { - CodecSpecificInfo info; - info.codec_name = ImplementationName(); - encode_complete_callback_->OnEncodedImage(EncodedImage(), &info, + encode_complete_callback_->OnEncodedImage(EncodedImage(), nullptr, nullptr); } return encode_return_code_; @@ -91,15 +89,10 @@ class VideoEncoderSoftwareFallbackWrapperTest : public ::testing::Test { return WEBRTC_VIDEO_CODEC_OK; } - bool SupportsNativeHandle() const override { + EncoderInfo GetEncoderInfo() const override { ++supports_native_handle_count_; - return supports_native_handle_; - } - - const char* ImplementationName() const override { return "fake-encoder"; } - - VideoEncoder::ScalingSettings GetScalingSettings() const override { - return VideoEncoder::ScalingSettings(kLowThreshold, kHighThreshold); + return EncoderInfo(ScalingSettings(kLowThreshold, kHighThreshold), + supports_native_handle_, "fake-encoder"); } int init_encode_count_ = 0; @@ -121,11 +114,9 @@ class VideoEncoderSoftwareFallbackWrapperTest : public ::testing::Test { const CodecSpecificInfo* codec_specific_info, const RTPFragmentationHeader* fragmentation) override { ++callback_count_; - last_codec_name_ = codec_specific_info->codec_name; return Result(Result::OK, callback_count_); } int callback_count_ = 0; - std::string last_codec_name_; }; void UtilizeFallbackEncoder(); @@ -133,7 +124,8 @@ class VideoEncoderSoftwareFallbackWrapperTest : public ::testing::Test { void EncodeFrame(); void EncodeFrame(int expected_ret); void CheckLastEncoderName(const char* expected_name) { - EXPECT_STREQ(expected_name, callback_.last_codec_name_.c_str()); + EXPECT_EQ(expected_name, + fallback_wrapper_->GetEncoderInfo().implementation_name); } test::ScopedFieldTrials override_field_trials_; @@ -290,15 +282,19 @@ TEST_F(VideoEncoderSoftwareFallbackWrapperTest, TEST_F(VideoEncoderSoftwareFallbackWrapperTest, SupportsNativeHandleForwardedWithoutFallback) { - fallback_wrapper_->SupportsNativeHandle(); + fallback_wrapper_->GetEncoderInfo(); EXPECT_EQ(1, fake_encoder_->supports_native_handle_count_); } TEST_F(VideoEncoderSoftwareFallbackWrapperTest, SupportsNativeHandleNotForwardedDuringFallback) { + // Fake encoder signals support for native handle, default (libvpx) does not. + fake_encoder_->supports_native_handle_ = true; + EXPECT_TRUE(fallback_wrapper_->GetEncoderInfo().supports_native_handle); UtilizeFallbackEncoder(); - fallback_wrapper_->SupportsNativeHandle(); - EXPECT_EQ(0, fake_encoder_->supports_native_handle_count_); + EXPECT_FALSE(fallback_wrapper_->GetEncoderInfo().supports_native_handle); + // Both times, both encoders are queried. + EXPECT_EQ(2, fake_encoder_->supports_native_handle_count_); EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, fallback_wrapper_->Release()); } @@ -488,7 +484,7 @@ TEST_F(ForcedFallbackTestDisabled, GetScaleSettings) { EncodeFrameAndVerifyLastName("fake-encoder"); // Default min pixels per frame should be used. - const auto settings = fallback_wrapper_->GetScalingSettings(); + const auto settings = fallback_wrapper_->GetEncoderInfo().scaling_settings; EXPECT_TRUE(settings.thresholds.has_value()); EXPECT_EQ(kDefaultMinPixelsPerFrame, settings.min_pixels_per_frame); } @@ -499,7 +495,7 @@ TEST_F(ForcedFallbackTestEnabled, GetScaleSettingsWithNoFallback) { EncodeFrameAndVerifyLastName("fake-encoder"); // Configured min pixels per frame should be used. - const auto settings = fallback_wrapper_->GetScalingSettings(); + const auto settings = fallback_wrapper_->GetEncoderInfo().scaling_settings; EXPECT_EQ(kMinPixelsPerFrame, settings.min_pixels_per_frame); ASSERT_TRUE(settings.thresholds); EXPECT_EQ(kLowThreshold, settings.thresholds->low); @@ -512,7 +508,7 @@ TEST_F(ForcedFallbackTestEnabled, GetScaleSettingsWithFallback) { EncodeFrameAndVerifyLastName("libvpx"); // Configured min pixels per frame should be used. - const auto settings = fallback_wrapper_->GetScalingSettings(); + const auto settings = fallback_wrapper_->GetEncoderInfo().scaling_settings; EXPECT_TRUE(settings.thresholds.has_value()); EXPECT_EQ(kMinPixelsPerFrame, settings.min_pixels_per_frame); } @@ -524,7 +520,7 @@ TEST_F(ForcedFallbackTestEnabled, ScalingDisabledIfResizeOff) { EncodeFrameAndVerifyLastName("libvpx"); // Should be disabled for automatic resize off. - const auto settings = fallback_wrapper_->GetScalingSettings(); + const auto settings = fallback_wrapper_->GetEncoderInfo().scaling_settings; EXPECT_FALSE(settings.thresholds.has_value()); } diff --git a/api/video_codecs/video_encoder.cc b/api/video_codecs/video_encoder.cc index 33d35deb68..468d50b954 100644 --- a/api/video_codecs/video_encoder.cc +++ b/api/video_codecs/video_encoder.cc @@ -83,6 +83,20 @@ VideoEncoder::ScalingSettings::~ScalingSettings() {} constexpr VideoEncoder::ScalingSettings::KOff VideoEncoder::ScalingSettings::kOff; +VideoEncoder::EncoderInfo::EncoderInfo() + : scaling_settings(VideoEncoder::ScalingSettings::kOff), + supports_native_handle(false), + implementation_name("unknown") {} + +VideoEncoder::EncoderInfo::EncoderInfo(const ScalingSettings& scaling_settings, + bool supports_native_handle, + const std::string& implementation_name) + : scaling_settings(scaling_settings), + supports_native_handle(supports_native_handle), + implementation_name(implementation_name) {} + +VideoEncoder::EncoderInfo::~EncoderInfo() = default; + int32_t VideoEncoder::SetRates(uint32_t bitrate, uint32_t framerate) { RTC_NOTREACHED() << "SetRate(uint32_t, uint32_t) is deprecated."; return -1; @@ -105,4 +119,9 @@ bool VideoEncoder::SupportsNativeHandle() const { const char* VideoEncoder::ImplementationName() const { return "unknown"; } + +VideoEncoder::EncoderInfo VideoEncoder::GetEncoderInfo() const { + return EncoderInfo(GetScalingSettings(), SupportsNativeHandle(), + ImplementationName()); +} } // namespace webrtc diff --git a/api/video_codecs/video_encoder.h b/api/video_codecs/video_encoder.h index 8f7d619a7b..2f42b7d4bb 100644 --- a/api/video_codecs/video_encoder.h +++ b/api/video_codecs/video_encoder.h @@ -103,13 +103,13 @@ class RTC_EXPORT VideoEncoder { ScalingSettings(KOff); // NOLINT(runtime/explicit) ~ScalingSettings(); - const absl::optional thresholds; + absl::optional thresholds; // We will never ask for a resolution lower than this. // TODO(kthelgason): Lower this limit when better testing // on MediaCodec and fallback implementations are in place. // See https://bugs.chromium.org/p/webrtc/issues/detail?id=7206 - const int min_pixels_per_frame = 320 * 180; + int min_pixels_per_frame = 320 * 180; private: // Private constructor; to get an object without thresholds, use @@ -117,6 +117,28 @@ class RTC_EXPORT VideoEncoder { ScalingSettings(); }; + // Struct containing metadata about the encoder implementing this interface. + struct EncoderInfo { + EncoderInfo(); + EncoderInfo(const ScalingSettings& scaling_settings, + bool supports_native_handle, + const std::string& implementation_name); + // EncoderInfo(const EncoderInfo& rhs); + // EncoderInfo& operator=(const EncoderInfo& rhs); + ~EncoderInfo(); + + // Any encoder implementation wishing to use the WebRTC provided + // quality scaler must populate this field. + ScalingSettings scaling_settings; + + // If true, encoder supports working with a native handle (e.g. texture + // handle for hw codecs) rather than requiring a raw I420 buffer. + bool supports_native_handle; + + // The name of this particular encoder implementation, e.g. "libvpx". + std::string implementation_name; + }; + static VideoCodecVP8 GetDefaultVp8Settings(); static VideoCodecVP9 GetDefaultVp9Settings(); static VideoCodecH264 GetDefaultH264Settings(); @@ -197,12 +219,13 @@ class RTC_EXPORT VideoEncoder { virtual int32_t SetRateAllocation(const VideoBitrateAllocation& allocation, uint32_t framerate); - // Any encoder implementation wishing to use the WebRTC provided - // quality scaler must implement this method. + // GetScalingSettings(), SupportsNativeHandle(), ImplementationName() are + // deprecated, use GetEncoderInfo() instead. virtual ScalingSettings GetScalingSettings() const; - virtual bool SupportsNativeHandle() const; virtual const char* ImplementationName() const; + + virtual EncoderInfo GetEncoderInfo() const; }; } // namespace webrtc #endif // API_VIDEO_CODECS_VIDEO_ENCODER_H_ diff --git a/api/video_codecs/video_encoder_software_fallback_wrapper.cc b/api/video_codecs/video_encoder_software_fallback_wrapper.cc index 99f29707e4..7bc69d6c8b 100644 --- a/api/video_codecs/video_encoder_software_fallback_wrapper.cc +++ b/api/video_codecs/video_encoder_software_fallback_wrapper.cc @@ -90,9 +90,7 @@ class VideoEncoderSoftwareFallbackWrapper final : public VideoEncoder { int32_t SetChannelParameters(uint32_t packet_loss, int64_t rtt) override; int32_t SetRateAllocation(const VideoBitrateAllocation& bitrate_allocation, uint32_t framerate) override; - bool SupportsNativeHandle() const override; - ScalingSettings GetScalingSettings() const override; - const char* ImplementationName() const override; + EncoderInfo GetEncoderInfo() const override; private: bool InitFallbackEncoder(); @@ -162,7 +160,7 @@ VideoEncoderSoftwareFallbackWrapper::VideoEncoderSoftwareFallbackWrapper( if (forced_fallback_possible_) { GetForcedFallbackParamsFromFieldTrialGroup( &forced_fallback_.min_pixels_, &forced_fallback_.max_pixels_, - encoder_->GetScalingSettings().min_pixels_per_frame - + encoder_->GetEncoderInfo().scaling_settings.min_pixels_per_frame - 1); // No HW below. } } @@ -294,29 +292,29 @@ int32_t VideoEncoderSoftwareFallbackWrapper::SetRateAllocation( return ret; } -bool VideoEncoderSoftwareFallbackWrapper::SupportsNativeHandle() const { - return use_fallback_encoder_ ? fallback_encoder_->SupportsNativeHandle() - : encoder_->SupportsNativeHandle(); -} +VideoEncoder::EncoderInfo VideoEncoderSoftwareFallbackWrapper::GetEncoderInfo() + const { + EncoderInfo fallback_encoder_info = fallback_encoder_->GetEncoderInfo(); + EncoderInfo default_encoder_info = encoder_->GetEncoderInfo(); + + EncoderInfo info = + use_fallback_encoder_ ? fallback_encoder_info : default_encoder_info; -VideoEncoder::ScalingSettings -VideoEncoderSoftwareFallbackWrapper::GetScalingSettings() const { if (forced_fallback_possible_) { const auto settings = forced_fallback_.active_ - ? fallback_encoder_->GetScalingSettings() - : encoder_->GetScalingSettings(); - return settings.thresholds - ? VideoEncoder::ScalingSettings(settings.thresholds->low, - settings.thresholds->high, - forced_fallback_.min_pixels_) - : VideoEncoder::ScalingSettings::kOff; + ? fallback_encoder_info.scaling_settings + : default_encoder_info.scaling_settings; + info.scaling_settings = + settings.thresholds + ? VideoEncoder::ScalingSettings(settings.thresholds->low, + settings.thresholds->high, + forced_fallback_.min_pixels_) + : VideoEncoder::ScalingSettings::kOff; + } else { + info.scaling_settings = default_encoder_info.scaling_settings; } - return encoder_->GetScalingSettings(); -} -const char* VideoEncoderSoftwareFallbackWrapper::ImplementationName() const { - return use_fallback_encoder_ ? fallback_encoder_->ImplementationName() - : encoder_->ImplementationName(); + return info; } bool VideoEncoderSoftwareFallbackWrapper::IsForcedFallbackActive() const { diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc index 80c2ef2f3c..0c507b683a 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -256,7 +256,8 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, if (i != 0) { implementation_name += ", "; } - implementation_name += streaminfos_[i].encoder->ImplementationName(); + implementation_name += + streaminfos_[i].encoder->GetEncoderInfo().implementation_name; } if (doing_simulcast) { @@ -451,7 +452,6 @@ EncodedImageCallback::Result SimulcastEncoderAdapter::OnEncodedImage( const RTPFragmentationHeader* fragmentation) { EncodedImage stream_image(encodedImage); CodecSpecificInfo stream_codec_specific = *codecSpecificInfo; - stream_codec_specific.codec_name = implementation_name_.c_str(); stream_image.SetSpatialIndex(stream_idx); @@ -519,7 +519,6 @@ void SimulcastEncoderAdapter::DestroyStoredEncoders() { bool SimulcastEncoderAdapter::SupportsNativeHandle() const { RTC_DCHECK_CALLED_SEQUENTIALLY(&encoder_queue_); // We should not be calling this method before streaminfos_ are configured. - RTC_DCHECK(!streaminfos_.empty()); for (const auto& streaminfo : streaminfos_) { if (!streaminfo.encoder->SupportsNativeHandle()) { return false; diff --git a/media/engine/simulcast_encoder_adapter_unittest.cc b/media/engine/simulcast_encoder_adapter_unittest.cc index 91d83e9729..85c946337c 100644 --- a/media/engine/simulcast_encoder_adapter_unittest.cc +++ b/media/engine/simulcast_encoder_adapter_unittest.cc @@ -735,9 +735,9 @@ TEST_F(TestSimulcastEncoderAdapterFake, SupportsNativeHandleForSingleStreams) { adapter_->RegisterEncodeCompleteCallback(this); ASSERT_EQ(1u, helper_->factory()->encoders().size()); helper_->factory()->encoders()[0]->set_supports_native_handle(true); - EXPECT_TRUE(adapter_->SupportsNativeHandle()); + EXPECT_TRUE(adapter_->GetEncoderInfo().supports_native_handle); helper_->factory()->encoders()[0]->set_supports_native_handle(false); - EXPECT_FALSE(adapter_->SupportsNativeHandle()); + EXPECT_FALSE(adapter_->GetEncoderInfo().supports_native_handle); } TEST_F(TestSimulcastEncoderAdapterFake, SetRatesUnderMinBitrate) { @@ -770,7 +770,8 @@ TEST_F(TestSimulcastEncoderAdapterFake, SetRatesUnderMinBitrate) { } TEST_F(TestSimulcastEncoderAdapterFake, SupportsImplementationName) { - EXPECT_STREQ("SimulcastEncoderAdapter", adapter_->ImplementationName()); + EXPECT_EQ("SimulcastEncoderAdapter", + adapter_->GetEncoderInfo().implementation_name); SimulcastTestFixtureImpl::DefaultSettings( &codec_, static_cast(kTestTemporalLayerProfile), kVideoCodecVP8); @@ -780,8 +781,8 @@ TEST_F(TestSimulcastEncoderAdapterFake, SupportsImplementationName) { encoder_names.push_back("codec3"); helper_->factory()->SetEncoderNames(encoder_names); EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); - EXPECT_STREQ("SimulcastEncoderAdapter (codec1, codec2, codec3)", - adapter_->ImplementationName()); + EXPECT_EQ("SimulcastEncoderAdapter (codec1, codec2, codec3)", + adapter_->GetEncoderInfo().implementation_name); // Single streams should not expose "SimulcastEncoderAdapter" in name. EXPECT_EQ(0, adapter_->Release()); @@ -789,7 +790,7 @@ TEST_F(TestSimulcastEncoderAdapterFake, SupportsImplementationName) { EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); adapter_->RegisterEncodeCompleteCallback(this); ASSERT_EQ(1u, helper_->factory()->encoders().size()); - EXPECT_STREQ("codec1", adapter_->ImplementationName()); + EXPECT_EQ("codec1", adapter_->GetEncoderInfo().implementation_name); } TEST_F(TestSimulcastEncoderAdapterFake, @@ -805,10 +806,10 @@ TEST_F(TestSimulcastEncoderAdapterFake, encoder->set_supports_native_handle(true); // If one encoder doesn't support it, then overall support is disabled. helper_->factory()->encoders()[0]->set_supports_native_handle(false); - EXPECT_FALSE(adapter_->SupportsNativeHandle()); + EXPECT_FALSE(adapter_->GetEncoderInfo().supports_native_handle); // Once all do, then the adapter claims support. helper_->factory()->encoders()[0]->set_supports_native_handle(true); - EXPECT_TRUE(adapter_->SupportsNativeHandle()); + EXPECT_TRUE(adapter_->GetEncoderInfo().supports_native_handle); } // TODO(nisse): Reuse definition in webrtc/test/fake_texture_handle.h. @@ -844,7 +845,7 @@ TEST_F(TestSimulcastEncoderAdapterFake, ASSERT_EQ(3u, helper_->factory()->encoders().size()); for (MockVideoEncoder* encoder : helper_->factory()->encoders()) encoder->set_supports_native_handle(true); - EXPECT_TRUE(adapter_->SupportsNativeHandle()); + EXPECT_TRUE(adapter_->GetEncoderInfo().supports_native_handle); rtc::scoped_refptr buffer( new rtc::RefCountedObject(1280, 720)); diff --git a/media/engine/vp8_encoder_simulcast_proxy_unittest.cc b/media/engine/vp8_encoder_simulcast_proxy_unittest.cc index fc6d5a634e..d0a0dfc8be 100644 --- a/media/engine/vp8_encoder_simulcast_proxy_unittest.cc +++ b/media/engine/vp8_encoder_simulcast_proxy_unittest.cc @@ -104,7 +104,8 @@ TEST(VP8EncoderSimulcastProxy, ChoosesCorrectImplementation) { SdpVideoFormat("VP8")); EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, simulcast_enabled_proxy.InitEncode(&codec_settings, 4, 1200)); - EXPECT_EQ(kImplementationName, simulcast_enabled_proxy.ImplementationName()); + EXPECT_EQ(kImplementationName, + simulcast_enabled_proxy.GetEncoderInfo().implementation_name); NiceMock* mock_encoder1 = new NiceMock(); NiceMock* mock_encoder2 = new NiceMock(); @@ -145,7 +146,7 @@ TEST(VP8EncoderSimulcastProxy, ChoosesCorrectImplementation) { EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, simulcast_disabled_proxy.InitEncode(&codec_settings, 4, 1200)); EXPECT_EQ(kSimulcastAdaptedImplementationName, - simulcast_disabled_proxy.ImplementationName()); + simulcast_disabled_proxy.GetEncoderInfo().implementation_name); // Cleanup. simulcast_enabled_proxy.Release(); diff --git a/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc b/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc index 263e4015d4..e8a666f075 100644 --- a/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc +++ b/modules/video_coding/codecs/test/videocodec_test_fixture_impl.cc @@ -671,7 +671,7 @@ void VideoCodecTestFixtureImpl::PrintSettings( std::string encoder_name; std::string decoder_name; task_queue->SendTask([this, &encoder_name, &decoder_name] { - encoder_name = encoder_->ImplementationName(); + encoder_name = encoder_->GetEncoderInfo().implementation_name; decoder_name = decoders_.at(0)->ImplementationName(); }); printf("enc_impl_name: %s\n", encoder_name.c_str()); diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc index a7f060fbe3..b92aa77952 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc @@ -843,7 +843,6 @@ void LibvpxVp8Encoder::PopulateCodecSpecific(CodecSpecificInfo* codec_specific, uint32_t timestamp) { assert(codec_specific != NULL); codec_specific->codecType = kVideoCodecVP8; - codec_specific->codec_name = ImplementationName(); CodecSpecificInfoVP8* vp8Info = &(codec_specific->codecSpecific.VP8); vp8Info->keyIdx = kNoKeyIdx; // TODO(hlundin) populate this vp8Info->nonReference = (pkt.data.frame.flags & VPX_FRAME_IS_DROPPABLE) != 0; diff --git a/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc b/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc index 5659a379b8..5eed5d380c 100644 --- a/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc +++ b/modules/video_coding/codecs/vp8/test/vp8_impl_unittest.cc @@ -77,7 +77,7 @@ class TestVp8Impl : public VideoCodecUnitTest { encoder_->Encode(input_frame, nullptr, &frame_types)); ASSERT_TRUE(WaitForEncodedFrame(encoded_frame, codec_specific_info)); VerifyQpParser(*encoded_frame); - EXPECT_STREQ("libvpx", codec_specific_info->codec_name); + EXPECT_EQ("libvpx", encoder_->GetEncoderInfo().implementation_name); EXPECT_EQ(kVideoCodecVP8, codec_specific_info->codecType); EXPECT_EQ(0, encoded_frame->SpatialIndex()); } @@ -334,7 +334,8 @@ TEST_F(TestVp8Impl, ScalingDisabledIfAutomaticResizeOff) { EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, encoder_->InitEncode(&codec_settings_, kNumCores, kMaxPayloadSize)); - VideoEncoder::ScalingSettings settings = encoder_->GetScalingSettings(); + VideoEncoder::ScalingSettings settings = + encoder_->GetEncoderInfo().scaling_settings; EXPECT_FALSE(settings.thresholds.has_value()); } @@ -344,7 +345,8 @@ TEST_F(TestVp8Impl, ScalingEnabledIfAutomaticResizeOn) { EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, encoder_->InitEncode(&codec_settings_, kNumCores, kMaxPayloadSize)); - VideoEncoder::ScalingSettings settings = encoder_->GetScalingSettings(); + VideoEncoder::ScalingSettings settings = + encoder_->GetEncoderInfo().scaling_settings; EXPECT_TRUE(settings.thresholds.has_value()); EXPECT_EQ(kDefaultMinPixelsPerFrame, settings.min_pixels_per_frame); } diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc index 3e58e44072..0cc527551f 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -837,7 +837,6 @@ void VP9EncoderImpl::PopulateCodecSpecific(CodecSpecificInfo* codec_specific, bool first_frame_in_picture) { RTC_CHECK(codec_specific != nullptr); codec_specific->codecType = kVideoCodecVP9; - codec_specific->codec_name = ImplementationName(); CodecSpecificInfoVP9* vp9_info = &(codec_specific->codecSpecific.VP9); vp9_info->first_frame_in_picture = first_frame_in_picture; diff --git a/modules/video_coding/generic_encoder.cc b/modules/video_coding/generic_encoder.cc index b1d7c28abd..485fe78f60 100644 --- a/modules/video_coding/generic_encoder.cc +++ b/modules/video_coding/generic_encoder.cc @@ -166,7 +166,7 @@ bool VCMGenericEncoder::InternalSource() const { bool VCMGenericEncoder::SupportsNativeHandle() const { RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); - return encoder_->SupportsNativeHandle(); + return encoder_->GetEncoderInfo().supports_native_handle; } VCMEncodedFrameCallback::VCMEncodedFrameCallback( diff --git a/modules/video_coding/include/video_codec_interface.h b/modules/video_coding/include/video_codec_interface.h index d2badf9201..38583bfa3b 100644 --- a/modules/video_coding/include/video_codec_interface.h +++ b/modules/video_coding/include/video_codec_interface.h @@ -79,6 +79,7 @@ struct CodecSpecificInfo { memset(&codecSpecific, 0, sizeof(codecSpecific)); } VideoCodecType codecType; + // |codec_name| is deprecated, use name provided by VideoEncoder instead. const char* codec_name; CodecSpecificInfoUnion codecSpecific; }; diff --git a/sdk/android/src/jni/videoencoderwrapper.cc b/sdk/android/src/jni/videoencoderwrapper.cc index 6381c63cb0..50437bef1b 100644 --- a/sdk/android/src/jni/videoencoderwrapper.cc +++ b/sdk/android/src/jni/videoencoderwrapper.cc @@ -385,7 +385,6 @@ CodecSpecificInfo VideoEncoderWrapper::ParseCodecSpecificInfo( CodecSpecificInfo info; memset(&info, 0, sizeof(info)); info.codecType = codec_settings_.codecType; - info.codec_name = implementation_name_.c_str(); switch (codec_settings_.codecType) { case kVideoCodecVP8: diff --git a/test/fake_encoder.cc b/test/fake_encoder.cc index ca99038e2d..c151e4c4a6 100644 --- a/test/fake_encoder.cc +++ b/test/fake_encoder.cc @@ -128,7 +128,6 @@ int32_t FakeEncoder::Encode(const VideoFrame& input_image, ? VideoContentType::SCREENSHARE : VideoContentType::UNSPECIFIED; encoded.SetSpatialIndex(i); - specifics.codec_name = ImplementationName(); if (callback->OnEncodedImage(encoded, &specifics, nullptr).error != EncodedImageCallback::Result::OK) { return -1; diff --git a/test/fake_vp8_encoder.cc b/test/fake_vp8_encoder.cc index 82ba88bc60..fedb9c1f2f 100644 --- a/test/fake_vp8_encoder.cc +++ b/test/fake_vp8_encoder.cc @@ -106,7 +106,6 @@ void FakeVP8Encoder::PopulateCodecSpecific(CodecSpecificInfo* codec_specific, uint32_t timestamp) { RTC_DCHECK_CALLED_SEQUENTIALLY(&sequence_checker_); codec_specific->codecType = kVideoCodecVP8; - codec_specific->codec_name = ImplementationName(); CodecSpecificInfoVP8* vp8Info = &(codec_specific->codecSpecific.VP8); vp8Info->keyIdx = kNoKeyIdx; vp8Info->nonReference = false; diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc index 9839ed7a3b..e11304bfdf 100644 --- a/video/send_statistics_proxy.cc +++ b/video/send_statistics_proxy.cc @@ -823,10 +823,13 @@ void SendStatisticsProxy::UpdateEncoderFallbackStats( const int64_t now_ms = clock_->TimeInMilliseconds(); bool is_active = fallback_info->is_active; - if (codec_info->codec_name != stats_.encoder_implementation_name) { + if (encoder_changed_) { // Implementation changed. - is_active = strcmp(codec_info->codec_name, kVp8SwCodecName) == 0; - if (!is_active && stats_.encoder_implementation_name != kVp8SwCodecName) { + const bool last_was_vp8_software = + encoder_changed_->previous_encoder_implementation == kVp8SwCodecName; + is_active = encoder_changed_->new_encoder_implementation == kVp8SwCodecName; + encoder_changed_.reset(); + if (!is_active && !last_was_vp8_software) { // First or not a VP8 SW change, update stats on next call. return; } @@ -865,7 +868,7 @@ void SendStatisticsProxy::UpdateFallbackDisabledStats( } if (!IsForcedFallbackPossible(codec_info, simulcast_index) || - strcmp(codec_info->codec_name, kVp8SwCodecName) == 0) { + stats_.encoder_implementation_name == kVp8SwCodecName) { uma_container_->fallback_info_disabled_.is_possible = false; return; } @@ -895,13 +898,9 @@ void SendStatisticsProxy::OnSendEncodedImage( rtc::CritScope lock(&crit_); ++stats_.frames_encoded; if (codec_info) { - if (codec_info->codec_name) { - UpdateEncoderFallbackStats( - codec_info, - encoded_image._encodedWidth * encoded_image._encodedHeight, - simulcast_idx); - stats_.encoder_implementation_name = codec_info->codec_name; - } + UpdateEncoderFallbackStats( + codec_info, encoded_image._encodedWidth * encoded_image._encodedHeight, + simulcast_idx); } if (static_cast(simulcast_idx) >= rtp_config_.ssrcs.size()) { @@ -981,6 +980,14 @@ void SendStatisticsProxy::OnSendEncodedImage( } } +void SendStatisticsProxy::OnEncoderImplementationChanged( + const std::string& implementation_name) { + rtc::CritScope lock(&crit_); + encoder_changed_ = EncoderChangeEvent{stats_.encoder_implementation_name, + implementation_name}; + stats_.encoder_implementation_name = implementation_name; +} + int SendStatisticsProxy::GetInputFrameRate() const { rtc::CritScope lock(&crit_); return round(uma_container_->input_frame_rate_tracker_.ComputeRate()); diff --git a/video/send_statistics_proxy.h b/video/send_statistics_proxy.h index b5868d0890..2fa87cccce 100644 --- a/video/send_statistics_proxy.h +++ b/video/send_statistics_proxy.h @@ -53,6 +53,10 @@ class SendStatisticsProxy : public VideoStreamEncoderObserver, void OnSendEncodedImage(const EncodedImage& encoded_image, const CodecSpecificInfo* codec_info) override; + + void OnEncoderImplementationChanged( + const std::string& implementation_name) override; + // Used to update incoming frame rate. void OnIncomingFrame(int width, int height) override; @@ -241,6 +245,14 @@ class SendStatisticsProxy : public VideoStreamEncoderObserver, absl::optional last_outlier_timestamp_ RTC_GUARDED_BY(crit_); + struct EncoderChangeEvent { + std::string previous_encoder_implementation; + std::string new_encoder_implementation; + }; + // Stores the last change in encoder implementation in an optional, so that + // the event can be consumed. + absl::optional encoder_changed_; + // Contains stats used for UMA histograms. These stats will be reset if // content type changes between real-time video and screenshare, since these // will be reported separately. diff --git a/video/send_statistics_proxy_unittest.cc b/video/send_statistics_proxy_unittest.cc index 3d27c712c6..586d57fb28 100644 --- a/video/send_statistics_proxy_unittest.cc +++ b/video/send_statistics_proxy_unittest.cc @@ -2167,13 +2167,9 @@ TEST_F(SendStatisticsProxyTest, FecBitrateNotReportedWhenNotEnabled) { } TEST_F(SendStatisticsProxyTest, GetStatsReportsEncoderImplementationName) { - const char* kName = "encoderName"; - EncodedImage encoded_image; - CodecSpecificInfo codec_info; - codec_info.codec_name = kName; - statistics_proxy_->OnSendEncodedImage(encoded_image, &codec_info); - EXPECT_STREQ( - kName, statistics_proxy_->GetStats().encoder_implementation_name.c_str()); + const std::string kName = "encoderName"; + statistics_proxy_->OnEncoderImplementationChanged(kName); + EXPECT_EQ(kName, statistics_proxy_->GetStats().encoder_implementation_name); } TEST_F(SendStatisticsProxyTest, Vp9SvcLowSpatialLayerDoesNotUpdateResolution) { @@ -2219,7 +2215,6 @@ class ForcedFallbackTest : public SendStatisticsProxyTest { : SendStatisticsProxyTest(field_trials) { codec_info_.codecType = kVideoCodecVP8; codec_info_.codecSpecific.VP8.temporalIdx = 0; - codec_info_.codec_name = "fake_codec"; encoded_image_._encodedWidth = kWidth; encoded_image_._encodedHeight = kHeight; encoded_image_.SetSpatialIndex(0); @@ -2229,6 +2224,8 @@ class ForcedFallbackTest : public SendStatisticsProxyTest { protected: void InsertEncodedFrames(int num_frames, int interval_ms) { + statistics_proxy_->OnEncoderImplementationChanged(codec_name_); + // First frame is not updating stats, insert initial frame. if (statistics_proxy_->GetStats().frames_encoded == 0) { statistics_proxy_->OnSendEncodedImage(encoded_image_, &codec_info_); @@ -2243,6 +2240,7 @@ class ForcedFallbackTest : public SendStatisticsProxyTest { EncodedImage encoded_image_; CodecSpecificInfo codec_info_; + std::string codec_name_; const std::string kPrefix = "WebRTC.Video.Encoder.ForcedSw"; const int kFrameIntervalMs = 1000; const int kMinFrames = 20; // Min run time 20 sec. @@ -2321,7 +2319,7 @@ TEST_F(ForcedFallbackEnabled, EnteredLowResolutionNotSetIfNotLibvpx) { } TEST_F(ForcedFallbackEnabled, EnteredLowResolutionSetIfLibvpx) { - codec_info_.codec_name = "libvpx"; + codec_name_ = "libvpx"; InsertEncodedFrames(1, kFrameIntervalMs); EXPECT_TRUE(statistics_proxy_->GetStats().has_entered_low_resolution); } @@ -2333,7 +2331,7 @@ TEST_F(ForcedFallbackDisabled, EnteredLowResolutionNotSetIfAboveMaxPixels) { } TEST_F(ForcedFallbackDisabled, EnteredLowResolutionNotSetIfLibvpx) { - codec_info_.codec_name = "libvpx"; + codec_name_ = "libvpx"; InsertEncodedFrames(1, kFrameIntervalMs); EXPECT_FALSE(statistics_proxy_->GetStats().has_entered_low_resolution); } @@ -2351,7 +2349,7 @@ TEST_F(ForcedFallbackEnabled, OneFallbackEvent) { EXPECT_FALSE(statistics_proxy_->GetStats().has_entered_low_resolution); InsertEncodedFrames(15, 1000); EXPECT_FALSE(statistics_proxy_->GetStats().has_entered_low_resolution); - codec_info_.codec_name = "libvpx"; + codec_name_ = "libvpx"; InsertEncodedFrames(5, 1000); EXPECT_TRUE(statistics_proxy_->GetStats().has_entered_low_resolution); @@ -2369,18 +2367,18 @@ TEST_F(ForcedFallbackEnabled, ThreeFallbackEvents) { // Three changes. Video: 60000 ms, fallback: 15000 ms (25%). InsertEncodedFrames(10, 1000); EXPECT_FALSE(statistics_proxy_->GetStats().has_entered_low_resolution); - codec_info_.codec_name = "libvpx"; + codec_name_ = "libvpx"; InsertEncodedFrames(15, 500); EXPECT_TRUE(statistics_proxy_->GetStats().has_entered_low_resolution); - codec_info_.codec_name = "notlibvpx"; + codec_name_ = "notlibvpx"; InsertEncodedFrames(20, 1000); InsertEncodedFrames(3, kMaxFrameDiffMs); // Should not be included. InsertEncodedFrames(10, 1000); EXPECT_TRUE(statistics_proxy_->GetStats().has_entered_low_resolution); - codec_info_.codec_name = "notlibvpx2"; + codec_name_ = "notlibvpx2"; InsertEncodedFrames(10, 500); EXPECT_TRUE(statistics_proxy_->GetStats().has_entered_low_resolution); - codec_info_.codec_name = "libvpx"; + codec_name_ = "libvpx"; InsertEncodedFrames(15, 500); EXPECT_TRUE(statistics_proxy_->GetStats().has_entered_low_resolution); @@ -2393,7 +2391,7 @@ TEST_F(ForcedFallbackEnabled, ThreeFallbackEvents) { TEST_F(ForcedFallbackEnabled, NoFallbackIfAboveMaxPixels) { encoded_image_._encodedWidth = kWidth + 1; - codec_info_.codec_name = "libvpx"; + codec_name_ = "libvpx"; InsertEncodedFrames(kMinFrames, kFrameIntervalMs); EXPECT_FALSE(statistics_proxy_->GetStats().has_entered_low_resolution); @@ -2404,7 +2402,7 @@ TEST_F(ForcedFallbackEnabled, NoFallbackIfAboveMaxPixels) { TEST_F(ForcedFallbackEnabled, FallbackIfAtMaxPixels) { encoded_image_._encodedWidth = kWidth; - codec_info_.codec_name = "libvpx"; + codec_name_ = "libvpx"; InsertEncodedFrames(kMinFrames, kFrameIntervalMs); EXPECT_TRUE(statistics_proxy_->GetStats().has_entered_low_resolution); diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 786ded4c82..6f734fb9d1 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -363,6 +363,8 @@ VideoStreamEncoder::VideoStreamEncoder( max_framerate_(-1), pending_encoder_reconfiguration_(false), pending_encoder_creation_(false), + crop_width_(0), + crop_height_(0), encoder_start_bitrate_bps_(0), max_data_payload_length_(0), last_observed_bitrate_bps_(0), @@ -376,6 +378,7 @@ VideoStreamEncoder::VideoStreamEncoder( last_frame_log_ms_(clock_->TimeInMilliseconds()), captured_frame_count_(0), dropped_frame_count_(0), + pending_frame_post_time_us_(0), bitrate_observer_(nullptr), encoder_queue_("EncoderQueue") { RTC_DCHECK(encoder_stats_observer); @@ -629,7 +632,7 @@ void VideoStreamEncoder::ReconfigureEncoder() { void VideoStreamEncoder::ConfigureQualityScaler() { RTC_DCHECK_RUN_ON(&encoder_queue_); - const auto scaling_settings = encoder_->GetScalingSettings(); + const auto scaling_settings = encoder_->GetEncoderInfo().scaling_settings; const bool quality_scaling_allowed = IsResolutionScalingEnabled(degradation_preference_) && scaling_settings.thresholds; @@ -887,6 +890,14 @@ void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame, overuse_detector_->FrameCaptured(out_frame, time_when_posted_us); + // Encoder metadata needs to be updated before encode complete callback. + VideoEncoder::EncoderInfo info = encoder_->GetEncoderInfo(); + if (info.implementation_name != encoder_info_.implementation_name) { + encoder_stats_observer_->OnEncoderImplementationChanged( + info.implementation_name); + } + encoder_info_ = info; + video_sender_.AddVideoFrame(out_frame, nullptr); } @@ -1085,7 +1096,7 @@ void VideoStreamEncoder::AdaptDown(AdaptReason reason) { bool min_pixels_reached = false; if (!source_proxy_->RequestResolutionLowerThan( adaptation_request.input_pixel_count_, - encoder_->GetScalingSettings().min_pixels_per_frame, + encoder_->GetEncoderInfo().scaling_settings.min_pixels_per_frame, &min_pixels_reached)) { if (min_pixels_reached) encoder_stats_observer_->OnMinPixelLimitReached(); diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h index 4008426bac..8c5efbe5c3 100644 --- a/video/video_stream_encoder.h +++ b/video/video_stream_encoder.h @@ -268,6 +268,8 @@ class VideoStreamEncoder : public VideoStreamEncoderInterface, absl::optional last_parameters_update_ms_ RTC_GUARDED_BY(&encoder_queue_); + VideoEncoder::EncoderInfo encoder_info_ RTC_GUARDED_BY(&encoder_queue_); + // All public methods are proxied to |encoder_queue_|. It must must be // destroyed first to make sure no tasks are run that use other members. rtc::TaskQueue encoder_queue_;