From 1e80ce438eefab6394fb176fdcb8938a13dda16c Mon Sep 17 00:00:00 2001 From: Danil Chapovalov Date: Fri, 19 Feb 2016 16:02:15 +0100 Subject: [PATCH] webrtc::RtpPacket name freed for better RtpPacket There were two different structures named RtpPacket in webrtc namespace: RtpPacket defined in fec_test_helper renamed to test::RawRtpPacket RtpPacket defined in rtp_sender_video and producer_fec removed as unused BUG=webrtc:5261 R=sprang@google.com, stefan@webrtc.org Review URL: https://codereview.webrtc.org/1710103004 . Cr-Commit-Position: refs/heads/master@{#11682} --- .../rtp_rtcp/source/fec_receiver_unittest.cc | 57 ++++++++++--------- .../rtp_rtcp/source/fec_test_helper.cc | 17 +++--- .../modules/rtp_rtcp/source/fec_test_helper.h | 21 +++---- .../modules/rtp_rtcp/source/producer_fec.cc | 5 -- webrtc/modules/rtp_rtcp/source/producer_fec.h | 2 - .../rtp_rtcp/source/producer_fec_unittest.cc | 11 ++-- .../rtp_rtcp/source/rtp_sender_video.cc | 5 -- .../rtp_rtcp/source/rtp_sender_video.h | 1 - 8 files changed, 58 insertions(+), 61 deletions(-) diff --git a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc index bb22e1d580..ee8f408720 100644 --- a/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/fec_receiver_unittest.cc @@ -26,6 +26,7 @@ using ::testing::_; using ::testing::Args; using ::testing::ElementsAreArray; using ::testing::Return; +using Packet = webrtc::ForwardErrorCorrection::Packet; namespace webrtc { @@ -46,8 +47,9 @@ class ReceiverFecTest : public ::testing::Test { ASSERT_EQ(num_fec_packets, fec_packets->size()); } - void GenerateFrame(int num_media_packets, int frame_offset, - std::list* media_rtp_packets, + void GenerateFrame(int num_media_packets, + int frame_offset, + std::list* media_rtp_packets, std::list* media_packets) { generator_->NewFrame(num_media_packets); for (int i = 0; i < num_media_packets; ++i) { @@ -57,7 +59,8 @@ class ReceiverFecTest : public ::testing::Test { } } - void VerifyReconstructedMediaPacket(const RtpPacket* packet, int times) { + void VerifyReconstructedMediaPacket(const test::RawRtpPacket* packet, + int 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 // of times in a row. @@ -67,8 +70,8 @@ class ReceiverFecTest : public ::testing::Test { .Times(times).WillRepeatedly(Return(true)); } - void BuildAndAddRedMediaPacket(RtpPacket* packet) { - RtpPacket* red_packet = generator_->BuildMediaRedPacket(packet); + void BuildAndAddRedMediaPacket(test::RawRtpPacket* packet) { + test::RawRtpPacket* red_packet = generator_->BuildMediaRedPacket(packet); EXPECT_EQ(0, receiver_fec_->AddReceivedRedPacket( red_packet->header.header, red_packet->data, red_packet->length, kFecPayloadType)); @@ -76,7 +79,7 @@ class ReceiverFecTest : public ::testing::Test { } void BuildAndAddRedFecPacket(Packet* packet) { - RtpPacket* red_packet = generator_->BuildFecRedPacket(packet); + test::RawRtpPacket* red_packet = generator_->BuildFecRedPacket(packet); EXPECT_EQ(0, receiver_fec_->AddReceivedRedPacket( red_packet->header.header, red_packet->data, red_packet->length, kFecPayloadType)); @@ -103,14 +106,14 @@ void DeletePackets(std::list* packets) { TEST_F(ReceiverFecTest, TwoMediaOneFec) { const unsigned int kNumFecPackets = 1u; - std::list media_rtp_packets; + std::list media_rtp_packets; std::list media_packets; GenerateFrame(2, 0, &media_rtp_packets, &media_packets); std::list fec_packets; GenerateFEC(&media_packets, &fec_packets, kNumFecPackets); // Recovery - std::list::iterator it = media_rtp_packets.begin(); + std::list::iterator it = media_rtp_packets.begin(); BuildAndAddRedMediaPacket(*it); VerifyReconstructedMediaPacket(*it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); @@ -134,7 +137,7 @@ void ReceiverFecTest::InjectGarbagePacketLength(size_t fec_garbage_offset) { .WillRepeatedly(Return(true)); const unsigned int kNumFecPackets = 1u; - std::list media_rtp_packets; + std::list media_rtp_packets; std::list media_packets; GenerateFrame(2, 0, &media_rtp_packets, &media_packets); std::list fec_packets; @@ -169,7 +172,7 @@ TEST_F(ReceiverFecTest, InjectGarbageFecLevelHeaderProtectionLength) { TEST_F(ReceiverFecTest, TwoMediaTwoFec) { const unsigned int kNumFecPackets = 2u; - std::list media_rtp_packets; + std::list media_rtp_packets; std::list media_packets; GenerateFrame(2, 0, &media_rtp_packets, &media_packets); std::list fec_packets; @@ -177,7 +180,7 @@ TEST_F(ReceiverFecTest, TwoMediaTwoFec) { // Recovery // Drop both media packets. - std::list::iterator it = media_rtp_packets.begin(); + std::list::iterator it = media_rtp_packets.begin(); std::list::iterator fec_it = fec_packets.begin(); BuildAndAddRedFecPacket(*fec_it); VerifyReconstructedMediaPacket(*it, 1); @@ -193,7 +196,7 @@ TEST_F(ReceiverFecTest, TwoMediaTwoFec) { TEST_F(ReceiverFecTest, TwoFramesOneFec) { const unsigned int kNumFecPackets = 1u; - std::list media_rtp_packets; + std::list media_rtp_packets; std::list media_packets; GenerateFrame(1, 0, &media_rtp_packets, &media_packets); GenerateFrame(1, 1, &media_rtp_packets, &media_packets); @@ -201,7 +204,7 @@ TEST_F(ReceiverFecTest, TwoFramesOneFec) { GenerateFEC(&media_packets, &fec_packets, kNumFecPackets); // Recovery - std::list::iterator it = media_rtp_packets.begin(); + std::list::iterator it = media_rtp_packets.begin(); BuildAndAddRedMediaPacket(media_rtp_packets.front()); VerifyReconstructedMediaPacket(*it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); @@ -216,7 +219,7 @@ TEST_F(ReceiverFecTest, TwoFramesOneFec) { TEST_F(ReceiverFecTest, OneCompleteOneUnrecoverableFrame) { const unsigned int kNumFecPackets = 1u; - std::list media_rtp_packets; + std::list media_rtp_packets; std::list media_packets; GenerateFrame(1, 0, &media_rtp_packets, &media_packets); GenerateFrame(2, 1, &media_rtp_packets, &media_packets); @@ -225,7 +228,7 @@ TEST_F(ReceiverFecTest, OneCompleteOneUnrecoverableFrame) { GenerateFEC(&media_packets, &fec_packets, kNumFecPackets); // Recovery - std::list::iterator it = media_rtp_packets.begin(); + std::list::iterator it = media_rtp_packets.begin(); BuildAndAddRedMediaPacket(*it); // First frame: one packet. VerifyReconstructedMediaPacket(*it, 1); EXPECT_EQ(0, receiver_fec_->ProcessReceivedFec()); @@ -240,7 +243,7 @@ TEST_F(ReceiverFecTest, OneCompleteOneUnrecoverableFrame) { TEST_F(ReceiverFecTest, MaxFramesOneFec) { const unsigned int kNumFecPackets = 1u; const unsigned int kNumMediaPackets = 48u; - std::list media_rtp_packets; + std::list media_rtp_packets; std::list media_packets; for (unsigned int i = 0; i < kNumMediaPackets; ++i) { GenerateFrame(1, i, &media_rtp_packets, &media_packets); @@ -249,7 +252,7 @@ TEST_F(ReceiverFecTest, MaxFramesOneFec) { GenerateFEC(&media_packets, &fec_packets, kNumFecPackets); // Recovery - std::list::iterator it = media_rtp_packets.begin(); + std::list::iterator it = media_rtp_packets.begin(); ++it; // Drop first packet. for (; it != media_rtp_packets.end(); ++it) { BuildAndAddRedMediaPacket(*it); @@ -267,7 +270,7 @@ TEST_F(ReceiverFecTest, MaxFramesOneFec) { TEST_F(ReceiverFecTest, TooManyFrames) { const unsigned int kNumFecPackets = 1u; const unsigned int kNumMediaPackets = 49u; - std::list media_rtp_packets; + std::list media_rtp_packets; std::list media_packets; for (unsigned int i = 0; i < kNumMediaPackets; ++i) { GenerateFrame(1, i, &media_rtp_packets, &media_packets); @@ -286,7 +289,7 @@ TEST_F(ReceiverFecTest, PacketNotDroppedTooEarly) { Packet* delayed_fec = NULL; const unsigned int kNumFecPacketsBatch1 = 1u; const unsigned int kNumMediaPacketsBatch1 = 2u; - std::list media_rtp_packets_batch1; + std::list media_rtp_packets_batch1; std::list media_packets_batch1; GenerateFrame(kNumMediaPacketsBatch1, 0, &media_rtp_packets_batch1, &media_packets_batch1); @@ -301,12 +304,13 @@ TEST_F(ReceiverFecTest, PacketNotDroppedTooEarly) { // Fill the FEC decoder. No packets should be dropped. const unsigned int kNumMediaPacketsBatch2 = 46u; - std::list media_rtp_packets_batch2; + std::list media_rtp_packets_batch2; std::list media_packets_batch2; for (unsigned int i = 0; i < kNumMediaPacketsBatch2; ++i) { GenerateFrame(1, i, &media_rtp_packets_batch2, &media_packets_batch2); } - for (std::list::iterator it = media_rtp_packets_batch2.begin(); + for (std::list::iterator it = + media_rtp_packets_batch2.begin(); it != media_rtp_packets_batch2.end(); ++it) { BuildAndAddRedMediaPacket(*it); EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)) @@ -330,7 +334,7 @@ TEST_F(ReceiverFecTest, PacketDroppedWhenTooOld) { Packet* delayed_fec = NULL; const unsigned int kNumFecPacketsBatch1 = 1u; const unsigned int kNumMediaPacketsBatch1 = 2u; - std::list media_rtp_packets_batch1; + std::list media_rtp_packets_batch1; std::list media_packets_batch1; GenerateFrame(kNumMediaPacketsBatch1, 0, &media_rtp_packets_batch1, &media_packets_batch1); @@ -345,12 +349,13 @@ TEST_F(ReceiverFecTest, PacketDroppedWhenTooOld) { // Fill the FEC decoder and force the last packet to be dropped. const unsigned int kNumMediaPacketsBatch2 = 48u; - std::list media_rtp_packets_batch2; + std::list media_rtp_packets_batch2; std::list media_packets_batch2; for (unsigned int i = 0; i < kNumMediaPacketsBatch2; ++i) { GenerateFrame(1, i, &media_rtp_packets_batch2, &media_packets_batch2); } - for (std::list::iterator it = media_rtp_packets_batch2.begin(); + for (std::list::iterator it = + media_rtp_packets_batch2.begin(); it != media_rtp_packets_batch2.end(); ++it) { BuildAndAddRedMediaPacket(*it); EXPECT_CALL(rtp_data_callback_, OnRecoveredPacket(_, _)) @@ -373,10 +378,10 @@ TEST_F(ReceiverFecTest, OldFecPacketDropped) { // 49 frames with 2 media packets and one FEC packet. All media packets // missing. const unsigned int kNumMediaPackets = 49 * 2; - std::list media_rtp_packets; + std::list media_rtp_packets; std::list media_packets; for (unsigned int i = 0; i < kNumMediaPackets / 2; ++i) { - std::list frame_media_rtp_packets; + std::list frame_media_rtp_packets; std::list frame_media_packets; std::list fec_packets; GenerateFrame(2, 0, &frame_media_rtp_packets, &frame_media_packets); diff --git a/webrtc/modules/rtp_rtcp/source/fec_test_helper.cc b/webrtc/modules/rtp_rtcp/source/fec_test_helper.cc index 9c14a213f1..e767f5a681 100644 --- a/webrtc/modules/rtp_rtcp/source/fec_test_helper.cc +++ b/webrtc/modules/rtp_rtcp/source/fec_test_helper.cc @@ -25,8 +25,8 @@ void FrameGenerator::NewFrame(int num_packets) { uint16_t FrameGenerator::NextSeqNum() { return ++seq_num_; } -RtpPacket* FrameGenerator::NextPacket(int offset, size_t length) { - RtpPacket* rtp_packet = new RtpPacket; +test::RawRtpPacket* FrameGenerator::NextPacket(int offset, size_t length) { + test::RawRtpPacket* rtp_packet = new test::RawRtpPacket; for (size_t i = 0; i < length; ++i) rtp_packet->data[i + kRtpHeaderSize] = offset + i; rtp_packet->length = length + kRtpHeaderSize; @@ -44,9 +44,10 @@ RtpPacket* FrameGenerator::NextPacket(int offset, size_t length) { } // Creates a new RtpPacket with the RED header added to the packet. -RtpPacket* FrameGenerator::BuildMediaRedPacket(const RtpPacket* packet) { +test::RawRtpPacket* FrameGenerator::BuildMediaRedPacket( + const test::RawRtpPacket* packet) { const size_t kHeaderLength = packet->header.header.headerLength; - RtpPacket* red_packet = new RtpPacket; + test::RawRtpPacket* red_packet = new test::RawRtpPacket; red_packet->header = packet->header; red_packet->length = packet->length + 1; // 1 byte RED header. memset(red_packet->data, 0, red_packet->length); @@ -61,10 +62,11 @@ RtpPacket* FrameGenerator::BuildMediaRedPacket(const RtpPacket* 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|. -RtpPacket* FrameGenerator::BuildFecRedPacket(const Packet* packet) { +test::RawRtpPacket* FrameGenerator::BuildFecRedPacket( + const ForwardErrorCorrection::Packet* packet) { // Create a fake media packet to get a correct header. 1 byte RED header. ++num_packets_; - RtpPacket* red_packet = NextPacket(0, packet->length + 1); + test::RawRtpPacket* 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); @@ -73,7 +75,8 @@ RtpPacket* FrameGenerator::BuildFecRedPacket(const Packet* packet) { return red_packet; } -void FrameGenerator::SetRedHeader(Packet* red_packet, uint8_t payload_type, +void FrameGenerator::SetRedHeader(ForwardErrorCorrection::Packet* red_packet, + uint8_t payload_type, size_t header_length) const { // Replace pltype. red_packet->data[1] &= 0x80; // Reset. diff --git a/webrtc/modules/rtp_rtcp/source/fec_test_helper.h b/webrtc/modules/rtp_rtcp/source/fec_test_helper.h index aacc2d1ecc..6af7f9cc7f 100644 --- a/webrtc/modules/rtp_rtcp/source/fec_test_helper.h +++ b/webrtc/modules/rtp_rtcp/source/fec_test_helper.h @@ -15,17 +15,16 @@ #include "webrtc/modules/rtp_rtcp/source/forward_error_correction.h" namespace webrtc { +namespace test { +struct RawRtpPacket : public ForwardErrorCorrection::Packet { + WebRtcRTPHeader header; +}; +} // namespace test const uint8_t kFecPayloadType = 96; const uint8_t kRedPayloadType = 97; const uint8_t kVp8PayloadType = 120; -typedef ForwardErrorCorrection::Packet Packet; - -struct RtpPacket : public Packet { - WebRtcRTPHeader header; -}; - class FrameGenerator { public: FrameGenerator(); @@ -34,17 +33,19 @@ class FrameGenerator { uint16_t NextSeqNum(); - RtpPacket* NextPacket(int offset, size_t length); + test::RawRtpPacket* NextPacket(int offset, size_t length); // Creates a new RtpPacket with the RED header added to the packet. - RtpPacket* BuildMediaRedPacket(const RtpPacket* packet); + test::RawRtpPacket* BuildMediaRedPacket(const test::RawRtpPacket* 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|. - RtpPacket* BuildFecRedPacket(const Packet* packet); + test::RawRtpPacket* BuildFecRedPacket( + const ForwardErrorCorrection::Packet* packet); - void SetRedHeader(Packet* red_packet, uint8_t payload_type, + void SetRedHeader(ForwardErrorCorrection::Packet* red_packet, + uint8_t payload_type, size_t header_length) const; private: diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.cc b/webrtc/modules/rtp_rtcp/source/producer_fec.cc index 6ec213ee43..69a28ed4db 100644 --- a/webrtc/modules/rtp_rtcp/source/producer_fec.cc +++ b/webrtc/modules/rtp_rtcp/source/producer_fec.cc @@ -32,11 +32,6 @@ enum { kHighProtectionThreshold = 80 }; // Corresponds to ~30 overhead, range // is 0 to 255, where 255 corresponds to 100% overhead (relative to number of // media packets). -struct RtpPacket { - uint16_t rtpHeaderLength; - ForwardErrorCorrection::Packet* pkt; -}; - RedPacket::RedPacket(size_t length) : data_(new uint8_t[length]), length_(length), diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec.h b/webrtc/modules/rtp_rtcp/source/producer_fec.h index b2fdfeccac..dcd71c8339 100644 --- a/webrtc/modules/rtp_rtcp/source/producer_fec.h +++ b/webrtc/modules/rtp_rtcp/source/producer_fec.h @@ -18,8 +18,6 @@ namespace webrtc { -struct RtpPacket; - class RedPacket { public: explicit RedPacket(size_t length); diff --git a/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc b/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc index be4b453454..fad0f502f5 100644 --- a/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc +++ b/webrtc/modules/rtp_rtcp/source/producer_fec_unittest.cc @@ -112,12 +112,12 @@ TEST_F(ProducerFecTest, OneFrameFec) { // media packets for 1 frame is at least |minimum_media_packets_fec_|. const int kNumPackets = 4; FecProtectionParams params = {15, false, 3}; - std::list rtp_packets; + std::list rtp_packets; generator_->NewFrame(kNumPackets); producer_->SetFecParameters(¶ms, 0); // Expecting one FEC packet. uint32_t last_timestamp = 0; for (int i = 0; i < kNumPackets; ++i) { - RtpPacket* rtp_packet = generator_->NextPacket(i, 10); + test::RawRtpPacket* rtp_packet = generator_->NextPacket(i, 10); rtp_packets.push_back(rtp_packet); EXPECT_EQ(0, producer_->AddRtpPacketAndGenerateFec(rtp_packet->data, rtp_packet->length, @@ -153,13 +153,14 @@ TEST_F(ProducerFecTest, TwoFrameFec) { const int kNumFrames = 2; FecProtectionParams params = {15, 0, 3}; - std::list rtp_packets; + std::list rtp_packets; producer_->SetFecParameters(¶ms, 0); // Expecting one FEC packet. uint32_t last_timestamp = 0; for (int i = 0; i < kNumFrames; ++i) { generator_->NewFrame(kNumPackets); for (int j = 0; j < kNumPackets; ++j) { - RtpPacket* rtp_packet = generator_->NextPacket(i * kNumPackets + j, 10); + test::RawRtpPacket* rtp_packet = + generator_->NextPacket(i * kNumPackets + j, 10); rtp_packets.push_back(rtp_packet); EXPECT_EQ(0, producer_->AddRtpPacketAndGenerateFec(rtp_packet->data, rtp_packet->length, @@ -186,7 +187,7 @@ TEST_F(ProducerFecTest, TwoFrameFec) { TEST_F(ProducerFecTest, BuildRedPacket) { generator_->NewFrame(1); - RtpPacket* packet = generator_->NextPacket(0, 10); + test::RawRtpPacket* packet = generator_->NextPacket(0, 10); rtc::scoped_ptr red_packet(producer_->BuildRedPacket( packet->data, packet->length - kRtpHeaderSize, kRtpHeaderSize, kRedPayloadType)); diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc index 1eecd0bca1..32ba26f54b 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.cc @@ -29,11 +29,6 @@ namespace webrtc { enum { REDForFECHeaderLength = 1 }; -struct RtpPacket { - uint16_t rtpHeaderLength; - ForwardErrorCorrection::Packet* pkt; -}; - RTPSenderVideo::RTPSenderVideo(Clock* clock, RTPSenderInterface* rtpSender) : _rtpSender(*rtpSender), crit_(CriticalSectionWrapper::CreateCriticalSection()), diff --git a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h index e3487f655b..dc1088a3f7 100644 --- a/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h +++ b/webrtc/modules/rtp_rtcp/source/rtp_sender_video.h @@ -28,7 +28,6 @@ namespace webrtc { class CriticalSectionWrapper; -struct RtpPacket; class RTPSenderVideo { public: