From 0c141c591ae829277053bc1192d136d8a9cb47b2 Mon Sep 17 00:00:00 2001 From: Johannes Kron Date: Mon, 26 Aug 2019 15:04:43 +0200 Subject: [PATCH] Fix frames dropped statistics MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The |frames_dropped| statistics contain not only frames that are dropped but also frames that are in internal queues. This CL changes that so that |frames_dropped| only contains frames that are dropped. Bug: chromium:990317 Change-Id: If222568501b277a75bc514661c4f8f861b56aaed Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/150111 Reviewed-by: Henrik Boström Reviewed-by: Ilya Nikolaevskiy Reviewed-by: Niels Moller Commit-Queue: Johannes Kron Cr-Commit-Position: refs/heads/master@{#28968} --- call/video_receive_stream.cc | 1 + call/video_receive_stream.h | 3 ++ media/base/media_channel.h | 1 + media/engine/webrtc_video_engine.cc | 1 + modules/video_coding/frame_buffer2.cc | 21 ++++++++++ .../video_coding/frame_buffer2_unittest.cc | 38 +++++++++++++++++++ modules/video_coding/generic_decoder.cc | 2 + .../include/video_coding_defines.h | 4 ++ modules/video_coding/video_coding_defines.cc | 1 + pc/rtc_stats_collector.cc | 5 +-- pc/rtc_stats_collector_unittest.cc | 3 +- video/receive_statistics_proxy.cc | 5 +++ video/receive_statistics_proxy.h | 1 + video/receive_statistics_proxy_unittest.cc | 10 +++++ video/video_stream_decoder.cc | 4 ++ video/video_stream_decoder.h | 1 + 16 files changed, 96 insertions(+), 5 deletions(-) diff --git a/call/video_receive_stream.cc b/call/video_receive_stream.cc index ed830bc3a3..261e5def5d 100644 --- a/call/video_receive_stream.cc +++ b/call/video_receive_stream.cc @@ -44,6 +44,7 @@ std::string VideoReceiveStream::Stats::ToString(int64_t time_ms) const { ss << "height: " << height << ", "; ss << "key: " << frame_counts.key_frames << ", "; ss << "delta: " << frame_counts.delta_frames << ", "; + ss << "frames_dropped: " << frames_dropped << ", "; ss << "network_fps: " << network_frame_rate << ", "; ss << "decode_fps: " << decode_frame_rate << ", "; ss << "render_fps: " << render_frame_rate << ", "; diff --git a/call/video_receive_stream.h b/call/video_receive_stream.h index ad702e69e0..fa37fe895f 100644 --- a/call/video_receive_stream.h +++ b/call/video_receive_stream.h @@ -84,6 +84,9 @@ class VideoReceiveStream { int min_playout_delay_ms = 0; int render_delay_ms = 10; int64_t interframe_delay_max_ms = -1; + // Frames dropped due to decoding failures or if the system is too slow. + // https://www.w3.org/TR/webrtc-stats/#dom-rtcvideoreceiverstats-framesdropped + uint32_t frames_dropped = 0; uint32_t frames_decoded = 0; // https://w3c.github.io/webrtc-stats/#dom-rtcinboundrtpstreamstats-totaldecodetime uint64_t total_decode_time_ms = 0; diff --git a/media/base/media_channel.h b/media/base/media_channel.h index 6ebb676520..c2378747aa 100644 --- a/media/base/media_channel.h +++ b/media/base/media_channel.h @@ -596,6 +596,7 @@ struct VideoReceiverInfo : public MediaReceiverInfo { // Framerate that the renderer reports. int framerate_render_output = 0; uint32_t frames_received = 0; + uint32_t frames_dropped = 0; uint32_t frames_decoded = 0; uint32_t key_frames_decoded = 0; uint32_t frames_rendered = 0; diff --git a/media/engine/webrtc_video_engine.cc b/media/engine/webrtc_video_engine.cc index a3e48d9447..ef4a2cbbc4 100644 --- a/media/engine/webrtc_video_engine.cc +++ b/media/engine/webrtc_video_engine.cc @@ -2824,6 +2824,7 @@ WebRtcVideoChannel::WebRtcVideoReceiveStream::GetVideoReceiverInfo( info.render_delay_ms = stats.render_delay_ms; info.frames_received = stats.frame_counts.key_frames + stats.frame_counts.delta_frames; + info.frames_dropped = stats.frames_dropped; info.frames_decoded = stats.frames_decoded; info.key_frames_decoded = stats.frame_counts.key_frames; info.frames_rendered = stats.frames_rendered; diff --git a/modules/video_coding/frame_buffer2.cc b/modules/video_coding/frame_buffer2.cc index 376cff3b65..ad776b6faa 100644 --- a/modules/video_coding/frame_buffer2.cc +++ b/modules/video_coding/frame_buffer2.cc @@ -286,6 +286,17 @@ EncodedFrame* FrameBuffer::GetNextFrame() { decoded_frames_history_.InsertDecoded(frame_it->first, frame->Timestamp()); // Remove decoded frame and all undecoded frames before it. + if (stats_callback_) { + unsigned int dropped_frames = std::count_if( + frames_.begin(), frame_it, + [](const std::pair& frame) { + return frame.second.frame != nullptr; + }); + if (dropped_frames > 0) { + stats_callback_->OnDroppedFrames(dropped_frames); + } + } + frames_.erase(frames_.begin(), ++frame_it); frames_out.push_back(frame); @@ -723,6 +734,16 @@ void FrameBuffer::UpdateTimingFrameInfo() { void FrameBuffer::ClearFramesAndHistory() { TRACE_EVENT0("webrtc", "FrameBuffer::ClearFramesAndHistory"); + if (stats_callback_) { + unsigned int dropped_frames = std::count_if( + frames_.begin(), frames_.end(), + [](const std::pair& frame) { + return frame.second.frame != nullptr; + }); + if (dropped_frames > 0) { + stats_callback_->OnDroppedFrames(dropped_frames); + } + } frames_.clear(); last_continuous_frame_.reset(); frames_to_decode_.clear(); diff --git a/modules/video_coding/frame_buffer2_unittest.cc b/modules/video_coding/frame_buffer2_unittest.cc index 1cc2ed551d..bc2fd8bc4c 100644 --- a/modules/video_coding/frame_buffer2_unittest.cc +++ b/modules/video_coding/frame_buffer2_unittest.cc @@ -111,6 +111,7 @@ class VCMReceiveStatisticsCallbackMock : public VCMReceiveStatisticsCallback { void(bool is_keyframe, size_t size_bytes, VideoContentType content_type)); + MOCK_METHOD1(OnDroppedFrames, void(uint32_t frames_dropped)); MOCK_METHOD1(OnDiscardedPacketsUpdated, void(int discarded_packets)); MOCK_METHOD1(OnFrameCountsUpdated, void(const FrameCounts& frame_counts)); MOCK_METHOD6(OnFrameBufferTimingsUpdated, @@ -406,6 +407,8 @@ TEST_F(TestFrameBuffer2, DropTemporalLayerSlowDecoder) { pid + i, pid + i - 1); } + EXPECT_CALL(stats_callback_, OnDroppedFrames(1)).Times(3); + for (int i = 0; i < 10; ++i) { ExtractFrame(); clock_.AdvanceTimeMilliseconds(70); @@ -423,6 +426,41 @@ TEST_F(TestFrameBuffer2, DropTemporalLayerSlowDecoder) { CheckNoFrame(9); } +TEST_F(TestFrameBuffer2, DropFramesIfSystemIsStalled) { + uint16_t pid = Rand(); + uint32_t ts = Rand(); + + InsertFrame(pid, 0, ts, false, true, kFrameSize); + InsertFrame(pid + 1, 0, ts + 1 * kFps10, false, true, kFrameSize, pid); + InsertFrame(pid + 2, 0, ts + 2 * kFps10, false, true, kFrameSize, pid + 1); + InsertFrame(pid + 3, 0, ts + 3 * kFps10, false, true, kFrameSize); + + ExtractFrame(); + // Jump forward in time, simulating the system being stalled for some reason. + clock_.AdvanceTimeMilliseconds(3 * kFps10); + // Extract one more frame, expect second and third frame to be dropped. + EXPECT_CALL(stats_callback_, OnDroppedFrames(2)).Times(1); + ExtractFrame(); + + CheckFrame(0, pid + 0, 0); + CheckFrame(1, pid + 3, 0); +} + +TEST_F(TestFrameBuffer2, DroppedFramesCountedOnClear) { + uint16_t pid = Rand(); + uint32_t ts = Rand(); + + InsertFrame(pid, 0, ts, false, true, kFrameSize); + for (int i = 1; i < 5; ++i) { + InsertFrame(pid + i, 0, ts + i * kFps10, false, true, kFrameSize, + pid + i - 1); + } + + // All frames should be dropped when Clear is called. + EXPECT_CALL(stats_callback_, OnDroppedFrames(5)).Times(1); + buffer_->Clear(); +} + TEST_F(TestFrameBuffer2, InsertLateFrame) { uint16_t pid = Rand(); uint32_t ts = Rand(); diff --git a/modules/video_coding/generic_decoder.cc b/modules/video_coding/generic_decoder.cc index 986c36c69f..d8e04342b0 100644 --- a/modules/video_coding/generic_decoder.cc +++ b/modules/video_coding/generic_decoder.cc @@ -78,6 +78,7 @@ void VCMDecodedFrameCallback::Decoded(VideoFrame& decodedImage, if (frameInfo == NULL) { RTC_LOG(LS_WARNING) << "Too many frames backed up in the decoder, dropping " "this one."; + _receiveCallback->OnDroppedFrames(1); return; } @@ -163,6 +164,7 @@ int32_t VCMDecodedFrameCallback::Pop(uint32_t timestamp) { if (_timestampMap.Pop(timestamp) == NULL) { return VCM_GENERAL_ERROR; } + _receiveCallback->OnDroppedFrames(1); return VCM_OK; } diff --git a/modules/video_coding/include/video_coding_defines.h b/modules/video_coding/include/video_coding_defines.h index 043d8c6f29..38707ee6da 100644 --- a/modules/video_coding/include/video_coding_defines.h +++ b/modules/video_coding/include/video_coding_defines.h @@ -67,6 +67,8 @@ class VCMReceiveCallback { return FrameToRender(videoFrame, qp, content_type); } + virtual void OnDroppedFrames(uint32_t frames_dropped); + // Called when the current receive codec changes. virtual void OnIncomingPayloadType(int payload_type); virtual void OnDecoderImplementationName(const char* implementation_name); @@ -83,6 +85,8 @@ class VCMReceiveStatisticsCallback { size_t size_bytes, VideoContentType content_type) = 0; + virtual void OnDroppedFrames(uint32_t frames_dropped) = 0; + virtual void OnFrameBufferTimingsUpdated(int max_decode_ms, int current_delay_ms, int target_delay_ms, diff --git a/modules/video_coding/video_coding_defines.cc b/modules/video_coding/video_coding_defines.cc index 0927697b4a..424b23f971 100644 --- a/modules/video_coding/video_coding_defines.cc +++ b/modules/video_coding/video_coding_defines.cc @@ -12,6 +12,7 @@ namespace webrtc { +void VCMReceiveCallback::OnDroppedFrames(uint32_t frames_dropped) {} void VCMReceiveCallback::OnIncomingPayloadType(int payload_type) {} void VCMReceiveCallback::OnDecoderImplementationName( const char* implementation_name) {} diff --git a/pc/rtc_stats_collector.cc b/pc/rtc_stats_collector.cc index c1b4878004..34911fbef3 100644 --- a/pc/rtc_stats_collector.cc +++ b/pc/rtc_stats_collector.cc @@ -691,10 +691,7 @@ ProduceMediaStreamTrackStatsFromVideoReceiverInfo( // received from. Since we don't support that, this is correct and is the same // value as "RTCInboundRTPStreamStats.framesDecoded". https://crbug.com/659137 video_track_stats->frames_decoded = video_receiver_info.frames_decoded; - RTC_DCHECK_GE(video_receiver_info.frames_received, - video_receiver_info.frames_rendered); - video_track_stats->frames_dropped = - video_receiver_info.frames_received - video_receiver_info.frames_rendered; + video_track_stats->frames_dropped = video_receiver_info.frames_dropped; video_track_stats->freeze_count = video_receiver_info.freeze_count; video_track_stats->pause_count = video_receiver_info.pause_count; video_track_stats->total_freezes_duration = diff --git a/pc/rtc_stats_collector_unittest.cc b/pc/rtc_stats_collector_unittest.cc index 4d59e7c11b..de66b951fa 100644 --- a/pc/rtc_stats_collector_unittest.cc +++ b/pc/rtc_stats_collector_unittest.cc @@ -1659,6 +1659,7 @@ TEST_F(RTCStatsCollectorTest, video_receiver_info_ssrc3.jitter_buffer_emitted_count = 25; video_receiver_info_ssrc3.frames_received = 1000; video_receiver_info_ssrc3.frames_decoded = 995; + video_receiver_info_ssrc3.frames_dropped = 10; video_receiver_info_ssrc3.frames_rendered = 990; video_receiver_info_ssrc3.freeze_count = 3; video_receiver_info_ssrc3.pause_count = 2; @@ -1708,7 +1709,7 @@ TEST_F(RTCStatsCollectorTest, expected_remote_video_track_ssrc3.jitter_buffer_emitted_count = 25; expected_remote_video_track_ssrc3.frames_received = 1000; expected_remote_video_track_ssrc3.frames_decoded = 995; - expected_remote_video_track_ssrc3.frames_dropped = 1000 - 990; + expected_remote_video_track_ssrc3.frames_dropped = 10; expected_remote_video_track_ssrc3.freeze_count = 3; expected_remote_video_track_ssrc3.pause_count = 2; expected_remote_video_track_ssrc3.total_freezes_duration = 1; diff --git a/video/receive_statistics_proxy.cc b/video/receive_statistics_proxy.cc index 4359b5d197..17cec1aba7 100644 --- a/video/receive_statistics_proxy.cc +++ b/video/receive_statistics_proxy.cc @@ -803,6 +803,11 @@ void ReceiveStatisticsProxy::OnCompleteFrame(bool is_keyframe, UpdateFramerate(now_ms); } +void ReceiveStatisticsProxy::OnDroppedFrames(uint32_t frames_dropped) { + rtc::CritScope lock(&crit_); + stats_.frames_dropped += frames_dropped; +} + void ReceiveStatisticsProxy::OnPreDecode(VideoCodecType codec_type, int qp) { RTC_DCHECK_RUN_ON(&decode_thread_); rtc::CritScope lock(&crit_); diff --git a/video/receive_statistics_proxy.h b/video/receive_statistics_proxy.h index f1a9f1ec7a..91ffdf6424 100644 --- a/video/receive_statistics_proxy.h +++ b/video/receive_statistics_proxy.h @@ -67,6 +67,7 @@ class ReceiveStatisticsProxy : public VCMReceiveStatisticsCallback, void OnCompleteFrame(bool is_keyframe, size_t size_bytes, VideoContentType content_type) override; + void OnDroppedFrames(uint32_t frames_dropped) override; void OnFrameBufferTimingsUpdated(int max_decode_ms, int current_delay_ms, int target_delay_ms, diff --git a/video/receive_statistics_proxy_unittest.cc b/video/receive_statistics_proxy_unittest.cc index 9834b3e143..e5727894f6 100644 --- a/video/receive_statistics_proxy_unittest.cc +++ b/video/receive_statistics_proxy_unittest.cc @@ -399,6 +399,16 @@ TEST_F(ReceiveStatisticsProxyTest, GetStatsReportsOnCompleteFrame) { EXPECT_EQ(0, stats.frame_counts.delta_frames); } +TEST_F(ReceiveStatisticsProxyTest, GetStatsReportsOnDroppedFrame) { + unsigned int dropped_frames = 0; + for (int i = 0; i < 10; ++i) { + statistics_proxy_->OnDroppedFrames(i); + dropped_frames += i; + } + VideoReceiveStream::Stats stats = statistics_proxy_->GetStats(); + EXPECT_EQ(dropped_frames, stats.frames_dropped); +} + TEST_F(ReceiveStatisticsProxyTest, GetStatsReportsDecodeTimingStats) { const int kMaxDecodeMs = 2; const int kCurrentDelayMs = 3; diff --git a/video/video_stream_decoder.cc b/video/video_stream_decoder.cc index c523a4759c..29156f4aca 100644 --- a/video/video_stream_decoder.cc +++ b/video/video_stream_decoder.cc @@ -52,6 +52,10 @@ int32_t VideoStreamDecoder::FrameToRender(VideoFrame& video_frame, return 0; } +void VideoStreamDecoder::OnDroppedFrames(uint32_t frames_dropped) { + receive_stats_callback_->OnDroppedFrames(frames_dropped); +} + void VideoStreamDecoder::OnIncomingPayloadType(int payload_type) { receive_stats_callback_->OnIncomingPayloadType(payload_type); } diff --git a/video/video_stream_decoder.h b/video/video_stream_decoder.h index b8ad933d9f..97c7a8c25c 100644 --- a/video/video_stream_decoder.h +++ b/video/video_stream_decoder.h @@ -44,6 +44,7 @@ class VideoStreamDecoder : public VCMReceiveCallback { absl::optional qp, int32_t decode_time_ms, VideoContentType content_type) override; + void OnDroppedFrames(uint32_t frames_dropped) override; void OnIncomingPayloadType(int payload_type) override; void OnDecoderImplementationName(const char* implementation_name) override;