Only return stats for the most recent unsignaled audio stream.

The track-level stats are currently implemented in terms of the stream-
level stats. Which is a problem if multiple unsignaled streams map to the
same track (see bug for more details). This CL fixes the problem
partially, but only returning stats for one of the unsignaled streams.
A better solution would be to return stats for both streams, but update
the track-level stats independently somehow. But that would require more
extensive changes, and it's not yet clear how we want to do it.

BUG=webrtc:8158

Review-Url: https://codereview.webrtc.org/3008373002
Cr-Commit-Position: refs/heads/master@{#19907}

media/engine/webrtcvoiceengine.cc

@@ -2265,6 +2265,23 @@ bool WebRtcVoiceMediaChannel::GetStats(VoiceMediaInfo* info) {
   // Get SSRC and stats for each receiver.
   RTC_DCHECK_EQ(info->receivers.size(), 0U);
   for (const auto& stream : recv_streams_) {
+    uint32_t ssrc = stream.first;
+    // When SSRCs are unsignaled, there's only one audio MediaStreamTrack, but
+    // multiple RTP streams can be received over time (if the SSRC changes for
+    // whatever reason). We only want the RTCMediaStreamTrackStats to represent
+    // the stats for the most recent stream (the one whose audio is actually
+    // routed to the MediaStreamTrack), so here we ignore any unsignaled SSRCs
+    // except for the most recent one (last in the vector). This is somewhat of
+    // a hack, and means you don't get *any* stats for these inactive streams,
+    // but it's slightly better than the previous behavior, which was "highest
+    // SSRC wins".
+    // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=8158
+    if (!unsignaled_recv_ssrcs_.empty()) {
+      auto end_it = --unsignaled_recv_ssrcs_.end();
+      if (std::find(unsignaled_recv_ssrcs_.begin(), end_it, ssrc) != end_it) {
+        continue;
+      }
+    }
     webrtc::AudioReceiveStream::Stats stats = stream.second->GetStats();
     VoiceReceiverInfo rinfo;
     rinfo.add_ssrc(stats.remote_ssrc);

pc/peerconnection_integrationtest.cc

@@ -1988,6 +1988,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 50 audio frames (500ms of audio) to be received by the callee.
+  ExpectNewFramesReceivedWithWait(0, 0, 25, 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;
+
+  // 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 25 more audio frames (250ms of audio) to be received, from the new
+  // SSRC.
+  ExpectNewFramesReceivedWithWait(0, 0, 25, 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 25%%. 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.25;
+  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;