FrameCadenceAdapter: ensure frame arrival after drop.
This CL accomplishes three things:
1) It enables feeding frame drop indications into the
AdaptedVideoTrackSource for the benefit of downstream projects.
2) Under zero hertz source delivery, a discarded frame ending a
sequence of frames which happened to contain important information
can be seen as a capture freeze. Avoid this by starting requesting
refresh frames after a grace period.
3) It changes the duration until first refresh frame requests on new
streams to three frame periods.
Bug: chromium:1324120, chromium:1336952
Change-Id: I0214852f1a26540588f6c193dd88a65c34ec0d99
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/265871
Reviewed-by: Erik Språng <sprang@webrtc.org>
Commit-Queue: Markus Handell <handellm@webrtc.org>
Cr-Commit-Position: refs/heads/main@{#37238}
diff --git a/media/base/adapted_video_track_source.cc b/media/base/adapted_video_track_source.cc
index f8f8f2d..816ada5 100644
--- a/media/base/adapted_video_track_source.cc
+++ b/media/base/adapted_video_track_source.cc
@@ -61,6 +61,10 @@
}
}
+void AdaptedVideoTrackSource::OnFrameDropped() {
+ broadcaster_.OnDiscardedFrame();
+}
+
void AdaptedVideoTrackSource::AddOrUpdateSink(
rtc::VideoSinkInterface<webrtc::VideoFrame>* sink,
const rtc::VideoSinkWants& wants) {
diff --git a/media/base/adapted_video_track_source.h b/media/base/adapted_video_track_source.h
index 1386fbd..1c3e0b6 100644
--- a/media/base/adapted_video_track_source.h
+++ b/media/base/adapted_video_track_source.h
@@ -45,6 +45,8 @@
// plain memory frame, it is rotated. Subclasses producing native frames must
// handle apply_rotation() themselves.
void OnFrame(const webrtc::VideoFrame& frame);
+ // Indication from source that a frame was dropped.
+ void OnFrameDropped();
// Reports the appropriate frame size after adaptation. Returns true
// if a frame is wanted. Returns false if there are no interested
diff --git a/video/frame_cadence_adapter.cc b/video/frame_cadence_adapter.cc
index b6e21d6..94fd318 100644
--- a/video/frame_cadence_adapter.cc
+++ b/video/frame_cadence_adapter.cc
@@ -120,6 +120,9 @@
absl::optional<uint32_t> GetInputFrameRateFps() override;
void UpdateFrameRate() override {}
+ // Notified on dropped frames.
+ void OnDiscardedFrame();
+
// Conditionally requests a refresh frame via
// Callback::RequestRefreshFrame.
void ProcessKeyFrameRequest();
@@ -182,6 +185,10 @@
// Returns the repeat duration depending on if it's an idle repeat or not.
TimeDelta RepeatDuration(bool idle_repeat) const
RTC_RUN_ON(sequence_checker_);
+ // Unless timer already running, starts repeatedly requesting refresh frames
+ // after a grace_period. If a frame appears before the grace_period has
+ // passed, the request is cancelled.
+ void MaybeStartRefreshFrameRequester() RTC_RUN_ON(sequence_checker_);
TaskQueueBase* const queue_;
Clock* const clock_;
@@ -233,7 +240,7 @@
// VideoFrameSink overrides.
void OnFrame(const VideoFrame& frame) override;
- void OnDiscardedFrame() override { callback_->OnDiscardedFrame(); }
+ void OnDiscardedFrame() override;
void OnConstraintsChanged(
const VideoTrackSourceConstraints& constraints) override;
@@ -301,12 +308,7 @@
double max_fps)
: queue_(queue), clock_(clock), callback_(callback), max_fps_(max_fps) {
sequence_checker_.Detach();
- refresh_frame_requester_ = RepeatingTaskHandle::Start(queue_, [this] {
- RTC_DLOG(LS_VERBOSE) << __func__ << " RequestRefreshFrame";
- if (callback_)
- callback_->RequestRefreshFrame();
- return frame_delay_;
- });
+ MaybeStartRefreshFrameRequester();
}
void ZeroHertzAdapterMode::ReconfigureParameters(
@@ -384,6 +386,17 @@
frame_delay_.ms());
}
+void ZeroHertzAdapterMode::OnDiscardedFrame() {
+ RTC_DCHECK_RUN_ON(&sequence_checker_);
+ RTC_DLOG(LS_VERBOSE) << "ZeroHertzAdapterMode::" << __func__;
+
+ // Under zero hertz source delivery, a discarded frame ending a sequence of
+ // frames which happened to contain important information can be seen as a
+ // capture freeze. Avoid this by starting requesting refresh frames after a
+ // grace period.
+ MaybeStartRefreshFrameRequester();
+}
+
absl::optional<uint32_t> ZeroHertzAdapterMode::GetInputFrameRateFps() {
RTC_DCHECK_RUN_ON(&sequence_checker_);
return max_fps_;
@@ -552,6 +565,23 @@
: frame_delay_;
}
+// RTC_RUN_ON(&sequence_checker_)
+void ZeroHertzAdapterMode::MaybeStartRefreshFrameRequester() {
+ RTC_DLOG(LS_VERBOSE) << __func__;
+ if (!refresh_frame_requester_.Running()) {
+ refresh_frame_requester_ = RepeatingTaskHandle::DelayedStart(
+ queue_,
+ FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod *
+ frame_delay_,
+ [this] {
+ RTC_DLOG(LS_VERBOSE) << __func__ << " RequestRefreshFrame";
+ if (callback_)
+ callback_->RequestRefreshFrame();
+ return frame_delay_;
+ });
+ }
+}
+
FrameCadenceAdapterImpl::FrameCadenceAdapterImpl(
Clock* clock,
TaskQueueBase* queue,
@@ -644,6 +674,16 @@
}));
}
+void FrameCadenceAdapterImpl::OnDiscardedFrame() {
+ callback_->OnDiscardedFrame();
+ queue_->PostTask(ToQueuedTask(safety_.flag(), [this] {
+ RTC_DCHECK_RUN_ON(queue_);
+ if (zero_hertz_adapter_) {
+ zero_hertz_adapter_->OnDiscardedFrame();
+ }
+ }));
+}
+
void FrameCadenceAdapterImpl::OnConstraintsChanged(
const VideoTrackSourceConstraints& constraints) {
RTC_LOG(LS_INFO) << __func__ << " this " << this << " min_fps "
diff --git a/video/frame_cadence_adapter.h b/video/frame_cadence_adapter.h
index 40e9805..149a915 100644
--- a/video/frame_cadence_adapter.h
+++ b/video/frame_cadence_adapter.h
@@ -41,6 +41,9 @@
// and some worst case RTT.
static constexpr TimeDelta kZeroHertzIdleRepeatRatePeriod =
TimeDelta::Millis(1000);
+ // The number of frame periods to wait for new frames until starting to
+ // request refresh frames.
+ static constexpr int kOnDiscardedFrameRefreshFramePeriod = 3;
struct ZeroHertzModeParams {
// The number of simulcast layers used in this configuration.
diff --git a/video/frame_cadence_adapter_unittest.cc b/video/frame_cadence_adapter_unittest.cc
index 9bd312a..3a2767a 100644
--- a/video/frame_cadence_adapter_unittest.cc
+++ b/video/frame_cadence_adapter_unittest.cc
@@ -368,9 +368,13 @@
adapter->Initialize(&callback);
adapter->SetZeroHertzModeEnabled(
FrameCadenceAdapterInterface::ZeroHertzModeParams{});
- adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, 10});
+ constexpr int kMaxFps = 10;
+ adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFps});
EXPECT_CALL(callback, RequestRefreshFrame);
- time_controller.AdvanceTime(TimeDelta::Zero());
+ time_controller.AdvanceTime(
+ TimeDelta::Seconds(1) *
+ FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod /
+ kMaxFps);
adapter->ProcessKeyFrameRequest();
}
@@ -400,10 +404,14 @@
constexpr int kMaxFps = 10;
adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFps});
- // We should see max_fps + 1 refresh frame requests during the one second we
- // wait until we send a single frame, after which refresh frame requests
- // should cease (we should see no such requests during a second).
- EXPECT_CALL(callback, RequestRefreshFrame).Times(kMaxFps + 1);
+ // We should see max_fps + 1 -
+ // FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod refresh
+ // frame requests during the one second we wait until we send a single frame,
+ // after which refresh frame requests should cease (we should see no such
+ // requests during a second).
+ EXPECT_CALL(callback, RequestRefreshFrame)
+ .Times(kMaxFps + 1 -
+ FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod);
time_controller.AdvanceTime(TimeDelta::Seconds(1));
Mock::VerifyAndClearExpectations(&callback);
adapter->OnFrame(CreateFrame());
@@ -411,6 +419,85 @@
time_controller.AdvanceTime(TimeDelta::Seconds(1));
}
+TEST(FrameCadenceAdapterTest, RequestsRefreshAfterFrameDrop) {
+ ZeroHertzFieldTrialEnabler enabler;
+ MockCallback callback;
+ GlobalSimulatedTimeController time_controller(Timestamp::Millis(0));
+ auto adapter = CreateAdapter(enabler, time_controller.GetClock());
+ adapter->Initialize(&callback);
+ adapter->SetZeroHertzModeEnabled(
+ FrameCadenceAdapterInterface::ZeroHertzModeParams{});
+ constexpr int kMaxFps = 10;
+ adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFps});
+
+ EXPECT_CALL(callback, RequestRefreshFrame).Times(0);
+
+ // Send a frame through to cancel the initial delayed timer waiting for first
+ // frame entry.
+ adapter->OnFrame(CreateFrame());
+ time_controller.AdvanceTime(TimeDelta::Seconds(1));
+ Mock::VerifyAndClearExpectations(&callback);
+
+ // Send a dropped frame indication without any following frames received.
+ // After FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod
+ // frame periods, we should receive a first refresh request.
+ adapter->OnDiscardedFrame();
+ EXPECT_CALL(callback, RequestRefreshFrame);
+ time_controller.AdvanceTime(
+ TimeDelta::Seconds(1) *
+ FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod /
+ kMaxFps);
+ Mock::VerifyAndClearExpectations(&callback);
+
+ // We will now receive a refresh frame request for every frame period.
+ EXPECT_CALL(callback, RequestRefreshFrame).Times(kMaxFps);
+ time_controller.AdvanceTime(TimeDelta::Seconds(1));
+ Mock::VerifyAndClearExpectations(&callback);
+
+ // After a frame is passed the requests will cease.
+ EXPECT_CALL(callback, RequestRefreshFrame).Times(0);
+ adapter->OnFrame(CreateFrame());
+ time_controller.AdvanceTime(TimeDelta::Seconds(1));
+}
+
+TEST(FrameCadenceAdapterTest, OmitsRefreshAfterFrameDropWithTimelyFrameEntry) {
+ ZeroHertzFieldTrialEnabler enabler;
+ MockCallback callback;
+ GlobalSimulatedTimeController time_controller(Timestamp::Millis(0));
+ auto adapter = CreateAdapter(enabler, time_controller.GetClock());
+ adapter->Initialize(&callback);
+ adapter->SetZeroHertzModeEnabled(
+ FrameCadenceAdapterInterface::ZeroHertzModeParams{});
+ constexpr int kMaxFps = 10;
+ adapter->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFps});
+
+ // Send a frame through to cancel the initial delayed timer waiting for first
+ // frame entry.
+ EXPECT_CALL(callback, RequestRefreshFrame).Times(0);
+ adapter->OnFrame(CreateFrame());
+ time_controller.AdvanceTime(TimeDelta::Seconds(1));
+ Mock::VerifyAndClearExpectations(&callback);
+
+ // Send a frame drop indication. No refresh frames should be requested
+ // until FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod
+ // intervals pass. Stop short of this.
+ EXPECT_CALL(callback, RequestRefreshFrame).Times(0);
+ adapter->OnDiscardedFrame();
+ time_controller.AdvanceTime(
+ TimeDelta::Seconds(1) *
+ FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod /
+ kMaxFps -
+ TimeDelta::Micros(1));
+ Mock::VerifyAndClearExpectations(&callback);
+
+ // Send a frame. The timer to request the refresh frame should be cancelled by
+ // the reception, so no refreshes should be requested.
+ EXPECT_CALL(callback, RequestRefreshFrame).Times(0);
+ adapter->OnFrame(CreateFrame());
+ time_controller.AdvanceTime(TimeDelta::Seconds(1));
+ Mock::VerifyAndClearExpectations(&callback);
+}
+
class FrameCadenceAdapterSimulcastLayersParamTest
: public ::testing::TestWithParam<int> {
public:
diff --git a/video/video_stream_encoder_unittest.cc b/video/video_stream_encoder_unittest.cc
index 5436df8..575a8e2 100644
--- a/video/video_stream_encoder_unittest.cc
+++ b/video/video_stream_encoder_unittest.cc
@@ -26,6 +26,7 @@
#include "api/test/mock_video_encoder.h"
#include "api/test/mock_video_encoder_factory.h"
#include "api/units/data_rate.h"
+#include "api/units/time_delta.h"
#include "api/video/builtin_video_bitrate_allocator_factory.h"
#include "api/video/i420_buffer.h"
#include "api/video/nv12_buffer.h"
@@ -9410,8 +9411,12 @@
// synchronized.
EXPECT_CALL(mock_source, RequestRefreshFrame);
video_stream_encoder->SendKeyFrame();
- adapter_ptr->OnConstraintsChanged(VideoTrackSourceConstraints{0, 30});
- factory.DepleteTaskQueues();
+ constexpr int kMaxFps = 30;
+ adapter_ptr->OnConstraintsChanged(VideoTrackSourceConstraints{0, kMaxFps});
+ factory.GetTimeController()->AdvanceTime(
+ TimeDelta::Seconds(1) *
+ FrameCadenceAdapterInterface::kOnDiscardedFrameRefreshFramePeriod /
+ kMaxFps);
}
} // namespace webrtc
