Notify jitter buffer about received FEC packets (to avoid sending NACK request for these packets).
Don't copy codec specific header for empty packets in the jitter buffer.
BUG=3135
R=pbos@webrtc.org, stefan@webrtc.org
Review URL: https://webrtc-codereview.appspot.com/37659004
Cr-Commit-Position: refs/heads/master@{#8184}
git-svn-id: http://webrtc.googlecode.com/svn/trunk@8184 4adac7df-926f-26a2-2b94-8c16560cd09d
diff --git a/webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc b/webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc
index 095022e..c0ba307 100644
--- a/webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc
+++ b/webrtc/modules/rtp_rtcp/source/rtcp_packet_unittest.cc
@@ -572,8 +572,6 @@
size_t len = 0;
uint8_t packet[kRrLength + kReportBlockLength - 1];
rr.Build(packet, &len, kRrLength + kReportBlockLength - 1);
- RtcpPacketParser parser;
- parser.Parse(packet, len);
EXPECT_EQ(0U, len);
}
diff --git a/webrtc/modules/video_coding/main/source/frame_buffer.cc b/webrtc/modules/video_coding/main/source/frame_buffer.cc
index fd35300..94b06f1 100644
--- a/webrtc/modules/video_coding/main/source/frame_buffer.cc
+++ b/webrtc/modules/video_coding/main/source/frame_buffer.cc
@@ -127,7 +127,9 @@
_encodedHeight = packet.height;
}
- CopyCodecSpecific(&packet.codecSpecificHeader);
+ // Don't copy payload specific data for empty packets (e.g padding packets).
+ if (packet.sizeBytes > 0)
+ CopyCodecSpecific(&packet.codecSpecificHeader);
int retVal = _sessionInfo.InsertPacket(packet, _buffer,
decode_error_mode,
diff --git a/webrtc/test/rtcp_packet_parser.cc b/webrtc/test/rtcp_packet_parser.cc
index 391be85..b9e430f 100644
--- a/webrtc/test/rtcp_packet_parser.cc
+++ b/webrtc/test/rtcp_packet_parser.cc
@@ -10,6 +10,8 @@
#include "webrtc/test/rtcp_packet_parser.h"
+#include "testing/gtest/include/gtest/gtest.h"
+
namespace webrtc {
namespace test {
@@ -20,6 +22,7 @@
void RtcpPacketParser::Parse(const void *data, size_t len) {
const uint8_t* packet = static_cast<const uint8_t*>(data);
RTCPUtility::RTCPParserV2 parser(packet, len, true);
+ EXPECT_TRUE(parser.IsValid());
for (RTCPUtility::RTCPPacketTypes type = parser.Begin();
type != RTCPUtility::kRtcpNotValidCode;
type = parser.Iterate()) {
diff --git a/webrtc/test/test.gyp b/webrtc/test/test.gyp
index 03e0529..83f2316 100644
--- a/webrtc/test/test.gyp
+++ b/webrtc/test/test.gyp
@@ -64,6 +64,7 @@
'rtp_file_writer.h',
],
'dependencies': [
+ '<(DEPTH)/testing/gtest.gyp:gtest',
'<(webrtc_root)/modules/modules.gyp:rtp_rtcp',
],
},
diff --git a/webrtc/video/end_to_end_tests.cc b/webrtc/video/end_to_end_tests.cc
index e888ed8..c01035d 100644
--- a/webrtc/video/end_to_end_tests.cc
+++ b/webrtc/video/end_to_end_tests.cc
@@ -35,6 +35,7 @@
#include "webrtc/test/frame_generator.h"
#include "webrtc/test/frame_generator_capturer.h"
#include "webrtc/test/null_transport.h"
+#include "webrtc/test/rtcp_packet_parser.h"
#include "webrtc/test/rtp_rtcp_observer.h"
#include "webrtc/test/testsupport/fileutils.h"
#include "webrtc/test/testsupport/gtest_disable.h"
@@ -75,6 +76,7 @@
void TestXrReceiverReferenceTimeReport(bool enable_rrtr);
void TestSendsSetSsrcs(size_t num_ssrcs, bool send_single_ssrc_first);
void TestRtpStatePreservation(bool use_rtx);
+ void TestReceivedFecPacketsNotNacked(const FakeNetworkPipe::Config& config);
};
TEST_F(EndToEndTest, ReceiverCanBeStartedTwice) {
@@ -536,6 +538,118 @@
RunBaseTest(&test);
}
+TEST_F(EndToEndTest, ReceivedFecPacketsNotNacked) {
+ // At low RTT (< kLowRttNackMs) -> NACK only, no FEC.
+ // Configure some network delay.
+ const int kNetworkDelayMs = 50;
+ FakeNetworkPipe::Config config;
+ config.queue_delay_ms = kNetworkDelayMs;
+ TestReceivedFecPacketsNotNacked(config);
+}
+
+void EndToEndTest::TestReceivedFecPacketsNotNacked(
+ const FakeNetworkPipe::Config& config) {
+ class FecNackObserver : public test::EndToEndTest {
+ public:
+ explicit FecNackObserver(const FakeNetworkPipe::Config& config)
+ : EndToEndTest(kDefaultTimeoutMs, config),
+ state_(kDropEveryOtherPacketUntilFec),
+ fec_sequence_number_(0),
+ has_last_sequence_number_(false),
+ last_sequence_number_(0) {}
+
+ private:
+ virtual Action OnSendRtp(const uint8_t* packet, size_t length) OVERRIDE {
+ RTPHeader header;
+ EXPECT_TRUE(parser_->Parse(packet, length, &header));
+ EXPECT_EQ(kRedPayloadType, header.payloadType);
+
+ int encapsulated_payload_type =
+ static_cast<int>(packet[header.headerLength]);
+ if (encapsulated_payload_type != kFakeSendPayloadType)
+ EXPECT_EQ(kUlpfecPayloadType, encapsulated_payload_type);
+
+ if (has_last_sequence_number_ &&
+ !IsNewerSequenceNumber(header.sequenceNumber,
+ last_sequence_number_)) {
+ // Drop retransmitted packets.
+ return DROP_PACKET;
+ }
+ last_sequence_number_ = header.sequenceNumber;
+ has_last_sequence_number_ = true;
+
+ bool fec_packet = encapsulated_payload_type == kUlpfecPayloadType;
+ switch (state_) {
+ case kDropEveryOtherPacketUntilFec:
+ if (fec_packet) {
+ state_ = kDropAllMediaPacketsUntilFec;
+ } else if (header.sequenceNumber % 2 == 0) {
+ return DROP_PACKET;
+ }
+ break;
+ case kDropAllMediaPacketsUntilFec:
+ if (!fec_packet)
+ return DROP_PACKET;
+ fec_sequence_number_ = header.sequenceNumber;
+ state_ = kVerifyFecPacketNotInNackList;
+ break;
+ case kVerifyFecPacketNotInNackList:
+ // Continue to drop packets. Make sure no frame can be decoded.
+ if (fec_packet || header.sequenceNumber % 2 == 0)
+ return DROP_PACKET;
+ break;
+ }
+ return SEND_PACKET;
+ }
+
+ virtual Action OnReceiveRtcp(const uint8_t* packet, size_t length)
+ OVERRIDE {
+ if (state_ == kVerifyFecPacketNotInNackList) {
+ test::RtcpPacketParser rtcp_parser;
+ rtcp_parser.Parse(packet, length);
+ std::vector<uint16_t> nacks = rtcp_parser.nack_item()->last_nack_list();
+ 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();
+ }
+ }
+ return SEND_PACKET;
+ }
+
+ virtual void ModifyConfigs(
+ VideoSendStream::Config* send_config,
+ std::vector<VideoReceiveStream::Config>* receive_configs,
+ VideoEncoderConfig* encoder_config) OVERRIDE {
+ // Configure hybrid NACK/FEC.
+ send_config->rtp.nack.rtp_history_ms = kNackRtpHistoryMs;
+ send_config->rtp.fec.red_payload_type = kRedPayloadType;
+ send_config->rtp.fec.ulpfec_payload_type = kUlpfecPayloadType;
+ (*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;
+ }
+
+ virtual void PerformTest() OVERRIDE {
+ EXPECT_EQ(kEventSignaled, Wait())
+ << "Timed out while waiting for FEC packets to be received.";
+ }
+
+ enum {
+ kDropEveryOtherPacketUntilFec,
+ kDropAllMediaPacketsUntilFec,
+ kVerifyFecPacketNotInNackList,
+ } state_;
+
+ uint16_t fec_sequence_number_;
+ bool has_last_sequence_number_;
+ uint16_t last_sequence_number_;
+ } test(config);
+
+ RunBaseTest(&test);
+}
+
// This test drops second RTP packet with a marker bit set, makes sure it's
// retransmitted and renders. Retransmission SSRCs are also checked.
void EndToEndTest::DecodesRetransmittedFrame(bool retransmit_over_rtx) {
diff --git a/webrtc/video_engine/vie_receiver.cc b/webrtc/video_engine/vie_receiver.cc
index cdff860..1d2485e 100644
--- a/webrtc/video_engine/vie_receiver.cc
+++ b/webrtc/video_engine/vie_receiver.cc
@@ -322,8 +322,11 @@
const RTPHeader& header) {
if (rtp_payload_registry_->IsRed(header)) {
int8_t ulpfec_pt = rtp_payload_registry_->ulpfec_payload_type();
- if (packet[header.headerLength] == ulpfec_pt)
+ if (packet[header.headerLength] == ulpfec_pt) {
rtp_receive_statistics_->FecPacketReceived(header, packet_length);
+ // Notify vcm about received FEC packets to avoid NACKing these packets.
+ NotifyReceiverOfFecPacket(header);
+ }
if (fec_receiver_->AddReceivedRedPacket(
header, packet, packet_length, ulpfec_pt) != 0) {
return false;
@@ -360,6 +363,28 @@
return false;
}
+void ViEReceiver::NotifyReceiverOfFecPacket(const RTPHeader& header) {
+ int8_t last_media_payload_type =
+ rtp_payload_registry_->last_received_media_payload_type();
+ if (last_media_payload_type < 0) {
+ LOG(LS_WARNING) << "Failed to get last media payload type.";
+ return;
+ }
+ // Fake an empty media packet.
+ WebRtcRTPHeader rtp_header = {};
+ rtp_header.header = header;
+ rtp_header.header.payloadType = last_media_payload_type;
+ rtp_header.header.paddingLength = 0;
+ PayloadUnion payload_specific;
+ if (!rtp_payload_registry_->GetPayloadSpecifics(last_media_payload_type,
+ &payload_specific)) {
+ LOG(LS_WARNING) << "Failed to get payload specifics.";
+ return;
+ }
+ rtp_header.type.Video.codec = payload_specific.Video.videoCodecType;
+ OnReceivedPayloadData(NULL, 0, &rtp_header);
+}
+
int ViEReceiver::InsertRTCPPacket(const uint8_t* rtcp_packet,
size_t rtcp_packet_length) {
{
diff --git a/webrtc/video_engine/vie_receiver.h b/webrtc/video_engine/vie_receiver.h
index b8e6569..2216df1 100644
--- a/webrtc/video_engine/vie_receiver.h
+++ b/webrtc/video_engine/vie_receiver.h
@@ -103,6 +103,7 @@
bool ParseAndHandleEncapsulatingHeader(const uint8_t* packet,
size_t packet_length,
const RTPHeader& header);
+ void NotifyReceiverOfFecPacket(const RTPHeader& header);
int InsertRTCPPacket(const uint8_t* rtcp_packet, size_t rtcp_packet_length);
bool IsPacketInOrder(const RTPHeader& header) const;
bool IsPacketRetransmitted(const RTPHeader& header, bool in_order) const;
