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}
diff --git a/pc/peerconnection_integrationtest.cc b/pc/peerconnection_integrationtest.cc
index fa99d38..641b2fa 100644
--- a/pc/peerconnection_integrationtest.cc
+++ b/pc/peerconnection_integrationtest.cc
@@ -1988,6 +1988,97 @@
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;
