diff --git a/media/base/mediachannel.h b/media/base/mediachannel.h index 32ab479546..3b78144c03 100644 --- a/media/base/mediachannel.h +++ b/media/base/mediachannel.h @@ -311,11 +311,15 @@ struct MediaSenderInfo { } return retval; } + // Returns true if the media has been connected. + bool connected() const { return local_stats.size() > 0; } // Utility accessor for clients that make the assumption only one ssrc // exists per media. // This will eventually go away. + // Call sites that compare this to zero should use connected() instead. + // https://bugs.webrtc.org/8694 uint32_t ssrc() const { - if (local_stats.size() > 0) { + if (connected()) { return local_stats[0].ssrc; } else { return 0; @@ -351,11 +355,15 @@ struct MediaReceiverInfo { } return retval; } + // Returns true if the media has been connected. + bool connected() const { return local_stats.size() > 0; } // Utility accessor for clients that make the assumption only one ssrc // exists per media. // This will eventually go away. + // Call sites that compare this to zero should use connected(); + // https://bugs.webrtc.org/8694 uint32_t ssrc() const { - if (local_stats.size() > 0) { + if (connected()) { return local_stats[0].ssrc; } else { return 0; diff --git a/pc/rtcstatscollector.cc b/pc/rtcstatscollector.cc index 10bb605f49..ff68f37aae 100644 --- a/pc/rtcstatscollector.cc +++ b/pc/rtcstatscollector.cc @@ -510,17 +510,21 @@ void ProduceSenderMediaTrackStats( // gathering at the time of detachment to get accurate stats and timestamps. // https://crbug.com/659137 for (auto const sender : senders) { - // Don't report on tracks before starting to send. - // https://bugs.webrtc.org/8673 - if (sender->ssrc() == 0) - continue; if (sender->media_type() == cricket::MEDIA_TYPE_AUDIO) { AudioTrackInterface* track = static_cast(sender->track().get()); if (!track) continue; - auto voice_sender_info = - track_media_info_map.GetVoiceSenderInfoBySsrc(sender->ssrc()); + cricket::VoiceSenderInfo null_sender_info; + const cricket::VoiceSenderInfo* voice_sender_info = &null_sender_info; + // TODO(hta): Checking on ssrc is not proper. There should be a way + // to see from a sender whether it's connected or not. + // Related to https://crbug.com/8694 (using ssrc 0 to indicate "none") + if (sender->ssrc()) { + voice_sender_info = + track_media_info_map.GetVoiceSenderInfoBySsrc(sender->ssrc()); + } + RTC_CHECK(voice_sender_info) << "No voice sender info for sender with ssrc " << sender->ssrc(); std::unique_ptr audio_track_stats = @@ -532,8 +536,13 @@ void ProduceSenderMediaTrackStats( static_cast(sender->track().get()); if (!track) continue; - auto video_sender_info = - track_media_info_map.GetVideoSenderInfoBySsrc(sender->ssrc()); + cricket::VideoSenderInfo null_sender_info; + const cricket::VideoSenderInfo* video_sender_info = &null_sender_info; + // TODO(hta): Check on state not ssrc when state is available + // Related to https://crbug.com/8694 (using ssrc 0 to indicate "none") + if (sender->ssrc()) + video_sender_info = + track_media_info_map.GetVideoSenderInfoBySsrc(sender->ssrc()); RTC_CHECK(video_sender_info) << "No video sender info for sender with ssrc " << sender->ssrc(); std::unique_ptr video_track_stats = @@ -1019,16 +1028,15 @@ void RTCStatsCollector::ProduceRTPStreamStats_n( // Inbound for (const cricket::VoiceReceiverInfo& voice_receiver_info : track_media_info_map.voice_media_info()->receivers) { - // TODO(nisse): SSRC == 0 currently means none. Delete check when that - // is fixed. - if (voice_receiver_info.ssrc() == 0) + if (!voice_receiver_info.connected()) continue; + std::unique_ptr inbound_audio( new RTCInboundRTPStreamStats(RTCInboundRTPStreamStatsIDFromSSRC( true, voice_receiver_info.ssrc()), timestamp_us)); - SetInboundRTPStreamStatsFromVoiceReceiverInfo( - voice_receiver_info, inbound_audio.get()); + SetInboundRTPStreamStatsFromVoiceReceiverInfo(voice_receiver_info, + inbound_audio.get()); // TODO(hta): This lookup should look for the sender, not the track. rtc::scoped_refptr audio_track = track_media_info_map_->GetAudioTrack(voice_receiver_info); @@ -1045,9 +1053,7 @@ void RTCStatsCollector::ProduceRTPStreamStats_n( // Outbound for (const cricket::VoiceSenderInfo& voice_sender_info : track_media_info_map.voice_media_info()->senders) { - // TODO(nisse): SSRC == 0 currently means none. Delete check when that - // is fixed. - if (voice_sender_info.ssrc() == 0) + if (!voice_sender_info.connected()) continue; std::unique_ptr outbound_audio( new RTCOutboundRTPStreamStats( @@ -1081,7 +1087,7 @@ void RTCStatsCollector::ProduceRTPStreamStats_n( track_media_info_map.video_media_info()->receivers) { // TODO(nisse): SSRC == 0 currently means none. Delete check when that // is fixed. - if (video_receiver_info.ssrc() == 0) + if (!video_receiver_info.connected()) continue; std::unique_ptr inbound_video( new RTCInboundRTPStreamStats( @@ -1105,9 +1111,7 @@ void RTCStatsCollector::ProduceRTPStreamStats_n( // Outbound for (const cricket::VideoSenderInfo& video_sender_info : track_media_info_map.video_media_info()->senders) { - // TODO(nisse): SSRC == 0 currently means none. Delete check when that - // is fixed. - if (video_sender_info.ssrc() == 0) + if (!video_sender_info.connected()) continue; std::unique_ptr outbound_video( new RTCOutboundRTPStreamStats(RTCOutboundRTPStreamStatsIDFromSSRC( diff --git a/pc/rtcstatscollector_unittest.cc b/pc/rtcstatscollector_unittest.cc index b77820fd21..20cfc59de6 100644 --- a/pc/rtcstatscollector_unittest.cc +++ b/pc/rtcstatscollector_unittest.cc @@ -2397,8 +2397,8 @@ TEST_F(RTCStatsCollectorTest, CollectNoStreamRTCOutboundRTPStreamStats_Audio) { // When the PC has not had SetLocalDescription done, tracks all have // SSRC 0, meaning "unconnected". -// We do not report stats on those tracks. https://bugs.webrtc.org/8673 -TEST_F(RTCStatsCollectorTest, StatsNotReportedOnZeroSsrc) { +// In this state, we report on track stats, but not RTP stats. +TEST_F(RTCStatsCollectorTest, StatsReportedOnZeroSsrc) { rtc::scoped_refptr track = CreateFakeTrack(cricket::MEDIA_TYPE_AUDIO, "audioTrack", MediaStreamTrackInterface::kLive); @@ -2410,7 +2410,10 @@ TEST_F(RTCStatsCollectorTest, StatsNotReportedOnZeroSsrc) { rtc::scoped_refptr report = GetStatsReport(); std::vector track_stats = report->GetStatsOfType(); - EXPECT_EQ(0, track_stats.size()); + EXPECT_EQ(1, track_stats.size()); + std::vector rtp_stream_stats = + report->GetStatsOfType(); + EXPECT_EQ(0, rtp_stream_stats.size()); } class RTCStatsCollectorTestWithFakeCollector : public testing::Test {