Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(365)

Unified Diff: webrtc/call/call.cc

Issue 2875703004: Add PeerConnectionInterface::UpdateCallBitrate with call tests. (Closed)
Patch Set: Add new test, make it pass. Created 3 years, 7 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « webrtc/call/call.h ('k') | webrtc/call/call_unittest.cc » ('j') | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: webrtc/call/call.cc
diff --git a/webrtc/call/call.cc b/webrtc/call/call.cc
index e594dcc6cadce83560ac48f325c60c4637a566d2..03706303b314487967f189f95f2d7d9548ec1dbd 100644
--- a/webrtc/call/call.cc
+++ b/webrtc/call/call.cc
@@ -142,6 +142,9 @@ class Call : public webrtc::Call,
void SetBitrateConfig(
const webrtc::Call::Config::BitrateConfig& bitrate_config) override;
+ void SetBitrateConfigMask(
+ const webrtc::Call::Config::BitrateConfigMask& bitrate_config) override;
+
void SignalChannelNetworkState(MediaType media, NetworkState state) override;
void OnTransportOverheadChanged(MediaType media,
@@ -188,6 +191,10 @@ class Call : public webrtc::Call,
void UpdateHistograms();
void UpdateAggregateNetworkState();
+ // Applies update to the BitrateConfig cached in |config_|, restarting
+ // bandwidth estimation from |new_start| if set.
+ void UpdateCurrentBitrateConfig(const rtc::Optional<int>& new_start);
+
Clock* const clock_;
const int num_cpu_cores_;
@@ -282,6 +289,14 @@ class Call : public webrtc::Call,
// and deleted before any other members.
rtc::TaskQueue worker_queue_;
+ // The config mask set by SetBitrateConfigMask.
+ // 0 <= min <= start <= max
+ Config::BitrateConfigMask bitrate_config_mask_;
+
+ // The config set by SetBitrateConfig.
+ // min >= 0, start != 0, max == -1 || max > 0
+ Config::BitrateConfig base_bitrate_config_;
+
RTC_DISALLOW_COPY_AND_ASSIGN(Call);
};
} // namespace internal
@@ -337,7 +352,8 @@ Call::Call(const Call::Config& config,
receive_side_cc_(clock_, transport_send->packet_router()),
video_send_delay_stats_(new SendDelayStats(clock_)),
start_ms_(clock_->TimeInMilliseconds()),
- worker_queue_("call_worker_queue") {
+ worker_queue_("call_worker_queue"),
+ base_bitrate_config_(config.bitrate_config) {
RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
RTC_DCHECK(config.event_log != nullptr);
RTC_DCHECK_GE(config.bitrate_config.min_bitrate_bps, 0);
@@ -862,29 +878,73 @@ void Call::SetBitrateConfig(
const webrtc::Call::Config::BitrateConfig& bitrate_config) {
TRACE_EVENT0("webrtc", "Call::SetBitrateConfig");
RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
+
RTC_DCHECK_GE(bitrate_config.min_bitrate_bps, 0);
- if (bitrate_config.max_bitrate_bps != -1)
+ RTC_DCHECK_NE(bitrate_config.start_bitrate_bps, 0);
+ if (bitrate_config.max_bitrate_bps != -1) {
RTC_DCHECK_GT(bitrate_config.max_bitrate_bps, 0);
- if (config_.bitrate_config.min_bitrate_bps ==
- bitrate_config.min_bitrate_bps &&
- (bitrate_config.start_bitrate_bps <= 0 ||
- config_.bitrate_config.start_bitrate_bps ==
- bitrate_config.start_bitrate_bps) &&
- config_.bitrate_config.max_bitrate_bps ==
- bitrate_config.max_bitrate_bps) {
- // Nothing new to set, early abort to avoid encoder reconfigurations.
+ }
+
+ rtc::Optional<int> new_start;
+ // Only update the "start" bitrate if it's set, and different from the old
+ // value. In practice, this value comes from the x-google-start-bitrate codec
+ // parameter in SDP, and setting the same remote description twice shouldn't
+ // restart bandwidth estimation.
+ if (bitrate_config.start_bitrate_bps != -1 &&
+ bitrate_config.start_bitrate_bps !=
+ base_bitrate_config_.start_bitrate_bps) {
+ new_start.emplace(bitrate_config.start_bitrate_bps);
+ }
+ base_bitrate_config_ = bitrate_config;
+ UpdateCurrentBitrateConfig(new_start);
+}
+
+void Call::SetBitrateConfigMask(
+ const webrtc::Call::Config::BitrateConfigMask& mask) {
+ TRACE_EVENT0("webrtc", "Call::SetBitrateConfigMask");
+ RTC_DCHECK(configuration_thread_checker_.CalledOnValidThread());
+
+ bitrate_config_mask_ = mask;
+ UpdateCurrentBitrateConfig(mask.start_bitrate_bps);
+}
+
+void Call::UpdateCurrentBitrateConfig(const rtc::Optional<int>& new_start) {
+ Config::BitrateConfig updated;
+ updated.min_bitrate_bps =
+ std::max(bitrate_config_mask_.min_bitrate_bps.value_or(0),
+ base_bitrate_config_.min_bitrate_bps);
+ updated.max_bitrate_bps =
+ cricket::MinPositive(bitrate_config_mask_.max_bitrate_bps.value_or(-1),
+ base_bitrate_config_.max_bitrate_bps);
+ // If the combined min ends up greater than the combined max, the max takes
+ // priority.
+ // TODO(zstein): This clamp logic is different than send_side_cc's (moves min
+ // instead of max, allows start to be greather than min) - check that it is a
+ // sensible difference.
+ if (updated.max_bitrate_bps != -1 &&
+ updated.min_bitrate_bps > updated.max_bitrate_bps) {
+ updated.min_bitrate_bps = updated.max_bitrate_bps;
+ }
+
+ // If there's nothing to update (min/max unchanged, no new bandwidth
+ // estimation start value), return early.
+ if (updated.min_bitrate_bps == config_.bitrate_config.min_bitrate_bps &&
+ updated.max_bitrate_bps == config_.bitrate_config.max_bitrate_bps &&
+ !new_start) {
return;
}
- config_.bitrate_config.min_bitrate_bps = bitrate_config.min_bitrate_bps;
- // Start bitrate of -1 means we should keep the old bitrate, which there is
- // no point in remembering for the future.
- if (bitrate_config.start_bitrate_bps > 0)
- config_.bitrate_config.start_bitrate_bps = bitrate_config.start_bitrate_bps;
- config_.bitrate_config.max_bitrate_bps = bitrate_config.max_bitrate_bps;
- RTC_DCHECK_NE(bitrate_config.start_bitrate_bps, 0);
- transport_send_->send_side_cc()->SetBweBitrates(
- bitrate_config.min_bitrate_bps, bitrate_config.start_bitrate_bps,
- bitrate_config.max_bitrate_bps);
+
+ if (new_start) {
+ updated.start_bitrate_bps = cricket::MinPositive(
+ std::max(*new_start, updated.min_bitrate_bps), updated.max_bitrate_bps);
+ } else {
+ updated.start_bitrate_bps = -1;
+ }
+
+ transport_send_->send_side_cc()->SetBweBitrates(updated.min_bitrate_bps,
+ updated.start_bitrate_bps,
+ updated.max_bitrate_bps);
+ config_.bitrate_config = updated;
}
void Call::SignalChannelNetworkState(MediaType media, NetworkState state) {
« no previous file with comments | « webrtc/call/call.h ('k') | webrtc/call/call_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698