Generate track stats when SSRC=0
This will generate an all-zeroes track stat when the sender
has not yet been connected (SSRC has not been assigned).
Bug: webrtc:8673
Change-Id: Id59e6941bc87eba6bb33b4d2a8fd808d985052c7
Reviewed-on: https://webrtc-review.googlesource.com/43080
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Fredrik Solenberg <solenberg@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21734}diff --git a/media/base/mediachannel.h b/media/base/mediachannel.h
index 32ab479..3b78144 100644
--- a/media/base/mediachannel.h
+++ b/media/base/mediachannel.h
@@ -311,11 +311,15 @@
}
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 @@
}
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 10bb605..ff68f37 100644
--- a/pc/rtcstatscollector.cc
+++ b/pc/rtcstatscollector.cc
@@ -510,17 +510,21 @@
// 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<AudioTrackInterface*>(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<RTCMediaStreamTrackStats> audio_track_stats =
@@ -532,8 +536,13 @@
static_cast<VideoTrackInterface*>(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<RTCMediaStreamTrackStats> video_track_stats =
@@ -1019,16 +1028,15 @@
// 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<RTCInboundRTPStreamStats> 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<AudioTrackInterface> audio_track =
track_media_info_map_->GetAudioTrack(voice_receiver_info);
@@ -1045,9 +1053,7 @@
// 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<RTCOutboundRTPStreamStats> outbound_audio(
new RTCOutboundRTPStreamStats(
@@ -1081,7 +1087,7 @@
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<RTCInboundRTPStreamStats> inbound_video(
new RTCInboundRTPStreamStats(
@@ -1105,9 +1111,7 @@
// 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<RTCOutboundRTPStreamStats> outbound_video(
new RTCOutboundRTPStreamStats(RTCOutboundRTPStreamStatsIDFromSSRC(
diff --git a/pc/rtcstatscollector_unittest.cc b/pc/rtcstatscollector_unittest.cc
index b77820f..20cfc59 100644
--- a/pc/rtcstatscollector_unittest.cc
+++ b/pc/rtcstatscollector_unittest.cc
@@ -2397,8 +2397,8 @@
// 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<MediaStreamTrackInterface> track =
CreateFakeTrack(cricket::MEDIA_TYPE_AUDIO, "audioTrack",
MediaStreamTrackInterface::kLive);
@@ -2410,7 +2410,10 @@
rtc::scoped_refptr<const RTCStatsReport> report = GetStatsReport();
std::vector<const RTCMediaStreamTrackStats*> track_stats =
report->GetStatsOfType<RTCMediaStreamTrackStats>();
- EXPECT_EQ(0, track_stats.size());
+ EXPECT_EQ(1, track_stats.size());
+ std::vector<const RTCRTPStreamStats*> rtp_stream_stats =
+ report->GetStatsOfType<RTCRTPStreamStats>();
+ EXPECT_EQ(0, rtp_stream_stats.size());
}
class RTCStatsCollectorTestWithFakeCollector : public testing::Test {
