From 0ad2d8af3976b8b039fd5f8c40f9f44e2c15990a Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=85sa=20Persson?= Date: Thu, 19 Apr 2018 11:06:11 +0200 Subject: [PATCH] Minor changes to QualityScaler. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - remove duplicated test, DoesNotDownscaleOnNormalQp - add test, KeepsScaleOnNormalQp - make member const Bug: none Change-Id: I6599e5eb0d59b67b0af55701accea25a80c7c875 Reviewed-on: https://webrtc-review.googlesource.com/70203 Commit-Queue: Åsa Persson Reviewed-by: Rasmus Brandt Cr-Commit-Position: refs/heads/master@{#22935} --- .../video_coding/utility/quality_scaler.cc | 13 ++-- modules/video_coding/utility/quality_scaler.h | 9 +-- .../utility/quality_scaler_unittest.cc | 74 +++++++++++++------ 3 files changed, 64 insertions(+), 32 deletions(-) diff --git a/modules/video_coding/utility/quality_scaler.cc b/modules/video_coding/utility/quality_scaler.cc index fe41129309..a5f6593e66 100644 --- a/modules/video_coding/utility/quality_scaler.cc +++ b/modules/video_coding/utility/quality_scaler.cc @@ -73,15 +73,15 @@ QualityScaler::QualityScaler(AdaptationObserverInterface* observer, // Protected ctor, should not be called directly. QualityScaler::QualityScaler(AdaptationObserverInterface* observer, VideoEncoder::QpThresholds thresholds, - int64_t sampling_period) + int64_t sampling_period_ms) : check_qp_task_(nullptr), observer_(observer), - sampling_period_ms_(sampling_period), + thresholds_(thresholds), + sampling_period_ms_(sampling_period_ms), fast_rampup_(true), // Arbitrarily choose size based on 30 fps for 5 seconds. average_qp_(5 * 30), - framedrop_percent_(5 * 30), - thresholds_(thresholds) { + framedrop_percent_(5 * 30) { RTC_DCHECK_CALLED_SEQUENTIALLY(&task_checker_); RTC_DCHECK(observer_ != nullptr); check_qp_task_ = new CheckQPTask(this); @@ -116,14 +116,15 @@ void QualityScaler::CheckQP() { // Should be set through InitEncode -> Should be set by now. RTC_DCHECK_GE(thresholds_.low, 0); - // If we have not observed at least this many frames we can't - // make a good scaling decision. + // If we have not observed at least this many frames we can't make a good + // scaling decision. if (framedrop_percent_.size() < kMinFramesNeededToScale) return; // Check if we should scale down due to high frame drop. const rtc::Optional drop_rate = framedrop_percent_.GetAverage(); if (drop_rate && *drop_rate >= kFramedropPercentThreshold) { + RTC_LOG(LS_INFO) << "Reporting high QP, framedrop percent " << *drop_rate; ReportQPHigh(); return; } diff --git a/modules/video_coding/utility/quality_scaler.h b/modules/video_coding/utility/quality_scaler.h index 5ac373608b..42cd1c31cf 100644 --- a/modules/video_coding/utility/quality_scaler.h +++ b/modules/video_coding/utility/quality_scaler.h @@ -49,16 +49,16 @@ class QualityScaler { QualityScaler(AdaptationObserverInterface* observer, VideoEncoder::QpThresholds thresholds); virtual ~QualityScaler(); - // Should be called each time the encoder drops a frame + // Should be called each time the encoder drops a frame. void ReportDroppedFrame(); // Inform the QualityScaler of the last seen QP. void ReportQP(int qp); - // The following members declared protected for testing purposes + // The following members declared protected for testing purposes. protected: QualityScaler(AdaptationObserverInterface* observer, VideoEncoder::QpThresholds thresholds, - int64_t sampling_period); + int64_t sampling_period_ms); private: class CheckQPTask; @@ -72,12 +72,11 @@ class QualityScaler { AdaptationObserverInterface* const observer_ RTC_GUARDED_BY(&task_checker_); rtc::SequencedTaskChecker task_checker_; + const VideoEncoder::QpThresholds thresholds_; const int64_t sampling_period_ms_; bool fast_rampup_ RTC_GUARDED_BY(&task_checker_); MovingAverage average_qp_ RTC_GUARDED_BY(&task_checker_); MovingAverage framedrop_percent_ RTC_GUARDED_BY(&task_checker_); - - VideoEncoder::QpThresholds thresholds_ RTC_GUARDED_BY(&task_checker_); }; } // namespace webrtc diff --git a/modules/video_coding/utility/quality_scaler_unittest.cc b/modules/video_coding/utility/quality_scaler_unittest.cc index d9da1b4b33..ea99a7d287 100644 --- a/modules/video_coding/utility/quality_scaler_unittest.cc +++ b/modules/video_coding/utility/quality_scaler_unittest.cc @@ -21,8 +21,8 @@ namespace webrtc { namespace { static const int kFramerate = 30; static const int kLowQp = 15; -static const int kLowQpThreshold = 18; static const int kHighQp = 40; +static const int kMinFramesNeededToScale = 60; // From quality_scaler.cc. static const size_t kDefaultTimeoutMs = 150; } // namespace @@ -66,6 +66,7 @@ class QualityScalerUnderTest : public QualityScaler { class QualityScalerTest : public ::testing::Test { protected: enum ScaleDirection { + kKeepScaleAboveLowQp, kKeepScaleAtHighQp, kScaleDown, kScaleDownAboveHighQp, @@ -77,8 +78,8 @@ class QualityScalerTest : public ::testing::Test { observer_(new MockAdaptationObserver()) { DO_SYNC(q_, { qs_ = std::unique_ptr(new QualityScalerUnderTest( - observer_.get(), - VideoEncoder::QpThresholds(kLowQpThreshold, kHighQp)));}); + observer_.get(), VideoEncoder::QpThresholds(kLowQp, kHighQp))); + }); } ~QualityScalerTest() { @@ -88,6 +89,9 @@ class QualityScalerTest : public ::testing::Test { void TriggerScale(ScaleDirection scale_direction) { for (int i = 0; i < kFramerate * 5; ++i) { switch (scale_direction) { + case kKeepScaleAboveLowQp: + qs_->ReportQP(kLowQp + 1); + break; case kScaleUp: qs_->ReportQP(kLowQp); break; @@ -113,6 +117,7 @@ TEST_F(QualityScalerTest, DownscalesAfterContinuousFramedrop) { DO_SYNC(q_, { TriggerScale(kScaleDown); }); EXPECT_TRUE(observer_->event.Wait(kDefaultTimeoutMs)); EXPECT_EQ(1, observer_->adapt_down_events_); + EXPECT_EQ(0, observer_->adapt_up_events_); } TEST_F(QualityScalerTest, KeepsScaleAtHighQp) { @@ -142,13 +147,6 @@ TEST_F(QualityScalerTest, DownscalesAfterTwoThirdsFramedrop) { EXPECT_EQ(0, observer_->adapt_up_events_); } -TEST_F(QualityScalerTest, DoesNotDownscaleOnNormalQp) { - DO_SYNC(q_, { TriggerScale(kScaleDownAboveHighQp); }); - EXPECT_TRUE(observer_->event.Wait(kDefaultTimeoutMs)); - EXPECT_EQ(1, observer_->adapt_down_events_); - EXPECT_EQ(0, observer_->adapt_up_events_); -} - TEST_F(QualityScalerTest, DoesNotDownscaleAfterHalfFramedrop) { DO_SYNC(q_, { for (int i = 0; i < kFramerate * 5; ++i) { @@ -161,6 +159,13 @@ TEST_F(QualityScalerTest, DoesNotDownscaleAfterHalfFramedrop) { EXPECT_EQ(0, observer_->adapt_up_events_); } +TEST_F(QualityScalerTest, KeepsScaleOnNormalQp) { + DO_SYNC(q_, { TriggerScale(kKeepScaleAboveLowQp); }); + EXPECT_FALSE(observer_->event.Wait(kDefaultTimeoutMs)); + EXPECT_EQ(0, observer_->adapt_down_events_); + EXPECT_EQ(0, observer_->adapt_up_events_); +} + TEST_F(QualityScalerTest, UpscalesAfterLowQp) { DO_SYNC(q_, { TriggerScale(kScaleUp); }); EXPECT_TRUE(observer_->event.Wait(kDefaultTimeoutMs)); @@ -181,22 +186,49 @@ TEST_F(QualityScalerTest, ScalesDownAndBackUp) { TEST_F(QualityScalerTest, DoesNotScaleUntilEnoughFramesObserved) { DO_SYNC(q_, { - // Send 30 frames. This should not be enough to make a decision. - for (int i = 0; i < kFramerate; ++i) { - qs_->ReportQP(kLowQp); - } - }); + // Not enough frames to make a decision. + for (int i = 0; i < kMinFramesNeededToScale - 1; ++i) { + qs_->ReportQP(kLowQp); + } + }); EXPECT_FALSE(observer_->event.Wait(kDefaultTimeoutMs)); DO_SYNC(q_, { - // Send 30 more. This should result in an adapt request as - // enough frames have now been observed. - for (int i = 0; i < kFramerate; ++i) { - qs_->ReportQP(kLowQp); - } - }); + // Send 1 more. Enough frames observed, should result in an adapt request. + qs_->ReportQP(kLowQp); + }); EXPECT_TRUE(observer_->event.Wait(kDefaultTimeoutMs)); EXPECT_EQ(0, observer_->adapt_down_events_); EXPECT_EQ(1, observer_->adapt_up_events_); + + // Samples should be cleared after an adapt request. + DO_SYNC(q_, { + // Not enough frames to make a decision. + qs_->ReportQP(kLowQp); + }); + EXPECT_FALSE(observer_->event.Wait(kDefaultTimeoutMs)); + EXPECT_EQ(0, observer_->adapt_down_events_); + EXPECT_EQ(1, observer_->adapt_up_events_); } + +TEST_F(QualityScalerTest, ScalesDownAndBackUpWithMinFramesNeeded) { + DO_SYNC(q_, { + for (int i = 0; i < kMinFramesNeededToScale; ++i) { + qs_->ReportQP(kHighQp + 1); + } + }); + EXPECT_TRUE(observer_->event.Wait(kDefaultTimeoutMs)); + EXPECT_EQ(1, observer_->adapt_down_events_); + EXPECT_EQ(0, observer_->adapt_up_events_); + // Samples cleared. + DO_SYNC(q_, { + for (int i = 0; i < kMinFramesNeededToScale; ++i) { + qs_->ReportQP(kLowQp); + } + }); + EXPECT_TRUE(observer_->event.Wait(kDefaultTimeoutMs)); + EXPECT_EQ(1, observer_->adapt_down_events_); + EXPECT_EQ(1, observer_->adapt_up_events_); +} + } // namespace webrtc #undef DO_SYNC