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}
This commit is contained in:
Steve Anton 2017-12-15 11:44:48 -08:00 committed by Commit Bot
parent 8ca5a446b9
commit 593e32551c
4 changed files with 31 additions and 23 deletions

View File

@ -207,6 +207,12 @@ class BaseChannel
// 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(); }

View File

@ -76,16 +76,6 @@ std::string RTCTransportStatsIDFromTransportChannel(
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 @@ void RTCStatsCollector::ProducePartialResultsOnNetworkThread(
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::ProducePeerConnectionStats_s(
}
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 @@ void RTCStatsCollector::ProduceRTPStreamStats_n(
}
// 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) {

View File

@ -118,10 +118,11 @@ class RTCStatsCollector : public virtual rtc::RefCountInterface,
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 @@ class RTCStatsCollector : public virtual rtc::RefCountInterface,
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

View File

@ -1807,6 +1807,7 @@ TEST_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Audio) {
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_F(RTCStatsCollectorTest, CollectRTCInboundRTPStreamStats_Video) {
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_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Audio) {
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_F(RTCStatsCollectorTest, CollectRTCOutboundRTPStreamStats_Video) {
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);