diff --git a/webrtc/media/base/fakemediaengine.h b/webrtc/media/base/fakemediaengine.h index 4984d1b88c..c0aacfce5c 100644 --- a/webrtc/media/base/fakemediaengine.h +++ b/webrtc/media/base/fakemediaengine.h @@ -604,7 +604,6 @@ 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 8741429fca..dadb55bfdb 100644 --- a/webrtc/media/base/mediachannel.h +++ b/webrtc/media/base/mediachannel.h @@ -862,11 +862,13 @@ struct VideoMediaInfo { void Clear() { senders.clear(); receivers.clear(); + bw_estimations.clear(); send_codecs.clear(); receive_codecs.clear(); } std::vector senders; std::vector receivers; + std::vector bw_estimations; RtpCodecParametersMap send_codecs; RtpCodecParametersMap receive_codecs; }; @@ -1080,15 +1082,6 @@ 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 4f3c283939..03e97f776b 100644 --- a/webrtc/media/engine/webrtcvideoengine2.cc +++ b/webrtc/media/engine/webrtcvideoengine2.cc @@ -1379,9 +1379,8 @@ 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; @@ -1416,13 +1415,22 @@ void WebRtcVideoChannel2::FillReceiverStats(VideoMediaInfo* video_media_info, } } -void WebRtcVideoChannel2::FillBitrateInfo(BandwidthEstimationInfo* bwe_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. rtc::CritScope stream_lock(&stream_crit_); for (std::map::iterator stream = send_streams_.begin(); stream != send_streams_.end(); ++stream) { - stream->second->FillBitrateInfo(bwe_info); + stream->second->FillBandwidthEstimationInfo(&bwe_info); } + video_media_info->bw_estimations.push_back(bwe_info); } void WebRtcVideoChannel2::FillSendAndReceiveCodecStats( @@ -2141,7 +2149,7 @@ VideoSenderInfo WebRtcVideoChannel2::WebRtcVideoSendStream::GetVideoSenderInfo( return info; } -void WebRtcVideoChannel2::WebRtcVideoSendStream::FillBitrateInfo( +void WebRtcVideoChannel2::WebRtcVideoSendStream::FillBandwidthEstimationInfo( 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 56431167a8..394646bc32 100644 --- a/webrtc/media/engine/webrtcvideoengine2.h +++ b/webrtc/media/engine/webrtcvideoengine2.h @@ -160,7 +160,6 @@ 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, @@ -285,7 +284,7 @@ class WebRtcVideoChannel2 : public VideoMediaChannel, public webrtc::Transport { const std::vector& GetSsrcs() const; VideoSenderInfo GetVideoSenderInfo(bool log_stats); - void FillBitrateInfo(BandwidthEstimationInfo* bwe_info); + void FillBandwidthEstimationInfo(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 3730e3dd04..95c37a1775 100644 --- a/webrtc/media/engine/webrtcvideoengine2_unittest.cc +++ b/webrtc/media/engine/webrtcvideoengine2_unittest.cc @@ -3709,19 +3709,17 @@ 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, - bwe_info.target_enc_bitrate); + info.bw_estimations[0].target_enc_bitrate); EXPECT_EQ(stats.media_bitrate_bps + stats2.media_bitrate_bps, - bwe_info.actual_enc_bitrate); - EXPECT_EQ(1 + 3 + 5 + 7, bwe_info.transmit_bitrate) + info.bw_estimations[0].actual_enc_bitrate); + EXPECT_EQ(1 + 3 + 5 + 7, info.bw_estimations[0].transmit_bitrate) << "Bandwidth stats should take all streams into account."; - EXPECT_EQ(2 + 4 + 6 + 8, bwe_info.retransmit_bitrate) + EXPECT_EQ(2 + 4 + 6 + 8, info.bw_estimations[0].retransmit_bitrate) << "Bandwidth stats should take all streams into account."; } diff --git a/webrtc/pc/channel.cc b/webrtc/pc/channel.cc index 86b5d5f5b6..f5428a43ef 100644 --- a/webrtc/pc/channel.cc +++ b/webrtc/pc/channel.cc @@ -443,42 +443,39 @@ 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) { @@ -1508,9 +1505,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 @@ -1530,22 +1527,20 @@ 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( @@ -1554,8 +1549,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 { @@ -1571,7 +1566,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)); } @@ -1596,7 +1591,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)); } @@ -1607,8 +1602,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 { @@ -1927,9 +1922,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 { @@ -1945,7 +1940,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)); } @@ -1970,7 +1965,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)); } @@ -1992,14 +1987,9 @@ 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) { @@ -2189,7 +2179,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 17a83a13bc..48259e5fd9 100644 --- a/webrtc/pc/channel.h +++ b/webrtc/pc/channel.h @@ -350,10 +350,11 @@ class BaseChannel virtual void OnConnectionMonitorUpdate(ConnectionMonitor* monitor, const std::vector& infos) = 0; - // 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); + // 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); } private: @@ -553,7 +554,6 @@ 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 ca995327d5..d9da0729ed 100644 --- a/webrtc/pc/rtcstatscollector.cc +++ b/webrtc/pc/rtcstatscollector.cc @@ -652,16 +652,9 @@ void RTCStatsCollector::GetStatsReport( // implemented to invoke on the signaling thread. track_to_id_ = PrepareTrackToID_s(); - // 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_, + 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); } } @@ -711,9 +704,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(), - call_stats_, report.get()); + ProduceIceCandidateAndPairStats_n( + timestamp_us, *session_stats, track_media_info_map_->video_media_info(), + report.get()); ProduceRTPStreamStats_n( timestamp_us, *session_stats, *track_media_info_map_, report.get()); ProduceTransportStats_n( @@ -842,11 +835,9 @@ void RTCStatsCollector::ProduceDataChannelStats_s( } void RTCStatsCollector::ProduceIceCandidateAndPairStats_n( - int64_t timestamp_us, - const SessionStats& session_stats, - const cricket::VideoMediaInfo* video_media_info, - const Call::Stats& call_stats, - RTCStatsReport* report) const { + int64_t timestamp_us, const SessionStats& session_stats, + const cricket::VideoMediaInfo* video_media_info, + 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) { @@ -888,18 +879,24 @@ void RTCStatsCollector::ProduceIceCandidateAndPairStats_n( static_cast(*info.current_round_trip_time_ms) / rtc::kNumMillisecsPerSec; } - if (info.best_connection) { + if (info.best_connection && video_media_info && + !video_media_info->bw_estimations.empty()) { // The bandwidth estimations we have are for the selected candidate // pair ("info.best_connection"). - 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) { + 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) { candidate_pair_stats->available_outgoing_bitrate = - static_cast(call_stats.send_bandwidth_bps); + static_cast(video_media_info->bw_estimations[0] + .available_send_bandwidth); } - if (call_stats.recv_bandwidth_bps > 0) { + if (video_media_info->bw_estimations[0].available_recv_bandwidth) { candidate_pair_stats->available_incoming_bitrate = - static_cast(call_stats.recv_bandwidth_bps); + static_cast(video_media_info->bw_estimations[0] + .available_recv_bandwidth); } } candidate_pair_stats->requests_received = diff --git a/webrtc/pc/rtcstatscollector.h b/webrtc/pc/rtcstatscollector.h index 9dce0fe79e..48e66baf7e 100644 --- a/webrtc/pc/rtcstatscollector.h +++ b/webrtc/pc/rtcstatscollector.h @@ -26,7 +26,6 @@ #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" @@ -105,10 +104,8 @@ 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( @@ -157,7 +154,6 @@ 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 7b78632126..1940da69da 100644 --- a/webrtc/pc/rtcstatscollector_unittest.cc +++ b/webrtc/pc/rtcstatscollector_unittest.cc @@ -1263,6 +1263,9 @@ 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()) @@ -1342,6 +1345,8 @@ 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(); @@ -1355,19 +1360,14 @@ TEST_F(RTCStatsCollectorTest, CollectRTCIceCandidatePairStats) { EXPECT_TRUE(report->Get(*expected_pair.transport_id)); // Set bandwidth and "GetStats" again. - 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)); + video_media_info.bw_estimations[0].available_send_bandwidth = 888; + video_media_info.bw_estimations[0].available_recv_bandwidth = 999; EXPECT_CALL(*video_media_channel, GetStats(_)) .WillOnce(DoAll(SetArgPointee<0>(video_media_info), Return(true))); collector_->ClearCachedStatsReport(); report = GetStatsReport(); - expected_pair.available_outgoing_bitrate = kSendBandwidth; - expected_pair.available_incoming_bitrate = kRecvBandwidth; + expected_pair.available_outgoing_bitrate = 888; + expected_pair.available_incoming_bitrate = 999; ASSERT_TRUE(report->Get(expected_pair.id())); EXPECT_EQ( expected_pair, diff --git a/webrtc/pc/statscollector.cc b/webrtc/pc/statscollector.cc index c0b18de6a1..e91f873824 100644 --- a/webrtc/pc/statscollector.cc +++ b/webrtc/pc/statscollector.cc @@ -287,6 +287,7 @@ 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); @@ -505,7 +506,6 @@ 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,28 +767,6 @@ 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()); @@ -849,6 +827,14 @@ 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 955bfddf5a..bf895ed811 100644 --- a/webrtc/pc/statscollector.h +++ b/webrtc/pc/statscollector.h @@ -110,7 +110,6 @@ 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 e9a2c5c9d5..7d75f65302 100644 --- a/webrtc/pc/statscollector_unittest.cc +++ b/webrtc/pc/statscollector_unittest.cc @@ -938,15 +938,12 @@ 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(_)) @@ -957,15 +954,9 @@ TEST_F(StatsCollectorTest, BandwidthEstimationInfoIsReported) { std::string result = ExtractSsrcStatsValue(reports, StatsReport::kStatsValueNameBytesSent); EXPECT_EQ(kBytesSentString, 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); + result = ExtractBweStatsValue(reports, + StatsReport::kStatsValueNameTargetEncBitrate); + EXPECT_EQ(kTargetEncBitrateString, 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 75e3b87c19..999c692de8 100644 --- a/webrtc/pc/test/mock_webrtcsession.h +++ b/webrtc/pc/test/mock_webrtcsession.h @@ -50,7 +50,6 @@ 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 1134c24939..cc424b222e 100644 --- a/webrtc/pc/webrtcsession.cc +++ b/webrtc/pc/webrtcsession.cc @@ -631,7 +631,6 @@ 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_); @@ -1896,15 +1895,6 @@ 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()); @@ -2327,7 +2317,6 @@ 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 cb5b69a152..dd8398e933 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/call/call.h" +#include "webrtc/media/base/mediachannel.h" #include "webrtc/p2p/base/candidate.h" #include "webrtc/p2p/base/transportcontroller.h" #include "webrtc/pc/datachannel.h" @@ -294,8 +294,6 @@ 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|,