Request refresh frame after unpausing encoder with native frame drop.
If a "normal" software buffer frame is dropped during paused state, we
store it as a pending frame and try encoding it after the pause state is
lifted. However, native frames are dropped entirely since keeping e.g.
texture handles for long time periods can lead to side effects.
Work around this by requesting a refresh frame after unpausing if the
dropped frame flag is set.
Bug: webrtc:14276
Change-Id: I9edd1e99454e082bcfe29f3d9041026dd8a390d0
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/270220
Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org>
Commit-Queue: Erik Språng <sprang@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37660}
diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc
index 91e5fb7..bde8881 100644
--- a/video/video_stream_encoder.cc
+++ b/video/video_stream_encoder.cc
@@ -1699,6 +1699,8 @@
pending_frame_.reset();
accumulated_update_rect_.Union(video_frame.update_rect());
accumulated_update_rect_is_valid_ &= video_frame.has_update_rect();
+ encoder_stats_observer_->OnFrameDropped(
+ VideoStreamEncoderObserver::DropReason::kEncoderQueue);
}
return;
}
@@ -1718,6 +1720,8 @@
TraceFrameDropStart();
accumulated_update_rect_.Union(video_frame.update_rect());
accumulated_update_rect_is_valid_ &= video_frame.has_update_rect();
+ encoder_stats_observer_->OnFrameDropped(
+ VideoStreamEncoderObserver::DropReason::kEncoderQueue);
}
return;
}
@@ -2185,14 +2189,22 @@
RTC_LOG(LS_INFO) << "Video suspend state changed to: "
<< (video_is_suspended ? "suspended" : "not suspended");
encoder_stats_observer_->OnSuspendChange(video_is_suspended);
- }
- if (video_suspension_changed && !video_is_suspended && pending_frame_ &&
- !DropDueToSize(pending_frame_->size())) {
- int64_t pending_time_us =
- clock_->CurrentTime().us() - pending_frame_post_time_us_;
- if (pending_time_us < kPendingFrameTimeoutMs * 1000)
- EncodeVideoFrame(*pending_frame_, pending_frame_post_time_us_);
- pending_frame_.reset();
+
+ if (!video_is_suspended && pending_frame_ &&
+ !DropDueToSize(pending_frame_->size())) {
+ // A pending stored frame can be processed.
+ int64_t pending_time_us =
+ clock_->CurrentTime().us() - pending_frame_post_time_us_;
+ if (pending_time_us < kPendingFrameTimeoutMs * 1000)
+ EncodeVideoFrame(*pending_frame_, pending_frame_post_time_us_);
+ pending_frame_.reset();
+ } else if (!video_is_suspended && !pending_frame_ &&
+ encoder_paused_and_dropped_frame_) {
+ // A frame was enqueued during pause-state, but since it was a native
+ // frame we could not store it in `pending_frame_` so request a
+ // refresh-frame instead.
+ RequestRefreshFrame();
+ }
}
}
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index 91c528c..190c1d7 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -604,12 +604,15 @@
test::FrameForwarder::AddOrUpdateSinkLocked(sink, wants);
}
+ void RequestRefreshFrame() override { ++refresh_frames_requested_; }
+
TimeController* const time_controller_;
cricket::VideoAdapter adapter_;
bool adaptation_enabled_ RTC_GUARDED_BY(mutex_);
rtc::VideoSinkWants last_wants_ RTC_GUARDED_BY(mutex_);
absl::optional<int> last_width_;
absl::optional<int> last_height_;
+ int refresh_frames_requested_{0};
};
// TODO(nisse): Mock only VideoStreamEncoderObserver.
@@ -8610,6 +8613,29 @@
video_stream_encoder_->Stop();
}
+TEST_F(VideoStreamEncoderTest,
+ RequestsRefreshFrameAfterEarlyDroppedNativeFrame) {
+ // Send a native frame before encoder rates have been set. The encoder is
+ // seen as paused at this time.
+ rtc::Event frame_destroyed_event;
+ video_source_.IncomingCapturedFrame(CreateFakeNativeFrame(
+ /*ntp_time_ms=*/1, &frame_destroyed_event, codec_width_, codec_height_));
+
+ // Frame should be dropped and destroyed.
+ ExpectDroppedFrame();
+ EXPECT_TRUE(frame_destroyed_event.Wait(kDefaultTimeoutMs));
+ EXPECT_EQ(video_source_.refresh_frames_requested_, 0);
+
+ // Set bitrates, unpausing the encoder and triggering a request for a refresh
+ // frame.
+ video_stream_encoder_->OnBitrateUpdatedAndWaitForManagedResources(
+ kTargetBitrate, kTargetBitrate, kTargetBitrate, 0, 0, 0);
+ video_stream_encoder_->WaitUntilTaskQueueIsIdle();
+ EXPECT_EQ(video_source_.refresh_frames_requested_, 1);
+
+ video_stream_encoder_->Stop();
+}
+
#endif // !defined(WEBRTC_IOS)
// Test parameters: (VideoCodecType codec, bool allow_i420_conversion)
