New api struct BitrateSettings.
Replaces both BitrateConstraintsMask and
PeerConnectionInterface::BitrateParameters. The latter is kept
temporarily for backwards compatibility.
Bug: None
Change-Id: Ibe1d043f2a76e56ff67809774e9c0f5e0ec9e00f
Reviewed-on: https://webrtc-review.googlesource.com/74020
Commit-Queue: Niels Moller <nisse@webrtc.org>
Reviewed-by: Karl Wiberg <kwiberg@webrtc.org>
Reviewed-by: Sebastian Jansson <srte@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#23148}diff --git a/api/BUILD.gn b/api/BUILD.gn
index d5da182..c394584 100644
--- a/api/BUILD.gn
+++ b/api/BUILD.gn
@@ -108,6 +108,7 @@
":video_frame_api",
"audio:audio_mixer_api",
"audio_codecs:audio_codecs_api",
+ "transport:bitrate_settings",
# Basically, don't add stuff here. You might break sensitive downstream
# targets like pnacl. API should not depend on anything outside of this
diff --git a/api/peerconnectioninterface.h b/api/peerconnectioninterface.h
index 6e39b4f..8ffb814 100644
--- a/api/peerconnectioninterface.h
+++ b/api/peerconnectioninterface.h
@@ -90,6 +90,7 @@
#include "api/setremotedescriptionobserverinterface.h"
#include "api/stats/rtcstatscollectorcallback.h"
#include "api/statstypes.h"
+#include "api/transport/bitrate_settings.h"
#include "api/turncustomizer.h"
#include "api/umametrics.h"
#include "logging/rtc_event_log/rtc_event_log_factory_interface.h"
@@ -994,7 +995,24 @@
//
// Setting |current_bitrate_bps| will reset the current bitrate estimate
// to the provided value.
- virtual RTCError SetBitrate(const BitrateParameters& bitrate) = 0;
+ virtual RTCError SetBitrate(const BitrateSettings& bitrate) {
+ BitrateParameters bitrate_parameters;
+ bitrate_parameters.min_bitrate_bps = bitrate.min_bitrate_bps;
+ bitrate_parameters.current_bitrate_bps = bitrate.start_bitrate_bps;
+ bitrate_parameters.max_bitrate_bps = bitrate.max_bitrate_bps;
+ return SetBitrate(bitrate_parameters);
+ }
+
+ // TODO(nisse): Deprecated - use version above. These two default
+ // implementations require subclasses to implement one or the other
+ // of the methods.
+ virtual RTCError SetBitrate(const BitrateParameters& bitrate_parameters) {
+ BitrateSettings bitrate;
+ bitrate.min_bitrate_bps = bitrate_parameters.min_bitrate_bps;
+ bitrate.start_bitrate_bps = bitrate_parameters.current_bitrate_bps;
+ bitrate.max_bitrate_bps = bitrate_parameters.max_bitrate_bps;
+ return SetBitrate(bitrate);
+ }
// Sets current strategy. If not set default WebRTC allocator will be used.
// May be changed during an active session. The strategy
diff --git a/api/peerconnectionproxy.h b/api/peerconnectionproxy.h
index 9325adc..3abcf96 100644
--- a/api/peerconnectionproxy.h
+++ b/api/peerconnectionproxy.h
@@ -135,7 +135,7 @@
PROXY_METHOD1(void, SetAudioPlayout, bool)
PROXY_METHOD1(void, SetAudioRecording, bool)
PROXY_METHOD1(void, RegisterUMAObserver, UMAObserver*)
- PROXY_METHOD1(RTCError, SetBitrate, const BitrateParameters&);
+ PROXY_METHOD1(RTCError, SetBitrate, const BitrateSettings&);
PROXY_METHOD1(void,
SetBitrateAllocationStrategy,
std::unique_ptr<rtc::BitrateAllocationStrategy>);
diff --git a/api/transport/BUILD.gn b/api/transport/BUILD.gn
new file mode 100644
index 0000000..4d29d75
--- /dev/null
+++ b/api/transport/BUILD.gn
@@ -0,0 +1,20 @@
+# Copyright (c) 2018 The WebRTC project authors. All Rights Reserved.
+#
+# Use of this source code is governed by a BSD-style license
+# that can be found in the LICENSE file in the root of the source
+# tree. An additional intellectual property rights grant can be found
+# in the file PATENTS. All contributing project authors may
+# be found in the AUTHORS file in the root of the source tree.
+
+import("../../webrtc.gni")
+
+rtc_source_set("bitrate_settings") {
+ visibility = [ "*" ]
+ sources = [
+ "bitrate_settings.cc",
+ "bitrate_settings.h",
+ ]
+ deps = [
+ "..:optional",
+ ]
+}
diff --git a/call/bitrate_constraints.cc b/api/transport/bitrate_settings.cc
similarity index 63%
rename from call/bitrate_constraints.cc
rename to api/transport/bitrate_settings.cc
index 1e6264e..c72bd82 100644
--- a/call/bitrate_constraints.cc
+++ b/api/transport/bitrate_settings.cc
@@ -8,11 +8,12 @@
* be found in the AUTHORS file in the root of the source tree.
*/
-#include "call/bitrate_constraints.h"
+#include "api/transport/bitrate_settings.h"
namespace webrtc {
-BitrateConstraintsMask::BitrateConstraintsMask() = default;
-BitrateConstraintsMask::~BitrateConstraintsMask() = default;
-BitrateConstraintsMask::BitrateConstraintsMask(const BitrateConstraintsMask&) =
- default;
+
+BitrateSettings::BitrateSettings() = default;
+BitrateSettings::~BitrateSettings() = default;
+BitrateSettings::BitrateSettings(const BitrateSettings&) = default;
+
} // namespace webrtc
diff --git a/api/transport/bitrate_settings.h b/api/transport/bitrate_settings.h
new file mode 100644
index 0000000..1a24d90
--- /dev/null
+++ b/api/transport/bitrate_settings.h
@@ -0,0 +1,35 @@
+/*
+ * Copyright (c) 2018 The WebRTC project authors. All Rights Reserved.
+ *
+ * Use of this source code is governed by a BSD-style license
+ * that can be found in the LICENSE file in the root of the source
+ * tree. An additional intellectual property rights grant can be found
+ * in the file PATENTS. All contributing project authors may
+ * be found in the AUTHORS file in the root of the source tree.
+ */
+
+#ifndef API_TRANSPORT_BITRATE_SETTINGS_H_
+#define API_TRANSPORT_BITRATE_SETTINGS_H_
+
+#include "api/optional.h"
+
+namespace webrtc {
+
+// Configuration of send bitrate. The |start_bitrate_bps| value is
+// used for multiple purposes, both as a prior in the bandwidth
+// estimator, and for initial configuration of the encoder. We may
+// want to create separate apis for those, and use a smaller struct
+// with only the min and max constraints.
+struct BitrateSettings {
+ BitrateSettings();
+ ~BitrateSettings();
+ BitrateSettings(const BitrateSettings&);
+ // 0 <= min <= start <= max should hold for set parameters.
+ rtc::Optional<int> min_bitrate_bps;
+ rtc::Optional<int> start_bitrate_bps;
+ rtc::Optional<int> max_bitrate_bps;
+};
+
+} // namespace webrtc
+
+#endif // API_TRANSPORT_BITRATE_SETTINGS_H_
diff --git a/call/BUILD.gn b/call/BUILD.gn
index cf3a592..5be4636 100644
--- a/call/BUILD.gn
+++ b/call/BUILD.gn
@@ -50,7 +50,6 @@
# when interfaces have stabilized. See also TODO for |mock_rtp_interfaces|.
rtc_source_set("rtp_interfaces") {
sources = [
- "bitrate_constraints.cc",
"bitrate_constraints.h",
"rtcp_packet_sink_interface.h",
"rtp_config.cc",
@@ -62,6 +61,7 @@
deps = [
"../api:array_view",
"../api:optional",
+ "../api/transport:bitrate_settings",
"../rtc_base:rtc_base_approved",
]
}
@@ -122,6 +122,7 @@
]
deps = [
":rtp_interfaces",
+ "../api/transport:bitrate_settings",
"../rtc_base:checks",
"../rtc_base:rtc_base_approved",
]
diff --git a/call/bitrate_constraints.h b/call/bitrate_constraints.h
index 7898a78..3da97a0 100644
--- a/call/bitrate_constraints.h
+++ b/call/bitrate_constraints.h
@@ -13,10 +13,8 @@
#include <algorithm>
-#include "api/optional.h"
-
namespace webrtc {
-// TODO(srte): BitrateConstraints and BitrateConstraintsMask should be merged.
+// TODO(srte): BitrateConstraints and BitrateSettings should be merged.
// Both represent the same kind data, but are using different default
// initializer and representation of unset values.
struct BitrateConstraints {
@@ -28,18 +26,6 @@
static constexpr int kDefaultStartBitrateBps = 300000;
};
-// BitrateConstraintsMask is used for the local client's bitrate preferences.
-// Semantically it carries the same kind of information as BitrateConstraints,
-// but is used in a slightly different way.
-struct BitrateConstraintsMask {
- BitrateConstraintsMask();
- ~BitrateConstraintsMask();
- BitrateConstraintsMask(const BitrateConstraintsMask&);
- rtc::Optional<int> min_bitrate_bps;
- rtc::Optional<int> start_bitrate_bps;
- rtc::Optional<int> max_bitrate_bps;
-};
-
// Like std::min, but considers non-positive values to be unset.
template <typename T>
static T MinPositive(T a, T b) {
diff --git a/call/rtp_bitrate_configurator.cc b/call/rtp_bitrate_configurator.cc
index 828b955..b3bc183 100644
--- a/call/rtp_bitrate_configurator.cc
+++ b/call/rtp_bitrate_configurator.cc
@@ -58,7 +58,7 @@
rtc::Optional<BitrateConstraints>
RtpBitrateConfigurator::UpdateWithClientPreferences(
- const BitrateConstraintsMask& bitrate_mask) {
+ const BitrateSettings& bitrate_mask) {
bitrate_config_mask_ = bitrate_mask;
return UpdateConstraints(bitrate_mask.start_bitrate_bps);
}
diff --git a/call/rtp_bitrate_configurator.h b/call/rtp_bitrate_configurator.h
index 9115243..14f312b 100644
--- a/call/rtp_bitrate_configurator.h
+++ b/call/rtp_bitrate_configurator.h
@@ -11,6 +11,7 @@
#ifndef CALL_RTP_BITRATE_CONFIGURATOR_H_
#define CALL_RTP_BITRATE_CONFIGURATOR_H_
+#include "api/transport/bitrate_settings.h"
#include "call/bitrate_constraints.h"
#include "rtc_base/constructormagic.h"
@@ -41,7 +42,7 @@
// Update the bitrate configuration
// The optional return value is set with new configuration if it was updated.
rtc::Optional<BitrateConstraints> UpdateWithClientPreferences(
- const BitrateConstraintsMask& bitrate_mask);
+ const BitrateSettings& bitrate_mask);
private:
// Applies update to the BitrateConstraints cached in |config_|, resetting
@@ -55,7 +56,7 @@
// The config mask set by SetClientBitratePreferences.
// 0 <= min <= start <= max
- BitrateConstraintsMask bitrate_config_mask_;
+ BitrateSettings bitrate_config_mask_;
// The config set by SetSdpBitrateParameters.
// min >= 0, start != 0, max == -1 || max > 0
diff --git a/call/rtp_bitrate_configurator_unittest.cc b/call/rtp_bitrate_configurator_unittest.cc
index fd4f3a5..d5eed23 100644
--- a/call/rtp_bitrate_configurator_unittest.cc
+++ b/call/rtp_bitrate_configurator_unittest.cc
@@ -35,7 +35,7 @@
EXPECT_EQ(result->max_bitrate_bps, max_bitrate_bps);
}
- void UpdateMaskMatches(BitrateConstraintsMask bitrate_mask,
+ void UpdateMaskMatches(BitrateSettings bitrate_mask,
rtc::Optional<int> min_bitrate_bps,
rtc::Optional<int> start_bitrate_bps,
rtc::Optional<int> max_bitrate_bps) {
@@ -120,13 +120,13 @@
}
TEST_F(RtpBitrateConfiguratorTest, BiggerMaskMinUsed) {
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.min_bitrate_bps = 1234;
UpdateMaskMatches(mask, *mask.min_bitrate_bps, nullopt, nullopt);
}
TEST_F(RtpBitrateConfiguratorTest, BiggerConfigMinUsed) {
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.min_bitrate_bps = 1000;
UpdateMaskMatches(mask, 1000, nullopt, nullopt);
@@ -137,7 +137,7 @@
// The last call to set start should be used.
TEST_F(RtpBitrateConfiguratorTest, LatestStartMaskPreferred) {
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.start_bitrate_bps = 1300;
UpdateMaskMatches(mask, nullopt, *mask.start_bitrate_bps, nullopt);
@@ -153,7 +153,7 @@
bitrate_config.max_bitrate_bps = bitrate_config.start_bitrate_bps + 2000;
configurator_.reset(new RtpBitrateConfigurator(bitrate_config));
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.max_bitrate_bps = bitrate_config.start_bitrate_bps + 1000;
UpdateMaskMatches(mask, nullopt, nullopt, *mask.max_bitrate_bps);
@@ -164,7 +164,7 @@
bitrate_config.max_bitrate_bps = bitrate_config.start_bitrate_bps + 1000;
configurator_.reset(new RtpBitrateConfigurator(bitrate_config));
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.max_bitrate_bps = bitrate_config.start_bitrate_bps + 2000;
// Expect no return because nothing changes
@@ -176,7 +176,7 @@
bitrate_config.min_bitrate_bps = 2000;
configurator_.reset(new RtpBitrateConfigurator(bitrate_config));
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.start_bitrate_bps = 1000;
UpdateMaskMatches(mask, 2000, 2000, nullopt);
}
@@ -186,7 +186,7 @@
bitrate_config.start_bitrate_bps = 2000;
configurator_.reset(new RtpBitrateConfigurator(bitrate_config));
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.max_bitrate_bps = 1000;
UpdateMaskMatches(mask, nullopt, -1, 1000);
@@ -197,14 +197,14 @@
bitrate_config.min_bitrate_bps = 2000;
configurator_.reset(new RtpBitrateConfigurator(bitrate_config));
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.max_bitrate_bps = 1000;
UpdateMaskMatches(mask, 1000, nullopt, 1000);
}
TEST_F(RtpBitrateConfiguratorTest, SettingMaskStartForcesUpdate) {
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.start_bitrate_bps = 1000;
// Config should be returned twice with the same params since
@@ -242,7 +242,7 @@
UpdateConfigMatches(config, nullopt, nullopt, 2000);
// Reduce effective max to 1000 with the mask.
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.max_bitrate_bps = 1000;
UpdateMaskMatches(mask, nullopt, nullopt, 1000);
@@ -255,7 +255,7 @@
// When the "start bitrate" mask is removed, new config shouldn't be returned
// again, since nothing's changing.
TEST_F(RtpBitrateConfiguratorTest, NewConfigNotReturnedWhenStartMaskRemoved) {
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.start_bitrate_bps = 1000;
UpdateMaskMatches(mask, 0, 1000, -1);
@@ -263,11 +263,11 @@
EXPECT_FALSE(configurator_->UpdateWithClientPreferences(mask).has_value());
}
-// Test that if a new config is returned after BitrateConstraintsMask applies a
+// Test that if a new config is returned after BitrateSettings applies a
// "start" value, the new config won't return that start value a
// second time.
TEST_F(RtpBitrateConfiguratorTest, NewConfigAfterBitrateConfigMaskWithStart) {
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.start_bitrate_bps = 1000;
UpdateMaskMatches(mask, 0, 1000, -1);
@@ -288,7 +288,7 @@
configurator_.reset(new RtpBitrateConfigurator(bitrate_config));
// Set min to 2000; it is clamped to the max (1000).
- BitrateConstraintsMask mask;
+ BitrateSettings mask;
mask.min_bitrate_bps = 2000;
UpdateMaskMatches(mask, 1000, -1, 1000);
diff --git a/call/rtp_transport_controller_send.cc b/call/rtp_transport_controller_send.cc
index e86cca3..78ede7e 100644
--- a/call/rtp_transport_controller_send.cc
+++ b/call/rtp_transport_controller_send.cc
@@ -248,7 +248,7 @@
}
void RtpTransportControllerSend::SetClientBitratePreferences(
- const BitrateConstraintsMask& preferences) {
+ const BitrateSettings& preferences) {
rtc::Optional<BitrateConstraints> updated =
bitrate_configurator_.UpdateWithClientPreferences(preferences);
if (updated.has_value()) {
diff --git a/call/rtp_transport_controller_send.h b/call/rtp_transport_controller_send.h
index ea4910d..d5bc25e 100644
--- a/call/rtp_transport_controller_send.h
+++ b/call/rtp_transport_controller_send.h
@@ -81,7 +81,7 @@
void SetSdpBitrateParameters(const BitrateConstraints& constraints) override;
void SetClientBitratePreferences(
- const BitrateConstraintsMask& preferences) override;
+ const BitrateSettings& preferences) override;
private:
const Clock* const clock_;
diff --git a/call/rtp_transport_controller_send_interface.h b/call/rtp_transport_controller_send_interface.h
index bb95bc9..ad3aa2b 100644
--- a/call/rtp_transport_controller_send_interface.h
+++ b/call/rtp_transport_controller_send_interface.h
@@ -16,6 +16,7 @@
#include <string>
#include "api/optional.h"
+#include "api/transport/bitrate_settings.h"
#include "call/bitrate_constraints.h"
namespace rtc {
@@ -108,7 +109,7 @@
virtual void SetSdpBitrateParameters(
const BitrateConstraints& constraints) = 0;
virtual void SetClientBitratePreferences(
- const BitrateConstraintsMask& preferences) = 0;
+ const BitrateSettings& preferences) = 0;
};
} // namespace webrtc
diff --git a/call/test/mock_rtp_transport_controller_send.h b/call/test/mock_rtp_transport_controller_send.h
index d903eef..7226093 100644
--- a/call/test/mock_rtp_transport_controller_send.h
+++ b/call/test/mock_rtp_transport_controller_send.h
@@ -51,7 +51,7 @@
MOCK_METHOD1(OnSentPacket, void(const rtc::SentPacket&));
MOCK_METHOD1(SetSdpBitrateParameters, void(const BitrateConstraints&));
MOCK_METHOD1(SetClientBitratePreferences,
- void(const BitrateConstraintsMask&));
+ void(const BitrateSettings&));
};
} // namespace webrtc
#endif // CALL_TEST_MOCK_RTP_TRANSPORT_CONTROLLER_SEND_H_
diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index ea76e3d..54de56c 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -3019,33 +3019,33 @@
}
}
-RTCError PeerConnection::SetBitrate(const BitrateParameters& bitrate) {
+RTCError PeerConnection::SetBitrate(const BitrateSettings& bitrate) {
if (!worker_thread()->IsCurrent()) {
return worker_thread()->Invoke<RTCError>(
- RTC_FROM_HERE, rtc::Bind(&PeerConnection::SetBitrate, this, bitrate));
+ RTC_FROM_HERE, [&](){ return SetBitrate(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);
+ const bool has_min = bitrate.min_bitrate_bps.has_value();
+ const bool has_start = bitrate.start_bitrate_bps.has_value();
+ const bool has_max = bitrate.max_bitrate_bps.has_value();
if (has_min && *bitrate.min_bitrate_bps < 0) {
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
"min_bitrate_bps <= 0");
}
- if (has_current) {
- if (has_min && *bitrate.current_bitrate_bps < *bitrate.min_bitrate_bps) {
+ if (has_start) {
+ if (has_min && *bitrate.start_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) {
+ "start_bitrate_bps < min_bitrate_bps");
+ } else if (*bitrate.start_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) {
+ if (has_start &&
+ *bitrate.max_bitrate_bps < *bitrate.start_bitrate_bps) {
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
- "max_bitrate_bps < current_bitrate_bps");
+ "max_bitrate_bps < start_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");
@@ -3055,13 +3055,8 @@
}
}
- BitrateConstraintsMask 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());
- call_->GetTransportControllerSend()->SetClientBitratePreferences(mask);
+ call_->GetTransportControllerSend()->SetClientBitratePreferences(bitrate);
return RTCError::OK();
}
diff --git a/pc/peerconnection.h b/pc/peerconnection.h
index a03e952..1c8c3e0 100644
--- a/pc/peerconnection.h
+++ b/pc/peerconnection.h
@@ -177,7 +177,7 @@
void RegisterUMAObserver(UMAObserver* observer) override;
- RTCError SetBitrate(const BitrateParameters& bitrate) override;
+ RTCError SetBitrate(const BitrateSettings& bitrate) override;
void SetBitrateAllocationStrategy(
std::unique_ptr<rtc::BitrateAllocationStrategy>
diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc
index 799baa6..ef8d47b 100644
--- a/pc/peerconnectioninterface_unittest.cc
+++ b/pc/peerconnectioninterface_unittest.cc
@@ -3772,7 +3772,7 @@
CreatePeerConnection(config, nullptr);
}
-// The current bitrate from Call's BitrateConstraintsMask is currently clamped
+// The current bitrate from BitrateSettings is currently clamped
// by Call's BitrateConstraints, which comes from the SDP or a default value.
// This test checks that a call to SetBitrate with a current bitrate that will
// be clamped succeeds.
diff --git a/pc/test/fakepeerconnectionbase.h b/pc/test/fakepeerconnectionbase.h
index 64164ba..203ab0f 100644
--- a/pc/test/fakepeerconnectionbase.h
+++ b/pc/test/fakepeerconnectionbase.h
@@ -196,7 +196,7 @@
void RegisterUMAObserver(UMAObserver* observer) override {}
- RTCError SetBitrate(const BitrateParameters& bitrate) override {
+ RTCError SetBitrate(const BitrateSettings& bitrate) override {
return RTCError(RTCErrorType::UNSUPPORTED_OPERATION, "Not implemented");
}
