From ccc1b57e32bd99e4f220a3db0e540713f4349ad9 Mon Sep 17 00:00:00 2001 From: Mirta Dvornicic Date: Tue, 15 Jan 2019 12:42:18 +0100 Subject: [PATCH] Poll is_hardware_accelerated from VideoEncoder instead of VideoEncoderFactory. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Currently, CPU overuse settings for HW encoders are sometimes being used even though the actual encoder is a SW encoder, e.g. in case of SW fallback when the encoder is initialized. Polling is_hardware_accelerated after the encoder has been created and initialized will improve choosing the correct CPU overuse settings. Bug: webrtc:10065 Change-Id: Ic6bd67630a040b5a121c13fa63dd074006973929 Reviewed-on: https://webrtc-review.googlesource.com/c/116688 Commit-Queue: Mirta Dvornicic Reviewed-by: Erik Språng Reviewed-by: Rasmus Brandt Cr-Commit-Position: refs/heads/master@{#26266} --- api/video_codecs/video_encoder.cc | 2 +- media/engine/fake_webrtc_video_engine.cc | 8 +++++++ media/engine/fake_webrtc_video_engine.h | 1 + .../multiplex/multiplex_encoder_adapter.cc | 6 ++++- video/video_stream_encoder.cc | 17 +++++++++----- video/video_stream_encoder_unittest.cc | 22 ++++++++++++++----- 6 files changed, 44 insertions(+), 12 deletions(-) diff --git a/api/video_codecs/video_encoder.cc b/api/video_codecs/video_encoder.cc index 6c6374b055..478bbe1351 100644 --- a/api/video_codecs/video_encoder.cc +++ b/api/video_codecs/video_encoder.cc @@ -88,7 +88,7 @@ VideoEncoder::EncoderInfo::EncoderInfo() supports_native_handle(false), implementation_name("unknown"), has_trusted_rate_controller(false), - is_hardware_accelerated(false), + is_hardware_accelerated(true), has_internal_source(false) {} VideoEncoder::EncoderInfo::EncoderInfo(const EncoderInfo&) = default; diff --git a/media/engine/fake_webrtc_video_engine.cc b/media/engine/fake_webrtc_video_engine.cc index 70bf9221d2..32ce1fa154 100644 --- a/media/engine/fake_webrtc_video_engine.cc +++ b/media/engine/fake_webrtc_video_engine.cc @@ -173,6 +173,14 @@ int32_t FakeWebRtcVideoEncoder::SetRateAllocation( return WEBRTC_VIDEO_CODEC_OK; } +webrtc::VideoEncoder::EncoderInfo FakeWebRtcVideoEncoder::GetEncoderInfo() + const { + EncoderInfo info; + info.is_hardware_accelerated = true; + info.has_internal_source = false; + return info; +} + bool FakeWebRtcVideoEncoder::WaitForInitEncode() { return init_encode_event_.Wait(kEventTimeoutMs); } diff --git a/media/engine/fake_webrtc_video_engine.h b/media/engine/fake_webrtc_video_engine.h index 1505f491fb..6d06923a9a 100644 --- a/media/engine/fake_webrtc_video_engine.h +++ b/media/engine/fake_webrtc_video_engine.h @@ -96,6 +96,7 @@ class FakeWebRtcVideoEncoder : public webrtc::VideoEncoder { int32_t Release() override; int32_t SetRateAllocation(const webrtc::VideoBitrateAllocation& allocation, uint32_t framerate) override; + webrtc::VideoEncoder::EncoderInfo GetEncoderInfo() const override; bool WaitForInitEncode(); webrtc::VideoCodec GetCodecSettings(); diff --git a/modules/video_coding/codecs/multiplex/multiplex_encoder_adapter.cc b/modules/video_coding/codecs/multiplex/multiplex_encoder_adapter.cc index 4aabec0300..450cc4b51b 100644 --- a/modules/video_coding/codecs/multiplex/multiplex_encoder_adapter.cc +++ b/modules/video_coding/codecs/multiplex/multiplex_encoder_adapter.cc @@ -116,11 +116,15 @@ int MultiplexEncoderAdapter::InitEncode(const VideoCodec* inst, if (i != kAlphaCodecStreams - 1) { encoder_info_.implementation_name += ", "; } + // Uses hardware support if any of the encoders uses it. + // For example, if we are having issues with down-scaling due to + // pipelining delay in HW encoders we need higher encoder usage + // thresholds in CPU adaptation. if (i == 0) { encoder_info_.is_hardware_accelerated = encoder_impl_info.is_hardware_accelerated; } else { - encoder_info_.is_hardware_accelerated &= + encoder_info_.is_hardware_accelerated |= encoder_impl_info.is_hardware_accelerated; } encoder_info_.has_internal_source = false; diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 2d4de8f052..ddb7631941 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -581,8 +581,10 @@ void VideoStreamEncoder::ReconfigureEncoder() { source_proxy_->SetMaxFramerate(max_framerate); // Keep the same encoder, as long as the video_format is unchanged. + // Encoder creation block is split in two since EncoderInfo needed to start + // CPU adaptation with the correct settings should be polled after + // encoder_->InitEncode(). if (pending_encoder_creation_) { - pending_encoder_creation_ = false; if (encoder_) { video_sender_.RegisterExternalEncoder(nullptr, false); } @@ -597,10 +599,6 @@ void VideoStreamEncoder::ReconfigureEncoder() { settings_.encoder_factory->QueryVideoEncoder( encoder_config_.video_format); - overuse_detector_->StopCheckForOveruse(); - overuse_detector_->StartCheckForOveruse( - GetCpuOveruseOptions(settings_, info.is_hardware_accelerated), this); - video_sender_.RegisterExternalEncoder(encoder_.get(), info.has_internal_source); } @@ -614,6 +612,15 @@ void VideoStreamEncoder::ReconfigureEncoder() { rate_allocator_.reset(); } + if (pending_encoder_creation_) { + overuse_detector_->StopCheckForOveruse(); + overuse_detector_->StartCheckForOveruse( + GetCpuOveruseOptions( + settings_, encoder_->GetEncoderInfo().is_hardware_accelerated), + this); + pending_encoder_creation_ = false; + } + int num_layers; if (codec.codecType == kVideoCodecVP8) { num_layers = codec.VP8()->numberOfTemporalLayers; diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc index e83b04e661..28e06c7980 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -525,11 +525,14 @@ class VideoStreamEncoderTest : public ::testing::Test { } VideoEncoder::EncoderInfo GetEncoderInfo() const override { - EncoderInfo info; rtc::CritScope lock(&local_crit_sect_); - if (quality_scaling_) { - info.scaling_settings = - VideoEncoder::ScalingSettings(1, 2, kMinPixelsPerFrame); + EncoderInfo info; + if (initialized_) { + if (quality_scaling_) { + info.scaling_settings = + VideoEncoder::ScalingSettings(1, 2, kMinPixelsPerFrame); + } + info.is_hardware_accelerated = is_hardware_accelerated_; } return info; } @@ -548,6 +551,11 @@ class VideoStreamEncoderTest : public ::testing::Test { quality_scaling_ = b; } + void SetIsHardwareAccelerated(bool is_hardware_accelerated) { + rtc::CritScope lock(&local_crit_sect_); + is_hardware_accelerated_ = is_hardware_accelerated; + } + void ForceInitEncodeFailure(bool force_failure) { rtc::CritScope lock(&local_crit_sect_); force_init_encode_failed_ = force_failure; @@ -606,6 +614,8 @@ class VideoStreamEncoderTest : public ::testing::Test { } if (force_init_encode_failed_) return -1; + + initialized_ = true; return res; } @@ -629,6 +639,7 @@ class VideoStreamEncoderTest : public ::testing::Test { } rtc::CriticalSection local_crit_sect_; + bool initialized_ RTC_GUARDED_BY(local_crit_sect_) = false; bool block_next_encode_ RTC_GUARDED_BY(local_crit_sect_) = false; rtc::Event continue_encode_event_; uint32_t timestamp_ RTC_GUARDED_BY(local_crit_sect_) = 0; @@ -636,6 +647,7 @@ class VideoStreamEncoderTest : public ::testing::Test { int last_input_width_ RTC_GUARDED_BY(local_crit_sect_) = 0; int last_input_height_ RTC_GUARDED_BY(local_crit_sect_) = 0; bool quality_scaling_ RTC_GUARDED_BY(local_crit_sect_) = true; + bool is_hardware_accelerated_ RTC_GUARDED_BY(local_crit_sect_) = false; std::vector> allocated_temporal_layers_ RTC_GUARDED_BY(local_crit_sect_); bool force_init_encode_failed_ RTC_GUARDED_BY(local_crit_sect_) = false; @@ -3239,7 +3251,7 @@ TEST_F(VideoStreamEncoderTest, CpuOveruseOptions hardware_options; hardware_options.low_encode_usage_threshold_percent = 150; hardware_options.high_encode_usage_threshold_percent = 200; - encoder_factory_.SetIsHardwareAccelerated(true); + fake_encoder_.SetIsHardwareAccelerated(true); video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0); video_source_.IncomingCapturedFrame(