From f89122c1fbfec9ca06f9668544729abb8ec2c211 Mon Sep 17 00:00:00 2001 From: Sergey Silkin Date: Tue, 13 Dec 2022 17:09:12 +0100 Subject: [PATCH] Call OnRenderedFrame after frame is passed to renderer This CL reverts change that was made in https://webrtc-review.googlesource.com/c/src/+/174220 where capturing of frame rendered time was moved into VideoFrameMetaData and construction of VideoFrameMetaData was placed before renderer->OnFrame(). That change made video freeze metrics (freeze and pause durations, harmonic frame rate) more off from the actual user experience. Bug: webrtc:11489, b/261512902 Change-Id: Ic92a0cc1bb6d7b3ee1023804a73aa282848f8c4b Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/287880 Reviewed-by: Philip Eliasson Commit-Queue: Sergey Silkin Cr-Commit-Position: refs/heads/main@{#38883} --- video/video_receive_stream2.cc | 13 ++++++++++--- 1 file changed, 10 insertions(+), 3 deletions(-) diff --git a/video/video_receive_stream2.cc b/video/video_receive_stream2.cc index 5d86455127..9a95c58a93 100644 --- a/video/video_receive_stream2.cc +++ b/video/video_receive_stream2.cc @@ -655,11 +655,20 @@ int VideoReceiveStream2::GetBaseMinimumPlayoutDelayMs() const { } void VideoReceiveStream2::OnFrame(const VideoFrame& video_frame) { - VideoFrameMetaData frame_meta(video_frame, clock_->CurrentTime()); + source_tracker_.OnFrameDelivered(video_frame.packet_infos()); + config_.renderer->OnFrame(video_frame); // TODO(bugs.webrtc.org/10739): we should set local capture clock offset for // `video_frame.packet_infos`. But VideoFrame is const qualified here. + // For frame delay metrics, calculated in `OnRenderedFrame`, to better reflect + // user experience measurements must be done as close as possible to frame + // rendering moment. Capture current time, which is used for calculation of + // delay metrics in `OnRenderedFrame`, right after frame is passed to + // renderer. Frame may or may be not rendered by this time. This results in + // inaccuracy but is still the best we can do in the absence of "frame + // rendered" callback from the renderer. + VideoFrameMetaData frame_meta(video_frame, clock_->CurrentTime()); call_->worker_thread()->PostTask( SafeTask(task_safety_.flag(), [frame_meta, this]() { RTC_DCHECK_RUN_ON(&worker_sequence_checker_); @@ -675,8 +684,6 @@ void VideoReceiveStream2::OnFrame(const VideoFrame& video_frame) { stats_proxy_.OnRenderedFrame(frame_meta); })); - source_tracker_.OnFrameDelivered(video_frame.packet_infos()); - config_.renderer->OnFrame(video_frame); webrtc::MutexLock lock(&pending_resolution_mutex_); if (pending_resolution_.has_value()) { if (!pending_resolution_->empty() &&