Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(120)

Unified Diff: webrtc/video/end_to_end_tests.cc

Issue 2017713002: Reenable EndToEndTest.ReceivedFecPacketsNotNacked (Closed) Base URL: https://chromium.googlesource.com/external/webrtc.git@master
Patch Set: comments Created 4 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « no previous file | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/video/end_to_end_tests.cc
diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc
index 607d654c929d896f3e8f3f02f6a0178b1b8bf020..592c31a675da6b158d4c0ce3c480b5b5f7edf643 100644
--- a/webrtc/video/end_to_end_tests.cc
+++ b/webrtc/video/end_to_end_tests.cc
@@ -584,8 +584,7 @@ TEST_F(EndToEndTest, CanReceiveFec) {
RunBaseTest(&test);
}
-// Flacky on all platforms. See webrtc:4328.
-TEST_F(EndToEndTest, DISABLED_ReceivedFecPacketsNotNacked) {
+TEST_F(EndToEndTest, ReceivedFecPacketsNotNacked) {
class FecNackObserver : public test::EndToEndTest {
public:
FecNackObserver()
@@ -593,7 +592,9 @@ TEST_F(EndToEndTest, DISABLED_ReceivedFecPacketsNotNacked) {
state_(kFirstPacket),
fec_sequence_number_(0),
has_last_sequence_number_(false),
- last_sequence_number_(0) {}
+ last_sequence_number_(0),
+ encoder_(VideoEncoder::Create(VideoEncoder::EncoderType::kVp8)),
+ decoder_(VP8Decoder::Create()) {}
private:
Action OnSendRtp(const uint8_t* packet, size_t length) override {
@@ -636,6 +637,20 @@ TEST_F(EndToEndTest, DISABLED_ReceivedFecPacketsNotNacked) {
if (!fec_packet)
return DROP_PACKET;
fec_sequence_number_ = header.sequenceNumber;
+ state_ = kDropOneMediaPacket;
+ break;
+ case kDropOneMediaPacket:
+ if (fec_packet)
+ return DROP_PACKET;
+ state_ = kPassOneMediaPacket;
+ return DROP_PACKET;
+ break;
+ case kPassOneMediaPacket:
+ if (fec_packet)
+ return DROP_PACKET;
+ // Pass one media packet after dropped packet after last FEC,
+ // otherwise receiver might never see a seq_no after
+ // |fec_sequence_number_|
state_ = kVerifyFecPacketNotInNackList;
break;
case kVerifyFecPacketNotInNackList:
@@ -653,10 +668,11 @@ TEST_F(EndToEndTest, DISABLED_ReceivedFecPacketsNotNacked) {
test::RtcpPacketParser rtcp_parser;
rtcp_parser.Parse(packet, length);
std::vector<uint16_t> nacks = rtcp_parser.nack_item()->last_nack_list();
+ EXPECT_TRUE(std::find(nacks.begin(), nacks.end(),
+ fec_sequence_number_) == nacks.end())
+ << "Got nack for FEC packet";
if (!nacks.empty() &&
IsNewerSequenceNumber(nacks.back(), fec_sequence_number_)) {
- EXPECT_TRUE(std::find(
- nacks.begin(), nacks.end(), fec_sequence_number_) == nacks.end());
observation_complete_.Set();
}
}
@@ -690,9 +706,24 @@ TEST_F(EndToEndTest, DISABLED_ReceivedFecPacketsNotNacked) {
send_config->rtp.nack.rtp_history_ms = kNackRtpHistoryMs;
send_config->rtp.fec.red_payload_type = kRedPayloadType;
send_config->rtp.fec.ulpfec_payload_type = kUlpfecPayloadType;
+ // Set codec to VP8, otherwise NACK/FEC hybrid will be disabled.
+ send_config->encoder_settings.encoder = encoder_.get();
+ send_config->encoder_settings.payload_name = "VP8";
+ send_config->encoder_settings.payload_type = kFakeVideoSendPayloadType;
+ encoder_config->streams[0].min_bitrate_bps = 50000;
+ encoder_config->streams[0].max_bitrate_bps =
+ encoder_config->streams[0].target_bitrate_bps = 2000000;
+
(*receive_configs)[0].rtp.nack.rtp_history_ms = kNackRtpHistoryMs;
(*receive_configs)[0].rtp.fec.red_payload_type = kRedPayloadType;
(*receive_configs)[0].rtp.fec.ulpfec_payload_type = kUlpfecPayloadType;
+
+ (*receive_configs)[0].decoders.resize(1);
+ (*receive_configs)[0].decoders[0].payload_type =
+ send_config->encoder_settings.payload_type;
+ (*receive_configs)[0].decoders[0].payload_name =
+ send_config->encoder_settings.payload_name;
+ (*receive_configs)[0].decoders[0].decoder = decoder_.get();
}
void PerformTest() override {
@@ -704,6 +735,8 @@ TEST_F(EndToEndTest, DISABLED_ReceivedFecPacketsNotNacked) {
kFirstPacket,
kDropEveryOtherPacketUntilFec,
kDropAllMediaPacketsUntilFec,
+ kDropOneMediaPacket,
+ kPassOneMediaPacket,
kVerifyFecPacketNotInNackList,
} state_;
@@ -711,6 +744,8 @@ TEST_F(EndToEndTest, DISABLED_ReceivedFecPacketsNotNacked) {
uint16_t fec_sequence_number_ GUARDED_BY(&crit_);
bool has_last_sequence_number_;
uint16_t last_sequence_number_;
+ std::unique_ptr<webrtc::VideoEncoder> encoder_;
+ std::unique_ptr<webrtc::VideoDecoder> decoder_;
} test;
RunBaseTest(&test);
« no previous file with comments | « no previous file | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698