diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 1bdd64056f..881c883b4a 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -4388,23 +4388,6 @@ bool PeerConnection::GetSslRole(const std::string& content_name, role); } -// TODO(steveanton): Eventually it'd be nice to store the channels as a single -// vector of BaseChannel pointers instead of separate voice and video channel -// vectors. At that point, this will become a simple getter. -std::vector PeerConnection::Channels() const { - std::vector channels; - if (voice_channel()) { - channels.push_back(voice_channel()); - } - if (video_channel()) { - channels.push_back(video_channel()); - } - if (rtp_data_channel_) { - channels.push_back(rtp_data_channel_); - } - return channels; -} - void PeerConnection::SetSessionError(SessionError error, const std::string& error_desc) { RTC_DCHECK_RUN_ON(signaling_thread()); diff --git a/pc/peerconnection.h b/pc/peerconnection.h index 05a11712d4..617aa37138 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -225,7 +225,7 @@ class PeerConnection : public PeerConnectionInternal, std::vector< rtc::scoped_refptr>> - GetTransceiversForTesting() const override { + GetTransceiversInternal() const override { return transceivers_; } @@ -723,9 +723,6 @@ class PeerConnection : public PeerConnectionInternal, return transport_controller_.get(); } - // Return all managed, non-null channels. - std::vector Channels() const; - // Non-const versions of local_description()/remote_description(), for use // internally. SessionDescriptionInterface* mutable_local_description() { diff --git a/pc/peerconnection_bundle_unittest.cc b/pc/peerconnection_bundle_unittest.cc index 116a598079..616a076643 100644 --- a/pc/peerconnection_bundle_unittest.cc +++ b/pc/peerconnection_bundle_unittest.cc @@ -76,8 +76,7 @@ class PeerConnectionWrapperForBundleTest : public PeerConnectionWrapper { } cricket::VoiceChannel* voice_channel() { - auto transceivers = - GetInternalPeerConnection()->GetTransceiversForTesting(); + auto transceivers = GetInternalPeerConnection()->GetTransceiversInternal(); for (auto transceiver : transceivers) { if (transceiver->media_type() == cricket::MEDIA_TYPE_AUDIO) { return static_cast( @@ -96,8 +95,7 @@ class PeerConnectionWrapperForBundleTest : public PeerConnectionWrapper { } cricket::VideoChannel* video_channel() { - auto transceivers = - GetInternalPeerConnection()->GetTransceiversForTesting(); + auto transceivers = GetInternalPeerConnection()->GetTransceiversInternal(); for (auto transceiver : transceivers) { if (transceiver->media_type() == cricket::MEDIA_TYPE_VIDEO) { return static_cast( diff --git a/pc/peerconnection_ice_unittest.cc b/pc/peerconnection_ice_unittest.cc index c1d7551321..06dd876dd7 100644 --- a/pc/peerconnection_ice_unittest.cc +++ b/pc/peerconnection_ice_unittest.cc @@ -196,7 +196,7 @@ class PeerConnectionIceBaseTest : public ::testing::Test { static_cast*>( pc_wrapper_ptr->pc()); PeerConnection* pc = static_cast(pc_proxy->internal()); - for (auto transceiver : pc->GetTransceiversForTesting()) { + for (auto transceiver : pc->GetTransceiversInternal()) { if (transceiver->media_type() == cricket::MEDIA_TYPE_AUDIO) { cricket::BaseChannel* channel = transceiver->internal()->channel(); if (channel) { diff --git a/pc/peerconnectioninternal.h b/pc/peerconnectioninternal.h index 18a24b8bf5..0f942d2c2c 100644 --- a/pc/peerconnectioninternal.h +++ b/pc/peerconnectioninternal.h @@ -36,14 +36,13 @@ class PeerConnectionInternal : public PeerConnectionInterface { // Returns true if we were the initial offerer. virtual bool initial_offerer() const = 0; - // TODO(steveanton): Remove these. + // TODO(steveanton): Remove these and replace with GetTransceiversInternal. virtual cricket::VoiceChannel* voice_channel() const = 0; virtual cricket::VideoChannel* video_channel() const = 0; - // Exposed for tests. virtual std::vector< rtc::scoped_refptr>> - GetTransceiversForTesting() const = 0; + GetTransceiversInternal() const = 0; // Get the id used as a media stream track's "id" field from ssrc. virtual bool GetLocalTrackIdBySsrc(uint32_t ssrc, std::string* track_id) = 0; diff --git a/pc/statscollector.cc b/pc/statscollector.cc index d81689986d..5154cbc3e3 100644 --- a/pc/statscollector.cc +++ b/pc/statscollector.cc @@ -569,8 +569,7 @@ StatsCollector::UpdateStats(PeerConnectionInterface::StatsOutputLevel level) { // the same thread (this class has no locks right now). ExtractSessionInfo(); ExtractBweInfo(); - ExtractVoiceInfo(); - ExtractVideoInfo(level); + ExtractMediaInfo(); ExtractSenderInfo(); ExtractDataInfo(); UpdateTrackReports(); @@ -846,59 +845,100 @@ void StatsCollector::ExtractBweInfo() { ExtractStats(bwe_info, stats_gathering_started_, report); } -void StatsCollector::ExtractVoiceInfo() { - RTC_DCHECK(pc_->signaling_thread()->IsCurrent()); +namespace { - if (!pc_->voice_channel()) { - return; - } - cricket::VoiceMediaInfo voice_info; - if (!pc_->voice_channel()->GetStats(&voice_info)) { - RTC_LOG(LS_ERROR) << "Failed to get voice channel stats."; - return; +struct VoiceChannelStatsInfo { + std::string transport_name; + cricket::VoiceMediaChannel* voice_media_channel; + cricket::VoiceMediaInfo voice_media_info; +}; + +struct VideoChannelStatsInfo { + std::string transport_name; + cricket::VideoMediaChannel* video_media_channel; + cricket::VideoMediaInfo video_media_info; +}; + +} // namespace + +void StatsCollector::ExtractMediaInfo() { + RTC_DCHECK_RUN_ON(pc_->signaling_thread()); + + std::vector voice_channel_infos; + std::vector video_channel_infos; + + { + rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls; + for (auto transceiver : pc_->GetTransceiversInternal()) { + if (!transceiver->internal()->channel()) { + continue; + } + cricket::MediaType media_type = transceiver->internal()->media_type(); + if (media_type == cricket::MEDIA_TYPE_AUDIO) { + auto* voice_channel = static_cast( + transceiver->internal()->channel()); + voice_channel_infos.emplace_back(); + VoiceChannelStatsInfo& info = voice_channel_infos.back(); + info.transport_name = voice_channel->transport_name(); + info.voice_media_channel = voice_channel->media_channel(); + } else { + RTC_DCHECK_EQ(media_type, cricket::MEDIA_TYPE_VIDEO); + auto* video_channel = static_cast( + transceiver->internal()->channel()); + video_channel_infos.emplace_back(); + VideoChannelStatsInfo& info = video_channel_infos.back(); + info.transport_name = video_channel->transport_name(); + info.video_media_channel = video_channel->media_channel(); + } + } } - // TODO(tommi): The above code should run on the worker thread and post the - // results back to the signaling thread, where we can add data to the reports. + pc_->worker_thread()->Invoke(RTC_FROM_HERE, [&] { + rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls; + for (auto it = voice_channel_infos.begin(); it != voice_channel_infos.end(); + /* incremented manually */) { + if (!it->voice_media_channel->GetStats(&it->voice_media_info)) { + RTC_LOG(LS_ERROR) << "Failed to get voice channel stats"; + it = voice_channel_infos.erase(it); + continue; + } + ++it; + } + for (auto it = video_channel_infos.begin(); it != video_channel_infos.end(); + /* incremented manually */) { + if (!it->video_media_channel->GetStats(&it->video_media_info)) { + RTC_LOG(LS_ERROR) << "Failed to get video channel stats"; + it = video_channel_infos.erase(it); + continue; + } + ++it; + } + }); + rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls; - StatsReport::Id transport_id = - StatsReport::NewComponentId(pc_->voice_channel()->transport_name(), - cricket::ICE_CANDIDATE_COMPONENT_RTP); - - ExtractStatsFromList(voice_info.receivers, transport_id, this, - StatsReport::kReceive); - ExtractStatsFromList(voice_info.senders, transport_id, this, - StatsReport::kSend); - - UpdateStatsFromExistingLocalAudioTracks(voice_info.receivers.size() > 0); -} - -void StatsCollector::ExtractVideoInfo( - PeerConnectionInterface::StatsOutputLevel level) { - RTC_DCHECK(pc_->signaling_thread()->IsCurrent()); - - if (!pc_->video_channel()) { - return; + bool has_remote_audio = false; + for (const auto& info : voice_channel_infos) { + StatsReport::Id transport_id = StatsReport::NewComponentId( + info.transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + ExtractStatsFromList(info.voice_media_info.receivers, transport_id, this, + StatsReport::kReceive); + ExtractStatsFromList(info.voice_media_info.senders, transport_id, this, + StatsReport::kSend); + if (!info.voice_media_info.receivers.empty()) { + has_remote_audio = true; + } } - cricket::VideoMediaInfo video_info; - if (!pc_->video_channel()->GetStats(&video_info)) { - RTC_LOG(LS_ERROR) << "Failed to get video channel stats."; - return; + for (const auto& info : video_channel_infos) { + StatsReport::Id transport_id = StatsReport::NewComponentId( + info.transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP); + ExtractStatsFromList(info.video_media_info.receivers, transport_id, this, + StatsReport::kReceive); + ExtractStatsFromList(info.video_media_info.senders, transport_id, this, + StatsReport::kSend); } - // TODO(tommi): The above code should run on the worker thread and post the - // results back to the signaling thread, where we can add data to the reports. - rtc::Thread::ScopedDisallowBlockingCalls no_blocking_calls; - - StatsReport::Id transport_id = - StatsReport::NewComponentId(pc_->video_channel()->transport_name(), - cricket::ICE_CANDIDATE_COMPONENT_RTP); - - ExtractStatsFromList(video_info.receivers, transport_id, this, - StatsReport::kReceive); - ExtractStatsFromList(video_info.senders, transport_id, this, - StatsReport::kSend); + UpdateStatsFromExistingLocalAudioTracks(has_remote_audio); } void StatsCollector::ExtractSenderInfo() { diff --git a/pc/statscollector.h b/pc/statscollector.h index c171dcae02..681a8f0098 100644 --- a/pc/statscollector.h +++ b/pc/statscollector.h @@ -111,10 +111,8 @@ class StatsCollector { void ExtractDataInfo(); void ExtractSessionInfo(); void ExtractBweInfo(); - void ExtractVoiceInfo(); - void ExtractVideoInfo(PeerConnectionInterface::StatsOutputLevel level); + void ExtractMediaInfo(); void ExtractSenderInfo(); - void BuildSsrcToTransportId(); webrtc::StatsReport* GetReport(const StatsReport::StatsType& type, const std::string& id, StatsReport::Direction direction); diff --git a/pc/test/fakepeerconnectionbase.h b/pc/test/fakepeerconnectionbase.h index 790aa8ff87..b1cee6d738 100644 --- a/pc/test/fakepeerconnectionbase.h +++ b/pc/test/fakepeerconnectionbase.h @@ -249,7 +249,7 @@ class FakePeerConnectionBase : public PeerConnectionInternal { std::vector< rtc::scoped_refptr>> - GetTransceiversForTesting() const override { + GetTransceiversInternal() const override { return {}; } diff --git a/pc/test/fakepeerconnectionforstats.h b/pc/test/fakepeerconnectionforstats.h index 989640a364..d73e734367 100644 --- a/pc/test/fakepeerconnectionforstats.h +++ b/pc/test/fakepeerconnectionforstats.h @@ -116,6 +116,10 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { std::move(voice_media_channel), mid, kDefaultRtcpMuxRequired, kDefaultSrtpRequired); voice_channel_->set_transport_name_for_testing(transport_name); + auto transceiver = RtpTransceiverProxyWithInternal::Create( + signaling_thread_, new RtpTransceiver(cricket::MEDIA_TYPE_AUDIO)); + transceiver->internal()->SetChannel(voice_channel_.get()); + transceivers_.push_back(transceiver); return voice_media_channel_ptr; } @@ -130,6 +134,10 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { std::move(video_media_channel), mid, kDefaultRtcpMuxRequired, kDefaultSrtpRequired); video_channel_->set_transport_name_for_testing(transport_name); + auto transceiver = RtpTransceiverProxyWithInternal::Create( + signaling_thread_, new RtpTransceiver(cricket::MEDIA_TYPE_VIDEO)); + transceiver->internal()->SetChannel(video_channel_.get()); + transceivers_.push_back(transceiver); return video_media_channel_ptr; } @@ -220,6 +228,12 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { return video_channel_.get(); } + std::vector< + rtc::scoped_refptr>> + GetTransceiversInternal() const override { + return transceivers_; + } + bool GetLocalTrackIdBySsrc(uint32_t ssrc, std::string* track_id) override { auto it = local_track_id_by_ssrc_.find(ssrc); if (it != local_track_id_by_ssrc_.end()) { @@ -316,6 +330,9 @@ class FakePeerConnectionForStats : public FakePeerConnectionBase { rtc::scoped_refptr local_streams_; rtc::scoped_refptr remote_streams_; + std::vector< + rtc::scoped_refptr>> + transceivers_; std::vector> senders_; std::vector> receivers_;