Index: webrtc/modules/congestion_controller/delay_based_bwe.cc |
diff --git a/webrtc/modules/congestion_controller/delay_based_bwe.cc b/webrtc/modules/congestion_controller/delay_based_bwe.cc |
index 7a20b3eedb4fea1556931d8935c5218591a71a8d..1edec365ea5a56bbf36c32f9c2751506111d6ad9 100644 |
--- a/webrtc/modules/congestion_controller/delay_based_bwe.cc |
+++ b/webrtc/modules/congestion_controller/delay_based_bwe.cc |
@@ -131,15 +131,21 @@ DelayBasedBwe::BitrateEstimator::BitrateEstimator() |
bitrate_estimate_(-1.0f), |
bitrate_estimate_var_(50.0f), |
old_estimator_(kBitrateWindowMs, 8000), |
- in_experiment_(BitrateEstimateExperimentIsEnabled()) {} |
+ in_experiment_(BitrateEstimateExperimentIsEnabled()), |
+ last_result_was_valid_(false) {} |
void DelayBasedBwe::BitrateEstimator::Update(int64_t now_ms, int bytes) { |
if (!in_experiment_) { |
old_estimator_.Update(bytes, now_ms); |
rtc::Optional<uint32_t> rate = old_estimator_.Rate(now_ms); |
bitrate_estimate_ = -1.0f; |
- if (rate) |
+ if (rate) { |
bitrate_estimate_ = *rate / 1000.0f; |
+ last_result_was_valid_ = true; |
terelius
2017/02/07 15:35:28
Please be aware that the "!in_experiment"-path wil
michaelt
2017/02/08 12:05:32
This just e fix for the old bitrate_estimate_. It
|
+ } else if (last_result_was_valid_) { |
+ old_estimator_.Reset(); |
+ last_result_was_valid_ = false; |
+ } |
return; |
} |
int rate_window_ms = kRateWindowMs; |
@@ -215,7 +221,6 @@ DelayBasedBwe::DelayBasedBwe(Clock* clock) |
trendline_estimator_(), |
detector_(), |
receiver_incoming_bitrate_(), |
- last_update_ms_(-1), |
last_seen_packet_ms_(-1), |
uma_recorded_(false), |
trendline_window_size_(kDefaultTrendlineWindowSize), |
@@ -252,6 +257,27 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketFeedbackVector( |
if (result.updated) |
aggregated_result = result; |
} |
+ |
terelius
2017/02/07 15:35:28
I like the idea of checking for overuse once per R
michaelt
2017/02/08 12:05:32
I hope my impl. change will not influence the beha
terelius
2017/02/08 12:19:52
Yes, it will get the same input and the detector w
|
+ rtc::Optional<uint32_t> acked_bitrate_bps = |
+ receiver_incoming_bitrate_.bitrate_bps(); |
+ int64_t now_ms = clock_->TimeInMilliseconds(); |
+ // Currently overusing the bandwidth. |
+ if (detector_.State() == kBwOverusing) { |
stefan-webrtc
2017/02/07 14:43:13
Given that you have to check the state also in Inc
michaelt
2017/02/07 15:07:07
The idea was to split the Update in two parts.
1.
stefan-webrtc
2017/02/07 15:10:42
Yes, I think that might be a good idea. It however
michaelt
2017/02/07 15:12:56
Ok will try to clean this up :)
|
+ if (acked_bitrate_bps && |
+ rate_control_.TimeToReduceFurther(now_ms, *acked_bitrate_bps)) { |
+ aggregated_result.updated = UpdateEstimate( |
+ now_ms, acked_bitrate_bps, &aggregated_result.target_bitrate_bps); |
+ } |
+ } else { |
+ if (!aggregated_result.updated) { |
+ aggregated_result.updated = UpdateEstimate( |
+ now_ms, acked_bitrate_bps, &aggregated_result.target_bitrate_bps); |
+ } |
+ } |
+ if (aggregated_result.updated) { |
+ BWE_TEST_LOGGING_PLOT(1, "target_bitrate_bps", now_ms, |
+ aggregated_result.target_bitrate_bps); |
+ } |
return aggregated_result; |
} |
@@ -313,46 +339,23 @@ DelayBasedBwe::Result DelayBasedBwe::IncomingPacketInfo( |
} |
} |
- int probing_bps = 0; |
if (info.probe_cluster_id != PacketInfo::kNotAProbe) { |
- probing_bps = probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info); |
- } |
- rtc::Optional<uint32_t> acked_bitrate_bps = |
- receiver_incoming_bitrate_.bitrate_bps(); |
- // Currently overusing the bandwidth. |
- if (detector_.State() == kBwOverusing) { |
- if (acked_bitrate_bps && |
- rate_control_.TimeToReduceFurther(now_ms, *acked_bitrate_bps)) { |
+ int probing_bps = |
+ probe_bitrate_estimator_.HandleProbeAndEstimateBitrate(info); |
+ if (probing_bps > 0) { |
+ // No overuse, but probing measured a bitrate. |
stefan-webrtc
2017/02/07 14:43:13
You have to check for overuse. Maybe you should mo
michaelt
2017/02/08 12:05:32
I used now the target_bitrate_bps to pass the prob
stefan-webrtc
2017/02/08 12:16:03
I don't see the change, did you forgot to upload?
|
+ rate_control_.SetEstimate(probing_bps, info.arrival_time_ms); |
+ result.probe = true; |
result.updated = |
- UpdateEstimate(info.arrival_time_ms, now_ms, acked_bitrate_bps, |
+ UpdateEstimate(now_ms, receiver_incoming_bitrate_.bitrate_bps(), |
&result.target_bitrate_bps); |
} |
- } else if (probing_bps > 0) { |
- // No overuse, but probing measured a bitrate. |
- rate_control_.SetEstimate(probing_bps, info.arrival_time_ms); |
- result.probe = true; |
- result.updated = |
- UpdateEstimate(info.arrival_time_ms, now_ms, acked_bitrate_bps, |
- &result.target_bitrate_bps); |
- } |
- if (!result.updated && |
- (last_update_ms_ == -1 || |
- now_ms - last_update_ms_ > rate_control_.GetFeedbackInterval())) { |
- result.updated = |
- UpdateEstimate(info.arrival_time_ms, now_ms, acked_bitrate_bps, |
- &result.target_bitrate_bps); |
- } |
- if (result.updated) { |
- last_update_ms_ = now_ms; |
- BWE_TEST_LOGGING_PLOT(1, "target_bitrate_bps", now_ms, |
- result.target_bitrate_bps); |
} |
return result; |
} |
-bool DelayBasedBwe::UpdateEstimate(int64_t arrival_time_ms, |
- int64_t now_ms, |
+bool DelayBasedBwe::UpdateEstimate(int64_t now_ms, |
rtc::Optional<uint32_t> acked_bitrate_bps, |
uint32_t* target_bitrate_bps) { |
// TODO(terelius): RateControlInput::noise_var is deprecated and will be |