Follow-up CL for r3007.

Fixes an initialization issue introduced with r3007 and adds a test for
detecting problems with reordering. Also some cleanup for better code.

TEST=remote_bitrate_estimator_unittests

BUG=1009

Review URL: https://webrtc-codereview.appspot.com/937007

git-svn-id: http://webrtc.googlecode.com/svn/trunk@3014 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
stefan@webrtc.org 2012-10-29 16:06:08 +00:00
parent e83d3111d4
commit 6dcef360f8
3 changed files with 72 additions and 61 deletions

View File

@ -110,14 +110,6 @@ void OveruseDetector::Update(uint16_t packet_size,
#endif #endif
bool new_timestamp = (timestamp != current_frame_.timestamp); bool new_timestamp = (timestamp != current_frame_.timestamp);
if (current_frame_.timestamp_ms == -1) {
const uint32_t timestamp_diff = timestamp - current_frame_.timestamp;
if (timestamp_diff > 0x80000000) {
// Assume that a diff this big must be due to reordering. Don't update
// with reordered samples.
return;
}
}
if (timestamp_ms >= 0) { if (timestamp_ms >= 0) {
if (prev_frame_.timestamp_ms == -1 && current_frame_.timestamp_ms == -1) { if (prev_frame_.timestamp_ms == -1 && current_frame_.timestamp_ms == -1) {
SwitchTimeBase(); SwitchTimeBase();
@ -125,24 +117,23 @@ void OveruseDetector::Update(uint16_t packet_size,
new_timestamp = (timestamp_ms != current_frame_.timestamp_ms); new_timestamp = (timestamp_ms != current_frame_.timestamp_ms);
} }
if (current_frame_.timestamp == -1) { if (current_frame_.timestamp == -1) {
// This is the first incoming packet. We don't have enough data to update
// the filter, so we store it until we have two frames of data to process.
current_frame_.timestamp = timestamp; current_frame_.timestamp = timestamp;
current_frame_.timestamp_ms = timestamp_ms; current_frame_.timestamp_ms = timestamp_ms;
} else if (!PacketInOrder(timestamp, timestamp_ms)) {
return;
} else if (new_timestamp) { } else if (new_timestamp) {
// First packet of a later frame, the previous frame sample is ready // First packet of a later frame, the previous frame sample is ready.
WEBRTC_TRACE(kTraceStream, kTraceRtpRtcp, -1, WEBRTC_TRACE(kTraceStream, kTraceRtpRtcp, -1, "Frame complete at %I64i",
"Frame complete at %I64i", current_frame_.complete_time_ms); current_frame_.complete_time_ms);
if (prev_frame_.complete_time_ms >= 0) { // This is our second frame if (prev_frame_.complete_time_ms >= 0) { // This is our second frame.
int64_t t_delta = 0; int64_t t_delta = 0;
double ts_delta = 0; double ts_delta = 0;
if (TimeDeltas(current_frame_, prev_frame_, TimeDeltas(current_frame_, prev_frame_, &t_delta, &ts_delta);
&t_delta, &ts_delta)) { UpdateKalman(t_delta, ts_delta, current_frame_.size, prev_frame_.size);
// No reordering.
UpdateKalman(t_delta, ts_delta, current_frame_.size, prev_frame_.size);
prev_frame_ = current_frame_;
}
} else {
prev_frame_ = current_frame_;
} }
prev_frame_ = current_frame_;
// The new timestamp is now the current frame. // The new timestamp is now the current frame.
current_frame_.timestamp = timestamp; current_frame_.timestamp = timestamp;
current_frame_.timestamp_ms = timestamp_ms; current_frame_.timestamp_ms = timestamp_ms;
@ -182,7 +173,7 @@ void OveruseDetector::SwitchTimeBase() {
prev_frame_ = current_frame_; prev_frame_ = current_frame_;
} }
bool OveruseDetector::TimeDeltas(const FrameSample& current_frame, void OveruseDetector::TimeDeltas(const FrameSample& current_frame,
const FrameSample& prev_frame, const FrameSample& prev_frame,
int64_t* t_delta, int64_t* t_delta,
double* ts_delta) { double* ts_delta) {
@ -193,26 +184,34 @@ bool OveruseDetector::TimeDeltas(const FrameSample& current_frame,
num_of_deltas_ = 1000; num_of_deltas_ = 1000;
} }
if (current_frame.timestamp_ms == -1) { if (current_frame.timestamp_ms == -1) {
const uint32_t timestamp_diff = current_frame.timestamp - uint32_t timestamp_diff = current_frame.timestamp - prev_frame.timestamp;
prev_frame.timestamp;
if (timestamp_diff > 0x80000000) {
// Assume that a diff this big must be due to reordering. Don't update
// with reordered samples.
return false;
}
*ts_delta = timestamp_diff / 90.0; *ts_delta = timestamp_diff / 90.0;
} else { } else {
*ts_delta = current_frame.timestamp_ms - prev_frame.timestamp_ms; *ts_delta = current_frame.timestamp_ms - prev_frame.timestamp_ms;
if (*ts_delta < 0) {
// Frame reordering.
return false;
}
} }
*t_delta = current_frame.complete_time_ms - prev_frame.complete_time_ms; *t_delta = current_frame.complete_time_ms - prev_frame.complete_time_ms;
assert(*ts_delta > 0); assert(*ts_delta > 0);
}
bool OveruseDetector::PacketInOrder(uint32_t timestamp, int64_t timestamp_ms) {
if (current_frame_.timestamp_ms == -1 && current_frame_.timestamp > -1) {
return InOrderTimestamp(timestamp, current_frame_.timestamp);
} else if (current_frame_.timestamp_ms > 0) {
// Using timestamps converted to NTP time.
return timestamp_ms > current_frame_.timestamp_ms;
}
// This is the first packet.
return true; return true;
} }
bool OveruseDetector::InOrderTimestamp(uint32_t timestamp,
uint32_t prev_timestamp) {
uint32_t timestamp_diff = timestamp - prev_timestamp;
// Assume that a diff this big must be due to reordering. Don't update
// with reordered samples.
return (timestamp_diff < 0x80000000);
}
double OveruseDetector::CurrentDrift() { double OveruseDetector::CurrentDrift() {
return 1.0; return 1.0;
} }
@ -406,23 +405,4 @@ BandwidthUsage OveruseDetector::Detect(double ts_delta) {
} }
return hypothesis_; return hypothesis_;
} }
bool OveruseDetector::OldTimestamp(uint32_t new_timestamp,
uint32_t existing_timestamp,
bool* wrapped) {
bool tmpWrapped =
(new_timestamp < 0x0000ffff && existing_timestamp > 0xffff0000) ||
(new_timestamp > 0xffff0000 && existing_timestamp < 0x0000ffff);
*wrapped = tmpWrapped;
if (existing_timestamp > new_timestamp && !tmpWrapped) {
return true;
} else if (existing_timestamp <= new_timestamp && !tmpWrapped) {
return false;
} else if (existing_timestamp < new_timestamp && tmpWrapped) {
return true;
} else {
return false;
}
}
} // namespace webrtc } // namespace webrtc

View File

@ -59,15 +59,17 @@ class OveruseDetector {
#endif #endif
}; };
static bool OldTimestamp(uint32_t new_timestamp, // Returns true if |timestamp| represent a time which is later than
uint32_t existing_timestamp, // |prev_timestamp|.
bool* wrapped); static bool InOrderTimestamp(uint32_t timestamp, uint32_t prev_timestamp);
bool PacketInOrder(uint32_t timestamp, int64_t timestamp_ms);
// Prepares the overuse detector to start using timestamps in milliseconds // Prepares the overuse detector to start using timestamps in milliseconds
// instead of 90 kHz timestamps. // instead of 90 kHz timestamps.
void SwitchTimeBase(); void SwitchTimeBase();
bool TimeDeltas(const FrameSample& current_frame, void TimeDeltas(const FrameSample& current_frame,
const FrameSample& prev_frame, const FrameSample& prev_frame,
int64_t* t_delta, int64_t* t_delta,
double* ts_delta); double* ts_delta);

View File

@ -277,7 +277,7 @@ class RemoteBitrateEstimatorTest : public ::testing::Test {
RemoteBitrateEstimator::Create( RemoteBitrateEstimator::Create(
bitrate_observer_.get(), bitrate_observer_.get(),
over_use_detector_options_, over_use_detector_options_,
RemoteBitrateEstimator::kMultiStreamEstimation)); RemoteBitrateEstimator::kSingleStreamEstimation));
stream_generator_.reset(new StreamGenerator(1e6, // Capacity. stream_generator_.reset(new StreamGenerator(1e6, // Capacity.
time_now_)); time_now_));
} }
@ -415,12 +415,41 @@ TEST_F(RemoteBitrateEstimatorTest, TestInitialBehavior) {
time_now += 2; time_now += 2;
bitrate_estimator_->UpdateEstimate(kDefaultSsrc, time_now); bitrate_estimator_->UpdateEstimate(kDefaultSsrc, time_now);
EXPECT_TRUE(bitrate_estimator_->LatestEstimate(kDefaultSsrc, &bitrate_bps)); EXPECT_TRUE(bitrate_estimator_->LatestEstimate(kDefaultSsrc, &bitrate_bps));
EXPECT_EQ(20644u, bitrate_bps); EXPECT_EQ(20607u, bitrate_bps);
EXPECT_TRUE(bitrate_observer_->updated()); EXPECT_TRUE(bitrate_observer_->updated());
bitrate_observer_->Reset(); bitrate_observer_->Reset();
EXPECT_EQ(bitrate_observer_->latest_bitrate(), bitrate_bps); EXPECT_EQ(bitrate_observer_->latest_bitrate(), bitrate_bps);
} }
TEST_F(RemoteBitrateEstimatorTest, TestRateIncreaseReordering) {
int64_t time_now = 0;
uint32_t timestamp = 0;
const int framerate = 50; // 50 fps to avoid rounding errors.
const int frame_interval_ms = 1000 / framerate;
bitrate_estimator_->IncomingPacket(kDefaultSsrc, 1000, time_now, timestamp);
bitrate_estimator_->UpdateEstimate(kDefaultSsrc, time_now);
EXPECT_FALSE(bitrate_observer_->updated()); // No valid estimate.
// Increase time with 1 second to get a valid estimate.
time_now += 1000;
timestamp += 90 * 1000;
bitrate_estimator_->IncomingPacket(kDefaultSsrc, 1000, time_now, timestamp);
bitrate_estimator_->UpdateEstimate(kDefaultSsrc, time_now);
EXPECT_TRUE(bitrate_observer_->updated());
EXPECT_EQ(17645u, bitrate_observer_->latest_bitrate());
for (int i = 0; i < 10; ++i) {
time_now += 2 * frame_interval_ms;
timestamp += 2 * 90 * frame_interval_ms;
bitrate_estimator_->IncomingPacket(kDefaultSsrc, 1000, time_now, timestamp);
bitrate_estimator_->IncomingPacket(kDefaultSsrc,
1000,
time_now - frame_interval_ms,
timestamp - 90 * frame_interval_ms);
}
bitrate_estimator_->UpdateEstimate(kDefaultSsrc, time_now);
EXPECT_TRUE(bitrate_observer_->updated());
EXPECT_EQ(18985u, bitrate_observer_->latest_bitrate());
}
// Make sure we initially increase the bitrate as expected. // Make sure we initially increase the bitrate as expected.
TEST_F(RemoteBitrateEstimatorTest, TestRateIncreaseRtpTimestamps) { TEST_F(RemoteBitrateEstimatorTest, TestRateIncreaseRtpTimestamps) {
const int kExpectedIterations = 276; const int kExpectedIterations = 276;
@ -512,7 +541,7 @@ TEST_F(RemoteBitrateEstimatorTest, TestCapacityDropRtpTimestampsWrap) {
bitrate_observer_->Reset(); bitrate_observer_->Reset();
} }
} }
EXPECT_EQ(8366, bitrate_drop_time); EXPECT_EQ(8299, bitrate_drop_time);
} }
// Verify that the time it takes for the estimator to reduce the bitrate when // Verify that the time it takes for the estimator to reduce the bitrate when
@ -552,7 +581,7 @@ TEST_F(RemoteBitrateEstimatorTestAlign, TestCapacityDropRtpTimestampsWrap) {
bitrate_observer_->Reset(); bitrate_observer_->Reset();
} }
} }
EXPECT_EQ(8366, bitrate_drop_time); EXPECT_EQ(8299, bitrate_drop_time);
} }
// Verify that the time it takes for the estimator to reduce the bitrate when // Verify that the time it takes for the estimator to reduce the bitrate when
@ -605,7 +634,7 @@ TEST_F(RemoteBitrateEstimatorTestAlign, TwoStreamsCapacityDropWithWrap) {
bitrate_observer_->Reset(); bitrate_observer_->Reset();
} }
} }
EXPECT_EQ(4966, bitrate_drop_time); EXPECT_EQ(4933, bitrate_drop_time);
} }
// Verify that the time it takes for the estimator to reduce the bitrate when // Verify that the time it takes for the estimator to reduce the bitrate when
@ -666,7 +695,7 @@ TEST_F(RemoteBitrateEstimatorTestAlign, ThreeStreams) {
bitrate_observer_->Reset(); bitrate_observer_->Reset();
} }
} }
EXPECT_EQ(3900, bitrate_drop_time); EXPECT_EQ(3966, bitrate_drop_time);
} }
} // namespace webrtc } // namespace webrtc