Only return stats for the most recent unsignaled audio stream.

webrtc/pc/peerconnection_integrationtest.cc

Index: webrtc/pc/peerconnection_integrationtest.cc
diff --git a/webrtc/pc/peerconnection_integrationtest.cc b/webrtc/pc/peerconnection_integrationtest.cc
index 641be6f7f227f18dd4f5b667bcb5920ff75c3bb4..de2b163603cebd5d619228a1e31fc29aa1c162ec 100644
--- a/webrtc/pc/peerconnection_integrationtest.cc
+++ b/webrtc/pc/peerconnection_integrationtest.cc
@@ -1966,6 +1966,97 @@ TEST_F(PeerConnectionIntegrationTest,
   EXPECT_TRUE(media_stats[audio_index]->audio_level.is_defined());
 }
 
+// Helper for test below.
+void ModifySsrcs(cricket::SessionDescription* desc) {
+  for (ContentInfo& content : desc->contents()) {
+    MediaContentDescription* media_desc =
+        static_cast<MediaContentDescription*>(content.description);
+    for (cricket::StreamParams& stream : media_desc->mutable_streams()) {
+      for (uint32_t& ssrc : stream.ssrcs) {
+        ssrc = rtc::CreateRandomId();
+      }
+    }
+  }
+}
+
+// Test that the "RTCMediaSteamTrackStats"  object is updated correctly when
+// SSRCs are unsignaled, and the SSRC of the received (audio) stream changes.
+// This should result in two "RTCInboundRTPStreamStats", but only one
+// "RTCMediaStreamTrackStats", whose counters go up continuously rather than
+// being reset to 0 once the SSRC change occurs.
+//
+// Regression test for this bug:
+// https://bugs.chromium.org/p/webrtc/issues/detail?id=8158
+//
+// The bug causes the track stats to only represent one of the two streams:
+// whichever one has the higher SSRC. So with this bug, there was a 50% chance
+// that the track stat counters would reset to 0 when the new stream is
+// received, and a 50% chance that they'll stop updating (while
+// "concealed_samples" continues increasing, due to silence being generated for
+// the inactive stream).
+TEST_F(PeerConnectionIntegrationTest,
+       TrackStatsUpdatedCorrectlyWhenUnsignaledSsrcChanges) {
+  ASSERT_TRUE(CreatePeerConnectionWrappers());
+  ConnectFakeSignaling();
+  caller()->AddAudioOnlyMediaStream();
+  // Remove SSRCs and MSIDs from the received offer SDP, simulating an endpoint
+  // that doesn't signal SSRCs (from the callee's perspective).
+  callee()->SetReceivedSdpMunger(RemoveSsrcsAndMsids);
+  caller()->CreateAndSetAndSignalOffer();
+  ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
+  // Wait for 20 audio frames (200ms of audio) to be received by the callee.
+  ExpectNewFramesReceivedWithWait(0, 0, 20, 0, kMaxWaitForFramesMs);
+
+  // Some audio frames were received, so we should have nonzero "samples
+  // received" for the track.
+  rtc::scoped_refptr<const webrtc::RTCStatsReport> report =
+      callee()->NewGetStats();
+  ASSERT_NE(nullptr, report);
+  auto track_stats = report->GetStatsOfType<webrtc::RTCMediaStreamTrackStats>();
+  ASSERT_EQ(1U, track_stats.size());
+  ASSERT_TRUE(track_stats[0]->total_samples_received.is_defined());
+  ASSERT_GT(*track_stats[0]->total_samples_received, 0U);
+  // uint64_t prev_samples_received = *track_stats[0]->total_samples_received;

[the sun, 2017/09/18 18:59:35]

remove commented-out line

[Taylor Brandstetter, 2017/09/20 00:19:46]

Won't compile if I do (unused variable warning). I plan to uncomment the
relevant code here when the issue is fixed fully. See TODO below.

+
+  // Create a new offer and munge it to cause the caller to use a new SSRC.
+  caller()->SetGeneratedSdpMunger(ModifySsrcs);
+  caller()->CreateAndSetAndSignalOffer();
+  ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
+  // Wait for 10 more audio frames (100ms of audio) to be received, from the new
+  // SSRC.
+  ExpectNewFramesReceivedWithWait(0, 0, 10, 0, kMaxWaitForFramesMs);
+
+  report = callee()->NewGetStats();
+  ASSERT_NE(nullptr, report);
+  track_stats = report->GetStatsOfType<webrtc::RTCMediaStreamTrackStats>();
+  ASSERT_EQ(1U, track_stats.size());
+  ASSERT_TRUE(track_stats[0]->total_samples_received.is_defined());
+  // The "total samples received" stat should only be greater than it was
+  // before.
+  // TODO(deadbeef): Uncomment this assertion once the bug is completely fixed.
+  // Right now, the new SSRC will cause the counters to reset to 0.
+  // EXPECT_GT(*track_stats[0]->total_samples_received, prev_samples_received);
+
+  // Additionally, the percentage of concealed samples (samples generated to
+  // conceal packet loss) should be less than 10%. If it's greater, that's a
+  // good sign that we're seeing stats from the old stream that's no longer
+  // receiving packets, and is generating concealed samples of silence.
+  constexpr double kAcceptableConcealedSamplesPercentage = 0.1;
+  ASSERT_TRUE(track_stats[0]->concealed_samples.is_defined());
+  EXPECT_LT(*track_stats[0]->concealed_samples,
+            *track_stats[0]->total_samples_received *
+                kAcceptableConcealedSamplesPercentage);
+
+  // Also ensure that we have two "RTCInboundRTPStreamStats" as expected, as a
+  // sanity check that the SSRC really changed.
+  // TODO(deadbeef): This isn't working right now, because we're not returning
+  // *any* stats for the inactive stream. Uncomment when the bug is completely
+  // fixed.
+  // auto inbound_stream_stats =
+  //     report->GetStatsOfType<webrtc::RTCInboundRTPStreamStats>();
+  // ASSERT_EQ(2U, inbound_stream_stats.size());
+}
+
 // Test that DTLS 1.0 is used if both sides only support DTLS 1.0.
 TEST_F(PeerConnectionIntegrationTest, EndToEndCallWithDtls10) {
   PeerConnectionFactory::Options dtls_10_options;