Fix bw_limited_resolution in SendStatisticsProxy GetStats
Removed old code, which took care of VP8 case, but actually interferes with
VP9 case.
Now regardless of codec bw_limited_resolution is calculated based on signals
from the bitrate allocator.
Bug: webrtc:11015
Change-Id: Ic99dbb504ab2d1a4b5f15ca93193a1af05ae5924
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/160651
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#29937}
diff --git a/video/send_statistics_proxy.cc b/video/send_statistics_proxy.cc
index 58fb82e..a4f1754 100644
--- a/video/send_statistics_proxy.cc
+++ b/video/send_statistics_proxy.cc
@@ -215,9 +215,7 @@
}
}
-void SendStatisticsProxy::UmaSamplesContainer::RemoveOld(
- int64_t now_ms,
- bool* is_limited_in_resolution) {
+void SendStatisticsProxy::UmaSamplesContainer::RemoveOld(int64_t now_ms) {
while (!encoded_frames_.empty()) {
auto it = encoded_frames_.begin();
if (now_ms - it->second.send_ms < kMaxEncodedFrameWindowMs)
@@ -229,7 +227,6 @@
// Check number of encoded streams per timestamp.
if (num_streams_ > static_cast<size_t>(it->second.max_simulcast_idx)) {
- *is_limited_in_resolution = false;
if (num_streams_ > 1) {
int disabled_streams =
static_cast<int>(num_streams_ - 1 - it->second.max_simulcast_idx);
@@ -240,7 +237,6 @@
bw_limited_frame_counter_.Add(bw_limited_resolution);
if (bw_limited_resolution) {
bw_resolutions_disabled_counter_.Add(disabled_streams);
- *is_limited_in_resolution = true;
}
}
}
@@ -250,10 +246,9 @@
bool SendStatisticsProxy::UmaSamplesContainer::InsertEncodedFrame(
const EncodedImage& encoded_frame,
- int simulcast_idx,
- bool* is_limited_in_resolution) {
+ int simulcast_idx) {
int64_t now_ms = clock_->TimeInMilliseconds();
- RemoveOld(now_ms, is_limited_in_resolution);
+ RemoveOld(now_ms);
if (encoded_frames_.size() > kMaxEncodedFrameMapSize) {
encoded_frames_.clear();
}
@@ -984,16 +979,11 @@
media_byte_rate_tracker_.AddSamples(encoded_image.size());
- // Initialize to current since |is_limited_in_resolution| is only updated
- // when an encoded frame is removed from the EncodedFrameMap.
- bool is_limited_in_resolution = stats_.bw_limited_resolution;
- if (uma_container_->InsertEncodedFrame(encoded_image, simulcast_idx,
- &is_limited_in_resolution)) {
+ if (uma_container_->InsertEncodedFrame(encoded_image, simulcast_idx)) {
encoded_frame_rate_tracker_.AddSamples(1);
}
- stats_.bw_limited_resolution =
- is_limited_in_resolution || quality_downscales_ > 0;
+ stats_.bw_limited_resolution |= quality_downscales_ > 0;
if (quality_downscales_ != -1) {
uma_container_->quality_limited_frame_counter_.Add(quality_downscales_ > 0);
diff --git a/video/send_statistics_proxy.h b/video/send_statistics_proxy.h
index e690803..a67725e 100644
--- a/video/send_statistics_proxy.h
+++ b/video/send_statistics_proxy.h
@@ -290,9 +290,8 @@
void InitializeBitrateCounters(const VideoSendStream::Stats& stats);
bool InsertEncodedFrame(const EncodedImage& encoded_frame,
- int simulcast_idx,
- bool* is_limited_in_resolution);
- void RemoveOld(int64_t now_ms, bool* is_limited_in_resolution);
+ int simulcast_idx);
+ void RemoveOld(int64_t now_ms);
const std::string uma_prefix_;
Clock* const clock_;
diff --git a/video/send_statistics_proxy_unittest.cc b/video/send_statistics_proxy_unittest.cc
index 4823e95..3b2b3ad 100644
--- a/video/send_statistics_proxy_unittest.cc
+++ b/video/send_statistics_proxy_unittest.cc
@@ -2064,50 +2064,10 @@
stream2.height = kHeight;
statistics_proxy_->OnEncoderReconfigured(config, {stream1, stream2});
- const int64_t kMaxEncodedFrameWindowMs = 800;
- const int kFps = 20;
- const int kMinSamples = // Sample added when removed from EncodedFrameMap.
- kFps * kMaxEncodedFrameWindowMs / 1000;
-
// One stream encoded.
EncodedImage encoded_image;
encoded_image._encodedWidth = kWidth / 2;
encoded_image._encodedHeight = kHeight / 2;
- for (int i = 0; i < kMinSamples; ++i) {
- fake_clock_.AdvanceTimeMilliseconds(1000 / kFps);
- encoded_image.SetTimestamp(encoded_image.Timestamp() +
- (kRtpClockRateHz / kFps));
- statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
- EXPECT_FALSE(statistics_proxy_->GetStats().bw_limited_resolution);
- }
-
- // First frame removed from EncodedFrameMap, stats updated.
- fake_clock_.AdvanceTimeMilliseconds(1000 / kFps);
- encoded_image.SetTimestamp(encoded_image.Timestamp() + 1);
- statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
- EXPECT_TRUE(statistics_proxy_->GetStats().bw_limited_resolution);
-
- // Two streams encoded.
- for (int i = 0; i < kMinSamples; ++i) {
- fake_clock_.AdvanceTimeMilliseconds(1000 / kFps);
- encoded_image.SetTimestamp(encoded_image.Timestamp() +
- (kRtpClockRateHz / kFps));
- encoded_image._encodedWidth = kWidth;
- encoded_image._encodedHeight = kHeight;
- statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
- EXPECT_TRUE(statistics_proxy_->GetStats().bw_limited_resolution);
- encoded_image._encodedWidth = kWidth / 2;
- encoded_image._encodedHeight = kHeight / 2;
- statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
- EXPECT_TRUE(statistics_proxy_->GetStats().bw_limited_resolution);
- }
-
- // First frame with two streams removed, expect no resolution limit.
- fake_clock_.AdvanceTimeMilliseconds(1000 / kFps);
- encoded_image.SetTimestamp(encoded_image.Timestamp() +
- (kRtpClockRateHz / kFps));
- statistics_proxy_->OnSendEncodedImage(encoded_image, nullptr);
- EXPECT_FALSE(statistics_proxy_->GetStats().bw_limited_resolution);
// Resolution scaled due to quality.
SendStatisticsProxy::AdaptationSteps cpu_counts;
