Resolved Rebase Conflicts
This is just https://webrtc-codereview.appspot.com/53629004/
Remove a constructor of VCMJitterBuffer.
Remove unnecessary factory use
Comment Fix
Move frame incoming simulation to the clock
DCHECK typo fix
Coding Style Fix
Rephrased some comments, and removed some virtual for override function.
Coding Style Fix
Coding Style Fix
Add a unittest for VCMReceiver::FrameForDecoding. Mainly test the time control algorithm.
BUG=
TBR=holmer@chromium.org
Review URL: https://codereview.webrtc.org/1173253008.
Cr-Commit-Position: refs/heads/master@{#9470}
diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.cc b/webrtc/modules/video_coding/main/source/jitter_buffer.cc
index da2ca3c..9156cc1 100644
--- a/webrtc/modules/video_coding/main/source/jitter_buffer.cc
+++ b/webrtc/modules/video_coding/main/source/jitter_buffer.cc
@@ -113,11 +113,12 @@
}
}
-VCMJitterBuffer::VCMJitterBuffer(Clock* clock, EventFactory* event_factory)
+VCMJitterBuffer::VCMJitterBuffer(Clock* clock,
+ rtc::scoped_ptr<EventWrapper> event)
: clock_(clock),
running_(false),
crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
- frame_event_(event_factory->CreateEvent()),
+ frame_event_(event.Pass()),
max_number_of_frames_(kStartNumberOfFrames),
free_frames_(),
decodable_frames_(),
diff --git a/webrtc/modules/video_coding/main/source/jitter_buffer.h b/webrtc/modules/video_coding/main/source/jitter_buffer.h
index d382580..455ac26 100644
--- a/webrtc/modules/video_coding/main/source/jitter_buffer.h
+++ b/webrtc/modules/video_coding/main/source/jitter_buffer.h
@@ -76,8 +76,8 @@
class VCMJitterBuffer {
public:
- VCMJitterBuffer(Clock* clock,
- EventFactory* event_factory);
+ VCMJitterBuffer(Clock* clock, rtc::scoped_ptr<EventWrapper> event);
+
~VCMJitterBuffer();
// Initializes and starts jitter buffer.
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 280507c..7ba4d68 100644
--- a/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc
+++ b/webrtc/modules/video_coding/main/source/jitter_buffer_unittest.cc
@@ -29,8 +29,9 @@
protected:
virtual void SetUp() {
clock_.reset(new SimulatedClock(0));
- jitter_buffer_.reset(
- new VCMJitterBuffer(clock_.get(), &event_factory_));
+ jitter_buffer_.reset(new VCMJitterBuffer(
+ clock_.get(),
+ rtc::scoped_ptr<EventWrapper>(event_factory_.CreateEvent())));
jitter_buffer_->Start();
seq_num_ = 1234;
timestamp_ = 0;
@@ -128,7 +129,9 @@
clock_.reset(new SimulatedClock(0));
max_nack_list_size_ = 150;
oldest_packet_to_nack_ = 250;
- jitter_buffer_ = new VCMJitterBuffer(clock_.get(), &event_factory_);
+ jitter_buffer_ = new VCMJitterBuffer(
+ clock_.get(),
+ rtc::scoped_ptr<EventWrapper>(event_factory_.CreateEvent()));
stream_generator_ = new StreamGenerator(0, clock_->TimeInMilliseconds());
jitter_buffer_->Start();
jitter_buffer_->SetNackSettings(max_nack_list_size_,
diff --git a/webrtc/modules/video_coding/main/source/receiver.cc b/webrtc/modules/video_coding/main/source/receiver.cc
index fbd1fee..e26bd86 100644
--- a/webrtc/modules/video_coding/main/source/receiver.cc
+++ b/webrtc/modules/video_coding/main/source/receiver.cc
@@ -28,11 +28,21 @@
VCMReceiver::VCMReceiver(VCMTiming* timing,
Clock* clock,
EventFactory* event_factory)
+ : VCMReceiver(timing,
+ clock,
+ rtc::scoped_ptr<EventWrapper>(event_factory->CreateEvent()),
+ rtc::scoped_ptr<EventWrapper>(event_factory->CreateEvent())) {
+}
+
+VCMReceiver::VCMReceiver(VCMTiming* timing,
+ Clock* clock,
+ rtc::scoped_ptr<EventWrapper> receiver_event,
+ rtc::scoped_ptr<EventWrapper> jitter_buffer_event)
: crit_sect_(CriticalSectionWrapper::CreateCriticalSection()),
clock_(clock),
- jitter_buffer_(clock_, event_factory),
+ jitter_buffer_(clock_, jitter_buffer_event.Pass()),
timing_(timing),
- render_wait_event_(event_factory->CreateEvent()),
+ render_wait_event_(receiver_event.Pass()),
max_video_delay_ms_(kMaxVideoDelayMs) {
Reset();
}
diff --git a/webrtc/modules/video_coding/main/source/receiver.h b/webrtc/modules/video_coding/main/source/receiver.h
index efd66bc..ce8320d 100644
--- a/webrtc/modules/video_coding/main/source/receiver.h
+++ b/webrtc/modules/video_coding/main/source/receiver.h
@@ -28,6 +28,16 @@
VCMReceiver(VCMTiming* timing,
Clock* clock,
EventFactory* event_factory);
+
+ // Using this constructor, you can specify a different event factory for the
+ // jitter buffer. Useful for unit tests when you want to simulate incoming
+ // packets, in which case the jitter buffer's wait event is different from
+ // that of VCMReceiver itself.
+ VCMReceiver(VCMTiming* timing,
+ Clock* clock,
+ rtc::scoped_ptr<EventWrapper> receiver_event,
+ rtc::scoped_ptr<EventWrapper> jitter_buffer_event);
+
~VCMReceiver();
void Reset();
diff --git a/webrtc/modules/video_coding/main/source/receiver_unittest.cc b/webrtc/modules/video_coding/main/source/receiver_unittest.cc
index 2e16984..dc63e81 100644
--- a/webrtc/modules/video_coding/main/source/receiver_unittest.cc
+++ b/webrtc/modules/video_coding/main/source/receiver_unittest.cc
@@ -10,8 +10,10 @@
#include <string.h>
#include <list>
+#include <queue>
#include "testing/gtest/include/gtest/gtest.h"
+#include "webrtc/base/checks.h"
#include "webrtc/modules/video_coding/main/source/packet.h"
#include "webrtc/modules/video_coding/main/source/receiver.h"
#include "webrtc/modules/video_coding/main/source/test/stream_generator.h"
@@ -31,8 +33,9 @@
: clock_(new SimulatedClock(0)),
timing_(clock_.get()),
receiver_(&timing_, clock_.get(), &event_factory_) {
- stream_generator_.reset(
- new StreamGenerator(0, clock_->TimeInMilliseconds()));
+
+ stream_generator_.reset(new
+ StreamGenerator(0, clock_->TimeInMilliseconds()));
}
virtual void SetUp() {
@@ -314,4 +317,212 @@
std::vector<uint16_t> nack_list = receiver_.NackList(&request_key_frame);
EXPECT_FALSE(request_key_frame);
}
+
+// A simulated clock, when time elapses, will insert frames into the jitter
+// buffer, based on initial settings.
+class SimulatedClockWithFrames : public SimulatedClock {
+ public:
+ SimulatedClockWithFrames(StreamGenerator* stream_generator,
+ VCMReceiver* receiver)
+ : SimulatedClock(0),
+ stream_generator_(stream_generator),
+ receiver_(receiver) {}
+ virtual ~SimulatedClockWithFrames() {}
+
+ // If |stop_on_frame| is true and next frame arrives between now and
+ // now+|milliseconds|, the clock will be advanced to the arrival time of next
+ // frame.
+ // Otherwise, the clock will be advanced by |milliseconds|.
+ //
+ // For both cases, a frame will be inserted into the jitter buffer at the
+ // instant when the clock time is timestamps_.front().arrive_time.
+ //
+ // Return true if some frame arrives between now and now+|milliseconds|.
+ bool AdvanceTimeMilliseconds(int64_t milliseconds, bool stop_on_frame) {
+ return AdvanceTimeMicroseconds(milliseconds * 1000, stop_on_frame);
+ };
+
+ bool AdvanceTimeMicroseconds(int64_t microseconds, bool stop_on_frame) {
+ int64_t start_time = TimeInMicroseconds();
+ int64_t end_time = start_time + microseconds;
+ bool frame_injected = false;
+ while (!timestamps_.empty() &&
+ timestamps_.front().arrive_time <= end_time) {
+ DCHECK(timestamps_.front().arrive_time >= start_time);
+
+ SimulatedClock::AdvanceTimeMicroseconds(timestamps_.front().arrive_time -
+ TimeInMicroseconds());
+ GenerateAndInsertFrame((timestamps_.front().render_time + 500) / 1000);
+ timestamps_.pop();
+ frame_injected = true;
+
+ if (stop_on_frame)
+ return frame_injected;
+ }
+
+ if (TimeInMicroseconds() < end_time) {
+ SimulatedClock::AdvanceTimeMicroseconds(end_time - TimeInMicroseconds());
+ }
+ return frame_injected;
+ };
+
+ // Input timestamps are in unit Milliseconds.
+ // And |arrive_timestamps| must be positive and in increasing order.
+ // |arrive_timestamps| determine when we are going to insert frames into the
+ // jitter buffer.
+ // |render_timestamps| are the timestamps on the frame.
+ void SetFrames(const int64_t* arrive_timestamps,
+ const int64_t* render_timestamps,
+ size_t size) {
+ int64_t previous_arrive_timestamp = 0;
+ for (size_t i = 0; i < size; i++) {
+ CHECK(arrive_timestamps[i] >= previous_arrive_timestamp);
+ timestamps_.push(TimestampPair(arrive_timestamps[i] * 1000,
+ render_timestamps[i] * 1000));
+ previous_arrive_timestamp = arrive_timestamps[i];
+ }
+ }
+
+ private:
+ struct TimestampPair {
+ TimestampPair(int64_t arrive_timestamp, int64_t render_timestamp)
+ : arrive_time(arrive_timestamp), render_time(render_timestamp) {}
+
+ int64_t arrive_time;
+ int64_t render_time;
+ };
+
+ void GenerateAndInsertFrame(int64_t render_timestamp_ms) {
+ VCMPacket packet;
+ stream_generator_->GenerateFrame(FrameType::kVideoFrameKey,
+ 1, // media packets
+ 0, // empty packets
+ render_timestamp_ms);
+
+ bool packet_available = stream_generator_->PopPacket(&packet, 0);
+ EXPECT_TRUE(packet_available);
+ if (!packet_available)
+ return; // Return here to avoid crashes below.
+ receiver_->InsertPacket(packet, 640, 480);
+ }
+
+ std::queue<TimestampPair> timestamps_;
+ StreamGenerator* stream_generator_;
+ VCMReceiver* receiver_;
+};
+
+// Use a SimulatedClockWithFrames
+// Wait call will do either of these:
+// 1. If |stop_on_frame| is true, the clock will be turned to the exact instant
+// that the first frame comes and the frame will be inserted into the jitter
+// buffer, or the clock will be turned to now + |max_time| if no frame comes in
+// the window.
+// 2. If |stop_on_frame| is false, the clock will be turn to now + |max_time|,
+// and all the frames arriving between now and now + |max_time| will be
+// inserted into the jitter buffer.
+//
+// This is used to simulate the JitterBuffer getting packets from internet as
+// time elapses.
+
+class FrameInjectEvent : public EventWrapper {
+ public:
+ FrameInjectEvent(SimulatedClockWithFrames* clock, bool stop_on_frame)
+ : clock_(clock), stop_on_frame_(stop_on_frame) {}
+
+ bool Set() override { return true; }
+
+ EventTypeWrapper Wait(unsigned long max_time) override {
+ if (clock_->AdvanceTimeMilliseconds(max_time, stop_on_frame_) &&
+ stop_on_frame_) {
+ return EventTypeWrapper::kEventSignaled;
+ } else {
+ return EventTypeWrapper::kEventTimeout;
+ }
+ }
+
+ private:
+ SimulatedClockWithFrames* clock_;
+ bool stop_on_frame_;
+};
+
+class VCMReceiverTimingTest : public ::testing::Test {
+ protected:
+
+ VCMReceiverTimingTest()
+
+ : clock_(&stream_generator_, &receiver_),
+ stream_generator_(0, clock_.TimeInMilliseconds()),
+ timing_(&clock_),
+ receiver_(
+ &timing_,
+ &clock_,
+ rtc::scoped_ptr<EventWrapper>(new FrameInjectEvent(&clock_, false)),
+ rtc::scoped_ptr<EventWrapper>(
+ new FrameInjectEvent(&clock_, true))) {}
+
+
+ virtual void SetUp() { receiver_.Reset(); }
+
+ SimulatedClockWithFrames clock_;
+ StreamGenerator stream_generator_;
+ VCMTiming timing_;
+ VCMReceiver receiver_;
+};
+
+// Test whether VCMReceiver::FrameForDecoding handles parameter
+// |max_wait_time_ms| correctly:
+// 1. The function execution should never take more than |max_wait_time_ms|.
+// 2. If the function exit before now + |max_wait_time_ms|, a frame must be
+// returned.
+TEST_F(VCMReceiverTimingTest, FrameForDecoding) {
+ const size_t kNumFrames = 100;
+ const int kFramePeriod = 40;
+ int64_t arrive_timestamps[kNumFrames];
+ int64_t render_timestamps[kNumFrames];
+ int64_t next_render_time;
+
+ // Construct test samples.
+ // render_timestamps are the timestamps stored in the Frame;
+ // arrive_timestamps controls when the Frame packet got received.
+ for (size_t i = 0; i < kNumFrames; i++) {
+ // Preset frame rate to 25Hz.
+ // But we add a reasonable deviation to arrive_timestamps to mimic Internet
+ // fluctuation.
+ arrive_timestamps[i] =
+ (i + 1) * kFramePeriod + (i % 10) * ((i % 2) ? 1 : -1);
+ render_timestamps[i] = (i + 1) * kFramePeriod;
+ }
+
+ clock_.SetFrames(arrive_timestamps, render_timestamps, kNumFrames);
+
+ // Record how many frames we finally get out of the receiver.
+ size_t num_frames_return = 0;
+
+ const int64_t kMaxWaitTime = 30;
+
+ // Ideally, we should get all frames that we input in InitializeFrames.
+ // In the case that FrameForDecoding kills frames by error, we rely on the
+ // build bot to kill the test.
+ while (num_frames_return < kNumFrames) {
+ int64_t start_time = clock_.TimeInMilliseconds();
+ VCMEncodedFrame* frame =
+ receiver_.FrameForDecoding(kMaxWaitTime, next_render_time, false);
+ int64_t end_time = clock_.TimeInMilliseconds();
+
+ // In any case the FrameForDecoding should not wait longer than
+ // max_wait_time.
+ // In the case that we did not get a frame, it should have been waiting for
+ // exactly max_wait_time. (By the testing samples we constructed above, we
+ // are sure there is no timing error, so the only case it returns with NULL
+ // is that it runs out of time.)
+ if (frame) {
+ receiver_.ReleaseFrame(frame);
+ ++num_frames_return;
+ EXPECT_GE(kMaxWaitTime, end_time - start_time);
+ } else {
+ EXPECT_EQ(kMaxWaitTime, end_time - start_time);
+ }
+ }
+}
+
} // namespace webrtc
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 e747db4..dcf9b68 100644
--- a/webrtc/modules/video_coding/main/source/test/stream_generator.cc
+++ b/webrtc/modules/video_coding/main/source/test/stream_generator.cc
@@ -35,8 +35,8 @@
void StreamGenerator::GenerateFrame(FrameType type,
int num_media_packets,
int num_empty_packets,
- int64_t current_time) {
- uint32_t timestamp = 90 * (current_time - start_time_);
+ int64_t time_ms) {
+ uint32_t timestamp = 90 * (time_ms - start_time_);
for (int i = 0; i < num_media_packets; ++i) {
const int packet_size =
(kFrameSize + num_media_packets / 2) / num_media_packets;
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 5d80acf..e3a2e79 100644
--- a/webrtc/modules/video_coding/main/source/test/stream_generator.h
+++ b/webrtc/modules/video_coding/main/source/test/stream_generator.h
@@ -30,10 +30,13 @@
StreamGenerator(uint16_t start_seq_num, int64_t current_time);
void Init(uint16_t start_seq_num, int64_t current_time);
+ // |time_ms| denotes the timestamp you want to put on the frame, and the unit
+ // is millisecond. GenerateFrame will translate |time_ms| into a 90kHz
+ // timestamp and put it on the frame.
void GenerateFrame(FrameType type,
int num_media_packets,
int num_empty_packets,
- int64_t current_time);
+ int64_t time_ms);
bool PopPacket(VCMPacket* packet, int index);
void DropLastPacket();