Return SSRC stats with the old stats API when SSRCs are unsignaled.
This is the simplest possible fix, returning SSRC stats with a missing
track ID instead of returning no SSRC stats at all.
This means calling GetStats with the track selector argument will still not
work in this case.
Bug: webrtc:3342
Change-Id: I6b58fd5ac15b49274d3f1655e78ae36c4575e5fd
Reviewed-on: https://webrtc-review.googlesource.com/82260
Commit-Queue: Taylor Brandstetter <deadbeef@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23667}diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc
index 1e61604..7de0ad6 100644
--- a/pc/peerconnection_integrationtest.cc
+++ b/pc/peerconnection_integrationtest.cc
@@ -2581,6 +2581,27 @@
ASSERT_TRUE(inbound_stream_stats[0]->track_id.is_defined());
}
+// Same as above but for the legacy stats implementation.
+TEST_P(PeerConnectionIntegrationTest,
+ GetStatsForUnsignaledStreamWithOldStatsApi) {
+ ASSERT_TRUE(CreatePeerConnectionWrappers());
+ ConnectFakeSignaling();
+ caller()->AddAudioTrack();
+ // Remove SSRCs and MSIDs from the received offer SDP.
+ callee()->SetReceivedSdpMunger(RemoveSsrcsAndMsids);
+ caller()->CreateAndSetAndSignalOffer();
+ ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
+
+ // Note that, since the old stats implementation associates SSRCs with tracks
+ // using SDP, when SSRCs aren't signaled in SDP these stats won't have an
+ // associated track ID. So we can't use the track "selector" argument.
+ //
+ // Also, we use "EXPECT_TRUE_WAIT" because the stats collector may decide to
+ // return cached stats if not enough time has passed since the last update.
+ EXPECT_TRUE_WAIT(callee()->OldGetStats()->BytesReceived() > 0U,
+ kDefaultTimeout);
+}
+
// Test that we can successfully get the media related stats (audio level
// etc.) for the unsignaled stream.
TEST_P(PeerConnectionIntegrationTest,
diff --git a/pc/statscollector.cc b/pc/statscollector.cc
index fc00189..6c5cdab 100644
--- a/pc/statscollector.cc
+++ b/pc/statscollector.cc
@@ -554,6 +554,9 @@
const double kMinGatherStatsPeriod = 50;
if (stats_gathering_started_ != 0 &&
stats_gathering_started_ + kMinGatherStatsPeriod > time_now) {
+ RTC_LOG(LS_INFO)
+ << "Not updating stats again, since they were updated within "
+ << kMinGatherStatsPeriod << "ms.";
return;
}
stats_gathering_started_ = time_now;
@@ -586,18 +589,15 @@
// Use the ID of the track that is currently mapped to the SSRC, if any.
std::string track_id;
if (!GetTrackIdBySsrc(ssrc, &track_id, direction)) {
- if (!report) {
- // The ssrc is not used by any track or existing report, return NULL
- // in such case to indicate no report is prepared for the ssrc.
- return NULL;
+ // The SSRC is not used by any existing track (or lookup failed since the
+ // SSRC wasn't signaled in SDP). Try copying the track ID from a previous
+ // report: if one exists.
+ if (report) {
+ const StatsReport::Value* v =
+ report->FindValue(StatsReport::kStatsValueNameTrackId);
+ if (v)
+ track_id = v->string_val();
}
-
- // The ssrc is not used by any existing track. Keeps the old track id
- // since we want to report the stats for inactive ssrc.
- const StatsReport::Value* v =
- report->FindValue(StatsReport::kStatsValueNameTrackId);
- if (v)
- track_id = v->string_val();
}
if (!report)
@@ -607,7 +607,9 @@
report->set_timestamp(stats_gathering_started_);
report->AddInt64(StatsReport::kStatsValueNameSsrc, ssrc);
- report->AddString(StatsReport::kStatsValueNameTrackId, track_id);
+ if (!track_id.empty()) {
+ report->AddString(StatsReport::kStatsValueNameTrackId, track_id);
+ }
// Add the mapping of SSRC to transport.
report->AddId(StatsReport::kStatsValueNameTransportId, transport_id);
return report;
