Reland "Improve structuring of test for audio glitches."
This reverts commit 3ae09f541900f18a8b680e70372f7f1d5e06bd0a.
Reason for revert: Revert didn't actually stabilize test. Decreasing # of rounds instead.
Original change's description:
> Revert "Improve structuring of test for audio glitches."
>
> This reverts commit fdbaeda00362a385de85b4c08aa0b536062a8415.
>
> Reason for revert: Breaks downstream project, see https://bugs.chromium.org/p/webrtc/issues/detail?id=12371
>
> Original change's description:
> > Improve structuring of test for audio glitches.
> >
> > Bug: webrtc:12361
> > Change-Id: Ieddc3dafbb638b3bd73dd79bcafa499290fa4340
> > Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/201723
> > Reviewed-by: Henrik Boström <hbos@webrtc.org>
> > Commit-Queue: Harald Alvestrand <hta@webrtc.org>
> > Cr-Commit-Position: refs/heads/master@{#32973}
>
> TBR=hbos@webrtc.org,hta@webrtc.org
>
> Change-Id: Ie337de79a80113958607a7508d136c05fe6d9167
> No-Presubmit: true
> No-Tree-Checks: true
> No-Try: true
> Bug: webrtc:12361
> Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/202024
> Reviewed-by: Alex Loiko <aleloi@webrtc.org>
> Commit-Queue: Alex Loiko <aleloi@webrtc.org>
> Cr-Commit-Position: refs/heads/master@{#32993}
TBR=aleloi@webrtc.org,hbos@webrtc.org,hta@webrtc.org
# Not skipping CQ checks because original CL landed > 1 day ago.
Bug: webrtc:12361
Change-Id: Ice79f2144d76bd7576cb415538afdd210625cc4c
Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/202247
Reviewed-by: Harald Alvestrand <hta@webrtc.org>
Reviewed-by: Alex Loiko <aleloi@webrtc.org>
Reviewed-by: Henrik Boström <hbos@webrtc.org>
Commit-Queue: Harald Alvestrand <hta@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#33019}
diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc
index 6cf3b05..16cd16f 100644
--- a/pc/peer_connection_integrationtest.cc
+++ b/pc/peer_connection_integrationtest.cc
@@ -600,6 +600,46 @@
webrtc::CreateSessionDescription(SdpType::kRollback, ""));
}
+ // Functions for querying stats.
+ void StartWatchingDelayStats() {
+ // Get the baseline numbers for audio_packets and audio_delay.
+ auto received_stats = NewGetStats();
+ auto track_stats =
+ received_stats->GetStatsOfType<webrtc::RTCMediaStreamTrackStats>()[0];
+ ASSERT_TRUE(track_stats->relative_packet_arrival_delay.is_defined());
+ auto rtp_stats =
+ received_stats->GetStatsOfType<webrtc::RTCInboundRTPStreamStats>()[0];
+ ASSERT_TRUE(rtp_stats->packets_received.is_defined());
+ ASSERT_TRUE(rtp_stats->track_id.is_defined());
+ audio_track_stats_id_ = track_stats->id();
+ ASSERT_TRUE(received_stats->Get(audio_track_stats_id_));
+ rtp_stats_id_ = rtp_stats->id();
+ ASSERT_EQ(audio_track_stats_id_, *rtp_stats->track_id);
+ audio_packets_stat_ = *rtp_stats->packets_received;
+ audio_delay_stat_ = *track_stats->relative_packet_arrival_delay;
+ }
+
+ void UpdateDelayStats(std::string tag, int desc_size) {
+ auto report = NewGetStats();
+ auto track_stats =
+ report->GetAs<webrtc::RTCMediaStreamTrackStats>(audio_track_stats_id_);
+ ASSERT_TRUE(track_stats);
+ auto rtp_stats =
+ report->GetAs<webrtc::RTCInboundRTPStreamStats>(rtp_stats_id_);
+ ASSERT_TRUE(rtp_stats);
+ auto delta_packets = *rtp_stats->packets_received - audio_packets_stat_;
+ auto delta_rpad =
+ *track_stats->relative_packet_arrival_delay - audio_delay_stat_;
+ auto recent_delay = delta_packets > 0 ? delta_rpad / delta_packets : -1;
+ // An average relative packet arrival delay over the renegotiation of
+ // > 100 ms indicates that something is dramatically wrong, and will impact
+ // quality for sure.
+ ASSERT_GT(0.1, recent_delay) << tag << " size " << desc_size;
+ // Increment trailing counters
+ audio_packets_stat_ = *rtp_stats->packets_received;
+ audio_delay_stat_ = *track_stats->relative_packet_arrival_delay;
+ }
+
private:
explicit PeerConnectionWrapper(const std::string& debug_name)
: debug_name_(debug_name) {}
@@ -1068,6 +1108,12 @@
peer_connection_signaling_state_history_;
webrtc::FakeRtcEventLogFactory* event_log_factory_;
+ // Variables for tracking delay stats on an audio track
+ int audio_packets_stat_ = 0;
+ double audio_delay_stat_ = 0.0;
+ std::string rtp_stats_id_;
+ std::string audio_track_stats_id_;
+
rtc::AsyncInvoker invoker_;
friend class PeerConnectionIntegrationBaseTest;
@@ -1233,7 +1279,7 @@
}
~PeerConnectionIntegrationBaseTest() {
- // The PeerConnections should deleted before the TurnCustomizers.
+ // The PeerConnections should be deleted before the TurnCustomizers.
// A TurnPort is created with a raw pointer to a TurnCustomizer. The
// TurnPort has the same lifetime as the PeerConnection, so it's expected
// that the TurnCustomizer outlives the life of the PeerConnection or else
@@ -5474,7 +5520,8 @@
// Add more tracks until we get close to having issues.
// Issues have been seen at:
// - 32 tracks on android_arm64_rel and android_arm_dbg bots
- while (current_size < 16) {
+ // - 16 tracks on android_arm_dbg (flaky)
+ while (current_size < 8) {
// Double the number of tracks
for (int i = 0; i < current_size; i++) {
caller()->pc()->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
@@ -5535,6 +5582,7 @@
ASSERT_TRUE(CreatePeerConnectionWrappersWithConfig(config, config));
ConnectFakeSignaling();
caller()->AddAudioTrack();
+ callee()->AddAudioTrack();
caller()->CreateAndSetAndSignalOffer();
ASSERT_TRUE_WAIT(SignalingStateStable(), kDefaultTimeout);
// Wait until we can see the audio flowing.
@@ -5542,26 +5590,15 @@
media_expectations.CalleeExpectsSomeAudio();
ASSERT_TRUE(ExpectNewFrames(media_expectations));
- // Get the baseline numbers for audio_packets and audio_delay.
- auto received_stats = callee()->NewGetStats();
- auto track_stats =
- received_stats->GetStatsOfType<webrtc::RTCMediaStreamTrackStats>()[0];
- ASSERT_TRUE(track_stats->relative_packet_arrival_delay.is_defined());
- auto rtp_stats =
- received_stats->GetStatsOfType<webrtc::RTCInboundRTPStreamStats>()[0];
- ASSERT_TRUE(rtp_stats->packets_received.is_defined());
- ASSERT_TRUE(rtp_stats->track_id.is_defined());
- auto audio_track_stats_id = track_stats->id();
- ASSERT_TRUE(received_stats->Get(audio_track_stats_id));
- auto rtp_stats_id = rtp_stats->id();
- ASSERT_EQ(audio_track_stats_id, *rtp_stats->track_id);
- auto audio_packets = *rtp_stats->packets_received;
- auto audio_delay = *track_stats->relative_packet_arrival_delay;
+ // Get the baseline numbers for audio_packets and audio_delay
+ // in both directions.
+ caller()->StartWatchingDelayStats();
+ callee()->StartWatchingDelayStats();
int current_size = caller()->pc()->GetTransceivers().size();
// Add more tracks until we get close to having issues.
// Making this number very large makes the test very slow.
- while (current_size < 32) {
+ while (current_size < 16) {
// Double the number of tracks
for (int i = 0; i < current_size; i++) {
caller()->pc()->AddTransceiver(cricket::MEDIA_TYPE_VIDEO);
@@ -5578,22 +5615,8 @@
ASSERT_GT(5000, elapsed_time_ms)
<< "Video transceivers: Negotiation took too long after "
<< current_size << " tracks added";
- auto report = callee()->NewGetStats();
- track_stats =
- report->GetAs<webrtc::RTCMediaStreamTrackStats>(audio_track_stats_id);
- ASSERT_TRUE(track_stats);
- rtp_stats = report->GetAs<webrtc::RTCInboundRTPStreamStats>(rtp_stats_id);
- ASSERT_TRUE(rtp_stats);
- auto delta_packets = *rtp_stats->packets_received - audio_packets;
- auto delta_rpad = *track_stats->relative_packet_arrival_delay - audio_delay;
- auto recent_delay = delta_packets > 0 ? delta_rpad / delta_packets : -1;
- // An average relative packet arrival delay over the renegotiation of
- // > 100 ms indicates that something is dramatically wrong, and will impact
- // quality for sure.
- ASSERT_GT(0.1, recent_delay);
- // Increment trailing counters
- audio_packets = *rtp_stats->packets_received;
- audio_delay = *track_stats->relative_packet_arrival_delay;
+ caller()->UpdateDelayStats("caller reception", current_size);
+ callee()->UpdateDelayStats("callee reception", current_size);
}
}
