Smoother frame dropping when screenshare_layers limits fps
Currently, when input fps is higher than the configured target fps in
screenshare_layers, we drop frames with the help of a rate tracker using
a one second sliding window. This is not optimal.
For instance, given a 5fps limit and a 30fps capturer, the window will
not be saturated until we have added the first 5 frames. Then we will
drop all frames until the oldest one drops out, at which point we can
immediately fill it's place. This results in quick 5 frame bursts every
second.
In addition to rate tracker, also set a limit on minimum interval
required between input frames, based on target frame rate.
Bug: webrtc:4172
Change-Id: I49f0abf4d549673cc6b3fafe580b529ea3feaf57
Reviewed-on: https://webrtc-review.googlesource.com/34380
Commit-Queue: Erik Språng <sprang@webrtc.org>
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21325}diff --git a/modules/video_coding/codecs/vp8/screenshare_layers.cc b/modules/video_coding/codecs/vp8/screenshare_layers.cc
index 782dc77..e398687 100644
--- a/modules/video_coding/codecs/vp8/screenshare_layers.cc
+++ b/modules/video_coding/codecs/vp8/screenshare_layers.cc
@@ -113,14 +113,6 @@
}
const int64_t now_ms = clock_->TimeInMilliseconds();
- if (target_framerate_.value_or(0) > 0 &&
- encode_framerate_.Rate(now_ms).value_or(0) > *target_framerate_) {
- // Max framerate exceeded, drop frame.
- return TemporalLayers::FrameConfig(kNone, kNone, kNone);
- }
-
- if (stats_.first_frame_time_ms_ == -1)
- stats_.first_frame_time_ms_ = now_ms;
int64_t unwrapped_timestamp = time_wrap_handler_.Unwrap(timestamp);
int64_t ts_diff;
@@ -129,6 +121,24 @@
} else {
ts_diff = unwrapped_timestamp - last_timestamp_;
}
+
+ // If input frame rate exceeds target frame rate, either over a one second
+ // averaging window, or if frame interval is below 90% of desired value,
+ // drop frame.
+ if (target_framerate_) {
+ bool exceeds_average_rate =
+ encode_framerate_.Rate(now_ms).value_or(0) > *target_framerate_;
+ bool exceeds_inter_frame_delay =
+ ts_diff < (kOneSecond90Khz * 9) / (*target_framerate_ * 10);
+ if (exceeds_average_rate || exceeds_inter_frame_delay) {
+ // Max framerate exceeded, drop frame.
+ return TemporalLayers::FrameConfig(kNone, kNone, kNone);
+ }
+ }
+
+ if (stats_.first_frame_time_ms_ == -1)
+ stats_.first_frame_time_ms_ = now_ms;
+
// Make sure both frame droppers leak out bits.
layers_[0].UpdateDebt(ts_diff / 90);
layers_[1].UpdateDebt(ts_diff / 90);
diff --git a/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc b/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc
index 754235a..f426b69 100644
--- a/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc
+++ b/modules/video_coding/codecs/vp8/screenshare_layers_unittest.cc
@@ -442,7 +442,12 @@
ASSERT_GT(kStartTimestamp + 90 * (kLargeFrameSizeBytes * 8) / kLowBitrateKbps,
kStartTimestamp + (ScreenshareLayers::kMaxFrameIntervalMs * 90));
- EXPECT_TRUE(layers_->UpdateLayerConfig(kTwoSecondsLater).drop_frame);
+ // Expect drop one frame interval before the two second timeout. If we try
+ // any later, the frame will be dropped anyway by the frame rate throttling
+ // logic.
+ EXPECT_TRUE(layers_->UpdateLayerConfig(kTwoSecondsLater - kTimestampDelta5Fps)
+ .drop_frame);
+
// More than two seconds has passed since last frame, one should be emitted
// even if bitrate target is then exceeded.
EXPECT_EQ(kTl0Flags, VP8EncoderImpl::EncodeFlags(
@@ -472,8 +477,6 @@
// Simulate one overshoot.
layers_->FrameEncoded(0, 0);
overshoot = true;
- flags =
- VP8EncoderImpl::EncodeFlags(layers_->UpdateLayerConfig(timestamp));
}
if (flags == kTl0Flags) {
@@ -589,13 +592,6 @@
// Simulate overshoot of this frame.
layers_->FrameEncoded(0, -1);
- // Reencode, frame config, flags and codec specific info should remain the
- // same as for the dropped frame.
- timestamp_ -= kTimestampDelta5Fps; // Undo last timestamp increment.
- TemporalLayers::FrameConfig new_tl_config =
- layers_->UpdateLayerConfig(timestamp_);
- EXPECT_EQ(tl_config_, new_tl_config);
-
config_updated_ = layers_->UpdateConfiguration(&cfg_);
EXPECT_EQ(kTl1SyncFlags, VP8EncoderImpl::EncodeFlags(tl_config_));
@@ -604,4 +600,24 @@
EXPECT_TRUE(new_vp8_info.layerSync);
}
+TEST_F(ScreenshareLayerTest, DropOnTooShortFrameInterval) {
+ // Run grace period so we have existing frames in both TL0 and Tl1.
+ EXPECT_TRUE(RunGracePeriod());
+
+ // Add a large gap, so there's plenty of room in the rate tracker.
+ timestamp_ += kTimestampDelta5Fps * 3;
+ EXPECT_FALSE(layers_->UpdateLayerConfig(timestamp_).drop_frame);
+ layers_->FrameEncoded(frame_size_, kDefaultQp);
+
+ // Frame interval below 90% if desired time is not allowed, try inserting
+ // frame just before this limit.
+ const int64_t kMinFrameInterval = (kTimestampDelta5Fps * 9) / 10;
+ timestamp_ += kMinFrameInterval - 90;
+ EXPECT_TRUE(layers_->UpdateLayerConfig(timestamp_).drop_frame);
+
+ // Try again at the limit, now it should pass.
+ timestamp_ += 90;
+ EXPECT_FALSE(layers_->UpdateLayerConfig(timestamp_).drop_frame);
+}
+
} // namespace webrtc
