From 349c2bb223139bf136e75d0f47e76e6d470b9f07 Mon Sep 17 00:00:00 2001 From: Wan-Teh Chang Date: Fri, 5 Jun 2015 14:45:05 -0700 Subject: [PATCH] Remove the timestamp_ member of StreamGenerator. timestamp_ is only used in GenerateFrame() and its old value is discarded. So it just needs to be a local variable in GenerateFrame(). As a result, we can remove the start_timestamp parameter from the constructor and Init(). Also mark the GeneratePacket() method private because it is only used internally. R=stefan@webrtc.org BUG=none TEST=none Review URL: https://webrtc-codereview.appspot.com/50149004 Cr-Commit-Position: refs/heads/master@{#9386} --- .../main/source/jitter_buffer_unittest.cc | 18 +++++++-------- .../main/source/receiver_unittest.cc | 4 ++-- .../main/source/test/stream_generator.cc | 21 ++++++----------- .../main/source/test/stream_generator.h | 23 ++++++++----------- 4 files changed, 27 insertions(+), 39 deletions(-) diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc b/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc index b5f8b953ac..280507c57a 100644 --- a/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc +++ b/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc @@ -129,7 +129,7 @@ class TestRunningJitterBuffer : public ::testing::Test { max_nack_list_size_ = 150; oldest_packet_to_nack_ = 250; jitter_buffer_ = new VCMJitterBuffer(clock_.get(), &event_factory_); - stream_generator_ = new StreamGenerator(0, 0, clock_->TimeInMilliseconds()); + stream_generator_ = new StreamGenerator(0, clock_->TimeInMilliseconds()); jitter_buffer_->Start(); jitter_buffer_->SetNackSettings(max_nack_list_size_, oldest_packet_to_nack_, 0); @@ -1995,7 +1995,7 @@ TEST_F(TestJitterBufferNack, NoNackListReturnedBeforeFirstDecode) { } TEST_F(TestJitterBufferNack, NackListBuiltBeforeFirstDecode) { - stream_generator_->Init(0, 0, clock_->TimeInMilliseconds()); + stream_generator_->Init(0, clock_->TimeInMilliseconds()); InsertFrame(kVideoFrameKey); stream_generator_->GenerateFrame(kVideoFrameDelta, 2, 0, clock_->TimeInMilliseconds()); @@ -2008,7 +2008,7 @@ TEST_F(TestJitterBufferNack, NackListBuiltBeforeFirstDecode) { } TEST_F(TestJitterBufferNack, VerifyRetransmittedFlag) { - stream_generator_->Init(0, 0, clock_->TimeInMilliseconds()); + stream_generator_->Init(0, clock_->TimeInMilliseconds()); stream_generator_->GenerateFrame(kVideoFrameKey, 3, 0, clock_->TimeInMilliseconds()); VCMPacket packet; @@ -2033,7 +2033,7 @@ TEST_F(TestJitterBufferNack, VerifyRetransmittedFlag) { } TEST_F(TestJitterBufferNack, UseNackToRecoverFirstKeyFrame) { - stream_generator_->Init(0, 0, clock_->TimeInMilliseconds()); + stream_generator_->Init(0, clock_->TimeInMilliseconds()); stream_generator_->GenerateFrame(kVideoFrameKey, 3, 0, clock_->TimeInMilliseconds()); EXPECT_EQ(kIncomplete, InsertPacketAndPop(0)); @@ -2050,7 +2050,7 @@ TEST_F(TestJitterBufferNack, UseNackToRecoverFirstKeyFrame) { TEST_F(TestJitterBufferNack, UseNackToRecoverFirstKeyFrameSecondInQueue) { VCMPacket packet; - stream_generator_->Init(0, 0, clock_->TimeInMilliseconds()); + stream_generator_->Init(0, clock_->TimeInMilliseconds()); // First frame is delta. stream_generator_->GenerateFrame(kVideoFrameDelta, 3, 0, clock_->TimeInMilliseconds()); @@ -2114,7 +2114,7 @@ TEST_F(TestJitterBufferNack, NormalOperationWrap) { // ------- ------------------------------------------------------------ // | 65532 | | 65533 | 65534 | 65535 | x | 1 | .. | 9 | x | 11 |.....| 96 | // ------- ------------------------------------------------------------ - stream_generator_->Init(65532, 0, clock_->TimeInMilliseconds()); + stream_generator_->Init(65532, clock_->TimeInMilliseconds()); InsertFrame(kVideoFrameKey); EXPECT_FALSE(request_key_frame); EXPECT_TRUE(DecodeCompleteFrame()); @@ -2148,7 +2148,7 @@ TEST_F(TestJitterBufferNack, NormalOperationWrap2) { // ----------------------------------- // | 65532 | 65533 | 65534 | x | 0 | 1 | // ----------------------------------- - stream_generator_->Init(65532, 0, clock_->TimeInMilliseconds()); + stream_generator_->Init(65532, clock_->TimeInMilliseconds()); InsertFrame(kVideoFrameKey); EXPECT_FALSE(request_key_frame); EXPECT_TRUE(DecodeCompleteFrame()); @@ -2176,7 +2176,7 @@ TEST_F(TestJitterBufferNack, NormalOperationWrap2) { } TEST_F(TestJitterBufferNack, ResetByFutureKeyFrameDoesntError) { - stream_generator_->Init(0, 0, clock_->TimeInMilliseconds()); + stream_generator_->Init(0, clock_->TimeInMilliseconds()); InsertFrame(kVideoFrameKey); EXPECT_TRUE(DecodeCompleteFrame()); bool extended = false; @@ -2186,7 +2186,7 @@ TEST_F(TestJitterBufferNack, ResetByFutureKeyFrameDoesntError) { // Far-into-the-future video frame, could be caused by resetting the encoder // or otherwise restarting. This should not fail when error when the packet is // a keyframe, even if all of the nack list needs to be flushed. - stream_generator_->Init(10000, 0, clock_->TimeInMilliseconds()); + stream_generator_->Init(10000, clock_->TimeInMilliseconds()); clock_->AdvanceTimeMilliseconds(kDefaultFramePeriodMs); InsertFrame(kVideoFrameKey); EXPECT_TRUE(DecodeCompleteFrame()); diff --git a/webrtc/modules/video_coding/main/source/receiver_unittest.cc b/webrtc/modules/video_coding/main/source/receiver_unittest.cc index 6365ab3920..2e16984720 100644 --- a/webrtc/modules/video_coding/main/source/receiver_unittest.cc +++ b/webrtc/modules/video_coding/main/source/receiver_unittest.cc @@ -31,8 +31,8 @@ class TestVCMReceiver : public ::testing::Test { : clock_(new SimulatedClock(0)), timing_(clock_.get()), receiver_(&timing_, clock_.get(), &event_factory_) { - stream_generator_.reset(new - StreamGenerator(0, 0, clock_->TimeInMilliseconds())); + stream_generator_.reset( + new StreamGenerator(0, clock_->TimeInMilliseconds())); } virtual void SetUp() { diff --git a/webrtc/modules/video_coding/main/source/test/stream_generator.cc b/webrtc/modules/video_coding/main/source/test/stream_generator.cc index 4f85dffc50..00592cea86 100644 --- a/webrtc/modules/video_coding/main/source/test/stream_generator.cc +++ b/webrtc/modules/video_coding/main/source/test/stream_generator.cc @@ -21,20 +21,13 @@ namespace webrtc { -StreamGenerator::StreamGenerator(uint16_t start_seq_num, - uint32_t start_timestamp, - int64_t current_time) - : packets_(), - sequence_number_(start_seq_num), - timestamp_(start_timestamp), - start_time_(current_time) {} +StreamGenerator::StreamGenerator(uint16_t start_seq_num, int64_t current_time) + : packets_(), sequence_number_(start_seq_num), start_time_(current_time) { +} -void StreamGenerator::Init(uint16_t start_seq_num, - uint32_t start_timestamp, - int64_t current_time) { +void StreamGenerator::Init(uint16_t start_seq_num, int64_t current_time) { packets_.clear(); sequence_number_ = start_seq_num; - timestamp_ = start_timestamp; start_time_ = current_time; memset(&packet_buffer, 0, sizeof(packet_buffer)); } @@ -43,18 +36,18 @@ void StreamGenerator::GenerateFrame(FrameType type, int num_media_packets, int num_empty_packets, int64_t current_time) { - timestamp_ = 90 * (current_time - start_time_); + uint32_t timestamp = 90 * (current_time - start_time_); for (int i = 0; i < num_media_packets; ++i) { const int packet_size = (kFrameSize + num_media_packets / 2) / num_media_packets; bool marker_bit = (i == num_media_packets - 1); packets_.push_back(GeneratePacket( - sequence_number_, timestamp_, packet_size, (i == 0), marker_bit, type)); + sequence_number_, timestamp, packet_size, (i == 0), marker_bit, type)); ++sequence_number_; } for (int i = 0; i < num_empty_packets; ++i) { packets_.push_back(GeneratePacket( - sequence_number_, timestamp_, 0, false, false, kFrameEmpty)); + sequence_number_, timestamp, 0, false, false, kFrameEmpty)); ++sequence_number_; } } diff --git a/webrtc/modules/video_coding/main/source/test/stream_generator.h b/webrtc/modules/video_coding/main/source/test/stream_generator.h index 6565527db6..994dd5cb76 100644 --- a/webrtc/modules/video_coding/main/source/test/stream_generator.h +++ b/webrtc/modules/video_coding/main/source/test/stream_generator.h @@ -28,25 +28,14 @@ const int kDefaultFramePeriodMs = 1000 / kDefaultFrameRate; class StreamGenerator { public: - StreamGenerator(uint16_t start_seq_num, - uint32_t start_timestamp, - int64_t current_time); - void Init(uint16_t start_seq_num, - uint32_t start_timestamp, - int64_t current_time); + StreamGenerator(uint16_t start_seq_num, int64_t current_time); + void Init(uint16_t start_seq_num, int64_t current_time); void GenerateFrame(FrameType type, int num_media_packets, int num_empty_packets, int64_t current_time); - VCMPacket GeneratePacket(uint16_t sequence_number, - uint32_t timestamp, - unsigned int size, - bool first_packet, - bool marker_bit, - FrameType type); - bool PopPacket(VCMPacket* packet, int index); void DropLastPacket(); @@ -59,11 +48,17 @@ class StreamGenerator { int PacketsRemaining() const; private: + VCMPacket GeneratePacket(uint16_t sequence_number, + uint32_t timestamp, + unsigned int size, + bool first_packet, + bool marker_bit, + FrameType type); + std::list::iterator GetPacketIterator(int index); std::list packets_; uint16_t sequence_number_; - uint32_t timestamp_; int64_t start_time_; uint8_t packet_buffer[kMaxPacketSize];