Add PeerConnectionInterface::UpdateCallBitrate.

webrtc/pc/peerconnection.cc

Index: webrtc/pc/peerconnection.cc
diff --git a/webrtc/pc/peerconnection.cc b/webrtc/pc/peerconnection.cc
index 852dd39de5bc21eed1461340f796d4ef0132e55f..7111b1c53d5925cae4ddfc742a7da2a571840ee2 100644
--- a/webrtc/pc/peerconnection.cc
+++ b/webrtc/pc/peerconnection.cc
@@ -1242,6 +1242,54 @@ void PeerConnection::RegisterUMAObserver(UMAObserver* observer) {
   }
 }
 
+RTCError PeerConnection::SetBitrate(const BitrateParameters& bitrate) {

[tommi, 2017/08/17 07:32:52]

Instead of blocking, can this function be designed to be async? (is the return
value necessary?)

+  rtc::Thread* worker_thread = factory_->worker_thread();
+  if (!worker_thread->IsCurrent()) {
+    return worker_thread->Invoke<RTCError>(
+        RTC_FROM_HERE, rtc::Bind(&PeerConnection::SetBitrate, this, bitrate));
+  }
+
+  const bool has_min = static_cast<bool>(bitrate.min_bitrate_bps);
+  const bool has_current = static_cast<bool>(bitrate.current_bitrate_bps);
+  const bool has_max = static_cast<bool>(bitrate.max_bitrate_bps);
+  if (has_min && *bitrate.min_bitrate_bps < 0) {
+    LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,

[tommi, 2017/08/17 07:32:52]

hiding return statements in macros can be dangerous. It would be good to clean
that up.

+                         "min_bitrate_bps <= 0");
+  }
+  if (has_current) {
+    if (has_min && *bitrate.current_bitrate_bps < *bitrate.min_bitrate_bps) {
+      LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
+                           "current_bitrate_bps < min_bitrate_bps");
+    } else if (*bitrate.current_bitrate_bps < 0) {
+      LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
+                           "curent_bitrate_bps < 0");
+    }
+  }
+  if (has_max) {
+    if (has_current &&
+        *bitrate.max_bitrate_bps < *bitrate.current_bitrate_bps) {
+      LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
+                           "max_bitrate_bps < current_bitrate_bps");
+    } else if (has_min && *bitrate.max_bitrate_bps < *bitrate.min_bitrate_bps) {
+      LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
+                           "max_bitrate_bps < min_bitrate_bps");
+    } else if (*bitrate.max_bitrate_bps < 0) {
+      LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,

[tommi, 2017/08/17 07:32:52]

The code cost (i.e. code bloat) of each of these log statements, is actually
substantial.  Consider having a single log statement and just a const char*
variable for the text you want to log.

+                           "max_bitrate_bps < 0");
+    }
+  }
+
+  Call::Config::BitrateConfigMask mask;
+  mask.min_bitrate_bps = bitrate.min_bitrate_bps;
+  mask.start_bitrate_bps = bitrate.current_bitrate_bps;
+  mask.max_bitrate_bps = bitrate.max_bitrate_bps;
+
+  RTC_DCHECK(call_.get());

[tommi, 2017/08/17 07:32:52]

we don't DCHECK a pointer that we dereference anyway.

However, if this must always be valid at the start of the function call, you
could move the DCHECK there.  That way you check this condition first regardless
of whether you end up dereferencing the pointer or not. Could be useful since
there are so many potential early return paths.

+  call_->SetBitrateConfigMask(mask);
+
+  return RTCError::OK();
+}
+
 bool PeerConnection::StartRtcEventLog(rtc::PlatformFile file,
                                       int64_t max_size_bytes) {
   return factory_->worker_thread()->Invoke<bool>(