From 5e13d0599cbe70070961908a1f6548e35d6a24a2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Erik=20Spr=C3=A5ng?= Date: Tue, 2 Aug 2022 11:42:49 +0200 Subject: [PATCH] Request refresh frame after unpausing encoder with native frame drop. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 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 Commit-Queue: Erik Språng Cr-Commit-Position: refs/heads/main@{#37660} --- video/video_stream_encoder.cc | 28 ++++++++++++++++++-------- video/video_stream_encoder_unittest.cc | 26 ++++++++++++++++++++++++ 2 files changed, 46 insertions(+), 8 deletions(-) diff --git a/video/video_stream_encoder.cc b/video/video_stream_encoder.cc index 91e5fb77bc..bde8881fdb 100644 --- a/video/video_stream_encoder.cc +++ b/video/video_stream_encoder.cc @@ -1699,6 +1699,8 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, 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 @@ void VideoStreamEncoder::MaybeEncodeVideoFrame(const VideoFrame& video_frame, 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 @@ void VideoStreamEncoder::OnBitrateUpdated(DataRate target_bitrate, 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 91c528cf74..190c1d7ab2 100644 --- a/video/video_stream_encoder_unittest.cc +++ b/video/video_stream_encoder_unittest.cc @@ -604,12 +604,15 @@ class AdaptingFrameForwarder : public test::FrameForwarder { 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 last_width_; absl::optional last_height_; + int refresh_frames_requested_{0}; }; // TODO(nisse): Mock only VideoStreamEncoderObserver. @@ -8610,6 +8613,29 @@ TEST_F(VideoStreamEncoderTest, 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)