From 26b039a137be0a8703766f45b546b29323de714f Mon Sep 17 00:00:00 2001 From: minyue Date: Mon, 19 Sep 2016 09:56:35 -0700 Subject: [PATCH] Adding BitrateController to audio network adaptor. BUG=webrtc:6303 Review-Url: https://codereview.webrtc.org/2334613002 Cr-Commit-Position: refs/heads/master@{#14293} --- webrtc/modules/BUILD.gn | 32 +++- webrtc/modules/audio_coding/BUILD.gn | 2 + .../audio_network_adaptor.gypi | 2 + .../audio_network_adaptor_impl_unittest.cc | 1 + .../bitrate_controller.cc | 68 +++++++++ .../bitrate_controller.h | 42 ++++++ .../bitrate_controller_unittest.cc | 142 ++++++++++++++++++ .../audio_network_adaptor/controller.h | 1 + 8 files changed, 283 insertions(+), 7 deletions(-) create mode 100644 webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller.cc create mode 100644 webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller.h create mode 100644 webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller_unittest.cc diff --git a/webrtc/modules/BUILD.gn b/webrtc/modules/BUILD.gn index 89959340d9..36ae692fbb 100644 --- a/webrtc/modules/BUILD.gn +++ b/webrtc/modules/BUILD.gn @@ -233,6 +233,30 @@ if (rtc_include_tests) { } } + rtc_source_set("audio_network_adaptor_unittests") { + # Put sources for unittests of audio network adaptor in a separate + # rtc_source_set to solve name collision on bitrate_controller_unittest.cc. + testonly = true + sources = [ + "audio_coding/audio_network_adaptor/audio_network_adaptor_impl_unittest.cc", + "audio_coding/audio_network_adaptor/bitrate_controller_unittest.cc", + "audio_coding/audio_network_adaptor/channel_controller_unittest.cc", + "audio_coding/audio_network_adaptor/controller_manager_unittest.cc", + "audio_coding/audio_network_adaptor/dtx_controller_unittest.cc", + "audio_coding/audio_network_adaptor/mock/mock_controller.h", + "audio_coding/audio_network_adaptor/mock/mock_controller_manager.h", + ] + deps = [ + "audio_coding:audio_network_adaptor", + "//testing/gmock", + "//testing/gtest", + ] + if (is_clang) { + # Suppress warnings from the Chromium Clang plugin (bugs.webrtc.org/163). + suppressed_configs += [ "//build/config/clang:find_bad_constructs" ] + } + } + rtc_test("modules_unittests") { testonly = true @@ -245,12 +269,6 @@ if (rtc_include_tests) { "audio_coding/acm2/codec_manager_unittest.cc", "audio_coding/acm2/initial_delay_manager_unittest.cc", "audio_coding/acm2/rent_a_codec_unittest.cc", - "audio_coding/audio_network_adaptor/audio_network_adaptor_impl_unittest.cc", - "audio_coding/audio_network_adaptor/channel_controller_unittest.cc", - "audio_coding/audio_network_adaptor/controller_manager_unittest.cc", - "audio_coding/audio_network_adaptor/dtx_controller_unittest.cc", - "audio_coding/audio_network_adaptor/mock/mock_controller.h", - "audio_coding/audio_network_adaptor/mock/mock_controller_manager.h", "audio_coding/codecs/audio_decoder_factory_unittest.cc", "audio_coding/codecs/cng/audio_encoder_cng_unittest.cc", "audio_coding/codecs/cng/cng_unittest.cc", @@ -592,6 +610,7 @@ if (rtc_include_tests) { } deps += [ + ":audio_network_adaptor_unittests", "..:webrtc_common", "../base:rtc_base", # TODO(kjellander): Cleanup in bugs.webrtc.org/3806. "../common_audio", @@ -605,7 +624,6 @@ if (rtc_include_tests) { "audio_coding", "audio_coding:acm_receive_test", "audio_coding:acm_send_test", - "audio_coding:audio_network_adaptor", "audio_coding:builtin_audio_decoder_factory", "audio_coding:cng", "audio_coding:isac_fix", diff --git a/webrtc/modules/audio_coding/BUILD.gn b/webrtc/modules/audio_coding/BUILD.gn index b6e2db3ac3..4fe49e5b9d 100644 --- a/webrtc/modules/audio_coding/BUILD.gn +++ b/webrtc/modules/audio_coding/BUILD.gn @@ -699,6 +699,8 @@ source_set("audio_network_adaptor") { "audio_network_adaptor/audio_network_adaptor.cc", "audio_network_adaptor/audio_network_adaptor_impl.cc", "audio_network_adaptor/audio_network_adaptor_impl.h", + "audio_network_adaptor/bitrate_controller.cc", + "audio_network_adaptor/bitrate_controller.h", "audio_network_adaptor/channel_controller.cc", "audio_network_adaptor/channel_controller.h", "audio_network_adaptor/controller.cc", diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor.gypi b/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor.gypi index 6f20d61a72..8e87815287 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor.gypi +++ b/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor.gypi @@ -14,6 +14,8 @@ 'audio_network_adaptor.cc', 'audio_network_adaptor_impl.cc', 'audio_network_adaptor_impl.h', + 'bitrate_controller.h', + 'bitrate_controller.cc', 'channel_controller.cc', 'channel_controller.h', 'controller.h', diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl_unittest.cc b/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl_unittest.cc index c17d3894d4..65626743ad 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl_unittest.cc +++ b/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl_unittest.cc @@ -28,6 +28,7 @@ constexpr size_t kNumControllers = 2; MATCHER_P(NetworkMetricsIs, metric, "") { return arg.uplink_bandwidth_bps == metric.uplink_bandwidth_bps && + arg.target_audio_bitrate_bps == metric.target_audio_bitrate_bps && arg.uplink_packet_loss_fraction == metric.uplink_packet_loss_fraction; } diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller.cc b/webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller.cc new file mode 100644 index 0000000000..4cc49e8c0d --- /dev/null +++ b/webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller.cc @@ -0,0 +1,68 @@ +/* + * Copyright (c) 2016 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. + */ + +#include "webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller.h" + +#include + +#include "webrtc/base/checks.h" + +namespace webrtc { + +namespace { +// TODO(minyue): consider passing this from a higher layer through +// SetConstraints(). +// L2(14B) + IPv4(20B) + UDP(8B) + RTP(12B) + SRTP_AUTH(10B) = 64B = 512 bits +constexpr int kPacketOverheadBits = 512; +} + +BitrateController::Config::Config(int initial_bitrate_bps, + int initial_frame_length_ms) + : initial_bitrate_bps(initial_bitrate_bps), + initial_frame_length_ms(initial_frame_length_ms) {} + +BitrateController::Config::~Config() = default; + +BitrateController::BitrateController(const Config& config) + : config_(config), + bitrate_bps_(config_.initial_bitrate_bps), + overhead_rate_bps_(kPacketOverheadBits * 1000 / + config_.initial_frame_length_ms) { + RTC_DCHECK_GT(bitrate_bps_, 0); + RTC_DCHECK_GT(overhead_rate_bps_, 0); +} + +void BitrateController::MakeDecision( + const NetworkMetrics& metrics, + AudioNetworkAdaptor::EncoderRuntimeConfig* config) { + // Decision on |bitrate_bps| should not have been made. + RTC_DCHECK(!config->bitrate_bps); + + if (metrics.target_audio_bitrate_bps) { + int overhead_rate = + config->frame_length_ms + ? kPacketOverheadBits * 1000 / *config->frame_length_ms + : overhead_rate_bps_; + // If |metrics.target_audio_bitrate_bps| had included overhead, we would + // simply do: + // bitrate_bps_ = metrics.target_audio_bitrate_bps - overhead_rate; + // Follow https://bugs.chromium.org/p/webrtc/issues/detail?id=6315 to track + // progress regarding this. + // Now we assume that |metrics.target_audio_bitrate_bps| can handle the + // overhead of most recent packets. + bitrate_bps_ = std::max(0, *metrics.target_audio_bitrate_bps + + overhead_rate_bps_ - overhead_rate); + // TODO(minyue): apply a smoothing on the |overhead_rate_bps_|. + overhead_rate_bps_ = overhead_rate; + } + config->bitrate_bps = rtc::Optional(bitrate_bps_); +} + +} // namespace webrtc diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller.h b/webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller.h new file mode 100644 index 0000000000..cfc6fa8058 --- /dev/null +++ b/webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller.h @@ -0,0 +1,42 @@ +/* + * Copyright (c) 2016 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 WEBRTC_MODULES_AUDIO_CODING_AUDIO_NETWORK_ADAPTOR_BITRATE_CONTROLLER_H_ +#define WEBRTC_MODULES_AUDIO_CODING_AUDIO_NETWORK_ADAPTOR_BITRATE_CONTROLLER_H_ + +#include "webrtc/base/constructormagic.h" +#include "webrtc/modules/audio_coding/audio_network_adaptor/controller.h" + +namespace webrtc { + +class BitrateController final : public Controller { + public: + struct Config { + Config(int initial_bitrate_bps, int frame_length_ms); + ~Config(); + int initial_bitrate_bps; + int initial_frame_length_ms; + }; + + explicit BitrateController(const Config& config); + + void MakeDecision(const NetworkMetrics& metrics, + AudioNetworkAdaptor::EncoderRuntimeConfig* config) override; + + private: + const Config config_; + int bitrate_bps_; + int overhead_rate_bps_; + RTC_DISALLOW_COPY_AND_ASSIGN(BitrateController); +}; + +} // namespace webrtc + +#endif // WEBRTC_MODULES_AUDIO_CODING_AUDIO_NETWORK_ADAPTOR_BITRATE_CONTROLLER_H_ diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller_unittest.cc b/webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller_unittest.cc new file mode 100644 index 0000000000..5a3a74ad3e --- /dev/null +++ b/webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller_unittest.cc @@ -0,0 +1,142 @@ +/* + * Copyright (c) 2016 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. + */ + +#include "testing/gtest/include/gtest/gtest.h" +#include "webrtc/modules/audio_coding/audio_network_adaptor/bitrate_controller.h" + +namespace webrtc { + +namespace { + +// L2(14B) + IPv4(20B) + UDP(8B) + RTP(12B) + SRTP_AUTH(10B) = 64B = 512 bits +constexpr int kPacketOverheadBits = 512; + +void CheckDecision(BitrateController* controller, + const rtc::Optional& target_audio_bitrate_bps, + const rtc::Optional& frame_length_ms, + int expected_bitrate_bps) { + Controller::NetworkMetrics metrics; + metrics.target_audio_bitrate_bps = target_audio_bitrate_bps; + AudioNetworkAdaptor::EncoderRuntimeConfig config; + config.frame_length_ms = frame_length_ms; + controller->MakeDecision(metrics, &config); + EXPECT_EQ(rtc::Optional(expected_bitrate_bps), config.bitrate_bps); +} + +} // namespace + +// These tests are named AnaBitrateControllerTest to distinguish from +// BitrateControllerTest in +// modules/bitrate_controller/bitrate_controller_unittest.cc. + +TEST(AnaBitrateControllerTest, OutputInitValueWhenTargetBitrateUnknown) { + constexpr int kInitialBitrateBps = 32000; + constexpr int kInitialFrameLengthMs = 20; + BitrateController controller( + BitrateController::Config(kInitialBitrateBps, kInitialFrameLengthMs)); + CheckDecision(&controller, rtc::Optional(), + rtc::Optional(kInitialFrameLengthMs * 2), + kInitialBitrateBps); +} + +TEST(AnaBitrateControllerTest, ChangeBitrateOnTargetBitrateChanged) { + constexpr int kInitialBitrateBps = 32000; + constexpr int kInitialFrameLengthMs = 20; + BitrateController controller( + BitrateController::Config(kInitialBitrateBps, kInitialFrameLengthMs)); + constexpr int kTargetBitrateBps = 48000; + // Frame length unchanged, bitrate changes in accordance with + // |metrics.target_audio_bitrate_bps| + CheckDecision(&controller, rtc::Optional(kTargetBitrateBps), + rtc::Optional(kInitialFrameLengthMs), kTargetBitrateBps); +} + +TEST(AnaBitrateControllerTest, TreatUnknownFrameLengthAsFrameLengthUnchanged) { + constexpr int kInitialBitrateBps = 32000; + constexpr int kInitialFrameLengthMs = 20; + BitrateController controller( + BitrateController::Config(kInitialBitrateBps, kInitialFrameLengthMs)); + constexpr int kTargetBitrateBps = 48000; + CheckDecision(&controller, rtc::Optional(kTargetBitrateBps), + rtc::Optional(), kTargetBitrateBps); +} + +TEST(AnaBitrateControllerTest, IncreaseBitrateOnFrameLengthIncreased) { + constexpr int kInitialBitrateBps = 32000; + constexpr int kInitialFrameLengthMs = 20; + BitrateController controller( + BitrateController::Config(kInitialBitrateBps, kInitialFrameLengthMs)); + constexpr int kFrameLengthMs = 60; + constexpr int kPacketOverheadRateDiff = + kPacketOverheadBits * 1000 / kInitialFrameLengthMs - + kPacketOverheadBits * 1000 / kFrameLengthMs; + CheckDecision(&controller, rtc::Optional(kInitialBitrateBps), + rtc::Optional(kFrameLengthMs), + kInitialBitrateBps + kPacketOverheadRateDiff); +} + +TEST(AnaBitrateControllerTest, DecreaseBitrateOnFrameLengthDecreased) { + constexpr int kInitialBitrateBps = 32000; + constexpr int kInitialFrameLengthMs = 60; + BitrateController controller( + BitrateController::Config(kInitialBitrateBps, kInitialFrameLengthMs)); + constexpr int kFrameLengthMs = 20; + constexpr int kPacketOverheadRateDiff = + kPacketOverheadBits * 1000 / kInitialFrameLengthMs - + kPacketOverheadBits * 1000 / kFrameLengthMs; + CheckDecision(&controller, rtc::Optional(kInitialBitrateBps), + rtc::Optional(kFrameLengthMs), + kInitialBitrateBps + kPacketOverheadRateDiff); +} + +TEST(AnaBitrateControllerTest, CheckBehaviorOnChangingCondition) { + constexpr int kInitialBitrateBps = 32000; + constexpr int kInitialFrameLengthMs = 20; + BitrateController controller( + BitrateController::Config(kInitialBitrateBps, kInitialFrameLengthMs)); + + int last_overhead_bitrate = + kPacketOverheadBits * 1000 / kInitialFrameLengthMs; + int current_overhead_bitrate = kPacketOverheadBits * 1000 / 20; + // Start from an arbitrary overall bitrate. + int overall_bitrate = 34567; + CheckDecision( + &controller, rtc::Optional(overall_bitrate - last_overhead_bitrate), + rtc::Optional(20), overall_bitrate - current_overhead_bitrate); + + // Next: increase overall bitrate. + overall_bitrate += 100; + CheckDecision( + &controller, rtc::Optional(overall_bitrate - last_overhead_bitrate), + rtc::Optional(20), overall_bitrate - current_overhead_bitrate); + + // Next: change frame length. + current_overhead_bitrate = kPacketOverheadBits * 1000 / 60; + CheckDecision( + &controller, rtc::Optional(overall_bitrate - last_overhead_bitrate), + rtc::Optional(60), overall_bitrate - current_overhead_bitrate); + last_overhead_bitrate = current_overhead_bitrate; + + // Next: change frame length. + current_overhead_bitrate = kPacketOverheadBits * 1000 / 20; + CheckDecision( + &controller, rtc::Optional(overall_bitrate - last_overhead_bitrate), + rtc::Optional(20), overall_bitrate - current_overhead_bitrate); + last_overhead_bitrate = current_overhead_bitrate; + + // Next: decrease overall bitrate and frame length. + overall_bitrate -= 100; + current_overhead_bitrate = kPacketOverheadBits * 1000 / 60; + CheckDecision( + &controller, rtc::Optional(overall_bitrate - last_overhead_bitrate), + rtc::Optional(60), overall_bitrate - current_overhead_bitrate); +} + +} // namespace webrtc diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/controller.h b/webrtc/modules/audio_coding/audio_network_adaptor/controller.h index f6a6079efd..c1b16c72a9 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/controller.h +++ b/webrtc/modules/audio_coding/audio_network_adaptor/controller.h @@ -23,6 +23,7 @@ class Controller { ~NetworkMetrics(); rtc::Optional uplink_bandwidth_bps; rtc::Optional uplink_packet_loss_fraction; + rtc::Optional target_audio_bitrate_bps; }; struct Constraints {