From 2e161c4dd6547f5f7accc7241c0c3d224d1d0cb6 Mon Sep 17 00:00:00 2001 From: Mirko Bonadei Date: Thu, 20 Feb 2020 08:45:01 +0000 Subject: [PATCH] Revert "Remove ResourceAdaptationModule::OnMaybeEncodeFrame" MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This reverts commit 93d9ae8a17f2e7b90641cbac28e740afc67d383a. Reason for revert: Perf regression. Original change's description: > Remove ResourceAdaptationModule::OnMaybeEncodeFrame > > We can react just as well at OnEncodeVideoFrame, which is the same > behaviour except after checking if the Encoder is paused and the frame > dropper. > > For the initial frame drop, the frame dropper is irrelevant as the frame > can not be dropped until we are accepting frames. If we didn't drop the > frame, the encoder can't be paused as the data rate > is over 0. > > For the quality rampup experiment, similar for encoder paused - we can't > rampup if we are paused anyways since the data rate needs to be non-zero. > If we are dropping frames we likely don't want to do quality rampup > anyways. > > Bug: webrtc:11222 > Change-Id: Ie3e09d9d8d509dc17ba7a1443cf4747f61c04f6a > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168601 > Reviewed-by: Henrik Boström > Reviewed-by: Ilya Nikolaevskiy > Commit-Queue: Evan Shrubsole > Cr-Commit-Position: refs/heads/master@{#30539} TBR=ilnik@webrtc.org,hbos@webrtc.org,eshr@google.com # Not skipping CQ checks because original CL landed > 1 day ago. No-Try: True Bug: webrtc:11222 Change-Id: Ifb2fc74eb7572568fb0ee1b53a09e4180f87b30c Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/168880 Commit-Queue: Mirko Bonadei Reviewed-by: Mirko Bonadei Reviewed-by: Ilya Nikolaevskiy Cr-Commit-Position: refs/heads/master@{#30568} --- call/adaptation/resource_adaptation_module_interface.h | 10 +++++++++- ...veruse_frame_detector_resource_adaptation_module.cc | 9 ++++++--- ...overuse_frame_detector_resource_adaptation_module.h | 1 + video/video_stream_encoder.cc | 1 + 4 files changed, 17 insertions(+), 4 deletions(-) diff --git a/call/adaptation/resource_adaptation_module_interface.h b/call/adaptation/resource_adaptation_module_interface.h index 7bfe24f4f4..623a414324 100644 --- a/call/adaptation/resource_adaptation_module_interface.h +++ b/call/adaptation/resource_adaptation_module_interface.h @@ -119,7 +119,15 @@ class ResourceAdaptationModuleInterface { // TODO(hbos): If we take frame rate into account perhaps it would be valid to // adapt down in frame rate as well. virtual void OnFrameDroppedDueToSize() = 0; - // 2.ii) An input frame is about to be encoded. It may have been cropped and + // 2.ii) If the frame will not be dropped due to size then signal that it may + // get encoded. However the frame is not guaranteed to be encoded right away + // or ever (for example if encoding is paused). + // TODO(eshr): Try replace OnMaybeEncodeFrame and merge behaviour into + // EncodeStarted. + // TODO(eshr): Try to merge OnFrame, OnFrameDroppedDueToSize, and + // OnMaybeEncode frame into one method. + virtual void OnMaybeEncodeFrame() = 0; + // 2.iii) An input frame is about to be encoded. It may have been cropped and // have different dimensions than what was observed at OnFrame(). Next // up: encoding completes or fails, see OnEncodeCompleted(). There is // currently no signal for encode failure. diff --git a/video/overuse_frame_detector_resource_adaptation_module.cc b/video/overuse_frame_detector_resource_adaptation_module.cc index 2a63c4e1b6..9cbd79c392 100644 --- a/video/overuse_frame_detector_resource_adaptation_module.cc +++ b/video/overuse_frame_detector_resource_adaptation_module.cc @@ -367,7 +367,7 @@ class OveruseFrameDetectorResourceAdaptationModule::InitialFrameDropper { void OnFrameDroppedDueToSize() { ++initial_framedrop_; } - void OnEncodeStarted() { initial_framedrop_ = kMaxInitialFramedrop; } + void OnMaybeEncodeFrame() { initial_framedrop_ = kMaxInitialFramedrop; } void OnQualityScalerSettingsUpdated() { if (quality_scaler_resource_->is_started()) { @@ -536,8 +536,6 @@ void OveruseFrameDetectorResourceAdaptationModule::OnFrameDroppedDueToSize() { void OveruseFrameDetectorResourceAdaptationModule::OnEncodeStarted( const VideoFrame& cropped_frame, int64_t time_when_first_seen_us) { - initial_frame_dropper_->OnEncodeStarted(); - MaybePerformQualityRampupExperiment(); encode_usage_resource_->OnEncodeStarted(cropped_frame, time_when_first_seen_us); } @@ -565,6 +563,11 @@ bool OveruseFrameDetectorResourceAdaptationModule::DropInitialFrames() const { return initial_frame_dropper_->DropInitialFrames(); } +void OveruseFrameDetectorResourceAdaptationModule::OnMaybeEncodeFrame() { + initial_frame_dropper_->OnMaybeEncodeFrame(); + MaybePerformQualityRampupExperiment(); +} + void OveruseFrameDetectorResourceAdaptationModule::UpdateQualityScalerSettings( absl::optional qp_thresholds) { if (qp_thresholds.has_value()) { diff --git a/video/overuse_frame_detector_resource_adaptation_module.h b/video/overuse_frame_detector_resource_adaptation_module.h index ecb990f4b6..d10b8c111c 100644 --- a/video/overuse_frame_detector_resource_adaptation_module.h +++ b/video/overuse_frame_detector_resource_adaptation_module.h @@ -86,6 +86,7 @@ class OveruseFrameDetectorResourceAdaptationModule void OnFrame(const VideoFrame& frame) override; void OnFrameDroppedDueToSize() override; + void OnMaybeEncodeFrame() override; void OnEncodeStarted(const VideoFrame& cropped_frame, int64_t time_when_first_seen_us) override; void OnEncodeCompleted(const EncodedImage& encoded_image, diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index b5ad286b90..becf1df738 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -1080,6 +1080,7 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, } return; } + resource_adaptation_module_->OnMaybeEncodeFrame(); if (EncoderPaused()) { // Storing references to a native buffer risks blocking frame capture.