diff --git a/api/BUILD.gn b/api/BUILD.gn index d5da1821bb..c3945846cd 100644 --- a/api/BUILD.gn +++ b/api/BUILD.gn @@ -108,6 +108,7 @@ rtc_static_library("libjingle_peerconnection_api") { ":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 6e39b4fb8e..8ffb8144bb 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 @@ class PeerConnectionInterface : public rtc::RefCountInterface { // // 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 9325adcec3..3abcf96f60 100644 --- a/api/peerconnectionproxy.h +++ b/api/peerconnectionproxy.h @@ -135,7 +135,7 @@ BEGIN_SIGNALING_PROXY_MAP(PeerConnection) 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); diff --git a/api/transport/BUILD.gn b/api/transport/BUILD.gn new file mode 100644 index 0000000000..4d29d75049 --- /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 64% rename from call/bitrate_constraints.cc rename to api/transport/bitrate_settings.cc index 1e6264e2e9..c72bd82791 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 0000000000..1a24d9057f --- /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 min_bitrate_bps; + rtc::Optional start_bitrate_bps; + rtc::Optional max_bitrate_bps; +}; + +} // namespace webrtc + +#endif // API_TRANSPORT_BITRATE_SETTINGS_H_ diff --git a/call/BUILD.gn b/call/BUILD.gn index cf3a592bcc..5be4636439 100644 --- a/call/BUILD.gn +++ b/call/BUILD.gn @@ -50,7 +50,6 @@ rtc_source_set("call_interfaces") { # 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 @@ rtc_source_set("rtp_interfaces") { deps = [ "../api:array_view", "../api:optional", + "../api/transport:bitrate_settings", "../rtc_base:rtc_base_approved", ] } @@ -122,6 +122,7 @@ rtc_source_set("bitrate_configurator") { ] 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 7898a78624..3da97a0b23 100644 --- a/call/bitrate_constraints.h +++ b/call/bitrate_constraints.h @@ -13,10 +13,8 @@ #include -#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 @@ struct BitrateConstraints { 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 min_bitrate_bps; - rtc::Optional start_bitrate_bps; - rtc::Optional max_bitrate_bps; -}; - // Like std::min, but considers non-positive values to be unset. template static T MinPositive(T a, T b) { diff --git a/call/rtp_bitrate_configurator.cc b/call/rtp_bitrate_configurator.cc index 828b955ee3..b3bc183816 100644 --- a/call/rtp_bitrate_configurator.cc +++ b/call/rtp_bitrate_configurator.cc @@ -58,7 +58,7 @@ RtpBitrateConfigurator::UpdateWithSdpParameters( rtc::Optional 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 9115243cf7..14f312bbb4 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 @@ class RtpBitrateConfigurator { // Update the bitrate configuration // The optional return value is set with new configuration if it was updated. rtc::Optional UpdateWithClientPreferences( - const BitrateConstraintsMask& bitrate_mask); + const BitrateSettings& bitrate_mask); private: // Applies update to the BitrateConstraints cached in |config_|, resetting @@ -55,7 +56,7 @@ class RtpBitrateConfigurator { // 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 fd4f3a56d6..d5eed23d3d 100644 --- a/call/rtp_bitrate_configurator_unittest.cc +++ b/call/rtp_bitrate_configurator_unittest.cc @@ -35,7 +35,7 @@ class RtpBitrateConfiguratorTest : public testing::Test { EXPECT_EQ(result->max_bitrate_bps, max_bitrate_bps); } - void UpdateMaskMatches(BitrateConstraintsMask bitrate_mask, + void UpdateMaskMatches(BitrateSettings bitrate_mask, rtc::Optional min_bitrate_bps, rtc::Optional start_bitrate_bps, rtc::Optional max_bitrate_bps) { @@ -120,13 +120,13 @@ TEST_F(RtpBitrateConfiguratorTest, } 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 @@ TEST_F(RtpBitrateConfiguratorTest, BiggerConfigMinUsed) { // 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 @@ TEST_F(RtpBitrateConfiguratorTest, SmallerMaskMaxUsed) { 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 @@ TEST_F(RtpBitrateConfiguratorTest, SmallerConfigMaxUsed) { 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 @@ TEST_F(RtpBitrateConfiguratorTest, MaskStartLessThanConfigMinClamped) { 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 @@ TEST_F(RtpBitrateConfiguratorTest, MaskStartGreaterThanConfigMaxClamped) { 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 @@ TEST_F(RtpBitrateConfiguratorTest, MaskMinGreaterThanConfigMaxClamped) { 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 @@ TEST_F(RtpBitrateConfiguratorTest, 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 @@ TEST_F(RtpBitrateConfiguratorTest, // 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 @@ TEST_F(RtpBitrateConfiguratorTest, NewConfigNotReturnedWhenStartMaskRemoved) { 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 @@ TEST_F(RtpBitrateConfiguratorTest, 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 e86cca37c3..78ede7ea22 100644 --- a/call/rtp_transport_controller_send.cc +++ b/call/rtp_transport_controller_send.cc @@ -248,7 +248,7 @@ void RtpTransportControllerSend::SetSdpBitrateParameters( } void RtpTransportControllerSend::SetClientBitratePreferences( - const BitrateConstraintsMask& preferences) { + const BitrateSettings& preferences) { rtc::Optional 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 ea4910da9f..d5bc25ed45 100644 --- a/call/rtp_transport_controller_send.h +++ b/call/rtp_transport_controller_send.h @@ -81,7 +81,7 @@ class RtpTransportControllerSend final 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 bb95bc9137..ad3aa2b75d 100644 --- a/call/rtp_transport_controller_send_interface.h +++ b/call/rtp_transport_controller_send_interface.h @@ -16,6 +16,7 @@ #include #include "api/optional.h" +#include "api/transport/bitrate_settings.h" #include "call/bitrate_constraints.h" namespace rtc { @@ -108,7 +109,7 @@ class RtpTransportControllerSendInterface { 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 d903eef20b..7226093b34 100644 --- a/call/test/mock_rtp_transport_controller_send.h +++ b/call/test/mock_rtp_transport_controller_send.h @@ -51,7 +51,7 @@ class MockRtpTransportControllerSend 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 ea76e3da44..54de56c3d9 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -3019,33 +3019,33 @@ void PeerConnection::SetMetricObserver_n(UMAObserver* observer) { } } -RTCError PeerConnection::SetBitrate(const BitrateParameters& bitrate) { +RTCError PeerConnection::SetBitrate(const BitrateSettings& bitrate) { if (!worker_thread()->IsCurrent()) { return worker_thread()->Invoke( - RTC_FROM_HERE, rtc::Bind(&PeerConnection::SetBitrate, this, bitrate)); + RTC_FROM_HERE, [&](){ return SetBitrate(bitrate); }); } - const bool has_min = static_cast(bitrate.min_bitrate_bps); - const bool has_current = static_cast(bitrate.current_bitrate_bps); - const bool has_max = static_cast(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 @@ RTCError PeerConnection::SetBitrate(const BitrateParameters& bitrate) { } } - 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 a03e9522d8..1c8c3e01f3 100644 --- a/pc/peerconnection.h +++ b/pc/peerconnection.h @@ -177,7 +177,7 @@ class PeerConnection : public PeerConnectionInternal, void RegisterUMAObserver(UMAObserver* observer) override; - RTCError SetBitrate(const BitrateParameters& bitrate) override; + RTCError SetBitrate(const BitrateSettings& bitrate) override; void SetBitrateAllocationStrategy( std::unique_ptr diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc index 799baa641d..ef8d47ba11 100644 --- a/pc/peerconnectioninterface_unittest.cc +++ b/pc/peerconnectioninterface_unittest.cc @@ -3772,7 +3772,7 @@ TEST_P(PeerConnectionInterfaceTest, 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 64164ba85e..203ab0f652 100644 --- a/pc/test/fakepeerconnectionbase.h +++ b/pc/test/fakepeerconnectionbase.h @@ -196,7 +196,7 @@ class FakePeerConnectionBase : public PeerConnectionInternal { void RegisterUMAObserver(UMAObserver* observer) override {} - RTCError SetBitrate(const BitrateParameters& bitrate) override { + RTCError SetBitrate(const BitrateSettings& bitrate) override { return RTCError(RTCErrorType::UNSUPPORTED_OPERATION, "Not implemented"); }