From bc77ed7657e19fc916beaff7f830c4d712ac99e7 Mon Sep 17 00:00:00 2001 From: minyue Date: Thu, 22 Sep 2016 00:45:16 -0700 Subject: [PATCH] Adding reordering logic in audio network adaptor. BUG=webrtc:6303 Review-Url: https://codereview.webrtc.org/2349113002 Cr-Commit-Position: refs/heads/master@{#14344} --- .../controller_manager.cc | 122 ++++++++++++++- .../controller_manager.h | 38 ++++- .../controller_manager_unittest.cc | 139 ++++++++++++++++-- 3 files changed, 279 insertions(+), 20 deletions(-) diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.cc b/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.cc index 80079d73b3..9f05b7e735 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.cc +++ b/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.cc @@ -8,25 +8,46 @@ * be found in the AUTHORS file in the root of the source tree. */ +#include "webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h" + +#include #include -#include "webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h" +#include "webrtc/system_wrappers/include/clock.h" namespace webrtc { -ControllerManagerImpl::Config::Config() = default; +ControllerManagerImpl::Config::Config(int min_reordering_time_ms, + float min_reordering_squared_distance, + const Clock* clock) + : min_reordering_time_ms(min_reordering_time_ms), + min_reordering_squared_distance(min_reordering_squared_distance), + clock(clock) {} ControllerManagerImpl::Config::~Config() = default; ControllerManagerImpl::ControllerManagerImpl(const Config& config) - : config_(config) {} + : ControllerManagerImpl( + config, + std::vector>(), + std::map>()) {} ControllerManagerImpl::ControllerManagerImpl( const Config& config, - std::vector> controllers) - : config_(config), controllers_(std::move(controllers)) { - for (auto& controller : controllers_) { + std::vector>&& controllers, + const std::map>& + chracteristic_points) + : config_(config), + controllers_(std::move(controllers)), + last_reordering_time_ms_(rtc::Optional()), + last_scoring_point_(0, 0.0) { + for (auto& controller : controllers_) default_sorted_controllers_.push_back(controller.get()); + sorted_controllers_ = default_sorted_controllers_; + for (auto& controller_point : chracteristic_points) { + controller_scoring_points_.insert(std::make_pair( + controller_point.first, ScoringPoint(controller_point.second.first, + controller_point.second.second))); } } @@ -34,12 +55,97 @@ ControllerManagerImpl::~ControllerManagerImpl() = default; std::vector ControllerManagerImpl::GetSortedControllers( const Controller::NetworkMetrics& metrics) { - // TODO(minyue): Reorder controllers according to their significance. - return default_sorted_controllers_; + int64_t now_ms = config_.clock->TimeInMilliseconds(); + + if (!metrics.uplink_bandwidth_bps || !metrics.uplink_packet_loss_fraction) + return sorted_controllers_; + + if (last_reordering_time_ms_ && + now_ms - *last_reordering_time_ms_ < config_.min_reordering_time_ms) + return sorted_controllers_; + + ScoringPoint scoring_point(*metrics.uplink_bandwidth_bps, + *metrics.uplink_packet_loss_fraction); + + if (last_reordering_time_ms_ && + last_scoring_point_.SquaredDistanceTo(scoring_point) < + config_.min_reordering_squared_distance) + return sorted_controllers_; + + // Sort controllers according to the distances of |scoring_point| to the + // characteristic scoring points of controllers. + // + // A controller that does not associate with any scoring point + // are treated as if + // 1) they are less important than any controller that has a scoring point, + // 2) they are equally important to any controller that has no scoring point, + // and their relative order will follow |default_sorted_controllers_|. + std::vector sorted_controllers(default_sorted_controllers_); + std::stable_sort( + sorted_controllers.begin(), sorted_controllers.end(), + [this, &scoring_point](const Controller* lhs, const Controller* rhs) { + auto lhs_scoring_point = controller_scoring_points_.find(lhs); + auto rhs_scoring_point = controller_scoring_points_.find(rhs); + + if (lhs_scoring_point == controller_scoring_points_.end()) + return false; + + if (rhs_scoring_point == controller_scoring_points_.end()) + return true; + + return lhs_scoring_point->second.SquaredDistanceTo(scoring_point) < + rhs_scoring_point->second.SquaredDistanceTo(scoring_point); + }); + + if (sorted_controllers_ != sorted_controllers) { + sorted_controllers_ = sorted_controllers; + last_reordering_time_ms_ = rtc::Optional(now_ms); + last_scoring_point_ = scoring_point; + } + return sorted_controllers_; } std::vector ControllerManagerImpl::GetControllers() const { return default_sorted_controllers_; } +ControllerManagerImpl::ScoringPoint::ScoringPoint( + int uplink_bandwidth_bps, + float uplink_packet_loss_fraction) + : uplink_bandwidth_bps(uplink_bandwidth_bps), + uplink_packet_loss_fraction(uplink_packet_loss_fraction) {} + +namespace { + +constexpr int kMinUplinkBandwidthBps = 0; +constexpr int kMaxUplinkBandwidthBps = 120000; + +float NormalizeUplinkBandwidth(int uplink_bandwidth_bps) { + uplink_bandwidth_bps = + std::min(kMaxUplinkBandwidthBps, + std::max(kMinUplinkBandwidthBps, uplink_bandwidth_bps)); + return static_cast(uplink_bandwidth_bps - kMinUplinkBandwidthBps) / + (kMaxUplinkBandwidthBps - kMinUplinkBandwidthBps); +} + +float NormalizePacketLossFraction(float uplink_packet_loss_fraction) { + // |uplink_packet_loss_fraction| is seldom larger than 0.3, so we scale it up + // by 3.3333f. + return std::min(uplink_packet_loss_fraction * 3.3333f, 1.0f); +} + +} // namespace + +float ControllerManagerImpl::ScoringPoint::SquaredDistanceTo( + const ScoringPoint& scoring_point) const { + float diff_normalized_bitrate_bps = + NormalizeUplinkBandwidth(scoring_point.uplink_bandwidth_bps) - + NormalizeUplinkBandwidth(uplink_bandwidth_bps); + float diff_normalized_packet_loss = + NormalizePacketLossFraction(scoring_point.uplink_packet_loss_fraction) - + NormalizePacketLossFraction(uplink_packet_loss_fraction); + return std::pow(diff_normalized_bitrate_bps, 2) + + std::pow(diff_normalized_packet_loss, 2); +} + } // namespace webrtc diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h b/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h index 6027c42fd2..c2ac9e35ef 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h +++ b/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h @@ -11,6 +11,7 @@ #ifndef WEBRTC_MODULES_AUDIO_CODING_AUDIO_NETWORK_ADAPTOR_CONTROLLER_MANAGER_H_ #define WEBRTC_MODULES_AUDIO_CODING_AUDIO_NETWORK_ADAPTOR_CONTROLLER_MANAGER_H_ +#include #include #include @@ -19,6 +20,8 @@ namespace webrtc { +class Clock; + class ControllerManager { public: virtual ~ControllerManager() = default; @@ -33,15 +36,23 @@ class ControllerManager { class ControllerManagerImpl final : public ControllerManager { public: struct Config { - Config(); + Config(int min_reordering_time_ms, + float min_reordering_squared_distance, + const Clock* clock); ~Config(); + int min_reordering_time_ms; + float min_reordering_squared_distance; + const Clock* clock; }; explicit ControllerManagerImpl(const Config& config); // Dependency injection for testing. - ControllerManagerImpl(const Config& config, - std::vector> controllers); + ControllerManagerImpl( + const Config& config, + std::vector>&& controllers, + const std::map>& + chracteristic_points); ~ControllerManagerImpl() override; @@ -52,12 +63,33 @@ class ControllerManagerImpl final : public ControllerManager { std::vector GetControllers() const override; private: + // Scoring point is a subset of NetworkMetrics that is used for comparing the + // significance of controllers. + struct ScoringPoint { + ScoringPoint(int uplink_bandwidth_bps, float uplink_packet_loss_fraction); + + // Calculate the normalized [0,1] distance between two scoring points. + float SquaredDistanceTo(const ScoringPoint& scoring_point) const; + + int uplink_bandwidth_bps; + float uplink_packet_loss_fraction; + }; + const Config config_; std::vector> controllers_; + rtc::Optional last_reordering_time_ms_; + ScoringPoint last_scoring_point_; + std::vector default_sorted_controllers_; + std::vector sorted_controllers_; + + // |scoring_points_| saves the characteristic scoring points of various + // controllers. + std::map controller_scoring_points_; + RTC_DISALLOW_COPY_AND_ASSIGN(ControllerManagerImpl); }; diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager_unittest.cc b/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager_unittest.cc index 53a2c2ccbd..a6c8436847 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager_unittest.cc +++ b/webrtc/modules/audio_coding/audio_network_adaptor/controller_manager_unittest.cc @@ -13,6 +13,7 @@ #include "testing/gtest/include/gtest/gtest.h" #include "webrtc/modules/audio_coding/audio_network_adaptor/controller_manager.h" #include "webrtc/modules/audio_coding/audio_network_adaptor/mock/mock_controller.h" +#include "webrtc/system_wrappers/include/clock.h" namespace webrtc { @@ -20,16 +21,32 @@ using ::testing::NiceMock; namespace { -constexpr size_t kNumControllers = 3; +constexpr size_t kNumControllers = 4; +constexpr int kChracteristicBandwithBps[2] = {15000, 0}; +constexpr float kChracteristicPacketLossFraction[2] = {0.2f, 0.0f}; +constexpr int kMinReorderingTimeMs = 200; +constexpr int kFactor = 100; +constexpr float kMinReorderingSquareDistance = 1.0f / kFactor / kFactor; + +// |kMinUplinkBandwidthBps| and |kMaxUplinkBandwidthBps| are copied from +// controller_manager.cc +constexpr int kMinUplinkBandwidthBps = 0; +constexpr int kMaxUplinkBandwidthBps = 120000; +constexpr int kMinBandwithChangeBps = + (kMaxUplinkBandwidthBps - kMinUplinkBandwidthBps) / kFactor; + +constexpr int64_t kClockInitialTime = 123456789; struct ControllerManagerStates { std::unique_ptr controller_manager; std::vector mock_controllers; + std::unique_ptr simulated_clock; }; ControllerManagerStates CreateControllerManager() { ControllerManagerStates states; std::vector> controllers; + std::map> chracteristic_points; for (size_t i = 0; i < kNumControllers; ++i) { auto controller = std::unique_ptr(new NiceMock()); @@ -37,16 +54,53 @@ ControllerManagerStates CreateControllerManager() { states.mock_controllers.push_back(controller.get()); controllers.push_back(std::move(controller)); } + + // Assign characteristic points to the last two controllers. + chracteristic_points[states.mock_controllers[kNumControllers - 2]] = + std::make_pair(kChracteristicBandwithBps[0], + kChracteristicPacketLossFraction[0]); + chracteristic_points[states.mock_controllers[kNumControllers - 1]] = + std::make_pair(kChracteristicBandwithBps[1], + kChracteristicPacketLossFraction[1]); + + states.simulated_clock.reset(new SimulatedClock(kClockInitialTime)); states.controller_manager.reset(new ControllerManagerImpl( - ControllerManagerImpl::Config(), std::move(controllers))); + ControllerManagerImpl::Config(kMinReorderingTimeMs, + kMinReorderingSquareDistance, + states.simulated_clock.get()), + std::move(controllers), chracteristic_points)); return states; } +// |expected_order| contains the expected indices of all controllers in the +// vector of controllers returned by GetSortedControllers(). A negative index +// means that we do not care about its exact place, but we do check that it +// exists in the vector. +void CheckControllersOrder( + ControllerManagerStates* states, + const rtc::Optional& uplink_bandwidth_bps, + const rtc::Optional& uplink_packet_loss_fraction, + const std::vector& expected_order) { + RTC_DCHECK_EQ(kNumControllers, expected_order.size()); + Controller::NetworkMetrics metrics; + metrics.uplink_bandwidth_bps = uplink_bandwidth_bps; + metrics.uplink_packet_loss_fraction = uplink_packet_loss_fraction; + auto check = states->controller_manager->GetSortedControllers(metrics); + EXPECT_EQ(states->mock_controllers.size(), check.size()); + for (size_t i = 0; i < states->mock_controllers.size(); ++i) { + if (expected_order[i] >= 0) { + EXPECT_EQ(states->mock_controllers[i], check[expected_order[i]]); + } else { + EXPECT_NE(check.end(), std::find(check.begin(), check.end(), + states->mock_controllers[i])); + } + } +} + } // namespace TEST(ControllerManagerTest, GetControllersReturnAllControllers) { auto states = CreateControllerManager(); - auto check = states.controller_manager->GetControllers(); // Verify that controllers in |check| are one-to-one mapped to those in // |mock_controllers_|. @@ -59,14 +113,81 @@ TEST(ControllerManagerTest, GetControllersReturnAllControllers) { TEST(ControllerManagerTest, ControllersInDefaultOrderOnEmptyNetworkMetrics) { auto states = CreateControllerManager(); - // |network_metrics| are empty, and the controllers are supposed to follow the // default order. - Controller::NetworkMetrics network_metrics; - auto check = states.controller_manager->GetSortedControllers(network_metrics); - EXPECT_EQ(states.mock_controllers.size(), check.size()); - for (size_t i = 0; i < states.mock_controllers.size(); ++i) - EXPECT_EQ(states.mock_controllers[i], check[i]); + CheckControllersOrder(&states, rtc::Optional(), rtc::Optional(), + {0, 1, 2, 3}); +} + +TEST(ControllerManagerTest, ControllersWithoutCharPointAtEndAndInDefaultOrder) { + auto states = CreateControllerManager(); + CheckControllersOrder(&states, rtc::Optional(0), + rtc::Optional(0.0), + {kNumControllers - 2, kNumControllers - 1, -1, -1}); +} + +TEST(ControllerManagerTest, ControllersWithCharPointDependOnNetworkMetrics) { + auto states = CreateControllerManager(); + CheckControllersOrder( + &states, rtc::Optional(kChracteristicBandwithBps[1]), + rtc::Optional(kChracteristicPacketLossFraction[1]), + {kNumControllers - 2, kNumControllers - 1, 1, 0}); +} + +TEST(ControllerManagerTest, DoNotReorderBeforeMinReordingTime) { + auto states = CreateControllerManager(); + CheckControllersOrder( + &states, rtc::Optional(kChracteristicBandwithBps[0]), + rtc::Optional(kChracteristicPacketLossFraction[0]), + {kNumControllers - 2, kNumControllers - 1, 0, 1}); + states.simulated_clock->AdvanceTimeMilliseconds(kMinReorderingTimeMs - 1); + // Move uplink bandwidth and packet loss fraction to the other controller's + // characteristic point, which would cause controller manager to reorder the + // controllers if time had reached min reordering time. + CheckControllersOrder( + &states, rtc::Optional(kChracteristicBandwithBps[1]), + rtc::Optional(kChracteristicPacketLossFraction[1]), + {kNumControllers - 2, kNumControllers - 1, 0, 1}); +} + +TEST(ControllerManagerTest, ReorderBeyondMinReordingTimeAndMinDistance) { + auto states = CreateControllerManager(); + constexpr int kBandwidthBps = + (kChracteristicBandwithBps[0] + kChracteristicBandwithBps[1]) / 2; + constexpr float kPacketLossFraction = (kChracteristicPacketLossFraction[0] + + kChracteristicPacketLossFraction[1]) / + 2.0f; + // Set network metrics to be in the middle between the characteristic points + // of two controllers. + CheckControllersOrder(&states, rtc::Optional(kBandwidthBps), + rtc::Optional(kPacketLossFraction), + {kNumControllers - 2, kNumControllers - 1, 0, 1}); + states.simulated_clock->AdvanceTimeMilliseconds(kMinReorderingTimeMs); + // Then let network metrics move a little towards the other controller. + CheckControllersOrder( + &states, rtc::Optional(kBandwidthBps - kMinBandwithChangeBps - 1), + rtc::Optional(kPacketLossFraction), + {kNumControllers - 2, kNumControllers - 1, 1, 0}); +} + +TEST(ControllerManagerTest, DoNotReorderIfNetworkMetricsChangeTooSmall) { + auto states = CreateControllerManager(); + constexpr int kBandwidthBps = + (kChracteristicBandwithBps[0] + kChracteristicBandwithBps[1]) / 2; + constexpr float kPacketLossFraction = (kChracteristicPacketLossFraction[0] + + kChracteristicPacketLossFraction[1]) / + 2.0f; + // Set network metrics to be in the middle between the characteristic points + // of two controllers. + CheckControllersOrder(&states, rtc::Optional(kBandwidthBps), + rtc::Optional(kPacketLossFraction), + {kNumControllers - 2, kNumControllers - 1, 0, 1}); + states.simulated_clock->AdvanceTimeMilliseconds(kMinReorderingTimeMs); + // Then let network metrics move a little towards the other controller. + CheckControllersOrder( + &states, rtc::Optional(kBandwidthBps - kMinBandwithChangeBps + 1), + rtc::Optional(kPacketLossFraction), + {kNumControllers - 2, kNumControllers - 1, 0, 1}); } } // namespace webrtc