Reland "Storing frame if encoder is paused."
This is a reland of dcc7e88cc79ab4f7aeb87c13f402e007e1320fd8
Original change's description:
> Storing frame if encoder is paused.
>
> Adds a pending frame to VideoStreamEncoder that is used to store frames
> that are not sent because the encoder is paused. If the encoder is
> resumed within 200 ms, the pending frame will be encoded and sent. This
> ensures that resuming a stream instantly starts sending frames if it is
> possible.
>
> This also protects against a race between submitting the first frame
> and enabling the encoder that caused flakiness in end to end tests
> when using the task queue based congestion controller.
>
> Bug: webrtc:8415
> Change-Id: If4bd897187fbfdc4926855f39503230bdad4a93a
> Reviewed-on: https://webrtc-review.googlesource.com/67141
> Commit-Queue: Sebastian Jansson <srte@webrtc.org>
> Reviewed-by: Erik Språng <sprang@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#22781}
Bug: webrtc:8415
Change-Id: I0ea7d4d679e7845907cfbe9a120f128ff2180e4b
Reviewed-on: https://webrtc-review.googlesource.com/68580
Commit-Queue: Sebastian Jansson <srte@webrtc.org>
Reviewed-by: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22810}diff --git a/video/end_to_end_tests/call_operation_tests.cc b/video/end_to_end_tests/call_operation_tests.cc
index 495b658..d90b85e 100644
--- a/video/end_to_end_tests/call_operation_tests.cc
+++ b/video/end_to_end_tests/call_operation_tests.cc
@@ -154,16 +154,7 @@
});
}
-// TODO(bugs.webrtc.org/9060): Re-enable this test with the TaskQueue when it
-// is no longer flaky.
-class CallOperationEndToEndTestNoTaskQueueCongestionControl
- : public CallOperationEndToEndTest {};
-INSTANTIATE_TEST_CASE_P(FieldTrials,
- CallOperationEndToEndTestNoTaskQueueCongestionControl,
- ::testing::Values("WebRTC-RoundRobinPacing/Disabled/",
- "WebRTC-RoundRobinPacing/Enabled/"));
-TEST_P(CallOperationEndToEndTestNoTaskQueueCongestionControl,
- TransmitsFirstFrame) {
+TEST_P(CallOperationEndToEndTest, TransmitsFirstFrame) {
class Renderer : public rtc::VideoSinkInterface<VideoFrame> {
public:
Renderer() : event_(false, false) {}
@@ -219,10 +210,7 @@
});
}
-// TODO(bugs.webrtc.org/9060): Re-enable this test with the TaskQueue when it
-// is no longer flaky.
-TEST_P(CallOperationEndToEndTestNoTaskQueueCongestionControl,
- ObserversEncodedFrames) {
+TEST_P(CallOperationEndToEndTest, ObserversEncodedFrames) {
class EncodedFrameTestObserver : public EncodedFrameObserver {
public:
EncodedFrameTestObserver()
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index 6474fff..952d4ed 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -40,21 +40,13 @@
const int kMinFramerateFps = 2;
const int kMaxFramerateFps = 120;
+// Time to keep a single cached pending frame in paused state.
+const int64_t kPendingFrameTimeoutMs = 1000;
+
// The maximum number of frames to drop at beginning of stream
// to try and achieve desired bitrate.
const int kMaxInitialFramedrop = 4;
-uint32_t MaximumFrameSizeForBitrate(uint32_t kbps) {
- if (kbps > 0) {
- if (kbps < 300 /* qvga */) {
- return 320 * 240;
- } else if (kbps < 500 /* vga */) {
- return 640 * 480;
- }
- }
- return std::numeric_limits<uint32_t>::max();
-}
-
// Initial limits for kBalanced degradation preference.
int MinFps(int pixels) {
if (pixels <= 320 * 240) {
@@ -661,7 +653,7 @@
posted_frames_waiting_for_encode_.fetch_sub(1);
RTC_DCHECK_GT(posted_frames_waiting_for_encode, 0);
if (posted_frames_waiting_for_encode == 1) {
- EncodeVideoFrame(incoming_frame, post_time_us);
+ MaybeEncodeVideoFrame(incoming_frame, post_time_us);
} else {
// There is a newer frame in flight. Do not encode this frame.
RTC_LOG(LS_VERBOSE)
@@ -712,8 +704,8 @@
encoder_paused_and_dropped_frame_ = false;
}
-void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame,
- int64_t time_when_posted_us) {
+void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame,
+ int64_t time_when_posted_us) {
RTC_DCHECK_RUN_ON(&encoder_queue_);
if (pre_encode_callback_)
@@ -731,9 +723,7 @@
<< ", texture=" << last_frame_info_->is_texture << ".";
}
- if (initial_rampup_ < kMaxInitialFramedrop &&
- video_frame.size() >
- MaximumFrameSizeForBitrate(encoder_start_bitrate_bps_ / 1000)) {
+ if (DropDueToSize(video_frame.size())) {
RTC_LOG(LS_INFO) << "Dropping frame. Too large for target bitrate.";
int count = GetConstAdaptCounter().ResolutionCount(kQuality);
AdaptDown(kQuality);
@@ -741,6 +731,8 @@
stats_proxy_->OnInitialQualityResolutionAdaptDown();
}
++initial_rampup_;
+ pending_frame_ = video_frame;
+ pending_frame_post_time_us_ = time_when_posted_us;
return;
}
initial_rampup_ = kMaxInitialFramedrop;
@@ -758,9 +750,20 @@
}
if (EncoderPaused()) {
- TraceFrameDropStart();
+ if (pending_frame_)
+ TraceFrameDropStart();
+ pending_frame_ = video_frame;
+ pending_frame_post_time_us_ = time_when_posted_us;
return;
}
+
+ pending_frame_.reset();
+ EncodeVideoFrame(video_frame, time_when_posted_us);
+}
+
+void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame,
+ int64_t time_when_posted_us) {
+ RTC_DCHECK_RUN_ON(&encoder_queue_);
TraceFrameDropEnd();
VideoFrame out_frame(video_frame);
@@ -891,6 +894,25 @@
<< (video_is_suspended ? "suspended" : "not suspended");
stats_proxy_->OnSuspendChange(video_is_suspended);
}
+ if (video_suspension_changed && !video_is_suspended && pending_frame_ &&
+ !DropDueToSize(pending_frame_->size())) {
+ int64_t pending_time_us = rtc::TimeMicros() - pending_frame_post_time_us_;
+ if (pending_time_us < kPendingFrameTimeoutMs * 1000)
+ EncodeVideoFrame(*pending_frame_, pending_frame_post_time_us_);
+ pending_frame_.reset();
+ }
+}
+
+bool VideoStreamEncoder::DropDueToSize(uint32_t pixel_count) const {
+ if (initial_rampup_ < kMaxInitialFramedrop &&
+ encoder_start_bitrate_bps_ > 0) {
+ if (encoder_start_bitrate_bps_ < 300000 /* qvga */) {
+ return pixel_count > 320 * 240;
+ } else if (encoder_start_bitrate_bps_ < 500000 /* vga */) {
+ return pixel_count > 640 * 480;
+ }
+ }
+ return false;
}
void VideoStreamEncoder::AdaptDown(AdaptReason reason) {
diff --git a/video/video_stream_encoder.h b/video/video_stream_encoder.h
index f516541..a81160e 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -144,8 +144,14 @@
void OnFrame(const VideoFrame& video_frame) override;
void OnDiscardedFrame() override;
+ void MaybeEncodeVideoFrame(const VideoFrame& frame,
+ int64_t time_when_posted_in_ms);
+
void EncodeVideoFrame(const VideoFrame& frame,
int64_t time_when_posted_in_ms);
+ // Indicates wether frame should be dropped because the pixel count is too
+ // large for the current bitrate configuration.
+ bool DropDueToSize(uint32_t pixel_count) const RTC_RUN_ON(&encoder_queue_);
// Implements EncodedImageCallback.
EncodedImageCallback::Result OnEncodedImage(
@@ -281,6 +287,8 @@
int64_t last_frame_log_ms_ RTC_GUARDED_BY(incoming_frame_race_checker_);
int captured_frame_count_ RTC_GUARDED_BY(&encoder_queue_);
int dropped_frame_count_ RTC_GUARDED_BY(&encoder_queue_);
+ rtc::Optional<VideoFrame> pending_frame_ RTC_GUARDED_BY(&encoder_queue_);
+ int64_t pending_frame_post_time_us_ RTC_GUARDED_BY(&encoder_queue_);
VideoBitrateAllocationObserver* bitrate_observer_
RTC_GUARDED_BY(&encoder_queue_);
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index 09d259a..8a6d4e1 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -702,13 +702,20 @@
TEST_F(VideoStreamEncoderTest, DropsFramesBeforeFirstOnBitrateUpdated) {
// Dropped since no target bitrate has been set.
rtc::Event frame_destroyed_event(false, false);
+ // The encoder will cache up to one frame for a short duration. Adding two
+ // frames means that the first frame will be dropped and the second frame will
+ // be sent when the encoder is enabled.
video_source_.IncomingCapturedFrame(CreateFrame(1, &frame_destroyed_event));
+ video_source_.IncomingCapturedFrame(CreateFrame(2, nullptr));
EXPECT_TRUE(frame_destroyed_event.Wait(kDefaultTimeoutMs));
video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
- video_source_.IncomingCapturedFrame(CreateFrame(2, nullptr));
+ // The pending frame should be received.
WaitForEncodedFrame(2);
+ video_source_.IncomingCapturedFrame(CreateFrame(3, nullptr));
+
+ WaitForEncodedFrame(3);
video_stream_encoder_->Stop();
}
@@ -718,12 +725,16 @@
WaitForEncodedFrame(1);
video_stream_encoder_->OnBitrateUpdated(0, 0, 0);
- // Dropped since bitrate is zero.
+ // The encoder will cache up to one frame for a short duration. Adding two
+ // frames means that the first frame will be dropped and the second frame will
+ // be sent when the encoder is resumed.
video_source_.IncomingCapturedFrame(CreateFrame(2, nullptr));
+ video_source_.IncomingCapturedFrame(CreateFrame(3, nullptr));
video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
- video_source_.IncomingCapturedFrame(CreateFrame(3, nullptr));
WaitForEncodedFrame(3);
+ video_source_.IncomingCapturedFrame(CreateFrame(4, nullptr));
+ WaitForEncodedFrame(4);
video_stream_encoder_->Stop();
}
