Revert "Storing frame if encoder is paused."
This reverts commit dcc7e88cc79ab4f7aeb87c13f402e007e1320fd8.
Reason for revert: breaks downstream projects
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}
TBR=sprang@webrtc.org,srte@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:8415
Change-Id: I4449eca65a64e2bc2fb25d866e5775e9a085cee9
Reviewed-on: https://webrtc-review.googlesource.com/68280
Reviewed-by: Sergey Silkin <ssilkin@webrtc.org>
Commit-Queue: Sergey Silkin <ssilkin@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#22788}diff --git a/video/end_to_end_tests/call_operation_tests.cc b/video/end_to_end_tests/call_operation_tests.cc
index d90b85e..495b658 100644
--- a/video/end_to_end_tests/call_operation_tests.cc
+++ b/video/end_to_end_tests/call_operation_tests.cc
@@ -154,7 +154,16 @@
});
}
-TEST_P(CallOperationEndToEndTest, TransmitsFirstFrame) {
+// 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) {
class Renderer : public rtc::VideoSinkInterface<VideoFrame> {
public:
Renderer() : event_(false, false) {}
@@ -210,7 +219,10 @@
});
}
-TEST_P(CallOperationEndToEndTest, ObserversEncodedFrames) {
+// TODO(bugs.webrtc.org/9060): Re-enable this test with the TaskQueue when it
+// is no longer flaky.
+TEST_P(CallOperationEndToEndTestNoTaskQueueCongestionControl,
+ ObserversEncodedFrames) {
class EncodedFrameTestObserver : public EncodedFrameObserver {
public:
EncodedFrameTestObserver()
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index 8c966d8..6474fff 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -44,6 +44,17 @@
// 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) {
@@ -650,7 +661,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) {
- MaybeEncodeVideoFrame(incoming_frame, post_time_us);
+ EncodeVideoFrame(incoming_frame, post_time_us);
} else {
// There is a newer frame in flight. Do not encode this frame.
RTC_LOG(LS_VERBOSE)
@@ -701,8 +712,8 @@
encoder_paused_and_dropped_frame_ = false;
}
-void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame,
- int64_t time_when_posted_us) {
+void VideoStreamEncoder::EncodeVideoFrame(const VideoFrame& video_frame,
+ int64_t time_when_posted_us) {
RTC_DCHECK_RUN_ON(&encoder_queue_);
if (pre_encode_callback_)
@@ -720,7 +731,9 @@
<< ", texture=" << last_frame_info_->is_texture << ".";
}
- if (DropDueToSize(video_frame.size())) {
+ if (initial_rampup_ < kMaxInitialFramedrop &&
+ video_frame.size() >
+ MaximumFrameSizeForBitrate(encoder_start_bitrate_bps_ / 1000)) {
RTC_LOG(LS_INFO) << "Dropping frame. Too large for target bitrate.";
int count = GetConstAdaptCounter().ResolutionCount(kQuality);
AdaptDown(kQuality);
@@ -728,8 +741,6 @@
stats_proxy_->OnInitialQualityResolutionAdaptDown();
}
++initial_rampup_;
- pending_frame_ = video_frame;
- pending_frame_post_time_us_ = time_when_posted_us;
return;
}
initial_rampup_ = kMaxInitialFramedrop;
@@ -747,20 +758,9 @@
}
if (EncoderPaused()) {
- if (pending_frame_)
- TraceFrameDropStart();
- pending_frame_ = video_frame;
- pending_frame_post_time_us_ = time_when_posted_us;
+ TraceFrameDropStart();
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,26 +891,6 @@
<< (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())) {
- const int64_t kPendingFrameTimeoutUs = 200000;
- if (rtc::TimeMicros() - pending_frame_post_time_us_ <
- kPendingFrameTimeoutUs)
- 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 a81160e..f516541 100644
--- a/video/video_stream_encoder.h
+++ b/video/video_stream_encoder.h
@@ -144,14 +144,8 @@
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(
@@ -287,8 +281,6 @@
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 3d117a7..09d259a 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -703,18 +703,12 @@
// Dropped since no target bitrate has been set.
rtc::Event frame_destroyed_event(false, false);
video_source_.IncomingCapturedFrame(CreateFrame(1, &frame_destroyed_event));
- // Fill the pending frame cache with a new frame so the previous frame is
- // dropped.
- video_source_.IncomingCapturedFrame(CreateFrame(2, nullptr));
EXPECT_TRUE(frame_destroyed_event.Wait(kDefaultTimeoutMs));
video_stream_encoder_->OnBitrateUpdated(kTargetBitrateBps, 0, 0);
- // The pending frame should be received.
+ video_source_.IncomingCapturedFrame(CreateFrame(2, nullptr));
WaitForEncodedFrame(2);
- video_source_.IncomingCapturedFrame(CreateFrame(3, nullptr));
-
- WaitForEncodedFrame(3);
video_stream_encoder_->Stop();
}
