Change RTCStatsCollector to only access channels from signaling thread
Previously, the RTCStatsCollector needed to ask the voice/video
channel for its transport name in order to generate transport
level stats. That would happen on the networking thread which was
unsafe because the voice/video channel could have disappeared in
the duration of the asynchronous thread hop from the signaling
thread to the networking thread. This changes the networking stats
code to check a saved map that tracks the transport name for each
voice/video channel.
Bug: None
Change-Id: I1f03ba8c0526eaa4419f660f18b8b9da62c3f932
Reviewed-on: https://webrtc-review.googlesource.com/33660
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Peter Thatcher <pthatcher@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21332}diff --git a/pc/channel.h b/pc/channel.h
index 73769b0..9e42811 100644
--- a/pc/channel.h
+++ b/pc/channel.h
@@ -207,6 +207,12 @@
// an RtpTransport in a more explicit way.
bool HandlesPayloadType(int payload_type) const;
+ // Used by the RTCStatsCollector tests to set the transport name without
+ // creating RtpTransports.
+ void set_transport_name_for_testing(const std::string& transport_name) {
+ transport_name_ = transport_name;
+ }
+
protected:
virtual MediaChannel* media_channel() const { return media_channel_.get(); }
diff --git a/pc/rtcstatscollector.cc b/pc/rtcstatscollector.cc
index 915aabf..24444f3 100644
--- a/pc/rtcstatscollector.cc
+++ b/pc/rtcstatscollector.cc
@@ -76,16 +76,6 @@
rtc::ToString<>(channel_component);
}
-std::string RTCTransportStatsIDFromBaseChannel(
- const std::map<std::string, std::string>& proxy_to_transport,
- const cricket::BaseChannel& base_channel) {
- auto proxy_it = proxy_to_transport.find(base_channel.content_name());
- if (proxy_it == proxy_to_transport.cend())
- return "";
- return RTCTransportStatsIDFromTransportChannel(
- proxy_it->second, cricket::ICE_CANDIDATE_COMPONENT_RTP);
-}
-
std::string RTCInboundRTPStreamStatsIDFromSSRC(bool audio, uint32_t ssrc) {
return audio ? "RTCInboundRTPAudioStream_" + rtc::ToString<>(ssrc)
: "RTCInboundRTPVideoStream_" + rtc::ToString<>(ssrc);
@@ -753,8 +743,8 @@
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());
+ ProduceRTPStreamStats_n(timestamp_us, *session_stats, *channel_name_pairs_,
+ *track_media_info_map_, report.get());
ProduceTransportStats_n(
timestamp_us, *session_stats, transport_cert_stats, report.get());
}
@@ -993,16 +983,20 @@
}
void RTCStatsCollector::ProduceRTPStreamStats_n(
- int64_t timestamp_us, const SessionStats& session_stats,
+ int64_t timestamp_us,
+ const SessionStats& session_stats,
+ const ChannelNamePairs& channel_name_pairs,
const TrackMediaInfoMap& track_media_info_map,
RTCStatsReport* report) const {
RTC_DCHECK(network_thread_->IsCurrent());
// Audio
if (track_media_info_map.voice_media_info()) {
- std::string transport_id = RTCTransportStatsIDFromBaseChannel(
- session_stats.proxy_to_transport, *pc_->voice_channel());
- RTC_DCHECK(!transport_id.empty());
+ RTC_DCHECK(channel_name_pairs.voice);
+ const std::string& transport_name =
+ (*channel_name_pairs.voice).transport_name;
+ std::string transport_id = RTCTransportStatsIDFromTransportChannel(
+ transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP);
// Inbound
for (const cricket::VoiceReceiverInfo& voice_receiver_info :
track_media_info_map.voice_media_info()->receivers) {
@@ -1062,9 +1056,11 @@
}
// Video
if (track_media_info_map.video_media_info()) {
- std::string transport_id = RTCTransportStatsIDFromBaseChannel(
- session_stats.proxy_to_transport, *pc_->video_channel());
- RTC_DCHECK(!transport_id.empty());
+ RTC_DCHECK(channel_name_pairs.video);
+ const std::string& transport_name =
+ (*channel_name_pairs.video).transport_name;
+ std::string transport_id = RTCTransportStatsIDFromTransportChannel(
+ transport_name, cricket::ICE_CANDIDATE_COMPONENT_RTP);
// Inbound
for (const cricket::VideoReceiverInfo& video_receiver_info :
track_media_info_map.video_media_info()->receivers) {
diff --git a/pc/rtcstatscollector.h b/pc/rtcstatscollector.h
index 6effe73..d73a764 100644
--- a/pc/rtcstatscollector.h
+++ b/pc/rtcstatscollector.h
@@ -118,10 +118,11 @@
void ProducePeerConnectionStats_s(
int64_t timestamp_us, RTCStatsReport* report) const;
// Produces |RTCInboundRTPStreamStats| and |RTCOutboundRTPStreamStats|.
- void ProduceRTPStreamStats_n(
- int64_t timestamp_us, const SessionStats& session_stats,
- const TrackMediaInfoMap& track_media_info_map,
- RTCStatsReport* report) const;
+ void ProduceRTPStreamStats_n(int64_t timestamp_us,
+ const SessionStats& session_stats,
+ const ChannelNamePairs& channel_name_pairs,
+ const TrackMediaInfoMap& track_media_info_map,
+ RTCStatsReport* report) const;
// Produces |RTCTransportStats|.
void ProduceTransportStats_n(
int64_t timestamp_us, const SessionStats& session_stats,
@@ -158,6 +159,7 @@
std::unique_ptr<ChannelNamePairs> channel_name_pairs_;
std::unique_ptr<TrackMediaInfoMap> track_media_info_map_;
std::map<MediaStreamTrackInterface*, std::string> track_to_id_;
+
Call::Stats call_stats_;
// A timestamp, in microseconds, that is based on a timer that is
diff --git a/pc/rtcstatscollector_unittest.cc b/pc/rtcstatscollector_unittest.cc
index 8047dde..c50111c 100644
--- a/pc/rtcstatscollector_unittest.cc
+++ b/pc/rtcstatscollector_unittest.cc
@@ -1807,6 +1807,7 @@
test_->signaling_thread(), test_->media_engine(),
rtc::WrapUnique(voice_media_channel), "VoiceContentName",
kDefaultRtcpMuxRequired, kDefaultSrtpRequired);
+ voice_channel.set_transport_name_for_testing("TransportName");
test_->SetupRemoteTrackAndReceiver(
cricket::MEDIA_TYPE_AUDIO, "RemoteAudioTrackID", 1);
@@ -1886,6 +1887,7 @@
test_->worker_thread(), test_->network_thread(),
test_->signaling_thread(), rtc::WrapUnique(video_media_channel),
"VideoContentName", kDefaultRtcpMuxRequired, kDefaultSrtpRequired);
+ video_channel.set_transport_name_for_testing("TransportName");
test_->SetupRemoteTrackAndReceiver(
cricket::MEDIA_TYPE_VIDEO, "RemoteVideoTrackID", 1);
@@ -1988,6 +1990,7 @@
test_->signaling_thread(), test_->media_engine(),
rtc::WrapUnique(voice_media_channel), "VoiceContentName",
kDefaultRtcpMuxRequired, kDefaultSrtpRequired);
+ voice_channel.set_transport_name_for_testing("TransportName");
test_->SetupLocalTrackAndSender(
cricket::MEDIA_TYPE_AUDIO, "LocalAudioTrackID", 1);
@@ -2065,6 +2068,7 @@
test_->worker_thread(), test_->network_thread(),
test_->signaling_thread(), rtc::WrapUnique(video_media_channel),
"VideoContentName", kDefaultRtcpMuxRequired, kDefaultSrtpRequired);
+ video_channel.set_transport_name_for_testing("TransportName");
test_->SetupLocalTrackAndSender(
cricket::MEDIA_TYPE_VIDEO, "LocalVideoTrackID", 1);
