Add ability to specify if rate controller of video encoder is trusted.
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 <sprang@webrtc.org>
Reviewed-by: Niels Moller <nisse@webrtc.org>
Reviewed-by: Rasmus Brandt <brandtr@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#25570}diff --git a/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc b/modules/video_coding/codecs/vp8/libvpx_vp8_encoder.cc
index 5d5e20b..be5242c 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(std::unique_ptr<LibvpxInterface> 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 @@
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 8a8a12c..df2dbce 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 @@
const absl::optional<std::vector<CpuSpeedExperiment::Config>>
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 da9633d..996c733 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 @@
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 @@
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 3418fe3..e647f3f 100644
--- a/modules/video_coding/codecs/vp9/vp9_impl.h
+++ b/modules/video_coding/codecs/vp9/vp9_impl.h
@@ -117,6 +117,7 @@
bool is_svc_;
InterLayerPredMode inter_layer_pred_;
bool external_ref_control_;
+ const bool trusted_rate_controller_;
std::vector<FramerateController> framerate_controller_;
diff --git a/modules/video_coding/generic_encoder.cc b/modules/video_coding/generic_encoder.cc
index fc08c3e..d5cc0a8 100644
--- a/modules/video_coding/generic_encoder.cc
+++ b/modules/video_coding/generic_encoder.cc
@@ -152,9 +152,9 @@
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 151e93e..2f841b3 100644
--- a/modules/video_coding/generic_encoder.h
+++ b/modules/video_coding/generic_encoder.h
@@ -146,7 +146,7 @@
int32_t RequestFrame(const std::vector<FrameType>& 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 593c0b1..ac88166 100644
--- a/modules/video_coding/video_coding_impl.h
+++ b/modules/video_coding/video_coding_impl.h
@@ -112,7 +112,18 @@
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 4c1e243..f95d7fc 100644
--- a/modules/video_coding/video_sender.cc
+++ b/modules/video_coding/video_sender.cc
@@ -40,7 +40,8 @@
_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 @@
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 @@
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 @@
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::EnableFrameDropper(bool enable) {
rtc::CritScope lock(&encoder_crit_);
- frame_dropper_enabled_ = enable;
+ frame_dropper_requested_ = enable;
_mediaOpt.EnableFrameDropper(enable);
return VCM_OK;
}