From d40b0f39e08bf0e4d7427bf73b4e3f86bbeb34be Mon Sep 17 00:00:00 2001 From: brandtr Date: Mon, 6 Feb 2017 05:54:43 -0800 Subject: [PATCH] Improve and re-enable FEC end-to-end tests. These tests got flaky under the new jitter buffer. Enhancements: - Use send-side BWE. - Let BWE ramp up before applying packet loss. - Improve packet loss simulation for ULPFEC. - Add delay to fake network pipe for FlexFEC. (Not added for ULPFEC, since this makes those flaky...?) - Add FlexFEC+NACK test, using RTX instead of "raw retransmits". - Tighter checks of received packets' payload types and SSRCs. TESTED= $ ninja -C out/Debug video_engine_tests && third_party/gtest-parallel/gtest-parallel -r 1000 out/Debug/video_engine_tests --gtest_filter="*EndToEnd*Ulpfec*:*EndToEnd*Flexfec*" ninja: Entering directory `out/Debug' ninja: no work to do. [12000/12000] TestWithNewVideoJitterBuffer/EndToEndTest.RecoversWithFlexfecAndNack/1 (14935 ms) BUG=webrtc:7047 Review-Url: https://codereview.webrtc.org/2675573004 Cr-Commit-Position: refs/heads/master@{#16449} --- webrtc/video/end_to_end_tests.cc | 202 +++++++++++++++++++------------ 1 file changed, 124 insertions(+), 78 deletions(-) diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc index 6812467cb7..1a7525c186 100644 --- a/webrtc/video/end_to_end_tests.cc +++ b/webrtc/video/end_to_end_tests.cc @@ -603,14 +603,14 @@ TEST_P(EndToEndTest, ReceivesNackAndRetransmitsAudio) { RunBaseTest(&test); } -// Disable due to failure, see bugs.webrtc.org/7050 for -// details. -TEST_P(EndToEndTest, DISABLED_CanReceiveUlpfec) { +TEST_P(EndToEndTest, ReceivesUlpfec) { class UlpfecRenderObserver : public test::EndToEndTest, public rtc::VideoSinkInterface { public: UlpfecRenderObserver() - : EndToEndTest(kDefaultTimeoutMs), state_(kFirstPacket) {} + : EndToEndTest(kDefaultTimeoutMs), + random_(0xcafef00d1), + num_packets_sent_(0) {} private: Action OnSendRtp(const uint8_t* packet, size_t length) override { @@ -618,45 +618,36 @@ TEST_P(EndToEndTest, DISABLED_CanReceiveUlpfec) { RTPHeader header; EXPECT_TRUE(parser_->Parse(packet, length, &header)); + EXPECT_TRUE(header.payloadType == kFakeVideoSendPayloadType || + header.payloadType == kRedPayloadType) + << "Unknown payload type received."; + EXPECT_EQ(kVideoSendSsrcs[0], header.ssrc) << "Unknown SSRC received."; + + // Parse RED header. int encapsulated_payload_type = -1; if (header.payloadType == kRedPayloadType) { encapsulated_payload_type = static_cast(packet[header.headerLength]); - if (encapsulated_payload_type != kFakeVideoSendPayloadType) - EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type); - } else { - EXPECT_EQ(kFakeVideoSendPayloadType, header.payloadType); + + EXPECT_TRUE(encapsulated_payload_type == kFakeVideoSendPayloadType || + encapsulated_payload_type == kUlpfecPayloadType) + << "Unknown encapsulated payload type received."; } - if (protected_sequence_numbers_.count(header.sequenceNumber) != 0) { - // Retransmitted packet, should not count. - protected_sequence_numbers_.erase(header.sequenceNumber); - auto ts_it = protected_timestamps_.find(header.timestamp); - EXPECT_NE(ts_it, protected_timestamps_.end()); - protected_timestamps_.erase(ts_it); + // To reduce test flakiness, always let ULPFEC packets through. + if (encapsulated_payload_type == kUlpfecPayloadType) { return SEND_PACKET; } - switch (state_) { - case kFirstPacket: - state_ = kDropEveryOtherPacketUntilUlpfec; - break; - case kDropEveryOtherPacketUntilUlpfec: - if (encapsulated_payload_type == kUlpfecPayloadType) { - state_ = kDropNextMediaPacket; - return SEND_PACKET; - } - if (header.sequenceNumber % 2 == 0) - return DROP_PACKET; - break; - case kDropNextMediaPacket: - if (encapsulated_payload_type == kFakeVideoSendPayloadType) { - protected_sequence_numbers_.insert(header.sequenceNumber); - protected_timestamps_.insert(header.timestamp); - state_ = kDropEveryOtherPacketUntilUlpfec; - return DROP_PACKET; - } - break; + // Simulate 5% video packet loss after rampup period. Record the + // corresponding timestamps that were dropped. + if (num_packets_sent_++ > 100 && random_.Rand(1, 100) <= 5) { + if (encapsulated_payload_type == kFakeVideoSendPayloadType) { + dropped_sequence_numbers_.insert(header.sequenceNumber); + dropped_timestamps_.insert(header.timestamp); + } + + return DROP_PACKET; } return SEND_PACKET; @@ -666,27 +657,23 @@ TEST_P(EndToEndTest, DISABLED_CanReceiveUlpfec) { rtc::CritScope lock(&crit_); // Rendering frame with timestamp of packet that was dropped -> FEC // protection worked. - if (protected_timestamps_.count(video_frame.timestamp()) != 0) + auto it = dropped_timestamps_.find(video_frame.timestamp()); + if (it != dropped_timestamps_.end()) { observation_complete_.Set(); + } } - enum { - kFirstPacket, - kDropEveryOtherPacketUntilUlpfec, - kDropNextMediaPacket, - } state_; - void ModifyVideoConfigs( VideoSendStream::Config* send_config, std::vector* receive_configs, VideoEncoderConfig* encoder_config) override { - // TODO(pbos): Run this test with combined NACK/ULPFEC enabled as well. - // int rtp_history_ms = 1000; - // (*receive_configs)[0].rtp.nack.rtp_history_ms = rtp_history_ms; - // send_config->rtp.nack.rtp_history_ms = rtp_history_ms; + send_config->rtp.extensions.push_back( + RtpExtension(RtpExtension::kTransportSequenceNumberUri, + test::kTransportSequenceNumberExtensionId)); send_config->rtp.ulpfec.red_payload_type = kRedPayloadType; send_config->rtp.ulpfec.ulpfec_payload_type = kUlpfecPayloadType; + (*receive_configs)[0].rtp.transport_cc = true; (*receive_configs)[0].rtp.ulpfec.red_payload_type = kRedPayloadType; (*receive_configs)[0].rtp.ulpfec.ulpfec_payload_type = kUlpfecPayloadType; (*receive_configs)[0].renderer = this; @@ -698,10 +685,11 @@ TEST_P(EndToEndTest, DISABLED_CanReceiveUlpfec) { } rtc::CriticalSection crit_; - std::set protected_sequence_numbers_ GUARDED_BY(crit_); - // Since several packets can have the same timestamp a multiset is used - // instead of a set. - std::multiset protected_timestamps_ GUARDED_BY(crit_); + std::set dropped_sequence_numbers_ GUARDED_BY(crit_); + // Several packets can have the same timestamp. + std::multiset dropped_timestamps_ GUARDED_BY(crit_); + Random random_; + int num_packets_sent_; } test; RunBaseTest(&test); @@ -713,11 +701,13 @@ class FlexfecRenderObserver : public test::EndToEndTest, static constexpr uint32_t kVideoLocalSsrc = 123; static constexpr uint32_t kFlexfecLocalSsrc = 456; - explicit FlexfecRenderObserver(bool expect_flexfec_rtcp) + explicit FlexfecRenderObserver(bool enable_nack, bool expect_flexfec_rtcp) : test::EndToEndTest(test::CallTest::kDefaultTimeoutMs), + enable_nack_(enable_nack), expect_flexfec_rtcp_(expect_flexfec_rtcp), received_flexfec_rtcp_(false), - random_(0xcafef00d1) {} + random_(0xcafef00d1), + num_packets_sent_(0) {} size_t GetNumFlexfecStreams() const override { return 1; } @@ -727,33 +717,55 @@ class FlexfecRenderObserver : public test::EndToEndTest, RTPHeader header; EXPECT_TRUE(parser_->Parse(packet, length, &header)); - uint8_t payload_type = header.payloadType; - if (payload_type != test::CallTest::kFakeVideoSendPayloadType) { - EXPECT_EQ(test::CallTest::kFlexfecPayloadType, payload_type); + EXPECT_TRUE(header.payloadType == + test::CallTest::kFakeVideoSendPayloadType || + header.payloadType == test::CallTest::kFlexfecPayloadType || + (enable_nack_ && + header.payloadType == test::CallTest::kSendRtxPayloadType)) + << "Unknown payload type received."; + EXPECT_TRUE( + header.ssrc == test::CallTest::kVideoSendSsrcs[0] || + header.ssrc == test::CallTest::kFlexfecSendSsrc || + (enable_nack_ && header.ssrc == test::CallTest::kSendRtxSsrcs[0])) + << "Unknown SSRC received."; + + // To reduce test flakiness, always let FlexFEC packets through. + if (header.payloadType == test::CallTest::kFlexfecPayloadType) { + EXPECT_EQ(test::CallTest::kFlexfecSendSsrc, header.ssrc); + + return SEND_PACKET; } - // Is this a retransmitted media packet? From the perspective of FEC, this - // packet is then no longer dropped, so remove it from the list of - // dropped packets. - if (payload_type == test::CallTest::kFakeVideoSendPayloadType) { - auto seq_num_it = dropped_sequence_numbers_.find(header.sequenceNumber); + // To reduce test flakiness, always let RTX packets through. + if (header.payloadType == test::CallTest::kSendRtxPayloadType) { + EXPECT_EQ(test::CallTest::kSendRtxSsrcs[0], header.ssrc); + + // Parse RTX header. + uint16_t original_sequence_number = + ByteReader::ReadBigEndian(&packet[header.headerLength]); + + // From the perspective of FEC, a retransmitted packet is no longer + // dropped, so remove it from list of dropped packets. + auto seq_num_it = + dropped_sequence_numbers_.find(original_sequence_number); if (seq_num_it != dropped_sequence_numbers_.end()) { dropped_sequence_numbers_.erase(seq_num_it); auto ts_it = dropped_timestamps_.find(header.timestamp); EXPECT_NE(ts_it, dropped_timestamps_.end()); dropped_timestamps_.erase(ts_it); - - return SEND_PACKET; } + + return SEND_PACKET; } - // Simulate 5% packet loss. Record what media packets, and corresponding - // timestamps, that were dropped. - if (random_.Rand(1, 100) <= 5) { - if (payload_type == test::CallTest::kFakeVideoSendPayloadType) { - dropped_sequence_numbers_.insert(header.sequenceNumber); - dropped_timestamps_.insert(header.timestamp); - } + // Simulate 5% video packet loss after rampup period. Record the + // corresponding timestamps that were dropped. + if (num_packets_sent_++ > 100 && random_.Rand(1, 100) <= 5) { + EXPECT_EQ(test::CallTest::kFakeVideoSendPayloadType, header.payloadType); + EXPECT_EQ(test::CallTest::kVideoSendSsrcs[0], header.ssrc); + + dropped_sequence_numbers_.insert(header.sequenceNumber); + dropped_timestamps_.insert(header.timestamp); return DROP_PACKET; } @@ -781,6 +793,15 @@ class FlexfecRenderObserver : public test::EndToEndTest, return SEND_PACKET; } + test::PacketTransport* CreateSendTransport(Call* sender_call) override { + // At low RTT (< kLowRttNackMs) -> NACK only, no FEC. + const int kNetworkDelayMs = 100; + FakeNetworkPipe::Config config; + config.queue_delay_ms = kNetworkDelayMs; + return new test::PacketTransport(sender_call, this, + test::PacketTransport::kSender, config); + } + void OnFrame(const VideoFrame& video_frame) override { rtc::CritScope lock(&crit_); // Rendering frame with timestamp of packet that was dropped -> FEC @@ -797,8 +818,31 @@ class FlexfecRenderObserver : public test::EndToEndTest, VideoSendStream::Config* send_config, std::vector* receive_configs, VideoEncoderConfig* encoder_config) override { + send_config->rtp.extensions.push_back( + RtpExtension(RtpExtension::kTransportSequenceNumberUri, + test::kTransportSequenceNumberExtensionId)); + (*receive_configs)[0].rtp.local_ssrc = kVideoLocalSsrc; + (*receive_configs)[0].rtp.transport_cc = true; (*receive_configs)[0].renderer = this; + + if (enable_nack_) { + send_config->rtp.nack.rtp_history_ms = test::CallTest::kNackRtpHistoryMs; + send_config->rtp.ulpfec.red_rtx_payload_type = + test::CallTest::kRtxRedPayloadType; + send_config->rtp.rtx.ssrcs.push_back(test::CallTest::kSendRtxSsrcs[0]); + send_config->rtp.rtx.payload_type = test::CallTest::kSendRtxPayloadType; + + (*receive_configs)[0].rtp.nack.rtp_history_ms = + test::CallTest::kNackRtpHistoryMs; + (*receive_configs)[0].rtp.ulpfec.red_rtx_payload_type = + test::CallTest::kRtxRedPayloadType; + + (*receive_configs)[0].rtp.rtx_ssrc = test::CallTest::kSendRtxSsrcs[0]; + (*receive_configs)[0] + .rtp.rtx_payload_types[test::CallTest::kVideoSendPayloadType] = + test::CallTest::kSendRtxPayloadType; + } } void ModifyFlexfecConfigs( @@ -813,25 +857,27 @@ class FlexfecRenderObserver : public test::EndToEndTest, rtc::CriticalSection crit_; std::set dropped_sequence_numbers_ GUARDED_BY(crit_); - // Since several packets can have the same timestamp a multiset is used - // instead of a set. + // Several packets can have the same timestamp. std::multiset dropped_timestamps_ GUARDED_BY(crit_); + const bool enable_nack_; const bool expect_flexfec_rtcp_; bool received_flexfec_rtcp_ GUARDED_BY(crit_); Random random_; + int num_packets_sent_; }; -// Disable due to failure, see bugs.webrtc.org/7050 for -// details. -TEST_P(EndToEndTest, DISABLED_ReceivesFlexfec) { - FlexfecRenderObserver test(false); +TEST_P(EndToEndTest, RecoversWithFlexfec) { + FlexfecRenderObserver test(false, false); RunBaseTest(&test); } -// Disable due to failure, see bugs.webrtc.org/7050 for -// details. -TEST_P(EndToEndTest, DISABLED_ReceivesFlexfecAndSendsCorrespondingRtcp) { - FlexfecRenderObserver test(true); +TEST_P(EndToEndTest, RecoversWithFlexfecAndNack) { + FlexfecRenderObserver test(true, false); + RunBaseTest(&test); +} + +TEST_P(EndToEndTest, RecoversWithFlexfecAndSendsCorrespondingRtcp) { + FlexfecRenderObserver test(false, true); RunBaseTest(&test); }