From 5b4c651d67f5249ae68696ed87524bc76526c1ef Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Tue, 28 Feb 2023 08:25:34 +0000 Subject: [PATCH] Add integration test for NACK functionality MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This adds a test that sets the required feedback mechanisms to get NACK configured for video, connects, and then sets packet loss to 100%. The expected result is that the receiver sends NACK; this will cause the test to set packet loss to 0%, so the next NACK sent should get to the sender and cause retransmission. This is explicating a problematic case in splitting media channel. Bug: webrtc:13931 Change-Id: I0c23c4a89953976454d84b0211f0a7545bbb717a Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/293720 Commit-Queue: Harald Alvestrand Reviewed-by: Henrik Boström Cr-Commit-Position: refs/heads/main@{#39412} --- pc/peer_connection_integrationtest.cc | 129 ++++++++++++++++++++++++++ 1 file changed, 129 insertions(+) diff --git a/pc/peer_connection_integrationtest.cc b/pc/peer_connection_integrationtest.cc index 1d1bec3e93..e915ef833b 100644 --- a/pc/peer_connection_integrationtest.cc +++ b/pc/peer_connection_integrationtest.cc @@ -69,9 +69,11 @@ #include "p2p/base/test_turn_server.h" #include "p2p/base/transport_description.h" #include "p2p/base/transport_info.h" +#include "pc/channel.h" #include "pc/media_session.h" #include "pc/peer_connection.h" #include "pc/peer_connection_factory.h" +#include "pc/rtp_transceiver.h" #include "pc/session_description.h" #include "pc/test/fake_periodic_video_source.h" #include "pc/test/integration_test_helpers.h" @@ -3813,6 +3815,133 @@ TEST_P(PeerConnectionIntegrationTest, EndToEndRtpSenderVideoEncoderSelector) { EXPECT_TRUE(ExpectNewFrames(media_expectations)); } +int NacksReceivedCount(PeerConnectionIntegrationWrapper& pc) { + rtc::scoped_refptr report = pc.NewGetStats(); + auto sender_stats = report->GetStatsOfType(); + if (sender_stats.size() != 1) { + ADD_FAILURE(); + return 0; + } + if (!sender_stats[0]->nack_count.is_defined()) { + return 0; + } + return *sender_stats[0]->nack_count; +} + +int NacksSentCount(PeerConnectionIntegrationWrapper& pc) { + rtc::scoped_refptr report = pc.NewGetStats(); + auto receiver_stats = report->GetStatsOfType(); + if (receiver_stats.size() != 1) { + ADD_FAILURE(); + return 0; + } + if (!receiver_stats[0]->nack_count.is_defined()) { + return 0; + } + return *receiver_stats[0]->nack_count; +} + +// Test disabled because it is flaky. +TEST_F(PeerConnectionIntegrationTestUnifiedPlan, + DISABLED_AudioPacketLossCausesNack) { + RTCConfiguration config; + ASSERT_TRUE(CreatePeerConnectionWrappersWithConfig(config, config)); + ConnectFakeSignaling(); + auto audio_transceiver_or_error = + caller()->pc()->AddTransceiver(caller()->CreateLocalAudioTrack()); + ASSERT_TRUE(audio_transceiver_or_error.ok()); + auto send_transceiver = audio_transceiver_or_error.MoveValue(); + // Munge the SDP to include NACK and RRTR on Opus, and remove all other + // codecs. + caller()->SetGeneratedSdpMunger([](cricket::SessionDescription* desc) { + for (ContentInfo& content : desc->contents()) { + cricket::AudioContentDescription* media = + content.media_description()->as_audio(); + std::vector codecs = media->codecs(); + std::vector codecs_out; + for (cricket::AudioCodec codec : codecs) { + if (codec.name == "opus") { + codec.AddFeedbackParam(cricket::FeedbackParam( + cricket::kRtcpFbParamNack, cricket::kParamValueEmpty)); + codec.AddFeedbackParam(cricket::FeedbackParam( + cricket::kRtcpFbParamRrtr, cricket::kParamValueEmpty)); + codecs_out.push_back(codec); + } + } + EXPECT_FALSE(codecs_out.empty()); + media->set_codecs(codecs_out); + } + }); + + caller()->CreateAndSetAndSignalOffer(); + // Check for failure in helpers + ASSERT_FALSE(HasFailure()); + MediaExpectations media_expectations; + media_expectations.CalleeExpectsSomeAudio(1); + ExpectNewFrames(media_expectations); + ASSERT_FALSE(HasFailure()); + + virtual_socket_server()->set_drop_probability(0.2); + + // Wait until callee has sent at least one NACK. + // Note that due to stats caching, this might only be visible 50 ms + // after the nack was in fact sent. + EXPECT_TRUE_WAIT(NacksSentCount(*callee()) > 0, kDefaultTimeout); + ASSERT_FALSE(HasFailure()); + + virtual_socket_server()->set_drop_probability(0.0); + // Wait until caller has received at least one NACK + EXPECT_TRUE_WAIT(NacksReceivedCount(*caller()) > 0, kDefaultTimeout); +} + +TEST_F(PeerConnectionIntegrationTestUnifiedPlan, VideoPacketLossCausesNack) { + RTCConfiguration config; + ASSERT_TRUE(CreatePeerConnectionWrappersWithConfig(config, config)); + ConnectFakeSignaling(); + auto video_transceiver_or_error = + caller()->pc()->AddTransceiver(caller()->CreateLocalVideoTrack()); + ASSERT_TRUE(video_transceiver_or_error.ok()); + auto send_transceiver = video_transceiver_or_error.MoveValue(); + // Munge the SDP to include NACK and RRTR on VP8, and remove all other + // codecs. + caller()->SetGeneratedSdpMunger([](cricket::SessionDescription* desc) { + for (ContentInfo& content : desc->contents()) { + cricket::VideoContentDescription* media = + content.media_description()->as_video(); + std::vector codecs = media->codecs(); + std::vector codecs_out; + for (cricket::VideoCodec codec : codecs) { + if (codec.name == "VP8") { + ASSERT_TRUE(codec.HasFeedbackParam(cricket::FeedbackParam( + cricket::kRtcpFbParamNack, cricket::kParamValueEmpty))); + codecs_out.push_back(codec); + } + } + EXPECT_FALSE(codecs_out.empty()); + media->set_codecs(codecs_out); + } + }); + + caller()->CreateAndSetAndSignalOffer(); + // Check for failure in helpers + ASSERT_FALSE(HasFailure()); + MediaExpectations media_expectations; + media_expectations.CalleeExpectsSomeVideo(1); + ExpectNewFrames(media_expectations); + ASSERT_FALSE(HasFailure()); + + virtual_socket_server()->set_drop_probability(0.2); + + // Wait until callee has sent at least one NACK. + // Note that due to stats caching, this might only be visible 50 ms + // after the nack was in fact sent. + EXPECT_TRUE_WAIT(NacksSentCount(*callee()) > 0, kDefaultTimeout); + ASSERT_FALSE(HasFailure()); + + // Wait until caller has received at least one NACK + EXPECT_TRUE_WAIT(NacksReceivedCount(*caller()) > 0, kDefaultTimeout); +} + } // namespace } // namespace webrtc