From fcc640f8f6c844211fb91dea49cb3f727dc07a00 Mon Sep 17 00:00:00 2001 From: nisse Date: Fri, 1 Apr 2016 01:10:42 -0700 Subject: [PATCH] Get VideoCapturer stats via VideoTrackSourceInterface in StatsCollector, without involving the VideoMediaChannel. BUG=webrtc:5426 Review URL: https://codereview.webrtc.org/1827023002 Cr-Commit-Position: refs/heads/master@{#12193} --- webrtc/api/mediastreaminterface.h | 14 +++++++++ webrtc/api/rtpsender.h | 2 ++ webrtc/api/statscollector.cc | 36 +++++++++++++++++++++-- webrtc/api/statscollector.h | 1 + webrtc/api/videocapturertracksource.cc | 5 ++++ webrtc/api/videocapturertracksource.h | 2 ++ webrtc/api/videosourceproxy.h | 1 + webrtc/api/videotracksource.h | 2 ++ webrtc/media/base/mediachannel.h | 4 --- webrtc/media/base/videocapturer.cc | 22 ++++++++------ webrtc/media/base/videocapturer.h | 13 ++++---- webrtc/media/engine/webrtcvideoengine2.cc | 7 ----- 12 files changed, 80 insertions(+), 29 deletions(-) diff --git a/webrtc/api/mediastreaminterface.h b/webrtc/api/mediastreaminterface.h index 349fcc02bb..f1730c57be 100644 --- a/webrtc/api/mediastreaminterface.h +++ b/webrtc/api/mediastreaminterface.h @@ -88,6 +88,10 @@ class MediaStreamTrackInterface : public rtc::RefCountInterface, static const char kAudioKind[]; static const char kVideoKind[]; + // The kind() method must return kAudioKind only if the object is a + // subclass of AudioTrackInterface, and kVideoKind only if the + // object is a subclass of VideoTrackInterface. It is typically used + // to protect a static_cast<> to the corresponding subclass. virtual std::string kind() const = 0; virtual std::string id() const = 0; virtual bool enabled() const = 0; @@ -106,6 +110,11 @@ class VideoTrackSourceInterface : public MediaSourceInterface, public rtc::VideoSourceInterface { public: + struct Stats { + // Original size of captured frame, before video adaptation. + int input_width; + int input_height; + }; // Get access to the source implementation of cricket::VideoCapturer. // This can be used for receiving frames and state notifications. // But it should not be used for starting or stopping capturing. @@ -132,6 +141,11 @@ class VideoTrackSourceInterface // the encoder. virtual rtc::Optional needs_denoising() const = 0; + // Returns false if no stats are available, e.g, for a remote + // source, or a source which has not seen its first frame yet. + // Should avoid blocking. + virtual bool GetStats(Stats* stats) = 0; + protected: virtual ~VideoTrackSourceInterface() {} }; diff --git a/webrtc/api/rtpsender.h b/webrtc/api/rtpsender.h index 879a332209..3919e070dc 100644 --- a/webrtc/api/rtpsender.h +++ b/webrtc/api/rtpsender.h @@ -101,6 +101,8 @@ class AudioRtpSender : public ObserverInterface, bool SetParameters(const RtpParameters& parameters); private: + // TODO(nisse): Since SSRC == 0 is technically valid, figure out + // some other way to test if we have a valid SSRC. bool can_send_track() const { return track_ && ssrc_; } // Helper function to construct options for // AudioProviderInterface::SetAudioSend. diff --git a/webrtc/api/statscollector.cc b/webrtc/api/statscollector.cc index d77953b3be..0182a37620 100644 --- a/webrtc/api/statscollector.cc +++ b/webrtc/api/statscollector.cc @@ -236,11 +236,9 @@ void ExtractStats(const cricket::VideoSenderInfo& info, StatsReport* report) { { StatsReport::kStatsValueNameEncodeUsagePercent, info.encode_usage_percent }, { StatsReport::kStatsValueNameFirsReceived, info.firs_rcvd }, - { StatsReport::kStatsValueNameFrameHeightInput, info.input_frame_height }, { StatsReport::kStatsValueNameFrameHeightSent, info.send_frame_height }, { StatsReport::kStatsValueNameFrameRateInput, info.framerate_input }, { StatsReport::kStatsValueNameFrameRateSent, info.framerate_sent }, - { StatsReport::kStatsValueNameFrameWidthInput, info.input_frame_width }, { StatsReport::kStatsValueNameFrameWidthSent, info.send_frame_width }, { StatsReport::kStatsValueNameNacksReceived, info.nacks_rcvd }, { StatsReport::kStatsValueNamePacketsLost, info.packets_lost }, @@ -474,6 +472,7 @@ StatsCollector::UpdateStats(PeerConnectionInterface::StatsOutputLevel level) { ExtractSessionInfo(); ExtractVoiceInfo(); ExtractVideoInfo(level); + ExtractSenderInfo(); ExtractDataInfo(); UpdateTrackReports(); } @@ -828,6 +827,39 @@ void StatsCollector::ExtractVideoInfo( } } +void StatsCollector::ExtractSenderInfo() { + RTC_DCHECK(pc_->session()->signaling_thread()->IsCurrent()); + + for (const auto& sender : pc_->GetSenders()) { + // TODO(nisse): SSRC == 0 currently means none. Delete check when + // that is fixed. + if (!sender->ssrc()) { + continue; + } + const rtc::scoped_refptr track(sender->track()); + if (!track || track->kind() != MediaStreamTrackInterface::kVideoKind) { + continue; + } + // Safe, because kind() == kVideoKind implies a subclass of + // VideoTrackInterface; see mediastreaminterface.h. + VideoTrackSourceInterface* source = + static_cast(track.get())->GetSource(); + + VideoTrackSourceInterface::Stats stats; + if (!source->GetStats(&stats)) { + continue; + } + const StatsReport::Id stats_id = StatsReport::NewIdWithDirection( + StatsReport::kStatsReportTypeSsrc, + rtc::ToString(sender->ssrc()), StatsReport::kSend); + StatsReport* report = reports_.FindOrAddNew(stats_id); + report->AddInt(StatsReport::kStatsValueNameFrameWidthInput, + stats.input_width); + report->AddInt(StatsReport::kStatsValueNameFrameHeightInput, + stats.input_height); + } +} + void StatsCollector::ExtractDataInfo() { RTC_DCHECK(pc_->session()->signaling_thread()->IsCurrent()); diff --git a/webrtc/api/statscollector.h b/webrtc/api/statscollector.h index 37572799fe..8c359e48c4 100644 --- a/webrtc/api/statscollector.h +++ b/webrtc/api/statscollector.h @@ -113,6 +113,7 @@ class StatsCollector { void ExtractSessionInfo(); void ExtractVoiceInfo(); void ExtractVideoInfo(PeerConnectionInterface::StatsOutputLevel level); + void ExtractSenderInfo(); void BuildSsrcToTransportId(); webrtc::StatsReport* GetReport(const StatsReport::StatsType& type, const std::string& id, diff --git a/webrtc/api/videocapturertracksource.cc b/webrtc/api/videocapturertracksource.cc index 0543dff3f3..cb539614be 100644 --- a/webrtc/api/videocapturertracksource.cc +++ b/webrtc/api/videocapturertracksource.cc @@ -360,6 +360,11 @@ void VideoCapturerTrackSource::Initialize( // Initialize hasn't succeeded until a successful state change has occurred. } +bool VideoCapturerTrackSource::GetStats(Stats* stats) { + return video_capturer_->GetInputSize(&stats->input_width, + &stats->input_height); +} + void VideoCapturerTrackSource::Stop() { if (!started_) { return; diff --git a/webrtc/api/videocapturertracksource.h b/webrtc/api/videocapturertracksource.h index 3673bf7fd4..0d1142deba 100644 --- a/webrtc/api/videocapturertracksource.h +++ b/webrtc/api/videocapturertracksource.h @@ -58,6 +58,8 @@ class VideoCapturerTrackSource : public VideoTrackSource, return needs_denoising_; } + bool GetStats(Stats* stats) override; + void Stop() override; void Restart() override; diff --git a/webrtc/api/videosourceproxy.h b/webrtc/api/videosourceproxy.h index fa6c428842..f43c0db69f 100644 --- a/webrtc/api/videosourceproxy.h +++ b/webrtc/api/videosourceproxy.h @@ -28,6 +28,7 @@ PROXY_METHOD0(void, Stop) PROXY_METHOD0(void, Restart) PROXY_CONSTMETHOD0(bool, is_screencast) PROXY_CONSTMETHOD0(rtc::Optional, needs_denoising) +PROXY_METHOD1(bool, GetStats, Stats*) PROXY_METHOD2(void, AddOrUpdateSink, rtc::VideoSinkInterface*, diff --git a/webrtc/api/videotracksource.h b/webrtc/api/videotracksource.h index 6d23d2e8ca..108209dc2c 100644 --- a/webrtc/api/videotracksource.h +++ b/webrtc/api/videotracksource.h @@ -40,6 +40,8 @@ class VideoTrackSource : public Notifier { virtual rtc::Optional needs_denoising() const { return rtc::Optional(); } + bool GetStats(Stats* stats) override { return false; } + void AddOrUpdateSink(rtc::VideoSinkInterface* sink, const rtc::VideoSinkWants& wants) override; void RemoveSink(rtc::VideoSinkInterface* sink) override; diff --git a/webrtc/media/base/mediachannel.h b/webrtc/media/base/mediachannel.h index 5e0f2d8bc2..6a56bcdb02 100644 --- a/webrtc/media/base/mediachannel.h +++ b/webrtc/media/base/mediachannel.h @@ -660,8 +660,6 @@ struct VideoSenderInfo : public MediaSenderInfo { firs_rcvd(0), plis_rcvd(0), nacks_rcvd(0), - input_frame_width(0), - input_frame_height(0), send_frame_width(0), send_frame_height(0), framerate_input(0), @@ -680,8 +678,6 @@ struct VideoSenderInfo : public MediaSenderInfo { int firs_rcvd; int plis_rcvd; int nacks_rcvd; - int input_frame_width; - int input_frame_height; int send_frame_width; int send_frame_height; int framerate_input; diff --git a/webrtc/media/base/videocapturer.cc b/webrtc/media/base/videocapturer.cc index 6b182b3533..57dc79f7b1 100644 --- a/webrtc/media/base/videocapturer.cc +++ b/webrtc/media/base/videocapturer.cc @@ -179,9 +179,15 @@ void VideoCapturer::set_frame_factory(VideoFrameFactory* frame_factory) { } } -void VideoCapturer::GetStats(VideoFormat* last_captured_frame_format) { +bool VideoCapturer::GetInputSize(int* width, int* height) { rtc::CritScope cs(&frame_stats_crit_); - *last_captured_frame_format = last_captured_frame_format_; + if (!input_size_valid_) { + return false; + } + *width = input_width_; + *height = input_height_; + + return true; } void VideoCapturer::RemoveSink( @@ -391,7 +397,7 @@ void VideoCapturer::OnFrameCaptured(VideoCapturer*, } OnFrame(this, adapted_frame.get()); - UpdateStats(captured_frame); + UpdateInputSize(captured_frame); } void VideoCapturer::OnFrame(VideoCapturer* capturer, const VideoFrame* frame) { @@ -536,15 +542,13 @@ bool VideoCapturer::ShouldFilterFormat(const VideoFormat& format) const { format.height > max_format_->height; } -void VideoCapturer::UpdateStats(const CapturedFrame* captured_frame) { +void VideoCapturer::UpdateInputSize(const CapturedFrame* captured_frame) { // Update stats protected from fetches from different thread. rtc::CritScope cs(&frame_stats_crit_); - last_captured_frame_format_.width = captured_frame->width; - last_captured_frame_format_.height = captured_frame->height; - // TODO(ronghuawu): Useful to report interval as well? - last_captured_frame_format_.interval = 0; - last_captured_frame_format_.fourcc = captured_frame->fourcc; + input_size_valid_ = true; + input_width_ = captured_frame->width; + input_height_ = captured_frame->height; } } // namespace cricket diff --git a/webrtc/media/base/videocapturer.h b/webrtc/media/base/videocapturer.h index 4ad102b7f2..5ec743aaee 100644 --- a/webrtc/media/base/videocapturer.h +++ b/webrtc/media/base/videocapturer.h @@ -209,9 +209,7 @@ class VideoCapturer : public sigslot::has_slots<>, // Takes ownership. void set_frame_factory(VideoFrameFactory* frame_factory); - // TODO(nisse): Rename function? Or pass the frame format before - // adaptation in some other way. - void GetStats(VideoFormat* last_captured_frame_format); + bool GetInputSize(int* width, int* height); // Implements VideoSourceInterface void AddOrUpdateSink(rtc::VideoSinkInterface* sink, @@ -275,7 +273,7 @@ class VideoCapturer : public sigslot::has_slots<>, // Returns true if format doesn't fulfill all applied restrictions. bool ShouldFilterFormat(const VideoFormat& format) const; - void UpdateStats(const CapturedFrame* captured_frame); + void UpdateInputSize(const CapturedFrame* captured_frame); rtc::ThreadChecker thread_checker_; std::string id_; @@ -298,9 +296,10 @@ class VideoCapturer : public sigslot::has_slots<>, CoordinatedVideoAdapter video_adapter_; rtc::CriticalSection frame_stats_crit_; - - // The captured frame format before potential adapation. - VideoFormat last_captured_frame_format_; + // The captured frame size before potential adapation. + bool input_size_valid_ GUARDED_BY(frame_stats_crit_) = false; + int input_width_ GUARDED_BY(frame_stats_crit_); + int input_height_ GUARDED_BY(frame_stats_crit_); // Whether capturer should apply rotation to the frame before signaling it. bool apply_rotation_; diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index b977521f9e..41faf70587 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -2089,13 +2089,6 @@ WebRtcVideoChannel2::WebRtcVideoSendStream::GetVideoSenderInfo() { ? CoordinatedVideoAdapter::ADAPTREASON_NONE : CoordinatedVideoAdapter::ADAPTREASON_CPU; - if (capturer_) { - VideoFormat last_captured_frame_format; - capturer_->GetStats(&last_captured_frame_format); - info.input_frame_width = last_captured_frame_format.width; - info.input_frame_height = last_captured_frame_format.height; - } - // Get bandwidth limitation info from stream_->GetStats(). // Input resolution (output from video_adapter) can be further scaled down or // higher video layer(s) can be dropped due to bitrate constraints.