From f79ade1320d1f27dc85a1bf7da5ca430ca80ab85 Mon Sep 17 00:00:00 2001 From: stefan Date: Fri, 2 Jun 2017 06:44:03 -0700 Subject: [PATCH] Revert "Revert of Wire up BWE stats through WebrtcSession so that they are filled in both for audio and video calls. (patchset #8 id:140001 of https://codereview.webrtc.org/2863123002/ )" This reverts commit d72098a41971833e210bfdcffaab7a18ced4775f. BUG=webrtc:5079 Review-Url: https://codereview.webrtc.org/2915263002 Cr-Commit-Position: refs/heads/master@{#18411} --- webrtc/media/base/fakemediaengine.h | 1 + webrtc/media/base/mediachannel.h | 11 +++ webrtc/media/engine/webrtcvideoengine2.cc | 18 ++--- webrtc/media/engine/webrtcvideoengine2.h | 3 +- .../engine/webrtcvideoengine2_unittest.cc | 10 ++- webrtc/pc/channel.cc | 76 +++++++++++-------- webrtc/pc/channel.h | 10 +-- webrtc/pc/rtcstatscollector.cc | 45 ++++++----- webrtc/pc/rtcstatscollector.h | 6 +- webrtc/pc/rtcstatscollector_unittest.cc | 18 ++--- webrtc/pc/statscollector.cc | 32 +++++--- webrtc/pc/statscollector.h | 1 + webrtc/pc/statscollector_unittest.cc | 25 ++++-- webrtc/pc/test/mock_webrtcsession.h | 1 + webrtc/pc/webrtcsession.cc | 11 +++ webrtc/pc/webrtcsession.h | 4 +- 16 files changed, 167 insertions(+), 105 deletions(-) diff --git a/webrtc/media/base/fakemediaengine.h b/webrtc/media/base/fakemediaengine.h index c0aacfce5c..4984d1b88c 100644 --- a/webrtc/media/base/fakemediaengine.h +++ b/webrtc/media/base/fakemediaengine.h @@ -604,6 +604,7 @@ class FakeVideoMediaChannel : public RtpHelper { return true; } + void FillBitrateInfo(BandwidthEstimationInfo* bwe_info) override {} bool GetStats(VideoMediaInfo* info) override { return false; } private: diff --git a/webrtc/media/base/mediachannel.h b/webrtc/media/base/mediachannel.h index dadb55bfdb..40869594e5 100644 --- a/webrtc/media/base/mediachannel.h +++ b/webrtc/media/base/mediachannel.h @@ -868,6 +868,8 @@ struct VideoMediaInfo { } std::vector senders; std::vector receivers; + // Deprecated. + // TODO(holmer): Remove once upstream projects no longer use this. std::vector bw_estimations; RtpCodecParametersMap send_codecs; RtpCodecParametersMap receive_codecs; @@ -1082,6 +1084,15 @@ class VideoMediaChannel : public MediaChannel { // If SSRC is 0, the sink is used for the 'default' stream. virtual bool SetSink(uint32_t ssrc, rtc::VideoSinkInterface* sink) = 0; + // This fills the "bitrate parts" (rtx, video bitrate) of the + // BandwidthEstimationInfo, since that part that isn't possible to get + // through webrtc::Call::GetStats, as they are statistics of the send + // streams. + // TODO(holmer): We should change this so that either BWE graphs doesn't + // need access to bitrates of the streams, or change the (RTC)StatsCollector + // so that it's getting the send stream stats separately by calling + // GetStats(), and merges with BandwidthEstimationInfo by itself. + virtual void FillBitrateInfo(BandwidthEstimationInfo* bwe_info) = 0; // Gets quality stats for the channel. virtual bool GetStats(VideoMediaInfo* info) = 0; }; diff --git a/webrtc/media/engine/webrtcvideoengine2.cc b/webrtc/media/engine/webrtcvideoengine2.cc index 03e97f776b..4f3c283939 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -1379,8 +1379,9 @@ bool WebRtcVideoChannel2::GetStats(VideoMediaInfo* info) { FillSenderStats(info, log_stats); FillReceiverStats(info, log_stats); FillSendAndReceiveCodecStats(info); + // TODO(holmer): We should either have rtt available as a metric on + // VideoSend/ReceiveStreams, or we should remove rtt from VideoSenderInfo. webrtc::Call::Stats stats = call_->GetStats(); - FillBandwidthEstimationStats(stats, info); if (stats.rtt_ms != -1) { for (size_t i = 0; i < info->senders.size(); ++i) { info->senders[i].rtt_ms = stats.rtt_ms; @@ -1415,22 +1416,13 @@ void WebRtcVideoChannel2::FillReceiverStats(VideoMediaInfo* video_media_info, } } -void WebRtcVideoChannel2::FillBandwidthEstimationStats( - const webrtc::Call::Stats& stats, - VideoMediaInfo* video_media_info) { - BandwidthEstimationInfo bwe_info; - bwe_info.available_send_bandwidth = stats.send_bandwidth_bps; - bwe_info.available_recv_bandwidth = stats.recv_bandwidth_bps; - bwe_info.bucket_delay = stats.pacer_delay_ms; - - // Get send stream bitrate stats. +void WebRtcVideoChannel2::FillBitrateInfo(BandwidthEstimationInfo* bwe_info) { rtc::CritScope stream_lock(&stream_crit_); for (std::map::iterator stream = send_streams_.begin(); stream != send_streams_.end(); ++stream) { - stream->second->FillBandwidthEstimationInfo(&bwe_info); + stream->second->FillBitrateInfo(bwe_info); } - video_media_info->bw_estimations.push_back(bwe_info); } void WebRtcVideoChannel2::FillSendAndReceiveCodecStats( @@ -2149,7 +2141,7 @@ VideoSenderInfo WebRtcVideoChannel2::WebRtcVideoSendStream::GetVideoSenderInfo( return info; } -void WebRtcVideoChannel2::WebRtcVideoSendStream::FillBandwidthEstimationInfo( +void WebRtcVideoChannel2::WebRtcVideoSendStream::FillBitrateInfo( BandwidthEstimationInfo* bwe_info) { RTC_DCHECK_RUN_ON(&thread_checker_); if (stream_ == NULL) { diff --git a/webrtc/media/engine/webrtcvideoengine2.h b/webrtc/media/engine/webrtcvideoengine2.h index 394646bc32..56431167a8 100644 --- a/webrtc/media/engine/webrtcvideoengine2.h +++ b/webrtc/media/engine/webrtcvideoengine2.h @@ -160,6 +160,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { bool RemoveRecvStream(uint32_t ssrc) override; bool SetSink(uint32_t ssrc, rtc::VideoSinkInterface* sink) override; + void FillBitrateInfo(BandwidthEstimationInfo* bwe_info) override; bool GetStats(VideoMediaInfo* info) override; void OnPacketReceived(rtc::CopyOnWriteBuffer* packet, @@ -284,7 +285,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { const std::vector& GetSsrcs() const; VideoSenderInfo GetVideoSenderInfo(bool log_stats); - void FillBandwidthEstimationInfo(BandwidthEstimationInfo* bwe_info); + void FillBitrateInfo(BandwidthEstimationInfo* bwe_info); private: // Parameters needed to reconstruct the underlying stream. diff --git a/webrtc/media/engine/webrtcvideoengine2_unittest.cc b/webrtc/media/engine/webrtcvideoengine2_unittest.cc index 95c37a1775..3730e3dd04 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -3709,17 +3709,19 @@ TEST_F(WebRtcVideoChannel2Test, TranslatesSenderBitrateStatsCorrectly) { cricket::VideoMediaInfo info; ASSERT_TRUE(channel_->GetStats(&info)); ASSERT_EQ(2u, info.senders.size()); + BandwidthEstimationInfo bwe_info; + channel_->FillBitrateInfo(&bwe_info); // Assuming stream and stream2 corresponds to senders[0] and [1] respectively // is OK as std::maps are sorted and AddSendStream() gives increasing SSRCs. EXPECT_EQ(stats.media_bitrate_bps, info.senders[0].nominal_bitrate); EXPECT_EQ(stats2.media_bitrate_bps, info.senders[1].nominal_bitrate); EXPECT_EQ(stats.target_media_bitrate_bps + stats2.target_media_bitrate_bps, - info.bw_estimations[0].target_enc_bitrate); + bwe_info.target_enc_bitrate); EXPECT_EQ(stats.media_bitrate_bps + stats2.media_bitrate_bps, - info.bw_estimations[0].actual_enc_bitrate); - EXPECT_EQ(1 + 3 + 5 + 7, info.bw_estimations[0].transmit_bitrate) + bwe_info.actual_enc_bitrate); + EXPECT_EQ(1 + 3 + 5 + 7, bwe_info.transmit_bitrate) << "Bandwidth stats should take all streams into account."; - EXPECT_EQ(2 + 4 + 6 + 8, info.bw_estimations[0].retransmit_bitrate) + EXPECT_EQ(2 + 4 + 6 + 8, bwe_info.retransmit_bitrate) << "Bandwidth stats should take all streams into account."; } diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc index a08d7ae350..cc4588a1cf 100644 --- a/webrtc/pc/channel.cc +++ b/webrtc/pc/channel.cc @@ -441,39 +441,42 @@ bool BaseChannel::Enable(bool enable) { } bool BaseChannel::AddRecvStream(const StreamParams& sp) { - return InvokeOnWorker(RTC_FROM_HERE, - Bind(&BaseChannel::AddRecvStream_w, this, sp)); + return InvokeOnWorker(RTC_FROM_HERE, + Bind(&BaseChannel::AddRecvStream_w, this, sp)); } bool BaseChannel::RemoveRecvStream(uint32_t ssrc) { - return InvokeOnWorker(RTC_FROM_HERE, - Bind(&BaseChannel::RemoveRecvStream_w, this, ssrc)); + return InvokeOnWorker( + RTC_FROM_HERE, Bind(&BaseChannel::RemoveRecvStream_w, this, ssrc)); } bool BaseChannel::AddSendStream(const StreamParams& sp) { - return InvokeOnWorker( + return InvokeOnWorker( RTC_FROM_HERE, Bind(&MediaChannel::AddSendStream, media_channel(), sp)); } bool BaseChannel::RemoveSendStream(uint32_t ssrc) { - return InvokeOnWorker(RTC_FROM_HERE, Bind(&MediaChannel::RemoveSendStream, - media_channel(), ssrc)); + return InvokeOnWorker( + RTC_FROM_HERE, + Bind(&MediaChannel::RemoveSendStream, media_channel(), ssrc)); } bool BaseChannel::SetLocalContent(const MediaContentDescription* content, ContentAction action, std::string* error_desc) { TRACE_EVENT0("webrtc", "BaseChannel::SetLocalContent"); - return InvokeOnWorker(RTC_FROM_HERE, Bind(&BaseChannel::SetLocalContent_w, - this, content, action, error_desc)); + return InvokeOnWorker( + RTC_FROM_HERE, + Bind(&BaseChannel::SetLocalContent_w, this, content, action, error_desc)); } bool BaseChannel::SetRemoteContent(const MediaContentDescription* content, ContentAction action, std::string* error_desc) { TRACE_EVENT0("webrtc", "BaseChannel::SetRemoteContent"); - return InvokeOnWorker(RTC_FROM_HERE, Bind(&BaseChannel::SetRemoteContent_w, - this, content, action, error_desc)); + return InvokeOnWorker( + RTC_FROM_HERE, Bind(&BaseChannel::SetRemoteContent_w, this, content, + action, error_desc)); } void BaseChannel::StartConnectionMonitor(int cms) { @@ -1467,9 +1470,9 @@ bool VoiceChannel::SetAudioSend(uint32_t ssrc, bool enable, const AudioOptions* options, AudioSource* source) { - return InvokeOnWorker(RTC_FROM_HERE, - Bind(&VoiceMediaChannel::SetAudioSend, media_channel(), - ssrc, enable, options, source)); + return InvokeOnWorker( + RTC_FROM_HERE, Bind(&VoiceMediaChannel::SetAudioSend, media_channel(), + ssrc, enable, options, source)); } // TODO(juberti): Handle early media the right way. We should get an explicit @@ -1489,20 +1492,22 @@ void VoiceChannel::SetEarlyMedia(bool enable) { } bool VoiceChannel::CanInsertDtmf() { - return InvokeOnWorker( + return InvokeOnWorker( RTC_FROM_HERE, Bind(&VoiceMediaChannel::CanInsertDtmf, media_channel())); } bool VoiceChannel::InsertDtmf(uint32_t ssrc, int event_code, int duration) { - return InvokeOnWorker(RTC_FROM_HERE, Bind(&VoiceChannel::InsertDtmf_w, this, - ssrc, event_code, duration)); + return InvokeOnWorker( + RTC_FROM_HERE, + Bind(&VoiceChannel::InsertDtmf_w, this, ssrc, event_code, duration)); } bool VoiceChannel::SetOutputVolume(uint32_t ssrc, double volume) { - return InvokeOnWorker(RTC_FROM_HERE, Bind(&VoiceMediaChannel::SetOutputVolume, - media_channel(), ssrc, volume)); + return InvokeOnWorker( + RTC_FROM_HERE, + Bind(&VoiceMediaChannel::SetOutputVolume, media_channel(), ssrc, volume)); } void VoiceChannel::SetRawAudioSink( @@ -1511,8 +1516,8 @@ void VoiceChannel::SetRawAudioSink( // We need to work around Bind's lack of support for unique_ptr and ownership // passing. So we invoke to our own little routine that gets a pointer to // our local variable. This is OK since we're synchronously invoking. - InvokeOnWorker(RTC_FROM_HERE, - Bind(&SetRawAudioSink_w, media_channel(), ssrc, &sink)); + InvokeOnWorker(RTC_FROM_HERE, + Bind(&SetRawAudioSink_w, media_channel(), ssrc, &sink)); } webrtc::RtpParameters VoiceChannel::GetRtpSendParameters(uint32_t ssrc) const { @@ -1528,7 +1533,7 @@ webrtc::RtpParameters VoiceChannel::GetRtpSendParameters_w( bool VoiceChannel::SetRtpSendParameters( uint32_t ssrc, const webrtc::RtpParameters& parameters) { - return InvokeOnWorker( + return InvokeOnWorker( RTC_FROM_HERE, Bind(&VoiceChannel::SetRtpSendParameters_w, this, ssrc, parameters)); } @@ -1553,7 +1558,7 @@ webrtc::RtpParameters VoiceChannel::GetRtpReceiveParameters_w( bool VoiceChannel::SetRtpReceiveParameters( uint32_t ssrc, const webrtc::RtpParameters& parameters) { - return InvokeOnWorker( + return InvokeOnWorker( RTC_FROM_HERE, Bind(&VoiceChannel::SetRtpReceiveParameters_w, this, ssrc, parameters)); } @@ -1564,8 +1569,8 @@ bool VoiceChannel::SetRtpReceiveParameters_w(uint32_t ssrc, } bool VoiceChannel::GetStats(VoiceMediaInfo* stats) { - return InvokeOnWorker(RTC_FROM_HERE, Bind(&VoiceMediaChannel::GetStats, - media_channel(), stats)); + return InvokeOnWorker(RTC_FROM_HERE, Bind(&VoiceMediaChannel::GetStats, + media_channel(), stats)); } std::vector VoiceChannel::GetSources(uint32_t ssrc) const { @@ -1882,9 +1887,9 @@ bool VideoChannel::SetVideoSend( bool mute, const VideoOptions* options, rtc::VideoSourceInterface* source) { - return InvokeOnWorker(RTC_FROM_HERE, - Bind(&VideoMediaChannel::SetVideoSend, media_channel(), - ssrc, mute, options, source)); + return InvokeOnWorker( + RTC_FROM_HERE, Bind(&VideoMediaChannel::SetVideoSend, media_channel(), + ssrc, mute, options, source)); } webrtc::RtpParameters VideoChannel::GetRtpSendParameters(uint32_t ssrc) const { @@ -1900,7 +1905,7 @@ webrtc::RtpParameters VideoChannel::GetRtpSendParameters_w( bool VideoChannel::SetRtpSendParameters( uint32_t ssrc, const webrtc::RtpParameters& parameters) { - return InvokeOnWorker( + return InvokeOnWorker( RTC_FROM_HERE, Bind(&VideoChannel::SetRtpSendParameters_w, this, ssrc, parameters)); } @@ -1925,7 +1930,7 @@ webrtc::RtpParameters VideoChannel::GetRtpReceiveParameters_w( bool VideoChannel::SetRtpReceiveParameters( uint32_t ssrc, const webrtc::RtpParameters& parameters) { - return InvokeOnWorker( + return InvokeOnWorker( RTC_FROM_HERE, Bind(&VideoChannel::SetRtpReceiveParameters_w, this, ssrc, parameters)); } @@ -1947,9 +1952,14 @@ void VideoChannel::UpdateMediaSendRecvState_w() { LOG(LS_INFO) << "Changing video state, send=" << send; } +void VideoChannel::FillBitrateInfo(BandwidthEstimationInfo* bwe_info) { + InvokeOnWorker(RTC_FROM_HERE, Bind(&VideoMediaChannel::FillBitrateInfo, + media_channel(), bwe_info)); +} + bool VideoChannel::GetStats(VideoMediaInfo* stats) { - return InvokeOnWorker(RTC_FROM_HERE, Bind(&VideoMediaChannel::GetStats, - media_channel(), stats)); + return InvokeOnWorker(RTC_FROM_HERE, Bind(&VideoMediaChannel::GetStats, + media_channel(), stats)); } void VideoChannel::StartMediaMonitor(int cms) { @@ -2139,7 +2149,7 @@ bool RtpDataChannel::Init_w( bool RtpDataChannel::SendData(const SendDataParams& params, const rtc::CopyOnWriteBuffer& payload, SendDataResult* result) { - return InvokeOnWorker( + return InvokeOnWorker( RTC_FROM_HERE, Bind(&DataMediaChannel::SendData, media_channel(), params, payload, result)); } diff --git a/webrtc/pc/channel.h b/webrtc/pc/channel.h index 0abdaf2b2a..35369d6f60 100644 --- a/webrtc/pc/channel.h +++ b/webrtc/pc/channel.h @@ -351,11 +351,10 @@ class BaseChannel virtual void OnConnectionMonitorUpdate(ConnectionMonitor* monitor, const std::vector& infos) = 0; - // Helper function for invoking bool-returning methods on the worker thread. - template - bool InvokeOnWorker(const rtc::Location& posted_from, - const FunctorT& functor) { - return worker_thread_->Invoke(posted_from, functor); + // Helper function template for invoking methods on the worker thread. + template + T InvokeOnWorker(const rtc::Location& posted_from, const FunctorT& functor) { + return worker_thread_->Invoke(posted_from, functor); } void AddHandledPayloadType(int payload_type); @@ -554,6 +553,7 @@ class VideoChannel : public BaseChannel { bool SetSink(uint32_t ssrc, rtc::VideoSinkInterface* sink); + void FillBitrateInfo(BandwidthEstimationInfo* bwe_info); // Get statistics about the current media session. bool GetStats(VideoMediaInfo* stats); diff --git a/webrtc/pc/rtcstatscollector.cc b/webrtc/pc/rtcstatscollector.cc index d9da0729ed..ca995327d5 100644 --- a/webrtc/pc/rtcstatscollector.cc +++ b/webrtc/pc/rtcstatscollector.cc @@ -652,9 +652,16 @@ void RTCStatsCollector::GetStatsReport( // implemented to invoke on the signaling thread. track_to_id_ = PrepareTrackToID_s(); - invoker_.AsyncInvoke(RTC_FROM_HERE, network_thread_, + // Prepare |call_stats_| here since GetCallStats() will hop to the worker + // thread. + // TODO(holmer): To avoid the hop we could move BWE and BWE stats to the + // network thread, where it more naturally belongs. + call_stats_ = pc_->session()->GetCallStats(); + + invoker_.AsyncInvoke( + RTC_FROM_HERE, network_thread_, rtc::Bind(&RTCStatsCollector::ProducePartialResultsOnNetworkThread, - rtc::scoped_refptr(this), timestamp_us)); + rtc::scoped_refptr(this), timestamp_us)); ProducePartialResultsOnSignalingThread(timestamp_us); } } @@ -704,9 +711,9 @@ void RTCStatsCollector::ProducePartialResultsOnNetworkThread( timestamp_us, transport_cert_stats, report.get()); ProduceCodecStats_n( timestamp_us, *track_media_info_map_, report.get()); - ProduceIceCandidateAndPairStats_n( - timestamp_us, *session_stats, track_media_info_map_->video_media_info(), - report.get()); + ProduceIceCandidateAndPairStats_n(timestamp_us, *session_stats, + track_media_info_map_->video_media_info(), + call_stats_, report.get()); ProduceRTPStreamStats_n( timestamp_us, *session_stats, *track_media_info_map_, report.get()); ProduceTransportStats_n( @@ -835,9 +842,11 @@ void RTCStatsCollector::ProduceDataChannelStats_s( } void RTCStatsCollector::ProduceIceCandidateAndPairStats_n( - int64_t timestamp_us, const SessionStats& session_stats, - const cricket::VideoMediaInfo* video_media_info, - RTCStatsReport* report) const { + int64_t timestamp_us, + const SessionStats& session_stats, + const cricket::VideoMediaInfo* video_media_info, + const Call::Stats& call_stats, + RTCStatsReport* report) const { RTC_DCHECK(network_thread_->IsCurrent()); for (const auto& transport_stats : session_stats.transport_stats) { for (const auto& channel_stats : transport_stats.second.channel_stats) { @@ -879,24 +888,18 @@ void RTCStatsCollector::ProduceIceCandidateAndPairStats_n( static_cast(*info.current_round_trip_time_ms) / rtc::kNumMillisecsPerSec; } - if (info.best_connection && video_media_info && - !video_media_info->bw_estimations.empty()) { + if (info.best_connection) { // The bandwidth estimations we have are for the selected candidate // pair ("info.best_connection"). - RTC_DCHECK_EQ(video_media_info->bw_estimations.size(), 1); - RTC_DCHECK_GE( - video_media_info->bw_estimations[0].available_send_bandwidth, 0); - RTC_DCHECK_GE( - video_media_info->bw_estimations[0].available_recv_bandwidth, 0); - if (video_media_info->bw_estimations[0].available_send_bandwidth) { + RTC_DCHECK_GE(call_stats.send_bandwidth_bps, 0); + RTC_DCHECK_GE(call_stats.recv_bandwidth_bps, 0); + if (call_stats.send_bandwidth_bps > 0) { candidate_pair_stats->available_outgoing_bitrate = - static_cast(video_media_info->bw_estimations[0] - .available_send_bandwidth); + static_cast(call_stats.send_bandwidth_bps); } - if (video_media_info->bw_estimations[0].available_recv_bandwidth) { + if (call_stats.recv_bandwidth_bps > 0) { candidate_pair_stats->available_incoming_bitrate = - static_cast(video_media_info->bw_estimations[0] - .available_recv_bandwidth); + static_cast(call_stats.recv_bandwidth_bps); } } candidate_pair_stats->requests_received = diff --git a/webrtc/pc/rtcstatscollector.h b/webrtc/pc/rtcstatscollector.h index 48e66baf7e..9dce0fe79e 100644 --- a/webrtc/pc/rtcstatscollector.h +++ b/webrtc/pc/rtcstatscollector.h @@ -26,6 +26,7 @@ #include "webrtc/base/sigslot.h" #include "webrtc/base/sslidentity.h" #include "webrtc/base/timeutils.h" +#include "webrtc/call/call.h" #include "webrtc/media/base/mediachannel.h" #include "webrtc/pc/datachannel.h" #include "webrtc/pc/trackmediainfomap.h" @@ -104,8 +105,10 @@ class RTCStatsCollector : public virtual rtc::RefCountInterface, int64_t timestamp_us, RTCStatsReport* report) const; // Produces |RTCIceCandidatePairStats| and |RTCIceCandidateStats|. void ProduceIceCandidateAndPairStats_n( - int64_t timestamp_us, const SessionStats& session_stats, + int64_t timestamp_us, + const SessionStats& session_stats, const cricket::VideoMediaInfo* video_media_info, + const Call::Stats& call_stats, RTCStatsReport* report) const; // Produces |RTCMediaStreamStats| and |RTCMediaStreamTrackStats|. void ProduceMediaStreamAndTrackStats_s( @@ -154,6 +157,7 @@ class RTCStatsCollector : public virtual rtc::RefCountInterface, std::unique_ptr channel_name_pairs_; std::unique_ptr track_media_info_map_; std::map track_to_id_; + Call::Stats call_stats_; // A timestamp, in microseconds, that is based on a timer that is // monotonically increasing. That is, even if the system clock is modified the diff --git a/webrtc/pc/rtcstatscollector_unittest.cc b/webrtc/pc/rtcstatscollector_unittest.cc index 1940da69da..7b78632126 100644 --- a/webrtc/pc/rtcstatscollector_unittest.cc +++ b/webrtc/pc/rtcstatscollector_unittest.cc @@ -1263,9 +1263,6 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { // Mock the session to return bandwidth estimation info. These should only // be used for a selected candidate pair. cricket::VideoMediaInfo video_media_info; - video_media_info.bw_estimations.push_back(cricket::BandwidthEstimationInfo()); - video_media_info.bw_estimations[0].available_send_bandwidth = 8888; - video_media_info.bw_estimations[0].available_recv_bandwidth = 9999; EXPECT_CALL(*video_media_channel, GetStats(_)) .WillOnce(DoAll(SetArgPointee<0>(video_media_info), Return(true))); EXPECT_CALL(test_->session(), video_channel()) @@ -1345,8 +1342,6 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { .channel_stats[0] .connection_infos[0] .best_connection = true; - video_media_info.bw_estimations[0].available_send_bandwidth = 0; - video_media_info.bw_estimations[0].available_recv_bandwidth = 0; EXPECT_CALL(*video_media_channel, GetStats(_)) .WillOnce(DoAll(SetArgPointee<0>(video_media_info), Return(true))); collector_->ClearCachedStatsReport(); @@ -1360,14 +1355,19 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { EXPECT_TRUE(report->Get(*expected_pair.transport_id)); // Set bandwidth and "GetStats" again. - video_media_info.bw_estimations[0].available_send_bandwidth = 888; - video_media_info.bw_estimations[0].available_recv_bandwidth = 999; + webrtc::Call::Stats call_stats; + const int kSendBandwidth = 888; + call_stats.send_bandwidth_bps = kSendBandwidth; + const int kRecvBandwidth = 999; + call_stats.recv_bandwidth_bps = kRecvBandwidth; + EXPECT_CALL(test_->session(), GetCallStats()) + .WillRepeatedly(Return(call_stats)); EXPECT_CALL(*video_media_channel, GetStats(_)) .WillOnce(DoAll(SetArgPointee<0>(video_media_info), Return(true))); collector_->ClearCachedStatsReport(); report = GetStatsReport(); - expected_pair.available_outgoing_bitrate = 888; - expected_pair.available_incoming_bitrate = 999; + expected_pair.available_outgoing_bitrate = kSendBandwidth; + expected_pair.available_incoming_bitrate = kRecvBandwidth; ASSERT_TRUE(report->Get(expected_pair.id())); EXPECT_EQ( expected_pair, diff --git a/webrtc/pc/statscollector.cc b/webrtc/pc/statscollector.cc index e91f873824..c0b18de6a1 100644 --- a/webrtc/pc/statscollector.cc +++ b/webrtc/pc/statscollector.cc @@ -287,7 +287,6 @@ void ExtractStats(const cricket::VideoSenderInfo& info, StatsReport* report) { void ExtractStats(const cricket::BandwidthEstimationInfo& info, double stats_gathering_started, - PeerConnectionInterface::StatsOutputLevel level, StatsReport* report) { RTC_DCHECK(report->type() == StatsReport::kStatsReportTypeBwe); @@ -506,6 +505,7 @@ StatsCollector::UpdateStats(PeerConnectionInterface::StatsOutputLevel level) { // since we'd be creating/updating the stats report objects consistently on // the same thread (this class has no locks right now). ExtractSessionInfo(); + ExtractBweInfo(); ExtractVoiceInfo(); ExtractVideoInfo(level); ExtractSenderInfo(); @@ -767,6 +767,28 @@ void StatsCollector::ExtractSessionInfo() { } } +void StatsCollector::ExtractBweInfo() { + RTC_DCHECK(pc_->session()->signaling_thread()->IsCurrent()); + + if (pc_->session()->state() == WebRtcSession::State::STATE_CLOSED) + return; + + webrtc::Call::Stats call_stats = pc_->session()->GetCallStats(); + cricket::BandwidthEstimationInfo bwe_info; + bwe_info.available_send_bandwidth = call_stats.send_bandwidth_bps; + bwe_info.available_recv_bandwidth = call_stats.recv_bandwidth_bps; + bwe_info.bucket_delay = call_stats.pacer_delay_ms; + // Fill in target encoder bitrate, actual encoder bitrate, rtx bitrate, etc. + // TODO(holmer): Also fill this in for audio. + if (!pc_->session()->video_channel()) { + return; + } + pc_->session()->video_channel()->FillBitrateInfo(&bwe_info); + StatsReport::Id report_id(StatsReport::NewBandwidthEstimationId()); + StatsReport* report = reports_.FindOrAddNew(report_id); + ExtractStats(bwe_info, stats_gathering_started_, report); +} + void StatsCollector::ExtractVoiceInfo() { RTC_DCHECK(pc_->session()->signaling_thread()->IsCurrent()); @@ -827,14 +849,6 @@ void StatsCollector::ExtractVideoInfo( StatsReport::kReceive); ExtractStatsFromList(video_info.senders, transport_id, this, StatsReport::kSend); - if (video_info.bw_estimations.size() != 1) { - LOG(LS_ERROR) << "BWEs count: " << video_info.bw_estimations.size(); - } else { - StatsReport::Id report_id(StatsReport::NewBandwidthEstimationId()); - StatsReport* report = reports_.FindOrAddNew(report_id); - ExtractStats( - video_info.bw_estimations[0], stats_gathering_started_, level, report); - } } void StatsCollector::ExtractSenderInfo() { diff --git a/webrtc/pc/statscollector.h b/webrtc/pc/statscollector.h index bf895ed811..955bfddf5a 100644 --- a/webrtc/pc/statscollector.h +++ b/webrtc/pc/statscollector.h @@ -110,6 +110,7 @@ class StatsCollector { void ExtractDataInfo(); void ExtractSessionInfo(); + void ExtractBweInfo(); void ExtractVoiceInfo(); void ExtractVideoInfo(PeerConnectionInterface::StatsOutputLevel level); void ExtractSenderInfo(); diff --git a/webrtc/pc/statscollector_unittest.cc b/webrtc/pc/statscollector_unittest.cc index 7d75f65302..e9a2c5c9d5 100644 --- a/webrtc/pc/statscollector_unittest.cc +++ b/webrtc/pc/statscollector_unittest.cc @@ -938,12 +938,15 @@ TEST_F(StatsCollectorTest, BandwidthEstimationInfoIsReported) { video_sender_info.add_ssrc(1234); video_sender_info.bytes_sent = kBytesSent; stats_read.senders.push_back(video_sender_info); - cricket::BandwidthEstimationInfo bwe; - const int kTargetEncBitrate = 123456; - const std::string kTargetEncBitrateString("123456"); - bwe.target_enc_bitrate = kTargetEncBitrate; - stats_read.bw_estimations.push_back(bwe); + webrtc::Call::Stats call_stats; + const int kSendBandwidth = 1234567; + const int kRecvBandwidth = 12345678; + const int kPacerDelay = 123; + call_stats.send_bandwidth_bps = kSendBandwidth; + call_stats.recv_bandwidth_bps = kRecvBandwidth; + call_stats.pacer_delay_ms = kPacerDelay; + EXPECT_CALL(session_, GetCallStats()).WillRepeatedly(Return(call_stats)); EXPECT_CALL(session_, video_channel()).WillRepeatedly(Return(&video_channel)); EXPECT_CALL(session_, voice_channel()).WillRepeatedly(ReturnNull()); EXPECT_CALL(*media_channel, GetStats(_)) @@ -954,9 +957,15 @@ TEST_F(StatsCollectorTest, BandwidthEstimationInfoIsReported) { std::string result = ExtractSsrcStatsValue(reports, StatsReport::kStatsValueNameBytesSent); EXPECT_EQ(kBytesSentString, result); - result = ExtractBweStatsValue(reports, - StatsReport::kStatsValueNameTargetEncBitrate); - EXPECT_EQ(kTargetEncBitrateString, result); + result = ExtractBweStatsValue( + reports, StatsReport::kStatsValueNameAvailableSendBandwidth); + EXPECT_EQ(rtc::ToString(kSendBandwidth), result); + result = ExtractBweStatsValue( + reports, StatsReport::kStatsValueNameAvailableReceiveBandwidth); + EXPECT_EQ(rtc::ToString(kRecvBandwidth), result); + result = + ExtractBweStatsValue(reports, StatsReport::kStatsValueNameBucketDelay); + EXPECT_EQ(rtc::ToString(kPacerDelay), result); } // This test verifies that an object of type "googSession" always diff --git a/webrtc/pc/test/mock_webrtcsession.h b/webrtc/pc/test/mock_webrtcsession.h index 999c692de8..75e3b87c19 100644 --- a/webrtc/pc/test/mock_webrtcsession.h +++ b/webrtc/pc/test/mock_webrtcsession.h @@ -50,6 +50,7 @@ class MockWebRtcSession : public webrtc::WebRtcSession { // track. MOCK_METHOD2(GetLocalTrackIdBySsrc, bool(uint32_t, std::string*)); MOCK_METHOD2(GetRemoteTrackIdBySsrc, bool(uint32_t, std::string*)); + MOCK_METHOD0(GetCallStats, Call::Stats()); MOCK_METHOD1(GetStats, std::unique_ptr(const ChannelNamePairs&)); MOCK_METHOD2(GetLocalCertificate, diff --git a/webrtc/pc/webrtcsession.cc b/webrtc/pc/webrtcsession.cc index cc424b222e..1134c24939 100644 --- a/webrtc/pc/webrtcsession.cc +++ b/webrtc/pc/webrtcsession.cc @@ -631,6 +631,7 @@ bool WebRtcSession::Initialize( void WebRtcSession::Close() { SetState(STATE_CLOSED); RemoveUnusedChannels(nullptr); + call_ = nullptr; RTC_DCHECK(!voice_channel_); RTC_DCHECK(!video_channel_); RTC_DCHECK(!rtp_data_channel_); @@ -1895,6 +1896,15 @@ bool WebRtcSession::CreateDataChannel(const cricket::ContentInfo* content, return true; } +Call::Stats WebRtcSession::GetCallStats() { + if (!worker_thread()->IsCurrent()) { + return worker_thread()->Invoke( + RTC_FROM_HERE, rtc::Bind(&WebRtcSession::GetCallStats, this)); + } + RTC_DCHECK(call_); + return call_->GetStats(); +} + std::unique_ptr WebRtcSession::GetStats_n( const ChannelNamePairs& channel_name_pairs) { RTC_DCHECK(network_thread()->IsCurrent()); @@ -2317,6 +2327,7 @@ void WebRtcSession::ReportNegotiatedCiphers( void WebRtcSession::OnSentPacket_w(const rtc::SentPacket& sent_packet) { RTC_DCHECK(worker_thread()->IsCurrent()); + RTC_DCHECK(call_); call_->OnSentPacket(sent_packet); } diff --git a/webrtc/pc/webrtcsession.h b/webrtc/pc/webrtcsession.h index dd8398e933..cb5b69a152 100644 --- a/webrtc/pc/webrtcsession.h +++ b/webrtc/pc/webrtcsession.h @@ -23,7 +23,7 @@ #include "webrtc/base/sigslot.h" #include "webrtc/base/sslidentity.h" #include "webrtc/base/thread.h" -#include "webrtc/media/base/mediachannel.h" +#include "webrtc/call/call.h" #include "webrtc/p2p/base/candidate.h" #include "webrtc/p2p/base/transportcontroller.h" #include "webrtc/pc/datachannel.h" @@ -294,6 +294,8 @@ class WebRtcSession : void RemoveSctpDataStream(int sid) override; bool ReadyToSendData() const override; + virtual Call::Stats GetCallStats(); + // Returns stats for all channels of all transports. // This avoids exposing the internal structures used to track them. // The parameterless version creates |ChannelNamePairs| from |voice_channel|,