From 0aabdac7431b578ad8dbec73c1bc5ca5d9a779a0 Mon Sep 17 00:00:00 2001 From: brandtr Date: Mon, 3 Oct 2016 06:36:43 -0700 Subject: [PATCH] Generalize UlpfecPacketGenerator into AugmentedPacketGenerator. Main changes: - Split out general functionality from UlpfecPacketGenerator into a new class AugmentedPacketGenerator. This will allow for the addition of a FlexfecPacketGenerator that inherits from AugmentedPacketGenerator. - Rename RawRtpPacket to AugmentedPacket. This name is more reflective of its purpose, i.e., an FEC packet with an additional WebRtcRTPHeader. - Return std::unique_ptr's instead of raw pointers. Minor changes: - Update names, based on RawRtpPacket -> AugmentedPacket name change, in FEC unit tests. - Rename |generator_| to |packet_generator_|, in FEC unit tests. - Change some int's to size_t's in the packet generator classes. - Use std::unique_ptr in ProducerFec unittest. No functionality or performance changes are expected due to this CL. BUG=webrtc:5654 Review-Url: https://codereview.webrtc.org/2271273004 Cr-Commit-Position: refs/heads/master@{#14477} --- .../rtp_rtcp/source/fec_receiver_unittest.cc | 122 +++++++++--------- .../rtp_rtcp/source/fec_test_helper.cc | 115 +++++++++-------- .../modules/rtp_rtcp/source/fec_test_helper.h | 66 +++++++--- .../rtp_rtcp/source/producer_fec_unittest.cc | 67 +++++----- 4 files changed, 198 insertions(+), 172 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc index 01ce2d1e0f..2c2b823977 100644 --- a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc @@ -30,18 +30,20 @@ using ::testing::Args; using ::testing::ElementsAreArray; using ::testing::Return; +using test::fec::AugmentedPacket; using Packet = ForwardErrorCorrection::Packet; -using test::fec::RawRtpPacket; using test::fec::UlpfecPacketGenerator; constexpr int kFecPayloadType = 96; +constexpr uint32_t kMediaSsrc = 835424; } // namespace class ReceiverFecTest : public ::testing::Test { protected: ReceiverFecTest() : fec_(ForwardErrorCorrection::CreateUlpfec()), - receiver_fec_(FecReceiver::Create(&rtp_data_callback_)) {} + receiver_fec_(FecReceiver::Create(&rtp_data_callback_)), + packet_generator_(kMediaSsrc) {} void EncodeFec(const ForwardErrorCorrection::PacketList& media_packets, size_t num_fec_packets, @@ -54,18 +56,18 @@ class ReceiverFecTest : public ::testing::Test { void GenerateFrame(size_t num_media_packets, size_t frame_offset, - std::list* media_rtp_packets, + std::list* augmented_packets, ForwardErrorCorrection::PacketList* media_packets) { - generator_.NewFrame(num_media_packets); + packet_generator_.NewFrame(num_media_packets); for (size_t i = 0; i < num_media_packets; ++i) { - std::unique_ptr next_packet( - generator_.NextPacket(frame_offset + i, kRtpHeaderSize + 10)); - media_rtp_packets->push_back(next_packet.get()); + std::unique_ptr next_packet( + packet_generator_.NextPacket(frame_offset + i, kRtpHeaderSize + 10)); + augmented_packets->push_back(next_packet.get()); media_packets->push_back(std::move(next_packet)); } } - void VerifyReconstructedMediaPacket(const RawRtpPacket& packet, + void VerifyReconstructedMediaPacket(const AugmentedPacket& packet, size_t times) { // Verify that the content of the reconstructed packet is equal to the // content of |packet|, and that the same content is received |times| number @@ -76,17 +78,17 @@ class ReceiverFecTest : public ::testing::Test { .WillRepeatedly(Return(true)); } - void BuildAndAddRedMediaPacket(RawRtpPacket* packet) { - std::unique_ptr red_packet( - generator_.BuildMediaRedPacket(packet)); + void BuildAndAddRedMediaPacket(AugmentedPacket* packet) { + std::unique_ptr red_packet( + UlpfecPacketGenerator::BuildMediaRedPacket(*packet)); EXPECT_EQ(0, receiver_fec_->AddReceivedRedPacket( red_packet->header.header, red_packet->data, red_packet->length, kFecPayloadType)); } void BuildAndAddRedFecPacket(Packet* packet) { - std::unique_ptr red_packet( - generator_.BuildFecRedPacket(packet)); + std::unique_ptr red_packet( + packet_generator_.BuildUlpfecRedPacket(*packet)); EXPECT_EQ(0, receiver_fec_->AddReceivedRedPacket( red_packet->header.header, red_packet->data, red_packet->length, kFecPayloadType)); @@ -100,19 +102,19 @@ class ReceiverFecTest : public ::testing::Test { MockRtpData rtp_data_callback_; std::unique_ptr fec_; std::unique_ptr receiver_fec_; - UlpfecPacketGenerator generator_; + UlpfecPacketGenerator packet_generator_; }; TEST_F(ReceiverFecTest, TwoMediaOneFec) { const size_t kNumFecPackets = 1; - std::list media_rtp_packets; + std::list augmented_media_packets; ForwardErrorCorrection::PacketList media_packets; - GenerateFrame(2, 0, &media_rtp_packets, &media_packets); + GenerateFrame(2, 0, &augmented_media_packets, &media_packets); std::list fec_packets; EncodeFec(media_packets, kNumFecPackets, &fec_packets); // Recovery - auto it = media_rtp_packets.begin(); + auto it = augmented_media_packets.begin(); BuildAndAddRedMediaPacket(*it); VerifyReconstructedMediaPacket(**it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); @@ -134,9 +136,9 @@ void ReceiverFecTest::InjectGarbagePacketLength(size_t fec_garbage_offset) { .WillRepeatedly(Return(true)); const size_t kNumFecPackets = 1; - std::list media_rtp_packets; + std::list augmented_media_packets; ForwardErrorCorrection::PacketList media_packets; - GenerateFrame(2, 0, &media_rtp_packets, &media_packets); + GenerateFrame(2, 0, &augmented_media_packets, &media_packets); std::list fec_packets; EncodeFec(media_packets, kNumFecPackets, &fec_packets); ByteWriter::WriteBigEndian( @@ -144,7 +146,7 @@ void ReceiverFecTest::InjectGarbagePacketLength(size_t fec_garbage_offset) { // Inject first media packet, then first FEC packet, skipping the second media // packet to cause a recovery from the FEC packet. - BuildAndAddRedMediaPacket(media_rtp_packets.front()); + BuildAndAddRedMediaPacket(augmented_media_packets.front()); BuildAndAddRedFecPacket(fec_packets.front()); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); @@ -167,15 +169,15 @@ TEST_F(ReceiverFecTest, InjectGarbageFecLevelHeaderProtectionLength) { TEST_F(ReceiverFecTest, TwoMediaTwoFec) { const size_t kNumFecPackets = 2; - std::list media_rtp_packets; + std::list augmented_media_packets; ForwardErrorCorrection::PacketList media_packets; - GenerateFrame(2, 0, &media_rtp_packets, &media_packets); + GenerateFrame(2, 0, &augmented_media_packets, &media_packets); std::list fec_packets; EncodeFec(media_packets, kNumFecPackets, &fec_packets); // Recovery // Drop both media packets. - auto it = media_rtp_packets.begin(); + auto it = augmented_media_packets.begin(); auto fec_it = fec_packets.begin(); BuildAndAddRedFecPacket(*fec_it); VerifyReconstructedMediaPacket(**it, 1); @@ -189,16 +191,16 @@ TEST_F(ReceiverFecTest, TwoMediaTwoFec) { TEST_F(ReceiverFecTest, TwoFramesOneFec) { const size_t kNumFecPackets = 1; - std::list media_rtp_packets; + std::list augmented_media_packets; ForwardErrorCorrection::PacketList media_packets; - GenerateFrame(1, 0, &media_rtp_packets, &media_packets); - GenerateFrame(1, 1, &media_rtp_packets, &media_packets); + GenerateFrame(1, 0, &augmented_media_packets, &media_packets); + GenerateFrame(1, 1, &augmented_media_packets, &media_packets); std::list fec_packets; EncodeFec(media_packets, kNumFecPackets, &fec_packets); // Recovery - auto it = media_rtp_packets.begin(); - BuildAndAddRedMediaPacket(media_rtp_packets.front()); + auto it = augmented_media_packets.begin(); + BuildAndAddRedMediaPacket(augmented_media_packets.front()); VerifyReconstructedMediaPacket(**it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); // Drop one media packet. @@ -210,16 +212,16 @@ TEST_F(ReceiverFecTest, TwoFramesOneFec) { TEST_F(ReceiverFecTest, OneCompleteOneUnrecoverableFrame) { const size_t kNumFecPackets = 1; - std::list media_rtp_packets; + std::list augmented_media_packets; ForwardErrorCorrection::PacketList media_packets; - GenerateFrame(1, 0, &media_rtp_packets, &media_packets); - GenerateFrame(2, 1, &media_rtp_packets, &media_packets); + GenerateFrame(1, 0, &augmented_media_packets, &media_packets); + GenerateFrame(2, 1, &augmented_media_packets, &media_packets); std::list fec_packets; EncodeFec(media_packets, kNumFecPackets, &fec_packets); // Recovery - auto it = media_rtp_packets.begin(); + auto it = augmented_media_packets.begin(); BuildAndAddRedMediaPacket(*it); // First frame: one packet. VerifyReconstructedMediaPacket(**it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); @@ -232,24 +234,24 @@ TEST_F(ReceiverFecTest, OneCompleteOneUnrecoverableFrame) { TEST_F(ReceiverFecTest, MaxFramesOneFec) { const size_t kNumFecPackets = 1; const size_t kNumMediaPackets = 48; - std::list media_rtp_packets; + std::list augmented_media_packets; ForwardErrorCorrection::PacketList media_packets; for (size_t i = 0; i < kNumMediaPackets; ++i) { - GenerateFrame(1, i, &media_rtp_packets, &media_packets); + GenerateFrame(1, i, &augmented_media_packets, &media_packets); } std::list fec_packets; EncodeFec(media_packets, kNumFecPackets, &fec_packets); // Recovery - auto it = media_rtp_packets.begin(); + auto it = augmented_media_packets.begin(); ++it; // Drop first packet. - for (; it != media_rtp_packets.end(); ++it) { + for (; it != augmented_media_packets.end(); ++it) { BuildAndAddRedMediaPacket(*it); VerifyReconstructedMediaPacket(**it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); } BuildAndAddRedFecPacket(fec_packets.front()); - it = media_rtp_packets.begin(); + it = augmented_media_packets.begin(); VerifyReconstructedMediaPacket(**it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); } @@ -257,10 +259,10 @@ TEST_F(ReceiverFecTest, MaxFramesOneFec) { TEST_F(ReceiverFecTest, TooManyFrames) { const size_t kNumFecPackets = 1; const size_t kNumMediaPackets = 49; - std::list media_rtp_packets; + std::list augmented_media_packets; ForwardErrorCorrection::PacketList media_packets; for (size_t i = 0; i < kNumMediaPackets; ++i) { - GenerateFrame(1, i, &media_rtp_packets, &media_packets); + GenerateFrame(1, i, &augmented_media_packets, &media_packets); } std::list fec_packets; EXPECT_EQ(-1, fec_->EncodeFec(media_packets, @@ -274,14 +276,14 @@ TEST_F(ReceiverFecTest, PacketNotDroppedTooEarly) { Packet* delayed_fec = nullptr; const size_t kNumFecPacketsBatch1 = 1; const size_t kNumMediaPacketsBatch1 = 2; - std::list media_rtp_packets_batch1; + std::list augmented_media_packets_batch1; ForwardErrorCorrection::PacketList media_packets_batch1; - GenerateFrame(kNumMediaPacketsBatch1, 0, &media_rtp_packets_batch1, + GenerateFrame(kNumMediaPacketsBatch1, 0, &augmented_media_packets_batch1, &media_packets_batch1); std::list fec_packets; EncodeFec(media_packets_batch1, kNumFecPacketsBatch1, &fec_packets); - BuildAndAddRedMediaPacket(media_rtp_packets_batch1.front()); + BuildAndAddRedMediaPacket(augmented_media_packets_batch1.front()); EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)) .Times(1).WillRepeatedly(Return(true)); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); @@ -289,13 +291,13 @@ TEST_F(ReceiverFecTest, PacketNotDroppedTooEarly) { // Fill the FEC decoder. No packets should be dropped. const size_t kNumMediaPacketsBatch2 = 46; - std::list media_rtp_packets_batch2; + std::list augmented_media_packets_batch2; ForwardErrorCorrection::PacketList media_packets_batch2; for (size_t i = 0; i < kNumMediaPacketsBatch2; ++i) { - GenerateFrame(1, i, &media_rtp_packets_batch2, &media_packets_batch2); + GenerateFrame(1, i, &augmented_media_packets_batch2, &media_packets_batch2); } - for (auto it = media_rtp_packets_batch2.begin(); - it != media_rtp_packets_batch2.end(); ++it) { + for (auto it = augmented_media_packets_batch2.begin(); + it != augmented_media_packets_batch2.end(); ++it) { BuildAndAddRedMediaPacket(*it); EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)) .Times(1).WillRepeatedly(Return(true)); @@ -315,14 +317,14 @@ TEST_F(ReceiverFecTest, PacketDroppedWhenTooOld) { Packet* delayed_fec = nullptr; const size_t kNumFecPacketsBatch1 = 1; const size_t kNumMediaPacketsBatch1 = 2; - std::list media_rtp_packets_batch1; + std::list augmented_media_packets_batch1; ForwardErrorCorrection::PacketList media_packets_batch1; - GenerateFrame(kNumMediaPacketsBatch1, 0, &media_rtp_packets_batch1, + GenerateFrame(kNumMediaPacketsBatch1, 0, &augmented_media_packets_batch1, &media_packets_batch1); std::list fec_packets; EncodeFec(media_packets_batch1, kNumFecPacketsBatch1, &fec_packets); - BuildAndAddRedMediaPacket(media_rtp_packets_batch1.front()); + BuildAndAddRedMediaPacket(augmented_media_packets_batch1.front()); EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)) .Times(1).WillRepeatedly(Return(true)); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); @@ -330,13 +332,13 @@ TEST_F(ReceiverFecTest, PacketDroppedWhenTooOld) { // Fill the FEC decoder and force the last packet to be dropped. const size_t kNumMediaPacketsBatch2 = 48; - std::list media_rtp_packets_batch2; + std::list augmented_media_packets_batch2; ForwardErrorCorrection::PacketList media_packets_batch2; for (size_t i = 0; i < kNumMediaPacketsBatch2; ++i) { - GenerateFrame(1, i, &media_rtp_packets_batch2, &media_packets_batch2); + GenerateFrame(1, i, &augmented_media_packets_batch2, &media_packets_batch2); } - for (auto it = media_rtp_packets_batch2.begin(); - it != media_rtp_packets_batch2.end(); ++it) { + for (auto it = augmented_media_packets_batch2.begin(); + it != augmented_media_packets_batch2.end(); ++it) { BuildAndAddRedMediaPacket(*it); EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)) .Times(1).WillRepeatedly(Return(true)); @@ -355,13 +357,13 @@ TEST_F(ReceiverFecTest, OldFecPacketDropped) { // 49 frames with 2 media packets and one FEC packet. All media packets // missing. const size_t kNumMediaPackets = 49 * 2; - std::list media_rtp_packets; + std::list augmented_media_packets; ForwardErrorCorrection::PacketList media_packets; for (size_t i = 0; i < kNumMediaPackets / 2; ++i) { - std::list frame_media_rtp_packets; + std::list frame_augmented_media_packets; ForwardErrorCorrection::PacketList frame_media_packets; std::list fec_packets; - GenerateFrame(2, 0, &frame_media_rtp_packets, &frame_media_packets); + GenerateFrame(2, 0, &frame_augmented_media_packets, &frame_media_packets); EncodeFec(frame_media_packets, 1, &fec_packets); for (auto it = fec_packets.begin(); it != fec_packets.end(); ++it) { // Only FEC packets inserted. No packets recoverable at this time. @@ -374,14 +376,14 @@ TEST_F(ReceiverFecTest, OldFecPacketDropped) { media_packets.insert(media_packets.end(), std::make_move_iterator(frame_media_packets.begin()), std::make_move_iterator(frame_media_packets.end())); - media_rtp_packets.insert(media_rtp_packets.end(), - frame_media_rtp_packets.begin(), - frame_media_rtp_packets.end()); + augmented_media_packets.insert(augmented_media_packets.end(), + frame_augmented_media_packets.begin(), + frame_augmented_media_packets.end()); } // Insert the oldest media packet. The corresponding FEC packet is too old // and should have been dropped. Only the media packet we inserted will be // returned. - BuildAndAddRedMediaPacket(media_rtp_packets.front()); + BuildAndAddRedMediaPacket(augmented_media_packets.front()); EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)) .Times(1).WillRepeatedly(Return(true)); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); diff --git a/webrtc/modules/rtp_rtcp/source/fec_test_helper.cc b/webrtc/modules/rtp_rtcp/source/fec_test_helper.cc index 3799c7f0c2..c2d2f9acfb 100644 --- a/webrtc/modules/rtp_rtcp/source/fec_test_helper.cc +++ b/webrtc/modules/rtp_rtcp/source/fec_test_helper.cc @@ -64,9 +64,8 @@ ForwardErrorCorrection::PacketList MediaPacketGenerator::ConstructMediaPackets( webrtc::ByteWriter::WriteBigEndian(&media_packet->data[8], ssrc_); // Generate random values for payload. - for (size_t j = 12; j < media_packet->length; ++j) { + for (size_t j = 12; j < media_packet->length; ++j) media_packet->data[j] = random_->Rand(); - } seq_num++; media_packets.push_back(std::move(media_packet)); } @@ -89,73 +88,91 @@ uint16_t MediaPacketGenerator::GetFecSeqNum() { return fec_seq_num_; } -UlpfecPacketGenerator::UlpfecPacketGenerator() - : num_packets_(0), seq_num_(0), timestamp_(0) {} +AugmentedPacketGenerator::AugmentedPacketGenerator(uint32_t ssrc) + : num_packets_(0), ssrc_(ssrc), seq_num_(0), timestamp_(0) {} -void UlpfecPacketGenerator::NewFrame(int num_packets) { +void AugmentedPacketGenerator::NewFrame(size_t num_packets) { num_packets_ = num_packets; timestamp_ += 3000; } -uint16_t UlpfecPacketGenerator::NextSeqNum() { +uint16_t AugmentedPacketGenerator::NextPacketSeqNum() { return ++seq_num_; } -RawRtpPacket* UlpfecPacketGenerator::NextPacket(int offset, size_t length) { - RawRtpPacket* rtp_packet = new RawRtpPacket; +std::unique_ptr AugmentedPacketGenerator::NextPacket( + size_t offset, + size_t length) { + std::unique_ptr packet(new AugmentedPacket()); + for (size_t i = 0; i < length; ++i) - rtp_packet->data[i + kRtpHeaderSize] = offset + i; - rtp_packet->length = length + kRtpHeaderSize; - memset(&rtp_packet->header, 0, sizeof(WebRtcRTPHeader)); - rtp_packet->header.frameType = kVideoFrameDelta; - rtp_packet->header.header.headerLength = kRtpHeaderSize; - rtp_packet->header.header.markerBit = (num_packets_ == 1); - rtp_packet->header.header.sequenceNumber = seq_num_; - rtp_packet->header.header.timestamp = timestamp_; - rtp_packet->header.header.payloadType = kVp8PayloadType; - BuildRtpHeader(rtp_packet->data, &rtp_packet->header.header); + packet->data[i + kRtpHeaderSize] = offset + i; + packet->length = length + kRtpHeaderSize; + memset(&packet->header, 0, sizeof(WebRtcRTPHeader)); + packet->header.frameType = kVideoFrameDelta; + packet->header.header.headerLength = kRtpHeaderSize; + packet->header.header.markerBit = (num_packets_ == 1); + packet->header.header.payloadType = kVp8PayloadType; + packet->header.header.sequenceNumber = seq_num_; + packet->header.header.timestamp = timestamp_; + packet->header.header.ssrc = ssrc_; + WriteRtpHeader(packet->header.header, packet->data); ++seq_num_; --num_packets_; - return rtp_packet; + + return packet; } -// Creates a new RtpPacket with the RED header added to the packet. -RawRtpPacket* UlpfecPacketGenerator::BuildMediaRedPacket( - const RawRtpPacket* packet) { - const size_t kHeaderLength = packet->header.header.headerLength; - RawRtpPacket* red_packet = new RawRtpPacket; - red_packet->header = packet->header; - red_packet->length = packet->length + 1; // 1 byte RED header. +void AugmentedPacketGenerator::WriteRtpHeader(const RTPHeader& header, + uint8_t* data) { + data[0] = 0x80; // Version 2. + data[1] = header.payloadType; + data[1] |= (header.markerBit ? kRtpMarkerBitMask : 0); + ByteWriter::WriteBigEndian(data + 2, header.sequenceNumber); + ByteWriter::WriteBigEndian(data + 4, header.timestamp); + ByteWriter::WriteBigEndian(data + 8, header.ssrc); +} + +UlpfecPacketGenerator::UlpfecPacketGenerator(uint32_t ssrc) + : AugmentedPacketGenerator(ssrc) {} + +std::unique_ptr UlpfecPacketGenerator::BuildMediaRedPacket( + const AugmentedPacket& packet) { + std::unique_ptr red_packet(new AugmentedPacket()); + + const size_t kHeaderLength = packet.header.header.headerLength; + red_packet->header = packet.header; + red_packet->length = packet.length + 1; // 1 byte RED header. memset(red_packet->data, 0, red_packet->length); // Copy RTP header. - memcpy(red_packet->data, packet->data, kHeaderLength); - SetRedHeader(red_packet, red_packet->data[1] & 0x7f, kHeaderLength); - memcpy(red_packet->data + kHeaderLength + 1, packet->data + kHeaderLength, - packet->length - kHeaderLength); + memcpy(red_packet->data, packet.data, kHeaderLength); + SetRedHeader(red_packet->data[1] & 0x7f, kHeaderLength, red_packet.get()); + memcpy(red_packet->data + kHeaderLength + 1, packet.data + kHeaderLength, + packet.length - kHeaderLength); + return red_packet; } -// Creates a new RtpPacket with FEC payload and RED header. Does this by -// creating a new fake media RtpPacket, clears the marker bit and adds a RED -// header. Finally replaces the payload with the content of |packet->data|. -RawRtpPacket* UlpfecPacketGenerator::BuildFecRedPacket( - const ForwardErrorCorrection::Packet* packet) { +std::unique_ptr UlpfecPacketGenerator::BuildUlpfecRedPacket( + const ForwardErrorCorrection::Packet& packet) { // Create a fake media packet to get a correct header. 1 byte RED header. ++num_packets_; - RawRtpPacket* red_packet = NextPacket(0, packet->length + 1); + std::unique_ptr red_packet = + NextPacket(0, packet.length + 1); + red_packet->data[1] &= ~0x80; // Clear marker bit. const size_t kHeaderLength = red_packet->header.header.headerLength; - SetRedHeader(red_packet, kFecPayloadType, kHeaderLength); - memcpy(red_packet->data + kHeaderLength + 1, packet->data, packet->length); - red_packet->length = kHeaderLength + 1 + packet->length; + SetRedHeader(kFecPayloadType, kHeaderLength, red_packet.get()); + memcpy(red_packet->data + kHeaderLength + 1, packet.data, packet.length); + red_packet->length = kHeaderLength + 1 + packet.length; + return red_packet; } -void UlpfecPacketGenerator::SetRedHeader( - ForwardErrorCorrection::Packet* red_packet, - uint8_t payload_type, - size_t header_length) const { - // Replace pltype. +void UlpfecPacketGenerator::SetRedHeader(uint8_t payload_type, + size_t header_length, + AugmentedPacket* red_packet) { + // Replace payload type. red_packet->data[1] &= 0x80; // Reset. red_packet->data[1] += kRedPayloadType; // Replace. @@ -163,16 +180,6 @@ void UlpfecPacketGenerator::SetRedHeader( red_packet->data[header_length] = payload_type; } -void UlpfecPacketGenerator::BuildRtpHeader(uint8_t* data, - const RTPHeader* header) { - data[0] = 0x80; // Version 2. - data[1] = header->payloadType; - data[1] |= (header->markerBit ? kRtpMarkerBitMask : 0); - ByteWriter::WriteBigEndian(data + 2, header->sequenceNumber); - ByteWriter::WriteBigEndian(data + 4, header->timestamp); - ByteWriter::WriteBigEndian(data + 8, header->ssrc); -} - } // namespace fec } // namespace test } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/fec_test_helper.h b/webrtc/modules/rtp_rtcp/source/fec_test_helper.h index 594d38ddff..36e40652c7 100644 --- a/webrtc/modules/rtp_rtcp/source/fec_test_helper.h +++ b/webrtc/modules/rtp_rtcp/source/fec_test_helper.h @@ -11,6 +11,8 @@ #ifndef WEBRTC_MODULES_RTP_RTCP_SOURCE_FEC_TEST_HELPER_H_ #define WEBRTC_MODULES_RTP_RTCP_SOURCE_FEC_TEST_HELPER_H_ +#include + #include "webrtc/base/basictypes.h" #include "webrtc/base/random.h" #include "webrtc/modules/include/module_common_types.h" @@ -20,10 +22,14 @@ namespace webrtc { namespace test { namespace fec { -struct RawRtpPacket : public ForwardErrorCorrection::Packet { +struct AugmentedPacket : public ForwardErrorCorrection::Packet { WebRtcRTPHeader header; }; +// TODO(brandtr): Consider merging MediaPacketGenerator and +// AugmentedPacketGenerator into a single class, since their functionality is +// similar. + // This class generates media packets corresponding to a single frame. class MediaPacketGenerator { public: @@ -55,38 +61,56 @@ class MediaPacketGenerator { uint16_t fec_seq_num_; }; -// This class generates media and ULPFEC packets (both encapsulated in RED) -// for a single frame. -class UlpfecPacketGenerator { +// This class generates media packets with a certain structure of the payload. +class AugmentedPacketGenerator { public: - UlpfecPacketGenerator(); + explicit AugmentedPacketGenerator(uint32_t ssrc); - void NewFrame(int num_packets); + // Prepare for generating a new set of packets, corresponding to a frame. + void NewFrame(size_t num_packets); - uint16_t NextSeqNum(); + // Increment and return the newly incremented sequence number. + uint16_t NextPacketSeqNum(); - RawRtpPacket* NextPacket(int offset, size_t length); + // Return the next packet in the current frame. + std::unique_ptr NextPacket(size_t offset, size_t length); - // Creates a new RtpPacket with the RED header added to the packet. - RawRtpPacket* BuildMediaRedPacket(const RawRtpPacket* packet); + protected: + // Given |header|, writes the appropriate RTP header fields in |data|. + static void WriteRtpHeader(const RTPHeader& header, uint8_t* data); - // Creates a new RtpPacket with FEC payload and red header. Does this by - // creating a new fake media RtpPacket, clears the marker bit and adds a RED - // header. Finally replaces the payload with the content of |packet->data|. - RawRtpPacket* BuildFecRedPacket(const ForwardErrorCorrection::Packet* packet); - - void SetRedHeader(ForwardErrorCorrection::Packet* red_packet, - uint8_t payload_type, - size_t header_length) const; + // Number of packets left to generate, in the current frame. + size_t num_packets_; private: - static void BuildRtpHeader(uint8_t* data, const RTPHeader* header); - - int num_packets_; + uint32_t ssrc_; uint16_t seq_num_; uint32_t timestamp_; }; +// This class generates media and ULPFEC packets (both encapsulated in RED) +// for a single frame. +class UlpfecPacketGenerator : public AugmentedPacketGenerator { + public: + explicit UlpfecPacketGenerator(uint32_t ssrc); + + // Creates a new AugmentedPacket with the RED header added to the packet. + static std::unique_ptr BuildMediaRedPacket( + const AugmentedPacket& packet); + + // Creates a new AugmentedPacket with FEC payload and RED header. Does this by + // creating a new fake media AugmentedPacket, clears the marker bit and adds a + // RED header. Finally replaces the payload with the content of + // |packet->data|. + std::unique_ptr BuildUlpfecRedPacket( + const ForwardErrorCorrection::Packet& packet); + + private: + static void SetRedHeader(uint8_t payload_type, + size_t header_length, + AugmentedPacket* red_packet); +}; + } // namespace fec } // namespace test } // namespace webrtc diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc b/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc index bf36c47c39..31304b4213 100644 --- a/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc @@ -10,6 +10,7 @@ #include #include +#include #include #include "webrtc/base/basictypes.h" @@ -22,11 +23,12 @@ namespace webrtc { namespace { -using test::fec::RawRtpPacket; -using test::fec::UlpfecPacketGenerator; +using test::fec::AugmentedPacket; +using test::fec::AugmentedPacketGenerator; constexpr int kFecPayloadType = 96; constexpr int kRedPayloadType = 97; +constexpr uint32_t kMediaSsrc = 835424; } // namespace void VerifyHeader(uint16_t seq_num, @@ -50,8 +52,10 @@ void VerifyHeader(uint16_t seq_num, class ProducerFecTest : public ::testing::Test { protected: + ProducerFecTest() : packet_generator_(kMediaSsrc) {} + ProducerFec producer_; - UlpfecPacketGenerator generator_; + AugmentedPacketGenerator packet_generator_; }; // Verifies bug found via fuzzing, where a gap in the packet sequence caused us @@ -106,30 +110,25 @@ TEST_F(ProducerFecTest, OneFrameFec) { // media packets for 1 frame is at least |minimum_media_packets_fec_|. constexpr size_t kNumPackets = 4; FecProtectionParams params = {15, 3, kFecMaskRandom}; - std::list rtp_packets; - generator_.NewFrame(kNumPackets); + packet_generator_.NewFrame(kNumPackets); producer_.SetFecParameters(¶ms, 0); // Expecting one FEC packet. uint32_t last_timestamp = 0; for (size_t i = 0; i < kNumPackets; ++i) { - RawRtpPacket* rtp_packet = generator_.NextPacket(i, 10); - rtp_packets.push_back(rtp_packet); + std::unique_ptr packet = + packet_generator_.NextPacket(i, 10); EXPECT_EQ(0, producer_.AddRtpPacketAndGenerateFec( - rtp_packet->data, rtp_packet->length, kRtpHeaderSize)); - last_timestamp = rtp_packet->header.header.timestamp; + packet->data, packet->length, kRtpHeaderSize)); + last_timestamp = packet->header.header.timestamp; } EXPECT_TRUE(producer_.FecAvailable()); - uint16_t seq_num = generator_.NextSeqNum(); - std::vector> packets = + uint16_t seq_num = packet_generator_.NextPacketSeqNum(); + std::vector> red_packets = producer_.GetFecPacketsAsRed(kRedPayloadType, kFecPayloadType, seq_num, kRtpHeaderSize); EXPECT_FALSE(producer_.FecAvailable()); - ASSERT_EQ(1u, packets.size()); + ASSERT_EQ(1u, red_packets.size()); VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType, - packets.front().get(), false); - while (!rtp_packets.empty()) { - delete rtp_packets.front(); - rtp_packets.pop_front(); - } + red_packets.front().get(), false); } TEST_F(ProducerFecTest, TwoFrameFec) { @@ -144,37 +143,32 @@ TEST_F(ProducerFecTest, TwoFrameFec) { constexpr size_t kNumFrames = 2; FecProtectionParams params = {15, 3, kFecMaskRandom}; - std::list rtp_packets; producer_.SetFecParameters(¶ms, 0); // Expecting one FEC packet. uint32_t last_timestamp = 0; for (size_t i = 0; i < kNumFrames; ++i) { - generator_.NewFrame(kNumPackets); + packet_generator_.NewFrame(kNumPackets); for (size_t j = 0; j < kNumPackets; ++j) { - RawRtpPacket* rtp_packet = generator_.NextPacket(i * kNumPackets + j, 10); - rtp_packets.push_back(rtp_packet); + std::unique_ptr packet = + packet_generator_.NextPacket(i * kNumPackets + j, 10); EXPECT_EQ(0, producer_.AddRtpPacketAndGenerateFec( - rtp_packet->data, rtp_packet->length, kRtpHeaderSize)); - last_timestamp = rtp_packet->header.header.timestamp; + packet->data, packet->length, kRtpHeaderSize)); + last_timestamp = packet->header.header.timestamp; } } EXPECT_TRUE(producer_.FecAvailable()); - uint16_t seq_num = generator_.NextSeqNum(); - std::vector> packets = + uint16_t seq_num = packet_generator_.NextPacketSeqNum(); + std::vector> red_packets = producer_.GetFecPacketsAsRed(kRedPayloadType, kFecPayloadType, seq_num, kRtpHeaderSize); EXPECT_FALSE(producer_.FecAvailable()); - ASSERT_EQ(1u, packets.size()); + ASSERT_EQ(1u, red_packets.size()); VerifyHeader(seq_num, last_timestamp, kRedPayloadType, kFecPayloadType, - packets.front().get(), false); - while (!rtp_packets.empty()) { - delete rtp_packets.front(); - rtp_packets.pop_front(); - } + red_packets.front().get(), false); } TEST_F(ProducerFecTest, BuildRedPacket) { - generator_.NewFrame(1); - RawRtpPacket* packet = generator_.NextPacket(0, 10); + packet_generator_.NewFrame(1); + std::unique_ptr packet = packet_generator_.NextPacket(0, 10); std::unique_ptr red_packet = ProducerFec::BuildRedPacket(packet->data, packet->length - kRtpHeaderSize, kRtpHeaderSize, kRedPayloadType); @@ -186,7 +180,6 @@ TEST_F(ProducerFecTest, BuildRedPacket) { for (int i = 0; i < 10; ++i) { EXPECT_EQ(i, red_packet->data()[kRtpHeaderSize + 1 + i]); } - delete packet; } TEST_F(ProducerFecTest, BuildRedPacketWithEmptyPayload) { @@ -194,9 +187,9 @@ TEST_F(ProducerFecTest, BuildRedPacketWithEmptyPayload) { constexpr size_t kPayloadLength = 0; constexpr size_t kRedForFecHeaderLength = 1; - generator_.NewFrame(kNumFrames); - std::unique_ptr packet( - generator_.NextPacket(0, kPayloadLength)); + packet_generator_.NewFrame(kNumFrames); + std::unique_ptr packet( + packet_generator_.NextPacket(0, kPayloadLength)); std::unique_ptr red_packet = ProducerFec::BuildRedPacket(packet->data, packet->length - kRtpHeaderSize, kRtpHeaderSize, kRedPayloadType);