From a6f495c7c20e8b5ae33766ebbb1265ba14389e5a Mon Sep 17 00:00:00 2001 From: minyue Date: Mon, 24 Oct 2016 09:19:14 -0700 Subject: [PATCH] Simplifying audio network adaptor by moving receiver frame length range to ctor. It turns out that that audio network adaptor can always be created with knowledge of receiver frame length range. There is no need to keep some infrastructure that is used for runtime setting of receiver frame length ranges. BUG=webrtc:6303 Review-Url: https://codereview.webrtc.org/2429503002 Cr-Commit-Position: refs/heads/master@{#14748} --- webrtc/base/array_view.h | 3 + .../audio_network_adaptor_impl.cc | 13 --- .../audio_network_adaptor_impl.h | 3 - .../audio_network_adaptor_impl_unittest.cc | 11 --- .../audio_network_adaptor/controller.cc | 14 --- .../audio_network_adaptor/controller.h | 14 --- .../frame_length_controller.cc | 55 ++---------- .../frame_length_controller.h | 9 +- .../frame_length_controller_unittest.cc | 87 +++++-------------- .../include/audio_network_adaptor.h | 3 - .../mock/mock_audio_network_adaptor.h | 3 - .../mock/mock_controller.h | 1 - .../codecs/opus/audio_encoder_opus.cc | 24 +++-- .../codecs/opus/audio_encoder_opus.h | 4 + .../opus/audio_encoder_opus_unittest.cc | 38 ++++---- 15 files changed, 74 insertions(+), 208 deletions(-) diff --git a/webrtc/base/array_view.h b/webrtc/base/array_view.h index 59e0395199..5c1ffe2489 100644 --- a/webrtc/base/array_view.h +++ b/webrtc/base/array_view.h @@ -73,6 +73,9 @@ namespace rtc { template class ArrayView final { public: + using value_type = T; + using const_iterator = const T*; + // Construct an empty ArrayView. ArrayView() : ArrayView(static_cast(nullptr), 0) {} ArrayView(std::nullptr_t) : ArrayView() {} diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc b/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc index 0db5429650..46c21d1e4c 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc +++ b/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.cc @@ -54,19 +54,6 @@ void AudioNetworkAdaptorImpl::SetRtt(int rtt_ms) { DumpNetworkMetrics(); } -void AudioNetworkAdaptorImpl::SetReceiverFrameLengthRange( - int min_frame_length_ms, - int max_frame_length_ms) { - Controller::Constraints constraints; - constraints.receiver_frame_length_range = - rtc::Optional( - Controller::Constraints::FrameLengthRange(min_frame_length_ms, - max_frame_length_ms)); - auto controllers = controller_manager_->GetControllers(); - for (auto& controller : controllers) - controller->SetConstraints(constraints); -} - AudioNetworkAdaptor::EncoderRuntimeConfig AudioNetworkAdaptorImpl::GetEncoderRuntimeConfig() { EncoderRuntimeConfig config; diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h b/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h index 0afba66f2b..8853db526d 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h +++ b/webrtc/modules/audio_coding/audio_network_adaptor/audio_network_adaptor_impl.h @@ -45,9 +45,6 @@ class AudioNetworkAdaptorImpl final : public AudioNetworkAdaptor { void SetTargetAudioBitrate(int target_audio_bitrate_bps) override; - void SetReceiverFrameLengthRange(int min_frame_length_ms, - int max_frame_length_ms) override; - EncoderRuntimeConfig GetEncoderRuntimeConfig() override; void StartDebugDump(FILE* file_handle) override; 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 d4aeadc185..c7700fd906 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 @@ -139,17 +139,6 @@ TEST(AudioNetworkAdaptorImplTest, states.audio_network_adaptor->GetEncoderRuntimeConfig(); } -TEST(AudioNetworkAdaptorImplTest, SetConstraintsIsCalledOnSetFrameLengthRange) { - auto states = CreateAudioNetworkAdaptor(); - - for (auto& mock_controller : states.mock_controllers) { - EXPECT_CALL(*mock_controller, - SetConstraints(ConstraintsReceiverFrameLengthRangeIs( - Controller::Constraints::FrameLengthRange(20, 120)))); - } - states.audio_network_adaptor->SetReceiverFrameLengthRange(20, 120); -} - TEST(AudioNetworkAdaptorImplTest, DumpEncoderRuntimeConfigIsCalledOnGetEncoderRuntimeConfig) { auto states = CreateAudioNetworkAdaptor(); diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/controller.cc b/webrtc/modules/audio_coding/audio_network_adaptor/controller.cc index 1151228a8b..549352f36a 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/controller.cc +++ b/webrtc/modules/audio_coding/audio_network_adaptor/controller.cc @@ -16,18 +16,4 @@ Controller::NetworkMetrics::NetworkMetrics() = default; Controller::NetworkMetrics::~NetworkMetrics() = default; -Controller::Constraints::Constraints() = default; - -Controller::Constraints::~Constraints() = default; - -Controller::Constraints::FrameLengthRange::FrameLengthRange( - int min_frame_length_ms, - int max_frame_length_ms) - : min_frame_length_ms(min_frame_length_ms), - max_frame_length_ms(max_frame_length_ms) {} - -Controller::Constraints::FrameLengthRange::~FrameLengthRange() = default; - -void Controller::SetConstraints(const Constraints& constraints) {} - } // 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 f27a391ad5..07806aa000 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/controller.h +++ b/webrtc/modules/audio_coding/audio_network_adaptor/controller.h @@ -27,25 +27,11 @@ class Controller { rtc::Optional rtt_ms; }; - struct Constraints { - Constraints(); - ~Constraints(); - struct FrameLengthRange { - FrameLengthRange(int min_frame_length_ms, int max_frame_length_ms); - ~FrameLengthRange(); - int min_frame_length_ms; - int max_frame_length_ms; - }; - rtc::Optional receiver_frame_length_range; - }; - virtual ~Controller() = default; virtual void MakeDecision( const NetworkMetrics& metrics, AudioNetworkAdaptor::EncoderRuntimeConfig* config) = 0; - - virtual void SetConstraints(const Constraints& constraints); }; } // namespace webrtc diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller.cc b/webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller.cc index ff271d58b6..01c995ddd0 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller.cc +++ b/webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller.cc @@ -10,7 +10,6 @@ #include "webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller.h" -#include #include #include "webrtc/base/checks.h" @@ -38,16 +37,11 @@ FrameLengthController::Config::~Config() = default; FrameLengthController::FrameLengthController(const Config& config) : config_(config) { + frame_length_ms_ = std::find(config_.encoder_frame_lengths_ms.begin(), + config_.encoder_frame_lengths_ms.end(), + config_.initial_frame_length_ms); // |encoder_frame_lengths_ms| must contain |initial_frame_length_ms|. - RTC_DCHECK(config_.encoder_frame_lengths_ms.end() != - std::find(config_.encoder_frame_lengths_ms.begin(), - config_.encoder_frame_lengths_ms.end(), - config_.initial_frame_length_ms)); - - // We start with assuming that the receiver only accepts - // |config_.initial_frame_length_ms|. - run_time_frame_lengths_ms_.push_back(config_.initial_frame_length_ms); - frame_length_ms_ = run_time_frame_lengths_ms_.begin(); + RTC_DCHECK(frame_length_ms_ != config_.encoder_frame_lengths_ms.end()); frame_length_change_criteria_.insert(std::make_pair( FrameLengthChange(20, 60), config_.fl_20ms_to_60ms_bandwidth_bps)); @@ -71,14 +65,6 @@ void FrameLengthController::MakeDecision( config->frame_length_ms = rtc::Optional(*frame_length_ms_); } -void FrameLengthController::SetConstraints(const Constraints& constraints) { - if (constraints.receiver_frame_length_range) { - SetReceiverFrameLengthRange( - constraints.receiver_frame_length_range->min_frame_length_ms, - constraints.receiver_frame_length_range->max_frame_length_ms); - } -} - FrameLengthController::FrameLengthChange::FrameLengthChange( int from_frame_length_ms, int to_frame_length_ms) @@ -94,35 +80,6 @@ bool FrameLengthController::FrameLengthChange::operator<( to_frame_length_ms < rhs.to_frame_length_ms); } -void FrameLengthController::SetReceiverFrameLengthRange( - int min_frame_length_ms, - int max_frame_length_ms) { - int frame_length_ms = *frame_length_ms_; - // Reset |run_time_frame_lengths_ms_| by filtering |config_.frame_lengths_ms| - // with the receiver frame length range. - run_time_frame_lengths_ms_.clear(); - std::copy_if(config_.encoder_frame_lengths_ms.begin(), - config_.encoder_frame_lengths_ms.end(), - std::back_inserter(run_time_frame_lengths_ms_), - [&](int frame_length_ms) { - return frame_length_ms >= min_frame_length_ms && - frame_length_ms <= max_frame_length_ms; - }); - RTC_DCHECK(std::is_sorted(run_time_frame_lengths_ms_.begin(), - run_time_frame_lengths_ms_.end())); - - // Keep the current frame length. If it has gone out of the new range, use - // the smallest available frame length. - frame_length_ms_ = - std::find(run_time_frame_lengths_ms_.begin(), - run_time_frame_lengths_ms_.end(), frame_length_ms); - if (frame_length_ms_ == run_time_frame_lengths_ms_.end()) { - LOG(LS_WARNING) - << "Actual frame length not in frame length range of the receiver"; - frame_length_ms_ = run_time_frame_lengths_ms_.begin(); - } -} - bool FrameLengthController::FrameLengthIncreasingDecision( const NetworkMetrics& metrics, const AudioNetworkAdaptor::EncoderRuntimeConfig& config) const { @@ -133,7 +90,7 @@ bool FrameLengthController::FrameLengthIncreasingDecision( // AND // 4. FEC is not decided or is OFF. auto longer_frame_length_ms = std::next(frame_length_ms_); - if (longer_frame_length_ms == run_time_frame_lengths_ms_.end()) + if (longer_frame_length_ms == config_.encoder_frame_lengths_ms.end()) return false; auto increase_threshold = frame_length_change_criteria_.find( @@ -158,7 +115,7 @@ bool FrameLengthController::FrameLengthDecreasingDecision( // 2. |uplink_bandwidth_bps| is known to be larger than a threshold, // 3. |uplink_packet_loss_fraction| is known to be larger than a threshold, // 4. FEC is decided ON. - if (frame_length_ms_ == run_time_frame_lengths_ms_.begin()) + if (frame_length_ms_ == config_.encoder_frame_lengths_ms.begin()) return false; auto shorter_frame_length_ms = std::prev(frame_length_ms_); diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller.h b/webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller.h index d19710254d..bce7f8ff03 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller.h +++ b/webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller.h @@ -52,8 +52,6 @@ class FrameLengthController final : public Controller { void MakeDecision(const NetworkMetrics& metrics, AudioNetworkAdaptor::EncoderRuntimeConfig* config) override; - void SetConstraints(const Constraints& constraints) override; - private: friend class FrameLengthControllerForTest; @@ -65,9 +63,6 @@ class FrameLengthController final : public Controller { int to_frame_length_ms; }; - void SetReceiverFrameLengthRange(int min_frame_length_ms, - int max_frame_length_ms); - bool FrameLengthIncreasingDecision( const NetworkMetrics& metrics, const AudioNetworkAdaptor::EncoderRuntimeConfig& config) const; @@ -78,9 +73,7 @@ class FrameLengthController final : public Controller { const Config config_; - std::vector run_time_frame_lengths_ms_; - - std::vector::iterator frame_length_ms_; + std::vector::const_iterator frame_length_ms_; std::map frame_length_change_criteria_; diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller_unittest.cc b/webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller_unittest.cc index 997382f750..a013c857b5 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller_unittest.cc +++ b/webrtc/modules/audio_coding/audio_network_adaptor/frame_length_controller_unittest.cc @@ -22,32 +22,19 @@ constexpr float kFlIncreasingPacketLossFraction = 0.04f; constexpr float kFlDecreasingPacketLossFraction = 0.05f; constexpr int kFl20msTo60msBandwidthBps = 22000; constexpr int kFl60msTo20msBandwidthBps = 88000; -constexpr int kMinReceiverFrameLengthMs = 10; -constexpr int kMaxReceiverFrameLengthMs = 120; constexpr int kMediumBandwidthBps = (kFl60msTo20msBandwidthBps + kFl20msTo60msBandwidthBps) / 2; constexpr float kMediumPacketLossFraction = (kFlDecreasingPacketLossFraction + kFlIncreasingPacketLossFraction) / 2; -void SetReceiverFrameLengthRange(FrameLengthController* controller, - int min_frame_length_ms, - int max_frame_length_ms) { - Controller::Constraints constraints; - using FrameLengthRange = Controller::Constraints::FrameLengthRange; - constraints.receiver_frame_length_range = rtc::Optional( - FrameLengthRange(min_frame_length_ms, max_frame_length_ms)); - controller->SetConstraints(constraints); -} - std::unique_ptr CreateController( + const std::vector& encoder_frame_lengths_ms, int initial_frame_length_ms) { std::unique_ptr controller( new FrameLengthController(FrameLengthController::Config( - {20, 60}, initial_frame_length_ms, kFlIncreasingPacketLossFraction, - kFlDecreasingPacketLossFraction, kFl20msTo60msBandwidthBps, - kFl60msTo20msBandwidthBps))); - SetReceiverFrameLengthRange(controller.get(), kMinReceiverFrameLengthMs, - kMaxReceiverFrameLengthMs); + encoder_frame_lengths_ms, initial_frame_length_ms, + kFlIncreasingPacketLossFraction, kFlDecreasingPacketLossFraction, + kFl20msTo60msBandwidthBps, kFl60msTo20msBandwidthBps))); return controller; } @@ -68,44 +55,28 @@ void CheckDecision(FrameLengthController* controller, } // namespace -TEST(FrameLengthControllerTest, - OutputInitValueWhenReceiverFrameLengthRangeUnset) { - constexpr int kInitialFrameLenghtMs = 60; - std::unique_ptr controller( - new FrameLengthController(FrameLengthController::Config( - {20, 60}, kInitialFrameLenghtMs, kFlIncreasingPacketLossFraction, - kFlDecreasingPacketLossFraction, kFl20msTo60msBandwidthBps, - kFl60msTo20msBandwidthBps))); - // Use a high uplink bandwidth that would cause frame length to decrease if - // receiver frame length is set. - CheckDecision(controller.get(), rtc::Optional(kFl60msTo20msBandwidthBps), - rtc::Optional(), rtc::Optional(), - kInitialFrameLenghtMs); -} - TEST(FrameLengthControllerTest, DecreaseTo20MsOnHighUplinkBandwidth) { - auto controller = CreateController(60); + auto controller = CreateController({20, 60}, 60); CheckDecision(controller.get(), rtc::Optional(kFl60msTo20msBandwidthBps), rtc::Optional(), rtc::Optional(), 20); } TEST(FrameLengthControllerTest, DecreaseTo20MsOnHighUplinkPacketLossFraction) { - auto controller = CreateController(60); + auto controller = CreateController({20, 60}, 60); CheckDecision(controller.get(), rtc::Optional(), rtc::Optional(kFlDecreasingPacketLossFraction), rtc::Optional(), 20); } TEST(FrameLengthControllerTest, DecreaseTo20MsWhenFecIsOn) { - auto controller = CreateController(60); + auto controller = CreateController({20, 60}, 60); CheckDecision(controller.get(), rtc::Optional(), rtc::Optional(), rtc::Optional(true), 20); } TEST(FrameLengthControllerTest, Maintain60MsIf20MsNotInReceiverFrameLengthRange) { - auto controller = CreateController(60); - SetReceiverFrameLengthRange(controller.get(), 21, 60); + auto controller = CreateController({60}, 60); // Set FEC on that would cause frame length to decrease if receiver frame // length range included 20ms. CheckDecision(controller.get(), rtc::Optional(), rtc::Optional(), @@ -117,7 +88,7 @@ TEST(FrameLengthControllerTest, Maintain60MsOnMultipleConditions) { // 1. |uplink_bandwidth_bps| is at medium level, // 2. |uplink_packet_loss_fraction| is at medium, // 3. FEC is not decided ON. - auto controller = CreateController(60); + auto controller = CreateController({20, 60}, 60); CheckDecision(controller.get(), rtc::Optional(kMediumBandwidthBps), rtc::Optional(kMediumPacketLossFraction), rtc::Optional(), 60); @@ -129,7 +100,7 @@ TEST(FrameLengthControllerTest, IncreaseTo60MsOnMultipleConditions) { // 2. |uplink_packet_loss_fraction| is known to be smaller than a threshold // AND // 3. FEC is not decided or OFF. - auto controller = CreateController(20); + auto controller = CreateController({20, 60}, 20); CheckDecision(controller.get(), rtc::Optional(kFl20msTo60msBandwidthBps), rtc::Optional(kFlIncreasingPacketLossFraction), rtc::Optional(), 60); @@ -137,8 +108,7 @@ TEST(FrameLengthControllerTest, IncreaseTo60MsOnMultipleConditions) { TEST(FrameLengthControllerTest, Maintain20MsIf60MsNotInReceiverFrameLengthRange) { - auto controller = CreateController(20); - SetReceiverFrameLengthRange(controller.get(), 10, 59); + auto controller = CreateController({20}, 20); // Use a low uplink bandwidth and a low uplink packet loss fraction that would // cause frame length to increase if receiver frame length included 60ms. CheckDecision(controller.get(), rtc::Optional(kFl20msTo60msBandwidthBps), @@ -147,14 +117,14 @@ TEST(FrameLengthControllerTest, } TEST(FrameLengthControllerTest, Maintain20MsOnMediumUplinkBandwidth) { - auto controller = CreateController(20); + auto controller = CreateController({20, 60}, 20); CheckDecision(controller.get(), rtc::Optional(kMediumBandwidthBps), rtc::Optional(kFlIncreasingPacketLossFraction), rtc::Optional(), 20); } TEST(FrameLengthControllerTest, Maintain20MsOnMediumUplinkPacketLossFraction) { - auto controller = CreateController(20); + auto controller = CreateController({20, 60}, 20); // Use a low uplink bandwidth that would cause frame length to increase if // uplink packet loss fraction was low. CheckDecision(controller.get(), rtc::Optional(kFl20msTo60msBandwidthBps), @@ -163,7 +133,7 @@ TEST(FrameLengthControllerTest, Maintain20MsOnMediumUplinkPacketLossFraction) { } TEST(FrameLengthControllerTest, Maintain20MsWhenFecIsOn) { - auto controller = CreateController(20); + auto controller = CreateController({20, 60}, 20); // Use a low uplink bandwidth and a low uplink packet loss fraction that would // cause frame length to increase if FEC was not ON. CheckDecision(controller.get(), rtc::Optional(kFl20msTo60msBandwidthBps), @@ -171,16 +141,6 @@ TEST(FrameLengthControllerTest, Maintain20MsWhenFecIsOn) { rtc::Optional(true), 20); } -TEST(FrameLengthControllerTest, - MaintainFrameLengthOnSetReceiverFrameLengthRange) { - auto controller = CreateController(60); - CheckDecision(controller.get(), rtc::Optional(), rtc::Optional(), - rtc::Optional(true), 20); - SetReceiverFrameLengthRange(controller.get(), 10, 60); - CheckDecision(controller.get(), rtc::Optional(), rtc::Optional(), - rtc::Optional(), 20); -} - namespace { constexpr int kFl60msTo120msBandwidthBps = 18000; constexpr int kFl120msTo60msBandwidthBps = 72000; @@ -191,9 +151,10 @@ class FrameLengthControllerForTest { // This class is to test multiple frame lengths. FrameLengthController is // implemented to support this, but is not enabled for the default constructor // for the time being. We use this class to test it. - explicit FrameLengthControllerForTest(int initial_frame_length_ms) + FrameLengthControllerForTest(const std::vector& encoder_frame_lengths_ms, + int initial_frame_length_ms) : frame_length_controller_( - FrameLengthController::Config({20, 60, 120}, + FrameLengthController::Config(encoder_frame_lengths_ms, initial_frame_length_ms, kFlIncreasingPacketLossFraction, kFlDecreasingPacketLossFraction, @@ -205,7 +166,6 @@ class FrameLengthControllerForTest { frame_length_controller_.frame_length_change_criteria_.insert( std::make_pair(FrameLengthController::FrameLengthChange(120, 60), kFl120msTo60msBandwidthBps)); - frame_length_controller_.SetReceiverFrameLengthRange(20, 120); } FrameLengthController* get() { return &frame_length_controller_; } @@ -214,7 +174,7 @@ class FrameLengthControllerForTest { }; TEST(FrameLengthControllerTest, From120MsTo20MsOnHighUplinkBandwidth) { - FrameLengthControllerForTest controller(120); + FrameLengthControllerForTest controller({20, 60, 120}, 120); // It takes two steps for frame length to go from 120ms to 20ms. CheckDecision(controller.get(), rtc::Optional(kFl60msTo20msBandwidthBps), rtc::Optional(), rtc::Optional(), 60); @@ -223,7 +183,7 @@ TEST(FrameLengthControllerTest, From120MsTo20MsOnHighUplinkBandwidth) { } TEST(FrameLengthControllerTest, From120MsTo20MsOnHighUplinkPacketLossFraction) { - FrameLengthControllerForTest controller(120); + FrameLengthControllerForTest controller({20, 60, 120}, 120); // It takes two steps for frame length to go from 120ms to 20ms. CheckDecision(controller.get(), rtc::Optional(), rtc::Optional(kFlDecreasingPacketLossFraction), @@ -234,7 +194,7 @@ TEST(FrameLengthControllerTest, From120MsTo20MsOnHighUplinkPacketLossFraction) { } TEST(FrameLengthControllerTest, From120MsTo20MsWhenFecIsOn) { - FrameLengthControllerForTest controller(120); + FrameLengthControllerForTest controller({20, 60, 120}, 120); // It takes two steps for frame length to go from 120ms to 20ms. CheckDecision(controller.get(), rtc::Optional(), rtc::Optional(), rtc::Optional(true), 60); @@ -248,7 +208,7 @@ TEST(FrameLengthControllerTest, From20MsTo120MsOnMultipleConditions) { // 2. |uplink_packet_loss_fraction| is known to be smaller than a threshold // AND // 3. FEC is not decided or OFF. - FrameLengthControllerForTest controller(20); + FrameLengthControllerForTest controller({20, 60, 120}, 20); // It takes two steps for frame length to go from 20ms to 120ms. CheckDecision(controller.get(), rtc::Optional(kFl60msTo120msBandwidthBps), @@ -261,8 +221,7 @@ TEST(FrameLengthControllerTest, From20MsTo120MsOnMultipleConditions) { } TEST(FrameLengthControllerTest, Stall60MsIf120MsNotInReceiverFrameLengthRange) { - FrameLengthControllerForTest controller(20); - SetReceiverFrameLengthRange(controller.get(), 20, 119); + FrameLengthControllerForTest controller({20, 60}, 20); CheckDecision(controller.get(), rtc::Optional(kFl60msTo120msBandwidthBps), rtc::Optional(kFlIncreasingPacketLossFraction), @@ -274,7 +233,7 @@ TEST(FrameLengthControllerTest, Stall60MsIf120MsNotInReceiverFrameLengthRange) { } TEST(FrameLengthControllerTest, CheckBehaviorOnChangingNetworkMetrics) { - FrameLengthControllerForTest controller(20); + FrameLengthControllerForTest controller({20, 60, 120}, 20); CheckDecision(controller.get(), rtc::Optional(kMediumBandwidthBps), rtc::Optional(kFlIncreasingPacketLossFraction), rtc::Optional(), 20); diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/include/audio_network_adaptor.h b/webrtc/modules/audio_coding/audio_network_adaptor/include/audio_network_adaptor.h index 1fcf8ddaac..f592ce8224 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/include/audio_network_adaptor.h +++ b/webrtc/modules/audio_coding/audio_network_adaptor/include/audio_network_adaptor.h @@ -47,9 +47,6 @@ class AudioNetworkAdaptor { virtual void SetTargetAudioBitrate(int target_audio_bitrate_bps) = 0; - virtual void SetReceiverFrameLengthRange(int min_frame_length_ms, - int max_frame_length_ms) = 0; - virtual EncoderRuntimeConfig GetEncoderRuntimeConfig() = 0; virtual void StartDebugDump(FILE* file_handle) = 0; diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/mock/mock_audio_network_adaptor.h b/webrtc/modules/audio_coding/audio_network_adaptor/mock/mock_audio_network_adaptor.h index 7d5b4e568d..44b63c0f7d 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/mock/mock_audio_network_adaptor.h +++ b/webrtc/modules/audio_coding/audio_network_adaptor/mock/mock_audio_network_adaptor.h @@ -30,9 +30,6 @@ class MockAudioNetworkAdaptor : public AudioNetworkAdaptor { MOCK_METHOD1(SetTargetAudioBitrate, void(int target_audio_bitrate_bps)); - MOCK_METHOD2(SetReceiverFrameLengthRange, - void(int min_frame_length_ms, int max_frame_length_ms)); - MOCK_METHOD0(GetEncoderRuntimeConfig, EncoderRuntimeConfig()); MOCK_METHOD1(StartDebugDump, void(FILE* file_handle)); diff --git a/webrtc/modules/audio_coding/audio_network_adaptor/mock/mock_controller.h b/webrtc/modules/audio_coding/audio_network_adaptor/mock/mock_controller.h index cf7c307326..21acadd248 100644 --- a/webrtc/modules/audio_coding/audio_network_adaptor/mock/mock_controller.h +++ b/webrtc/modules/audio_coding/audio_network_adaptor/mock/mock_controller.h @@ -23,7 +23,6 @@ class MockController : public Controller { MOCK_METHOD2(MakeDecision, void(const NetworkMetrics& metrics, AudioNetworkAdaptor::EncoderRuntimeConfig* config)); - MOCK_METHOD1(SetConstraints, void(const Constraints& constraints)); }; } // namespace webrtc diff --git a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc index 29f85ac470..f466926f19 100644 --- a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc +++ b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.cc @@ -11,6 +11,7 @@ #include "webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.h" #include +#include #include "webrtc/base/checks.h" #include "webrtc/base/exp_filter.h" @@ -42,6 +43,7 @@ AudioEncoderOpus::Config CreateConfig(const CodecInst& codec_inst) { config.payload_type = codec_inst.pltype; config.application = config.num_channels == 1 ? AudioEncoderOpus::kVoip : AudioEncoderOpus::kAudio; + config.supported_frame_lengths_ms.push_back(config.frame_size_ms); return config; } @@ -297,11 +299,21 @@ void AudioEncoderOpus::OnReceivedRtt(int rtt_ms) { void AudioEncoderOpus::SetReceiverFrameLengthRange(int min_frame_length_ms, int max_frame_length_ms) { - if (!audio_network_adaptor_) - return; - audio_network_adaptor_->SetReceiverFrameLengthRange(min_frame_length_ms, - max_frame_length_ms); - ApplyAudioNetworkAdaptor(); + // Ensure that |SetReceiverFrameLengthRange| is called before + // |EnableAudioNetworkAdaptor|, otherwise we need to recreate + // |audio_network_adaptor_|, which is not a needed use case. + RTC_DCHECK(!audio_network_adaptor_); + + config_.supported_frame_lengths_ms.clear(); + std::copy_if(std::begin(kSupportedFrameLengths), + std::end(kSupportedFrameLengths), + std::back_inserter(config_.supported_frame_lengths_ms), + [&](int frame_length_ms) { + return frame_length_ms >= min_frame_length_ms && + frame_length_ms <= max_frame_length_ms; + }); + RTC_DCHECK(std::is_sorted(config_.supported_frame_lengths_ms.begin(), + config_.supported_frame_lengths_ms.end())); } AudioEncoder::EncodedInfo AudioEncoderOpus::EncodeImpl( @@ -447,7 +459,7 @@ AudioEncoderOpus::DefaultAudioNetworkAdaptorCreator( config.clock = clock; return std::unique_ptr(new AudioNetworkAdaptorImpl( config, ControllerManagerImpl::Create( - config_string, NumChannels(), kSupportedFrameLengths, + config_string, NumChannels(), supported_frame_lengths_ms(), num_channels_to_encode_, next_frame_length_ms_, GetTargetBitrate(), config_.fec_enabled, GetDtx(), clock))); } diff --git a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.h b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.h index 342668e780..81ca17f7e0 100644 --- a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.h +++ b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.h @@ -49,6 +49,7 @@ class AudioEncoderOpus final : public AudioEncoder { int max_playback_rate_hz = 48000; int complexity = kDefaultComplexity; bool dtx_enabled = false; + std::vector supported_frame_lengths_ms; const Clock* clock = nullptr; private: @@ -102,6 +103,9 @@ class AudioEncoderOpus final : public AudioEncoder { void OnReceivedRtt(int rtt_ms) override; void SetReceiverFrameLengthRange(int min_frame_length_ms, int max_frame_length_ms) override; + rtc::ArrayView supported_frame_lengths_ms() const { + return config_.supported_frame_lengths_ms; + } // Getters for testing. double packet_loss_rate() const { return packet_loss_rate_; } diff --git a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc index 6a4c47c0e3..54529ad985 100644 --- a/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc +++ b/webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus_unittest.cc @@ -14,6 +14,7 @@ #include "webrtc/common_types.h" #include "webrtc/modules/audio_coding/audio_network_adaptor/mock/mock_audio_network_adaptor.h" #include "webrtc/modules/audio_coding/codecs/opus/audio_encoder_opus.h" +#include "webrtc/test/gmock.h" #include "webrtc/test/gtest.h" #include "webrtc/system_wrappers/include/clock.h" @@ -34,6 +35,7 @@ AudioEncoderOpus::Config CreateConfig(const CodecInst& codec_inst) { config.payload_type = codec_inst.pltype; config.application = config.num_channels == 1 ? AudioEncoderOpus::kVoip : AudioEncoderOpus::kAudio; + config.supported_frame_lengths_ms.push_back(config.frame_size_ms); return config; } @@ -222,9 +224,25 @@ TEST(AudioEncoderOpusTest, PacketLossRateOptimized) { // clang-format on } +TEST(AudioEncoderOpusTest, SetReceiverFrameLengthRange) { + auto states = CreateCodec(2); + // Before calling to |SetReceiverFrameLengthRange|, + // |supported_frame_lengths_ms| should contain only the frame length being + // used. + using ::testing::ElementsAre; + EXPECT_THAT(states.encoder->supported_frame_lengths_ms(), + ElementsAre(states.encoder->next_frame_length_ms())); + states.encoder->SetReceiverFrameLengthRange(0, 12345); + EXPECT_THAT(states.encoder->supported_frame_lengths_ms(), + ElementsAre(20, 60)); + states.encoder->SetReceiverFrameLengthRange(21, 60); + EXPECT_THAT(states.encoder->supported_frame_lengths_ms(), ElementsAre(60)); + states.encoder->SetReceiverFrameLengthRange(20, 59); + EXPECT_THAT(states.encoder->supported_frame_lengths_ms(), ElementsAre(20)); +} + TEST(AudioEncoderOpusTest, InvokeAudioNetworkAdaptorOnSetUplinkBandwidth) { auto states = CreateCodec(2); - printf("passed!\n"); states.encoder->EnableAudioNetworkAdaptor("", nullptr); auto config = CreateEncoderRuntimeConfig(); @@ -291,24 +309,6 @@ TEST(AudioEncoderOpusTest, InvokeAudioNetworkAdaptorOnSetRtt) { CheckEncoderRuntimeConfig(states.encoder.get(), config); } -TEST(AudioEncoderOpusTest, - InvokeAudioNetworkAdaptorOnSetReceiverFrameLengthRange) { - auto states = CreateCodec(2); - states.encoder->EnableAudioNetworkAdaptor("", nullptr); - - auto config = CreateEncoderRuntimeConfig(); - EXPECT_CALL(**states.mock_audio_network_adaptor, GetEncoderRuntimeConfig()) - .WillOnce(Return(config)); - - constexpr int kMinFrameLength = 10; - constexpr int kMaxFrameLength = 60; - EXPECT_CALL(**states.mock_audio_network_adaptor, - SetReceiverFrameLengthRange(kMinFrameLength, kMaxFrameLength)); - states.encoder->SetReceiverFrameLengthRange(kMinFrameLength, kMaxFrameLength); - - CheckEncoderRuntimeConfig(states.encoder.get(), config); -} - TEST(AudioEncoderOpusTest, PacketLossFractionSmoothedOnSetUplinkPacketLossFraction) { auto states = CreateCodec(2);