From d3438aa1ed938746bd021ebcbe87890bea8f8150 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Thu, 8 Nov 2018 16:56:43 +0100 Subject: [PATCH] Add ability to specify if rate controller of video encoder is trusted. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit If rate controller is trusted, we disable the frame dropper in the media optimization module. This is a re-land of https://webrtc-review.googlesource.com/c/src/+/105020 Bug: webrtc:9890 Change-Id: I418e47a43a1a98cb2fd5295c03360b954f2288f2 Reviewed-on: https://webrtc-review.googlesource.com/c/109141 Commit-Queue: Erik SprÃ¥ng Reviewed-by: Niels Moller Reviewed-by: Rasmus Brandt Cr-Commit-Position: refs/heads/master@{#25570} --- api/video_codecs/test/BUILD.gn | 1 + ...oder_software_fallback_wrapper_unittest.cc | 79 +++++++++++++++++++ api/video_codecs/video_encoder.cc | 3 +- api/video_codecs/video_encoder.h | 17 ++++ media/engine/simulcast_encoder_adapter.cc | 7 ++ .../simulcast_encoder_adapter_unittest.cc | 51 ++++++++++++ .../vp8_encoder_simulcast_proxy_unittest.cc | 50 +++++++++--- .../codecs/vp8/libvpx_vp8_encoder.cc | 7 ++ .../codecs/vp8/libvpx_vp8_encoder.h | 1 + modules/video_coding/codecs/vp9/vp9_impl.cc | 12 ++- modules/video_coding/codecs/vp9/vp9_impl.h | 1 + modules/video_coding/generic_encoder.cc | 4 +- modules/video_coding/generic_encoder.h | 2 +- modules/video_coding/video_coding_impl.h | 13 ++- modules/video_coding/video_sender.cc | 28 ++++--- video/full_stack_tests.cc | 58 ++++++++++++-- 16 files changed, 299 insertions(+), 35 deletions(-) diff --git a/api/video_codecs/test/BUILD.gn b/api/video_codecs/test/BUILD.gn index fa191625a1..6f9dcaa2d8 100644 --- a/api/video_codecs/test/BUILD.gn +++ b/api/video_codecs/test/BUILD.gn @@ -21,6 +21,7 @@ if (rtc_include_tests) { "..:builtin_video_encoder_factory", "..:rtc_software_fallback_wrappers", "..:video_codecs_api", + "../..:mock_video_encoder", "../../../modules/video_coding:video_codec_interface", "../../../modules/video_coding:video_coding_utility", "../../../modules/video_coding:webrtc_vp8", 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 555914ea23..b374f3d1e1 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 @@ -10,6 +10,7 @@ #include +#include "api/test/mock_video_encoder.h" #include "api/video/i420_buffer.h" #include "api/video/video_bitrate_allocation.h" #include "api/video_codecs/video_encoder_software_fallback_wrapper.h" @@ -21,9 +22,12 @@ #include "rtc_base/checks.h" #include "rtc_base/fakeclock.h" #include "test/field_trial.h" +#include "test/gmock.h" #include "test/gtest.h" namespace webrtc { +using ::testing::Return; + namespace { const int kWidth = 320; const int kHeight = 240; @@ -33,6 +37,12 @@ const size_t kMaxPayloadSize = 800; const int kDefaultMinPixelsPerFrame = 320 * 180; const int kLowThreshold = 10; const int kHighThreshold = 20; + +VideoEncoder::EncoderInfo GetEncoderInfo(bool trusted_rate_controller) { + VideoEncoder::EncoderInfo info; + info.has_trusted_rate_controller = trusted_rate_controller; + return info; +} } // namespace class VideoEncoderSoftwareFallbackWrapperTest : public ::testing::Test { @@ -512,4 +522,73 @@ TEST_F(ForcedFallbackTestEnabled, ScalingDisabledIfResizeOff) { EXPECT_FALSE(settings.thresholds.has_value()); } +TEST(SoftwareFallbackEncoderTest, BothRateControllersNotTrusted) { + auto* sw_encoder = new testing::NiceMock(); + auto* hw_encoder = new testing::NiceMock(); + + EXPECT_CALL(*sw_encoder, GetEncoderInfo()) + .WillRepeatedly(Return(GetEncoderInfo(false))); + EXPECT_CALL(*hw_encoder, GetEncoderInfo()) + .WillRepeatedly(Return(GetEncoderInfo(false))); + + std::unique_ptr wrapper = + CreateVideoEncoderSoftwareFallbackWrapper( + std::unique_ptr(sw_encoder), + std::unique_ptr(hw_encoder)); + EXPECT_FALSE(wrapper->GetEncoderInfo().has_trusted_rate_controller); +} + +TEST(SoftwareFallbackEncoderTest, SwRateControllerTrusted) { + auto* sw_encoder = new testing::NiceMock(); + auto* hw_encoder = new testing::NiceMock(); + EXPECT_CALL(*sw_encoder, GetEncoderInfo()) + .WillRepeatedly(Return(GetEncoderInfo(true))); + EXPECT_CALL(*hw_encoder, GetEncoderInfo()) + .WillRepeatedly(Return(GetEncoderInfo(false))); + + std::unique_ptr wrapper = + CreateVideoEncoderSoftwareFallbackWrapper( + std::unique_ptr(sw_encoder), + std::unique_ptr(hw_encoder)); + EXPECT_FALSE(wrapper->GetEncoderInfo().has_trusted_rate_controller); +} + +TEST(SoftwareFallbackEncoderTest, HwRateControllerTrusted) { + auto* sw_encoder = new testing::NiceMock(); + auto* hw_encoder = new testing::NiceMock(); + EXPECT_CALL(*sw_encoder, GetEncoderInfo()) + .WillRepeatedly(Return(GetEncoderInfo(false))); + EXPECT_CALL(*hw_encoder, GetEncoderInfo()) + .WillRepeatedly(Return(GetEncoderInfo(true))); + + std::unique_ptr wrapper = + CreateVideoEncoderSoftwareFallbackWrapper( + std::unique_ptr(sw_encoder), + std::unique_ptr(hw_encoder)); + EXPECT_TRUE(wrapper->GetEncoderInfo().has_trusted_rate_controller); + + // Trigger fallback to software. + EXPECT_CALL(*hw_encoder, Encode) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_FALLBACK_SOFTWARE)); + VideoFrame frame = VideoFrame::Builder().build(); + wrapper->Encode(frame, nullptr, nullptr); + + EXPECT_FALSE(wrapper->GetEncoderInfo().has_trusted_rate_controller); +} + +TEST(SoftwareFallbackEncoderTest, BothRateControllersTrusted) { + auto* sw_encoder = new testing::NiceMock(); + auto* hw_encoder = new testing::NiceMock(); + EXPECT_CALL(*sw_encoder, GetEncoderInfo()) + .WillRepeatedly(Return(GetEncoderInfo(true))); + EXPECT_CALL(*hw_encoder, GetEncoderInfo()) + .WillRepeatedly(Return(GetEncoderInfo(true))); + + std::unique_ptr wrapper = + CreateVideoEncoderSoftwareFallbackWrapper( + std::unique_ptr(sw_encoder), + std::unique_ptr(hw_encoder)); + EXPECT_TRUE(wrapper->GetEncoderInfo().has_trusted_rate_controller); +} + } // namespace webrtc diff --git a/api/video_codecs/video_encoder.cc b/api/video_codecs/video_encoder.cc index 0963886894..363edbfd2f 100644 --- a/api/video_codecs/video_encoder.cc +++ b/api/video_codecs/video_encoder.cc @@ -86,7 +86,8 @@ constexpr VideoEncoder::ScalingSettings::KOff VideoEncoder::EncoderInfo::EncoderInfo() : scaling_settings(VideoEncoder::ScalingSettings::kOff), supports_native_handle(false), - implementation_name("unknown") {} + implementation_name("unknown"), + has_trusted_rate_controller(false) {} VideoEncoder::EncoderInfo::~EncoderInfo() = default; diff --git a/api/video_codecs/video_encoder.h b/api/video_codecs/video_encoder.h index f8f0313c3a..d881f8ac64 100644 --- a/api/video_codecs/video_encoder.h +++ b/api/video_codecs/video_encoder.h @@ -132,6 +132,19 @@ class RTC_EXPORT VideoEncoder { // The name of this particular encoder implementation, e.g. "libvpx". std::string implementation_name; + + // If this field is true, the encoder rate controller must perform + // well even in difficult situations, and produce close to the specified + // target bitrate seen over a reasonable time window, drop frames if + // necessary in order to keep the rate correct, and react quickly to + // changing bitrate targets. If this method returns true, we disable the + // frame dropper in the media optimization module and rely entirely on the + // encoder to produce media at a bitrate that closely matches the target. + // Any overshooting may result in delay buildup. If this method returns + // false (default behavior), the media opt frame dropper will drop input + // frames if it suspect encoder misbehavior. Misbehavior is common, + // especially in hardware codecs. Disable media opt at your own risk. + bool has_trusted_rate_controller; }; static VideoCodecVP8 GetDefaultVp8Settings(); @@ -220,6 +233,10 @@ class RTC_EXPORT VideoEncoder { virtual bool SupportsNativeHandle() const; virtual const char* ImplementationName() const; + // Returns meta-data about the encoder, such as implementation name. + // The output of this method may change during runtime. For instance if a + // hardware encoder fails, it may fall back to doing software encoding using + // an implementation with different characteristics. virtual EncoderInfo GetEncoderInfo() const; }; } // namespace webrtc diff --git a/media/engine/simulcast_encoder_adapter.cc b/media/engine/simulcast_encoder_adapter.cc index e4d0b0bbe9..6241ff9d36 100644 --- a/media/engine/simulcast_encoder_adapter.cc +++ b/media/engine/simulcast_encoder_adapter.cc @@ -249,6 +249,7 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, Release(); return ret; } + std::unique_ptr callback( new AdapterEncodedImageCallback(this, i)); encoder->RegisterEncodeCompleteCallback(callback.get()); @@ -275,6 +276,8 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, encoder_info_.supports_native_handle = encoder_impl_info.supports_native_handle; + encoder_info_.has_trusted_rate_controller = + encoder_impl_info.has_trusted_rate_controller; } else { encoder_info_.implementation_name += ", "; encoder_info_.implementation_name += @@ -283,6 +286,10 @@ int SimulcastEncoderAdapter::InitEncode(const VideoCodec* inst, // Native handle supported only if all encoders supports it. encoder_info_.supports_native_handle &= encoder_impl_info.supports_native_handle; + + // Trusted rate controller only if all encoders have it. + encoder_info_.has_trusted_rate_controller &= + encoder_impl_info.has_trusted_rate_controller; } } } diff --git a/media/engine/simulcast_encoder_adapter_unittest.cc b/media/engine/simulcast_encoder_adapter_unittest.cc index 4854725a70..59541755fa 100644 --- a/media/engine/simulcast_encoder_adapter_unittest.cc +++ b/media/engine/simulcast_encoder_adapter_unittest.cc @@ -216,6 +216,7 @@ class MockVideoEncoder : public VideoEncoder { info.supports_native_handle = supports_native_handle_; info.implementation_name = implementation_name_; info.scaling_settings = scaling_settings_; + info.has_trusted_rate_controller = has_trusted_rate_controller_; return info; } @@ -250,6 +251,10 @@ class MockVideoEncoder : public VideoEncoder { scaling_settings_ = settings; } + void set_has_trusted_rate_controller(bool trusted) { + has_trusted_rate_controller_ = trusted; + } + VideoBitrateAllocation last_set_bitrate() const { return last_set_bitrate_; } private: @@ -257,6 +262,7 @@ class MockVideoEncoder : public VideoEncoder { bool supports_native_handle_ = false; std::string implementation_name_ = "unknown"; VideoEncoder::ScalingSettings scaling_settings_; + bool has_trusted_rate_controller_ = false; int32_t init_encode_return_value_ = 0; VideoBitrateAllocation last_set_bitrate_; @@ -954,5 +960,50 @@ TEST_F(TestSimulcastEncoderAdapterFake, ActivatesCorrectStreamsInInitEncode) { frame_types.resize(3, kVideoFrameKey); EXPECT_EQ(0, adapter_->Encode(input_frame, nullptr, &frame_types)); } + +TEST_F(TestSimulcastEncoderAdapterFake, TrustedRateControl) { + // Set up common settings for three streams. + SimulcastTestFixtureImpl::DefaultSettings( + &codec_, static_cast(kTestTemporalLayerProfile), + kVideoCodecVP8); + rate_allocator_.reset(new SimulcastRateAllocator(codec_)); + adapter_->RegisterEncodeCompleteCallback(this); + + // Only enough start bitrate for the lowest stream. + ASSERT_EQ(3u, codec_.numberOfSimulcastStreams); + codec_.startBitrate = codec_.simulcastStream[0].targetBitrate + + codec_.simulcastStream[1].minBitrate - 1; + + // Input data. + rtc::scoped_refptr buffer(I420Buffer::Create(1280, 720)); + VideoFrame input_frame(buffer, 100, 1000, kVideoRotation_180); + + // No encoder trusted, so simulcast adapter should not be either. + EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); + EXPECT_FALSE(adapter_->GetEncoderInfo().has_trusted_rate_controller); + + // Encode with three streams. + std::vector original_encoders = + helper_->factory()->encoders(); + + // All encoders are trusted, so simulcast adapter should be too. + original_encoders[0]->set_has_trusted_rate_controller(true); + original_encoders[1]->set_has_trusted_rate_controller(true); + original_encoders[2]->set_has_trusted_rate_controller(true); + EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); + EXPECT_TRUE(adapter_->GetEncoderInfo().has_trusted_rate_controller); + + // One encoder not trusted, so simulcast adapter should not be either. + original_encoders[2]->set_has_trusted_rate_controller(false); + EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); + EXPECT_FALSE(adapter_->GetEncoderInfo().has_trusted_rate_controller); + + // No encoder trusted, so simulcast adapter should not be either. + original_encoders[0]->set_has_trusted_rate_controller(false); + original_encoders[1]->set_has_trusted_rate_controller(false); + EXPECT_EQ(0, adapter_->InitEncode(&codec_, 1, 1200)); + EXPECT_FALSE(adapter_->GetEncoderInfo().has_trusted_rate_controller); +} + } // namespace test } // namespace webrtc diff --git a/media/engine/vp8_encoder_simulcast_proxy_unittest.cc b/media/engine/vp8_encoder_simulcast_proxy_unittest.cc index b89c6573a3..d1b26bbe93 100644 --- a/media/engine/vp8_encoder_simulcast_proxy_unittest.cc +++ b/media/engine/vp8_encoder_simulcast_proxy_unittest.cc @@ -51,7 +51,7 @@ class MockEncoder : public VideoEncoder { const CodecSpecificInfo* codecSpecificInfo, const std::vector* frame_types) /* override */); - MOCK_CONST_METHOD0(ImplementationName, const char*()); + MOCK_CONST_METHOD0(GetEncoderInfo, VideoEncoder::EncoderInfo(void)); }; TEST(VP8EncoderSimulcastProxy, ChoosesCorrectImplementation) { @@ -91,8 +91,10 @@ TEST(VP8EncoderSimulcastProxy, ChoosesCorrectImplementation) { EXPECT_CALL(*mock_encoder, InitEncode(_, _, _)) .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); - EXPECT_CALL(*mock_encoder, ImplementationName()) - .WillRepeatedly(Return(kImplementationName.c_str())); + VideoEncoder::EncoderInfo encoder_info; + encoder_info.implementation_name = kImplementationName; + EXPECT_CALL(*mock_encoder, GetEncoderInfo()) + .WillRepeatedly(Return(encoder_info)); EXPECT_CALL(simulcast_factory, CreateVideoEncoderProxy(_)) .Times(1) @@ -114,23 +116,23 @@ TEST(VP8EncoderSimulcastProxy, ChoosesCorrectImplementation) { EXPECT_CALL(*mock_encoder1, InitEncode(_, _, _)) .WillOnce( Return(WEBRTC_VIDEO_CODEC_ERR_SIMULCAST_PARAMETERS_NOT_SUPPORTED)); - EXPECT_CALL(*mock_encoder1, ImplementationName()) - .WillRepeatedly(Return(kImplementationName.c_str())); + EXPECT_CALL(*mock_encoder1, GetEncoderInfo()) + .WillRepeatedly(Return(encoder_info)); EXPECT_CALL(*mock_encoder2, InitEncode(_, _, _)) .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); - EXPECT_CALL(*mock_encoder2, ImplementationName()) - .WillRepeatedly(Return(kImplementationName.c_str())); + EXPECT_CALL(*mock_encoder2, GetEncoderInfo()) + .WillRepeatedly(Return(encoder_info)); EXPECT_CALL(*mock_encoder3, InitEncode(_, _, _)) .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); - EXPECT_CALL(*mock_encoder3, ImplementationName()) - .WillRepeatedly(Return(kImplementationName.c_str())); + EXPECT_CALL(*mock_encoder3, GetEncoderInfo()) + .WillRepeatedly(Return(encoder_info)); EXPECT_CALL(*mock_encoder4, InitEncode(_, _, _)) .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); - EXPECT_CALL(*mock_encoder4, ImplementationName()) - .WillRepeatedly(Return(kImplementationName.c_str())); + EXPECT_CALL(*mock_encoder4, GetEncoderInfo()) + .WillRepeatedly(Return(encoder_info)); EXPECT_CALL(nonsimulcast_factory, CreateVideoEncoderProxy(_)) .Times(4) @@ -151,5 +153,31 @@ TEST(VP8EncoderSimulcastProxy, ChoosesCorrectImplementation) { simulcast_disabled_proxy.Release(); } +TEST(VP8EncoderSimulcastProxy, ForwardsTrustedSetting) { + NiceMock* mock_encoder = new NiceMock(); + NiceMock simulcast_factory; + + EXPECT_CALL(*mock_encoder, InitEncode(_, _, _)) + .WillOnce(Return(WEBRTC_VIDEO_CODEC_OK)); + + EXPECT_CALL(simulcast_factory, CreateVideoEncoderProxy(_)) + .Times(1) + .WillOnce(Return(mock_encoder)); + + VP8EncoderSimulcastProxy simulcast_enabled_proxy(&simulcast_factory, + SdpVideoFormat("VP8")); + VideoCodec codec_settings; + webrtc::test::CodecSettings(kVideoCodecVP8, &codec_settings); + EXPECT_EQ(WEBRTC_VIDEO_CODEC_OK, + simulcast_enabled_proxy.InitEncode(&codec_settings, 4, 1200)); + + VideoEncoder::EncoderInfo info; + info.has_trusted_rate_controller = true; + EXPECT_CALL(*mock_encoder, GetEncoderInfo()).WillRepeatedly(Return(info)); + + EXPECT_TRUE( + simulcast_enabled_proxy.GetEncoderInfo().has_trusted_rate_controller); +} + } // namespace testing } // namespace webrtc diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc index 5d5e20b9e2..be5242c1b9 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc @@ -23,11 +23,15 @@ #include "rtc_base/checks.h" #include "rtc_base/timeutils.h" #include "rtc_base/trace_event.h" +#include "system_wrappers/include/field_trial.h" #include "third_party/libyuv/include/libyuv/convert.h" #include "third_party/libyuv/include/libyuv/scale.h" namespace webrtc { namespace { +const char kVp8TrustedRateControllerFieldTrial[] = + "WebRTC-LibvpxVp8TrustedRateController"; + // QP is obtained from VP8-bitstream for HW, so the QP corresponds to the // bitstream range of [0, 127] and not the user-level range of [0,63]. constexpr int kLowVp8QpThreshold = 29; @@ -143,6 +147,8 @@ LibvpxVp8Encoder::LibvpxVp8Encoder() LibvpxVp8Encoder::LibvpxVp8Encoder(std::unique_ptr interface) : libvpx_(std::move(interface)), experimental_cpu_speed_config_arm_(CpuSpeedExperiment::GetConfigs()), + trusted_rate_controller_( + field_trial::IsEnabled(kVp8TrustedRateControllerFieldTrial)), encoded_complete_callback_(nullptr), inited_(false), timestamp_(0), @@ -916,6 +922,7 @@ VideoEncoder::EncoderInfo LibvpxVp8Encoder::GetEncoderInfo() const { EncoderInfo info; info.supports_native_handle = false; info.implementation_name = "libvpx"; + info.has_trusted_rate_controller = trusted_rate_controller_; const bool enable_scaling = encoders_.size() == 1 && configurations_[0].rc_dropframe_thresh > 0 && diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h index 8a8a12c7ec..df2dbcee59 100644 --- a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h +++ b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.h @@ -86,6 +86,7 @@ class LibvpxVp8Encoder : public VideoEncoder { const absl::optional> experimental_cpu_speed_config_arm_; + const bool trusted_rate_controller_; EncodedImageCallback* encoded_complete_callback_; VideoCodec codec_; diff --git a/modules/video_coding/codecs/vp9/vp9_impl.cc b/modules/video_coding/codecs/vp9/vp9_impl.cc index da9633d2f1..996c733f71 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.cc +++ b/modules/video_coding/codecs/vp9/vp9_impl.cc @@ -37,6 +37,9 @@ namespace webrtc { namespace { +const char kVp9TrustedRateControllerFieldTrial[] = + "WebRTC-LibvpxVp9TrustedRateController"; + // Maps from gof_idx to encoder internal reference frame buffer index. These // maps work for 1,2 and 3 temporal layers with GOF length of 1,2 and 4 frames. uint8_t kRefBufIdx[4] = {0, 0, 0, 1}; @@ -162,12 +165,14 @@ VP9EncoderImpl::VP9EncoderImpl(const cricket::VideoCodec& codec) num_temporal_layers_(0), num_spatial_layers_(0), num_active_spatial_layers_(0), - layer_deactivation_requires_key_frame_(webrtc::field_trial::IsEnabled( - "WebRTC-Vp9IssueKeyFrameOnLayerDeactivation")), + layer_deactivation_requires_key_frame_( + field_trial::IsEnabled("WebRTC-Vp9IssueKeyFrameOnLayerDeactivation")), is_svc_(false), inter_layer_pred_(InterLayerPredMode::kOn), external_ref_control_( - webrtc::field_trial::IsEnabled("WebRTC-Vp9ExternalRefCtrl")), + field_trial::IsEnabled("WebRTC-Vp9ExternalRefCtrl")), + trusted_rate_controller_( + field_trial::IsEnabled(kVp9TrustedRateControllerFieldTrial)), is_flexible_mode_(false) { memset(&codec_, 0, sizeof(codec_)); memset(&svc_params_, 0, sizeof(vpx_svc_extra_cfg_t)); @@ -1250,6 +1255,7 @@ VideoEncoder::EncoderInfo VP9EncoderImpl::GetEncoderInfo() const { info.supports_native_handle = false; info.implementation_name = "libvpx"; info.scaling_settings = VideoEncoder::ScalingSettings::kOff; + info.has_trusted_rate_controller = trusted_rate_controller_; return info; } diff --git a/modules/video_coding/codecs/vp9/vp9_impl.h b/modules/video_coding/codecs/vp9/vp9_impl.h index 3418fe37a6..e647f3f074 100644 --- a/modules/video_coding/codecs/vp9/vp9_impl.h +++ b/modules/video_coding/codecs/vp9/vp9_impl.h @@ -117,6 +117,7 @@ class VP9EncoderImpl : public VP9Encoder { bool is_svc_; InterLayerPredMode inter_layer_pred_; bool external_ref_control_; + const bool trusted_rate_controller_; std::vector framerate_controller_; diff --git a/modules/video_coding/generic_encoder.cc b/modules/video_coding/generic_encoder.cc index fc08c3ea1d..d5cc0a8ba0 100644 --- a/modules/video_coding/generic_encoder.cc +++ b/modules/video_coding/generic_encoder.cc @@ -152,9 +152,9 @@ bool VCMGenericEncoder::InternalSource() const { return internal_source_; } -bool VCMGenericEncoder::SupportsNativeHandle() const { +VideoEncoder::EncoderInfo VCMGenericEncoder::GetEncoderInfo() const { RTC_DCHECK_RUNS_SERIALIZED(&race_checker_); - return encoder_->GetEncoderInfo().supports_native_handle; + return encoder_->GetEncoderInfo(); } VCMEncodedFrameCallback::VCMEncodedFrameCallback( diff --git a/modules/video_coding/generic_encoder.h b/modules/video_coding/generic_encoder.h index 151e93e497..2f841b34dd 100644 --- a/modules/video_coding/generic_encoder.h +++ b/modules/video_coding/generic_encoder.h @@ -146,7 +146,7 @@ class VCMGenericEncoder { int32_t RequestFrame(const std::vector& frame_types); bool InternalSource() const; - bool SupportsNativeHandle() const; + VideoEncoder::EncoderInfo GetEncoderInfo() const; private: rtc::RaceChecker race_checker_; diff --git a/modules/video_coding/video_coding_impl.h b/modules/video_coding/video_coding_impl.h index 593c0b1ec3..ac88166394 100644 --- a/modules/video_coding/video_coding_impl.h +++ b/modules/video_coding/video_coding_impl.h @@ -112,7 +112,18 @@ class VideoSender { VCMEncodedFrameCallback _encodedFrameCallback RTC_GUARDED_BY(encoder_crit_); EncodedImageCallback* const post_encode_callback_; VCMEncoderDataBase _codecDataBase RTC_GUARDED_BY(encoder_crit_); - bool frame_dropper_enabled_ RTC_GUARDED_BY(encoder_crit_); + + // |frame_dropper_requested_| specifies if the user of this class has + // requested frame dropping to be enabled, via EnableFrameDropper(). + // Depending on video encoder configuration, this setting may be overridden + // and the frame dropper be force disabled. If so, + // |force_disable_frame_dropper_| will be set to true. + // If frame dropper is requested, and is not force disabled, frame dropping + // might still be disabled if VideoEncoder::GetEncoderInfo() indicates that + // the encoder has a trusted rate controller. This is determined on a + // per-frame basis, as the encoder behavior might dynamically change. + bool frame_dropper_requested_ RTC_GUARDED_BY(encoder_crit_); + bool force_disable_frame_dropper_ RTC_GUARDED_BY(encoder_crit_); // Must be accessed on the construction thread of VideoSender. VideoCodec current_codec_; diff --git a/modules/video_coding/video_sender.cc b/modules/video_coding/video_sender.cc index 4c1e243612..f95d7fcf15 100644 --- a/modules/video_coding/video_sender.cc +++ b/modules/video_coding/video_sender.cc @@ -40,7 +40,8 @@ VideoSender::VideoSender(Clock* clock, _encodedFrameCallback(post_encode_callback, &_mediaOpt), post_encode_callback_(post_encode_callback), _codecDataBase(&_encodedFrameCallback), - frame_dropper_enabled_(true), + frame_dropper_requested_(true), + force_disable_frame_dropper_(false), current_codec_(), encoder_params_({VideoBitrateAllocation(), 0, 0, 0}), encoder_has_internal_source_(false), @@ -96,15 +97,12 @@ int32_t VideoSender::RegisterSendCodec(const VideoCodec* sendCodec, numLayers = 1; } - // If we have screensharing and we have layers, we disable frame dropper. - const bool disable_frame_dropper = + // Force-disable frame dropper if either: + // * We have screensharing with layers. + // * "WebRTC-FrameDropper" field trial is "Disabled". + force_disable_frame_dropper_ = field_trial::IsDisabled(kFrameDropperFieldTrial) || (numLayers > 1 && sendCodec->mode == VideoCodecMode::kScreensharing); - if (disable_frame_dropper) { - _mediaOpt.EnableFrameDropper(false); - } else if (frame_dropper_enabled_) { - _mediaOpt.EnableFrameDropper(true); - } { rtc::CritScope cs(¶ms_crit_); @@ -262,6 +260,16 @@ int32_t VideoSender::AddVideoFrame(const VideoFrame& videoFrame, if (_encoder == nullptr) return VCM_UNINITIALIZED; SetEncoderParameters(encoder_params, encoder_has_internal_source); + + const VideoEncoder::EncoderInfo encoder_info = _encoder->GetEncoderInfo(); + + // Frame dropping is enabled iff frame dropping has been requested, and + // frame dropping is not force-disabled, and rate controller is not trusted. + const bool frame_dropping_enabled = frame_dropper_requested_ && + !force_disable_frame_dropper_ && + !encoder_info.has_trusted_rate_controller; + _mediaOpt.EnableFrameDropper(frame_dropping_enabled); + if (_mediaOpt.DropFrame()) { RTC_LOG(LS_VERBOSE) << "Drop Frame " << "target bitrate " @@ -287,7 +295,7 @@ int32_t VideoSender::AddVideoFrame(const VideoFrame& videoFrame, const bool is_buffer_type_supported = buffer_type == VideoFrameBuffer::Type::kI420 || (buffer_type == VideoFrameBuffer::Type::kNative && - _encoder->SupportsNativeHandle()); + encoder_info.supports_native_handle); if (!is_buffer_type_supported) { // This module only supports software encoding. // TODO(pbos): Offload conversion from the encoder thread. @@ -355,7 +363,7 @@ int32_t VideoSender::IntraFrameRequest(size_t stream_index) { int32_t VideoSender::EnableFrameDropper(bool enable) { rtc::CritScope lock(&encoder_crit_); - frame_dropper_enabled_ = enable; + frame_dropper_requested_ = enable; _mediaOpt.EnableFrameDropper(enable); return VCM_OK; } diff --git a/video/full_stack_tests.cc b/video/full_stack_tests.cc index 347fbd1412..3f7eee523a 100644 --- a/video/full_stack_tests.cc +++ b/video/full_stack_tests.cc @@ -52,6 +52,8 @@ namespace { static const int kFullStackTestDurationSecs = 45; const char kPacerPushBackExperiment[] = "WebRTC-PacerPushbackExperiment/Enabled/"; +const char kVp8TrustedRateControllerFieldTrial[] = + "WebRTC-LibvpxVp8TrustedRateController/Enabled/"; struct ParamsWithLogging : public VideoQualityTest::Params { public: @@ -235,6 +237,25 @@ TEST_P(GenericDescriptorTest, ForemanCif30kbpsWithoutPacketLoss) { fixture->RunWithAnalyzer(foreman_cif); } +// TODO(webrtc:9722): Remove when experiment is cleaned up. +TEST_P(GenericDescriptorTest, + ForemanCif30kbpsWithoutPacketLossTrustedRateControl) { + test::ScopedFieldTrials override_field_trials( + AppendFieldTrials(kVp8TrustedRateControllerFieldTrial)); + auto fixture = CreateVideoQualityTestFixture(); + + ParamsWithLogging foreman_cif; + foreman_cif.call.send_side_bwe = true; + foreman_cif.video[0] = {true, 352, 288, 10, 30000, 30000, 30000, + false, "VP8", 1, 0, 0, false, false, + false, "foreman_cif"}; + foreman_cif.analyzer = { + GetTestName("foreman_cif_30kbps_net_delay_0_0_plr_0_trusted_rate_ctrl"), + 0.0, 0.0, kFullStackTestDurationSecs}; + foreman_cif.call.generic_descriptor = GenericDescriptorEnabled(); + fixture->RunWithAnalyzer(foreman_cif); +} + // Link capacity below default start rate. Automatic down scaling enabled. TEST(FullStackTest, ForemanCifLink150kbpsWithoutPacketLoss) { auto fixture = CreateVideoQualityTestFixture(); @@ -366,9 +387,9 @@ TEST_P(GenericDescriptorTest, ForemanCifPlr5H264) { } TEST(FullStackTest, ForemanCifPlr5H264SpsPpsIdrIsKeyframe) { - auto fixture = CreateVideoQualityTestFixture(); test::ScopedFieldTrials override_field_trials( AppendFieldTrials("WebRTC-SpsPpsIdrIsH264Keyframe/Enabled/")); + auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging foreman_cif; foreman_cif.call.send_side_bwe = true; @@ -522,16 +543,21 @@ TEST(FullStackTest, ConferenceMotionHd2000kbps100msLimitedQueue) { fixture->RunWithAnalyzer(conf_motion_hd); } -TEST(FullStackTest, ConferenceMotionHd1TLModerateLimits) { +// TODO(webrtc:9722): Remove when experiment is cleaned up. +TEST(FullStackTest, ConferenceMotionHd1TLModerateLimitsWhitelistVp8) { + test::ScopedFieldTrials override_field_trials( + AppendFieldTrials(kVp8TrustedRateControllerFieldTrial)); auto fixture = CreateVideoQualityTestFixture(); + ParamsWithLogging conf_motion_hd; conf_motion_hd.call.send_side_bwe = true; conf_motion_hd.video[0] = { true, 1280, 720, 50, 30000, 3000000, 3000000, false, "VP8", 1, -1, 0, false, false, false, "ConferenceMotion_1280_720_50"}; - conf_motion_hd.analyzer = {"conference_motion_hd_1tl_moderate_limits", 0.0, - 0.0, kFullStackTestDurationSecs}; + conf_motion_hd.analyzer = { + "conference_motion_hd_1tl_moderate_limits_trusted_rate_ctrl", 0.0, 0.0, + kFullStackTestDurationSecs}; conf_motion_hd.config->queue_length_packets = 50; conf_motion_hd.config->loss_percent = 3; conf_motion_hd.config->queue_delay_ms = 100; @@ -593,9 +619,9 @@ TEST(FullStackTest, ConferenceMotionHd4TLModerateLimits) { } TEST(FullStackTest, ConferenceMotionHd3TLModerateLimitsAltTLPattern) { - auto fixture = CreateVideoQualityTestFixture(); test::ScopedFieldTrials field_trial( AppendFieldTrials("WebRTC-UseShortVP8TL3Pattern/Enabled/")); + auto fixture = CreateVideoQualityTestFixture(); ParamsWithLogging conf_motion_hd; conf_motion_hd.call.send_side_bwe = true; conf_motion_hd.video[0] = { @@ -672,8 +698,9 @@ const char kScreenshareSimulcastExperiment[] = "WebRTC-SimulcastScreenshare/Enabled/"; TEST(FullStackTest, ScreenshareSlidesVP8_3TL_Simulcast) { + test::ScopedFieldTrials field_trial( + AppendFieldTrials(kScreenshareSimulcastExperiment)); auto fixture = CreateVideoQualityTestFixture(); - test::ScopedFieldTrials field_trial(kScreenshareSimulcastExperiment); ParamsWithLogging screenshare; screenshare.call.send_side_bwe = true; screenshare.screenshare[0] = {true, false, 10}; @@ -921,6 +948,25 @@ TEST(FullStackTest, VP9KSVC_3SL_Medium_Network_Restricted) { simulcast.config->queue_delay_ms = 100; fixture->RunWithAnalyzer(simulcast); } + +// TODO(webrtc:9722): Remove when experiment is cleaned up. +TEST(FullStackTest, VP9KSVC_3SL_Medium_Network_Restricted_Trusted_Rate) { + webrtc::test::ScopedFieldTrials override_trials( + AppendFieldTrials("WebRTC-Vp9IssueKeyFrameOnLayerDeactivation/Enabled/" + "WebRTC-LibvpxVp9TrustedRateController/Enabled/")); + auto fixture = CreateVideoQualityTestFixture(); + ParamsWithLogging simulcast; + simulcast.call.send_side_bwe = true; + simulcast.video[0] = kSvcVp9Video; + simulcast.analyzer = {"vp9ksvc_3sl_medium_network_restricted_trusted_rate", + 0.0, 0.0, kFullStackTestDurationSecs}; + simulcast.ss[0] = { + std::vector(), 0, 3, -1, InterLayerPredMode::kOnKeyPic, + std::vector(), false}; + simulcast.config->link_capacity_kbps = 1000; + simulcast.config->queue_delay_ms = 100; + fixture->RunWithAnalyzer(simulcast); +} #endif // !defined(WEBRTC_MAC) #endif // !defined(RTC_DISABLE_VP9)