Fixes a potential BWE clock mismatch bug.
Since libjingle provides a packet arrival timestamp to webrtc, and the clock in remote bitrate estimator and the clock used for packet arrival timestamp can be different. This can cause the bandwidth estimator to malfunction. This CL changes the remote bitrate estimator so that packet arrival timestamps never are compared to the time taken from the internal clock. BUG=3527 R=mflodman@webrtc.org, pbos@webrtc.org Review URL: https://webrtc-codereview.appspot.com/20789004 git-svn-id: http://webrtc.googlecode.com/svn/trunk@6571 4adac7df-926f-26a2-2b94-8c16560cd09d
This commit is contained in:
parent
6d21ddca5f
commit
5779ca478d
@ -71,6 +71,7 @@ class RemoteBitrateEstimator : public CallStatsObserver, public Module {
|
||||
// estimate and the over-use detector. If an over-use is detected the
|
||||
// remote bitrate estimate will be updated. Note that |payload_size| is the
|
||||
// packet size excluding headers.
|
||||
// Note that |arrival_time_ms| can be of an arbitrary time base.
|
||||
virtual void IncomingPacket(int64_t arrival_time_ms,
|
||||
int payload_size,
|
||||
const RTPHeader& header) = 0;
|
||||
|
@ -36,8 +36,7 @@ OveruseDetector::OveruseDetector(const OverUseDetectorOptions& options)
|
||||
prev_offset_(0.0),
|
||||
time_over_using_(-1),
|
||||
over_use_counter_(0),
|
||||
hypothesis_(kBwNormal),
|
||||
time_of_last_received_packet_(-1) {
|
||||
hypothesis_(kBwNormal) {
|
||||
memcpy(E_, options_.initial_e, sizeof(E_));
|
||||
memcpy(process_noise_, options_.initial_process_noise,
|
||||
sizeof(process_noise_));
|
||||
@ -50,8 +49,7 @@ OveruseDetector::~OveruseDetector() {
|
||||
void OveruseDetector::Update(uint16_t packet_size,
|
||||
int64_t timestamp_ms,
|
||||
uint32_t timestamp,
|
||||
const int64_t now_ms) {
|
||||
time_of_last_received_packet_ = now_ms;
|
||||
const int64_t arrival_time_ms) {
|
||||
bool new_timestamp = (timestamp != current_frame_.timestamp);
|
||||
if (timestamp_ms >= 0) {
|
||||
if (prev_frame_.timestamp_ms == -1 && current_frame_.timestamp_ms == -1) {
|
||||
@ -82,7 +80,7 @@ void OveruseDetector::Update(uint16_t packet_size,
|
||||
}
|
||||
// Accumulate the frame size
|
||||
current_frame_.size += packet_size;
|
||||
current_frame_.complete_time_ms = now_ms;
|
||||
current_frame_.complete_time_ms = arrival_time_ms;
|
||||
}
|
||||
|
||||
BandwidthUsage OveruseDetector::State() const {
|
||||
@ -107,10 +105,6 @@ void OveruseDetector::SetRateControlRegion(RateControlRegion region) {
|
||||
}
|
||||
}
|
||||
|
||||
int64_t OveruseDetector::time_of_last_received_packet() const {
|
||||
return time_of_last_received_packet_;
|
||||
}
|
||||
|
||||
void OveruseDetector::SwitchTimeBase() {
|
||||
current_frame_.size = 0;
|
||||
current_frame_.complete_time_ms = -1;
|
||||
|
@ -28,11 +28,10 @@ class OveruseDetector {
|
||||
void Update(uint16_t packet_size,
|
||||
int64_t timestamp_ms,
|
||||
uint32_t rtp_timestamp,
|
||||
int64_t now_ms);
|
||||
int64_t arrival_time_ms);
|
||||
BandwidthUsage State() const;
|
||||
double NoiseVar() const;
|
||||
void SetRateControlRegion(RateControlRegion region);
|
||||
int64_t time_of_last_received_packet() const;
|
||||
|
||||
private:
|
||||
struct FrameSample {
|
||||
@ -89,7 +88,6 @@ class OveruseDetector {
|
||||
double time_over_using_;
|
||||
uint16_t over_use_counter_;
|
||||
BandwidthUsage hypothesis_;
|
||||
int64_t time_of_last_received_packet_;
|
||||
};
|
||||
} // namespace webrtc
|
||||
|
||||
|
@ -59,10 +59,27 @@ class RemoteBitrateEstimatorSingleStream : public RemoteBitrateEstimator {
|
||||
ReceiveBandwidthEstimatorStats* output) const OVERRIDE;
|
||||
|
||||
private:
|
||||
typedef std::map<unsigned int, OveruseDetector> SsrcOveruseDetectorMap;
|
||||
// Map from SSRC to over-use detector and last incoming packet time in
|
||||
// milliseconds, taken from clock_.
|
||||
typedef std::map<unsigned int, std::pair<OveruseDetector, int64_t> >
|
||||
SsrcOveruseDetectorMap;
|
||||
|
||||
static OveruseDetector* GetDetector(
|
||||
const SsrcOveruseDetectorMap::iterator it) {
|
||||
return &it->second.first;
|
||||
}
|
||||
|
||||
static int64_t GetPacketTimeMs(const SsrcOveruseDetectorMap::iterator it) {
|
||||
return it->second.second;
|
||||
}
|
||||
|
||||
static void SetPacketTimeMs(SsrcOveruseDetectorMap::iterator it,
|
||||
int64_t time_ms) {
|
||||
it->second.second = time_ms;
|
||||
}
|
||||
|
||||
// Triggers a new estimate calculation.
|
||||
void UpdateEstimate(int64_t time_now);
|
||||
void UpdateEstimate(int64_t now_ms);
|
||||
|
||||
void GetSsrcs(std::vector<unsigned int>* ssrcs) const;
|
||||
|
||||
@ -95,6 +112,7 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(
|
||||
uint32_t ssrc = header.ssrc;
|
||||
uint32_t rtp_timestamp = header.timestamp +
|
||||
header.extension.transmissionTimeOffset;
|
||||
int64_t now_ms = clock_->TimeInMilliseconds();
|
||||
CriticalSectionScoped cs(crit_sect_.get());
|
||||
SsrcOveruseDetectorMap::iterator it = overuse_detectors_.find(ssrc);
|
||||
if (it == overuse_detectors_.end()) {
|
||||
@ -105,22 +123,23 @@ void RemoteBitrateEstimatorSingleStream::IncomingPacket(
|
||||
// automatically cleaned up when we have one RemoteBitrateEstimator per REMB
|
||||
// group.
|
||||
std::pair<SsrcOveruseDetectorMap::iterator, bool> insert_result =
|
||||
overuse_detectors_.insert(std::make_pair(ssrc, OveruseDetector(
|
||||
OverUseDetectorOptions())));
|
||||
overuse_detectors_.insert(std::make_pair(ssrc,
|
||||
std::make_pair(OveruseDetector(OverUseDetectorOptions()), now_ms)));
|
||||
it = insert_result.first;
|
||||
}
|
||||
OveruseDetector* overuse_detector = &it->second;
|
||||
incoming_bitrate_.Update(payload_size, arrival_time_ms);
|
||||
SetPacketTimeMs(it, now_ms);
|
||||
OveruseDetector* overuse_detector = GetDetector(it);
|
||||
incoming_bitrate_.Update(payload_size, now_ms);
|
||||
const BandwidthUsage prior_state = overuse_detector->State();
|
||||
overuse_detector->Update(payload_size, -1, rtp_timestamp, arrival_time_ms);
|
||||
if (overuse_detector->State() == kBwOverusing) {
|
||||
unsigned int incoming_bitrate = incoming_bitrate_.Rate(arrival_time_ms);
|
||||
unsigned int incoming_bitrate = incoming_bitrate_.Rate(now_ms);
|
||||
if (prior_state != kBwOverusing ||
|
||||
remote_rate_.TimeToReduceFurther(arrival_time_ms, incoming_bitrate)) {
|
||||
remote_rate_.TimeToReduceFurther(now_ms, incoming_bitrate)) {
|
||||
// The first overuse should immediately trigger a new estimate.
|
||||
// We also have to update the estimate immediately if we are overusing
|
||||
// and the target bitrate is too high compared to what we are receiving.
|
||||
UpdateEstimate(arrival_time_ms);
|
||||
UpdateEstimate(now_ms);
|
||||
}
|
||||
}
|
||||
}
|
||||
@ -129,8 +148,9 @@ int32_t RemoteBitrateEstimatorSingleStream::Process() {
|
||||
if (TimeUntilNextProcess() > 0) {
|
||||
return 0;
|
||||
}
|
||||
UpdateEstimate(clock_->TimeInMilliseconds());
|
||||
last_process_time_ = clock_->TimeInMilliseconds();
|
||||
int64_t now_ms = clock_->TimeInMilliseconds();
|
||||
UpdateEstimate(now_ms);
|
||||
last_process_time_ = now_ms;
|
||||
return 0;
|
||||
}
|
||||
|
||||
@ -141,25 +161,24 @@ int32_t RemoteBitrateEstimatorSingleStream::TimeUntilNextProcess() {
|
||||
return last_process_time_ + kProcessIntervalMs - clock_->TimeInMilliseconds();
|
||||
}
|
||||
|
||||
void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t time_now) {
|
||||
void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t now_ms) {
|
||||
CriticalSectionScoped cs(crit_sect_.get());
|
||||
BandwidthUsage bw_state = kBwNormal;
|
||||
double sum_noise_var = 0.0;
|
||||
SsrcOveruseDetectorMap::iterator it = overuse_detectors_.begin();
|
||||
while (it != overuse_detectors_.end()) {
|
||||
const int64_t time_of_last_received_packet =
|
||||
it->second.time_of_last_received_packet();
|
||||
if (time_of_last_received_packet >= 0 &&
|
||||
time_now - time_of_last_received_packet > kStreamTimeOutMs) {
|
||||
if (GetPacketTimeMs(it) >= 0 &&
|
||||
now_ms - GetPacketTimeMs(it) > kStreamTimeOutMs) {
|
||||
// This over-use detector hasn't received packets for |kStreamTimeOutMs|
|
||||
// milliseconds and is considered stale.
|
||||
overuse_detectors_.erase(it++);
|
||||
} else {
|
||||
sum_noise_var += it->second.NoiseVar();
|
||||
OveruseDetector* overuse_detector = GetDetector(it);
|
||||
sum_noise_var += overuse_detector->NoiseVar();
|
||||
// Make sure that we trigger an over-use if any of the over-use detectors
|
||||
// is detecting over-use.
|
||||
if (it->second.State() > bw_state) {
|
||||
bw_state = it->second.State();
|
||||
if (overuse_detector->State() > bw_state) {
|
||||
bw_state = overuse_detector->State();
|
||||
}
|
||||
++it;
|
||||
}
|
||||
@ -172,17 +191,17 @@ void RemoteBitrateEstimatorSingleStream::UpdateEstimate(int64_t time_now) {
|
||||
double mean_noise_var = sum_noise_var /
|
||||
static_cast<double>(overuse_detectors_.size());
|
||||
const RateControlInput input(bw_state,
|
||||
incoming_bitrate_.Rate(time_now),
|
||||
incoming_bitrate_.Rate(now_ms),
|
||||
mean_noise_var);
|
||||
const RateControlRegion region = remote_rate_.Update(&input, time_now);
|
||||
unsigned int target_bitrate = remote_rate_.UpdateBandwidthEstimate(time_now);
|
||||
const RateControlRegion region = remote_rate_.Update(&input, now_ms);
|
||||
unsigned int target_bitrate = remote_rate_.UpdateBandwidthEstimate(now_ms);
|
||||
if (remote_rate_.ValidEstimate()) {
|
||||
std::vector<unsigned int> ssrcs;
|
||||
GetSsrcs(&ssrcs);
|
||||
observer_->OnReceiveBitrateChanged(ssrcs, target_bitrate);
|
||||
}
|
||||
for (it = overuse_detectors_.begin(); it != overuse_detectors_.end(); ++it) {
|
||||
it->second.SetRateControlRegion(region);
|
||||
GetDetector(it).SetRateControlRegion(region);
|
||||
}
|
||||
}
|
||||
|
||||
|
@ -226,7 +226,8 @@ void RemoteBitrateEstimatorTest::IncomingPacket(uint32_t ssrc,
|
||||
header.ssrc = ssrc;
|
||||
header.timestamp = rtp_timestamp;
|
||||
header.extension.absoluteSendTime = absolute_send_time;
|
||||
bitrate_estimator_->IncomingPacket(arrival_time, payload_size, header);
|
||||
bitrate_estimator_->IncomingPacket(arrival_time + kArrivalTimeClockOffsetMs,
|
||||
payload_size, header);
|
||||
}
|
||||
|
||||
// Generates a frame of packets belonging to a stream at a given bitrate and
|
||||
@ -245,6 +246,10 @@ bool RemoteBitrateEstimatorTest::GenerateAndProcessFrame(unsigned int ssrc,
|
||||
while (!packets.empty()) {
|
||||
testing::RtpStream::RtpPacket* packet = packets.front();
|
||||
bitrate_observer_->Reset();
|
||||
// The simulated clock should match the time of packet->arrival_time
|
||||
// since both are used in IncomingPacket().
|
||||
clock_.AdvanceTimeMicroseconds(packet->arrival_time -
|
||||
clock_.TimeInMicroseconds());
|
||||
IncomingPacket(packet->ssrc,
|
||||
packet->size,
|
||||
(packet->arrival_time + 500) / 1000,
|
||||
@ -256,8 +261,6 @@ bool RemoteBitrateEstimatorTest::GenerateAndProcessFrame(unsigned int ssrc,
|
||||
overuse = true;
|
||||
EXPECT_LE(bitrate_observer_->latest_bitrate(), bitrate_bps);
|
||||
}
|
||||
clock_.AdvanceTimeMicroseconds(packet->arrival_time -
|
||||
clock_.TimeInMicroseconds());
|
||||
delete packet;
|
||||
packets.pop_front();
|
||||
}
|
||||
|
@ -198,6 +198,7 @@ class RemoteBitrateEstimatorTest : public ::testing::Test {
|
||||
unsigned int expected_bitrate_drop_delta);
|
||||
|
||||
static const unsigned int kDefaultSsrc;
|
||||
static const int kArrivalTimeClockOffsetMs = 60000;
|
||||
|
||||
SimulatedClock clock_; // Time at the receiver.
|
||||
scoped_ptr<testing::TestBitrateObserver> bitrate_observer_;
|
||||
|
Loading…
x
Reference in New Issue
Block a user