Resolve cyclic dependency between audio network adaptor and event log api

BUG=webrtc:7257

Review-Url: https://codereview.webrtc.org/2745473003
Cr-Commit-Position: refs/heads/master@{#17565}
This commit is contained in:
michaelt
2017-04-06 05:59:10 -07:00
committed by Commit bot
parent 28dc285f22
commit cde46b7278
46 changed files with 101 additions and 108 deletions

View File

@ -12,11 +12,11 @@
namespace webrtc {
AudioNetworkAdaptor::EncoderRuntimeConfig::EncoderRuntimeConfig() = default;
AudioEncoderRuntimeConfig::AudioEncoderRuntimeConfig() = default;
AudioNetworkAdaptor::EncoderRuntimeConfig::~EncoderRuntimeConfig() = default;
AudioEncoderRuntimeConfig::~AudioEncoderRuntimeConfig() = default;
AudioNetworkAdaptor::EncoderRuntimeConfig::EncoderRuntimeConfig(
const EncoderRuntimeConfig& other) = default;
AudioEncoderRuntimeConfig::AudioEncoderRuntimeConfig(
const AudioEncoderRuntimeConfig& other) = default;
} // namespace webrtc

View File

@ -112,9 +112,8 @@ void AudioNetworkAdaptorImpl::SetOverhead(size_t overhead_bytes_per_packet) {
UpdateNetworkMetrics(network_metrics);
}
AudioNetworkAdaptor::EncoderRuntimeConfig
AudioNetworkAdaptorImpl::GetEncoderRuntimeConfig() {
EncoderRuntimeConfig config;
AudioEncoderRuntimeConfig AudioNetworkAdaptorImpl::GetEncoderRuntimeConfig() {
AudioEncoderRuntimeConfig config;
for (auto& controller :
controller_manager_->GetSortedControllers(last_metrics_))
controller->MakeDecision(&config);

View File

@ -54,7 +54,7 @@ class AudioNetworkAdaptorImpl final : public AudioNetworkAdaptor {
void SetOverhead(size_t overhead_bytes_per_packet) override;
EncoderRuntimeConfig GetEncoderRuntimeConfig() override;
AudioEncoderRuntimeConfig GetEncoderRuntimeConfig() override;
void StartDebugDump(FILE* file_handle) override;

View File

@ -182,7 +182,7 @@ TEST(AudioNetworkAdaptorImplTest,
DumpEncoderRuntimeConfigIsCalledOnGetEncoderRuntimeConfig) {
auto states = CreateAudioNetworkAdaptor();
AudioNetworkAdaptor::EncoderRuntimeConfig config;
AudioEncoderRuntimeConfig config;
config.bitrate_bps = rtc::Optional<int>(32000);
config.enable_fec = rtc::Optional<bool>(true);
@ -255,7 +255,7 @@ TEST(AudioNetworkAdaptorImplTest,
TEST(AudioNetworkAdaptorImplTest, LogRuntimeConfigOnGetEncoderRuntimeConfig) {
auto states = CreateAudioNetworkAdaptor();
AudioNetworkAdaptor::EncoderRuntimeConfig config;
AudioEncoderRuntimeConfig config;
config.bitrate_bps = rtc::Optional<int>(32000);
config.enable_fec = rtc::Optional<bool>(true);

View File

@ -43,8 +43,7 @@ void BitrateController::UpdateNetworkMetrics(
overhead_bytes_per_packet_ = network_metrics.overhead_bytes_per_packet;
}
void BitrateController::MakeDecision(
AudioNetworkAdaptor::EncoderRuntimeConfig* config) {
void BitrateController::MakeDecision(AudioEncoderRuntimeConfig* config) {
// Decision on |bitrate_bps| should not have been made.
RTC_DCHECK(!config->bitrate_bps);
if (target_audio_bitrate_bps_ && overhead_bytes_per_packet_) {

View File

@ -32,7 +32,7 @@ class BitrateController final : public Controller {
void UpdateNetworkMetrics(const NetworkMetrics& network_metrics) override;
void MakeDecision(AudioNetworkAdaptor::EncoderRuntimeConfig* config) override;
void MakeDecision(AudioEncoderRuntimeConfig* config) override;
private:
const Config config_;

View File

@ -39,7 +39,7 @@ void UpdateNetworkMetrics(
void CheckDecision(BitrateController* controller,
const rtc::Optional<int>& frame_length_ms,
int expected_bitrate_bps) {
AudioNetworkAdaptor::EncoderRuntimeConfig config;
AudioEncoderRuntimeConfig config;
config.frame_length_ms = frame_length_ms;
controller->MakeDecision(&config);
EXPECT_EQ(rtc::Optional<int>(expected_bitrate_bps), config.bitrate_bps);

View File

@ -41,8 +41,7 @@ void ChannelController::UpdateNetworkMetrics(
uplink_bandwidth_bps_ = network_metrics.uplink_bandwidth_bps;
}
void ChannelController::MakeDecision(
AudioNetworkAdaptor::EncoderRuntimeConfig* config) {
void ChannelController::MakeDecision(AudioEncoderRuntimeConfig* config) {
// Decision on |num_channels| should not have been made.
RTC_DCHECK(!config->num_channels);

View File

@ -39,7 +39,7 @@ class ChannelController final : public Controller {
void UpdateNetworkMetrics(const NetworkMetrics& network_metrics) override;
void MakeDecision(AudioNetworkAdaptor::EncoderRuntimeConfig* config) override;
void MakeDecision(AudioEncoderRuntimeConfig* config) override;
private:
const Config config_;

View File

@ -39,7 +39,7 @@ void CheckDecision(ChannelController* controller,
network_metrics.uplink_bandwidth_bps = uplink_bandwidth_bps;
controller->UpdateNetworkMetrics(network_metrics);
}
AudioNetworkAdaptor::EncoderRuntimeConfig config;
AudioEncoderRuntimeConfig config;
controller->MakeDecision(&config);
EXPECT_EQ(rtc::Optional<size_t>(expected_num_channels), config.num_channels);
}

View File

@ -35,8 +35,7 @@ class Controller {
// indicates an update on the corresponding network metric.
virtual void UpdateNetworkMetrics(const NetworkMetrics& network_metrics) = 0;
virtual void MakeDecision(
AudioNetworkAdaptor::EncoderRuntimeConfig* config) = 0;
virtual void MakeDecision(AudioEncoderRuntimeConfig* config) = 0;
};
} // namespace webrtc

View File

@ -299,10 +299,10 @@ void CheckControllersOrder(const std::vector<Controller*>& controllers,
ASSERT_EQ(expected_types.size(), controllers.size());
// We also check that the controllers follow the initial settings.
AudioNetworkAdaptor::EncoderRuntimeConfig encoder_config;
AudioEncoderRuntimeConfig encoder_config;
for (size_t i = 0; i < controllers.size(); ++i) {
AudioNetworkAdaptor::EncoderRuntimeConfig encoder_config;
AudioEncoderRuntimeConfig encoder_config;
// We check the order of |controllers| by judging their decisions.
controllers[i]->MakeDecision(&encoder_config);

View File

@ -49,9 +49,8 @@ class DebugDumpWriterImpl final : public DebugDumpWriter {
explicit DebugDumpWriterImpl(FILE* file_handle);
~DebugDumpWriterImpl() override = default;
void DumpEncoderRuntimeConfig(
const AudioNetworkAdaptor::EncoderRuntimeConfig& config,
int64_t timestamp) override;
void DumpEncoderRuntimeConfig(const AudioEncoderRuntimeConfig& config,
int64_t timestamp) override;
void DumpNetworkMetrics(const Controller::NetworkMetrics& metrics,
int64_t timestamp) override;
@ -104,7 +103,7 @@ void DebugDumpWriterImpl::DumpNetworkMetrics(
}
void DebugDumpWriterImpl::DumpEncoderRuntimeConfig(
const AudioNetworkAdaptor::EncoderRuntimeConfig& config,
const AudioEncoderRuntimeConfig& config,
int64_t timestamp) {
#ifdef WEBRTC_AUDIO_NETWORK_ADAPTOR_DEBUG_DUMP
Event event;

View File

@ -27,9 +27,8 @@ class DebugDumpWriter {
virtual ~DebugDumpWriter() = default;
virtual void DumpEncoderRuntimeConfig(
const AudioNetworkAdaptor::EncoderRuntimeConfig& config,
int64_t timestamp) = 0;
virtual void DumpEncoderRuntimeConfig(const AudioEncoderRuntimeConfig& config,
int64_t timestamp) = 0;
virtual void DumpNetworkMetrics(const Controller::NetworkMetrics& metrics,
int64_t timestamp) = 0;

View File

@ -31,8 +31,7 @@ void DtxController::UpdateNetworkMetrics(
uplink_bandwidth_bps_ = network_metrics.uplink_bandwidth_bps;
}
void DtxController::MakeDecision(
AudioNetworkAdaptor::EncoderRuntimeConfig* config) {
void DtxController::MakeDecision(AudioEncoderRuntimeConfig* config) {
// Decision on |enable_dtx| should not have been made.
RTC_DCHECK(!config->enable_dtx);

View File

@ -35,7 +35,7 @@ class DtxController final : public Controller {
void UpdateNetworkMetrics(const NetworkMetrics& network_metrics) override;
void MakeDecision(AudioNetworkAdaptor::EncoderRuntimeConfig* config) override;
void MakeDecision(AudioEncoderRuntimeConfig* config) override;
private:
const Config config_;

View File

@ -37,7 +37,7 @@ void CheckDecision(DtxController* controller,
network_metrics.uplink_bandwidth_bps = uplink_bandwidth_bps;
controller->UpdateNetworkMetrics(network_metrics);
}
AudioNetworkAdaptor::EncoderRuntimeConfig config;
AudioEncoderRuntimeConfig config;
controller->MakeDecision(&config);
EXPECT_EQ(rtc::Optional<bool>(expected_dtx_enabled), config.enable_dtx);
}

View File

@ -30,7 +30,7 @@ EventLogWriter::EventLogWriter(RtcEventLog* event_log,
EventLogWriter::~EventLogWriter() = default;
void EventLogWriter::MaybeLogEncoderConfig(
const AudioNetworkAdaptor::EncoderRuntimeConfig& config) {
const AudioEncoderRuntimeConfig& config) {
if (last_logged_config_.num_channels != config.num_channels)
return LogEncoderConfig(config);
if (last_logged_config_.enable_dtx != config.enable_dtx)
@ -59,8 +59,7 @@ void EventLogWriter::MaybeLogEncoderConfig(
}
}
void EventLogWriter::LogEncoderConfig(
const AudioNetworkAdaptor::EncoderRuntimeConfig& config) {
void EventLogWriter::LogEncoderConfig(const AudioEncoderRuntimeConfig& config) {
event_log_->LogAudioNetworkAdaptation(config);
last_logged_config_ = config;
}

View File

@ -24,18 +24,16 @@ class EventLogWriter final {
float min_bitrate_change_fraction,
float min_packet_loss_change_fraction);
~EventLogWriter();
void MaybeLogEncoderConfig(
const AudioNetworkAdaptor::EncoderRuntimeConfig& config);
void MaybeLogEncoderConfig(const AudioEncoderRuntimeConfig& config);
private:
void LogEncoderConfig(
const AudioNetworkAdaptor::EncoderRuntimeConfig& config);
void LogEncoderConfig(const AudioEncoderRuntimeConfig& config);
RtcEventLog* const event_log_;
const int min_bitrate_change_bps_;
const float min_bitrate_change_fraction_;
const float min_packet_loss_change_fraction_;
AudioNetworkAdaptor::EncoderRuntimeConfig last_logged_config_;
AudioEncoderRuntimeConfig last_logged_config_;
RTC_DISALLOW_COPY_AND_ASSIGN(EventLogWriter);
};

View File

@ -43,7 +43,7 @@ MATCHER_P(EncoderRuntimeConfigIs, config, "") {
struct EventLogWriterStates {
std::unique_ptr<EventLogWriter> event_log_writer;
std::unique_ptr<testing::StrictMock<MockRtcEventLog>> event_log;
AudioNetworkAdaptor::EncoderRuntimeConfig runtime_config;
AudioEncoderRuntimeConfig runtime_config;
};
EventLogWriterStates CreateEventLogWriter() {

View File

@ -79,8 +79,7 @@ void FecControllerPlrBased::UpdateNetworkMetrics(
}
}
void FecControllerPlrBased::MakeDecision(
AudioNetworkAdaptor::EncoderRuntimeConfig* config) {
void FecControllerPlrBased::MakeDecision(AudioEncoderRuntimeConfig* config) {
RTC_DCHECK(!config->enable_fec);
RTC_DCHECK(!config->uplink_packet_loss_fraction);

View File

@ -55,7 +55,7 @@ class FecControllerPlrBased final : public Controller {
void UpdateNetworkMetrics(const NetworkMetrics& network_metrics) override;
void MakeDecision(AudioNetworkAdaptor::EncoderRuntimeConfig* config) override;
void MakeDecision(AudioEncoderRuntimeConfig* config) override;
private:
bool FecEnablingDecision(const rtc::Optional<float>& packet_loss) const;

View File

@ -96,7 +96,7 @@ void UpdateNetworkMetrics(FecControllerPlrBasedTestStates* states,
void CheckDecision(FecControllerPlrBasedTestStates* states,
bool expected_enable_fec,
float expected_uplink_packet_loss_fraction) {
AudioNetworkAdaptor::EncoderRuntimeConfig config;
AudioEncoderRuntimeConfig config;
states->controller->MakeDecision(&config);
EXPECT_EQ(rtc::Optional<bool>(expected_enable_fec), config.enable_fec);
EXPECT_EQ(rtc::Optional<float>(expected_uplink_packet_loss_fraction),

View File

@ -42,8 +42,7 @@ void FecControllerRplrBased::UpdateNetworkMetrics(
}
}
void FecControllerRplrBased::MakeDecision(
AudioNetworkAdaptor::EncoderRuntimeConfig* config) {
void FecControllerRplrBased::MakeDecision(AudioEncoderRuntimeConfig* config) {
RTC_DCHECK(!config->enable_fec);
RTC_DCHECK(!config->uplink_packet_loss_fraction);

View File

@ -47,7 +47,7 @@ class FecControllerRplrBased final : public Controller {
void UpdateNetworkMetrics(const NetworkMetrics& network_metrics) override;
void MakeDecision(AudioNetworkAdaptor::EncoderRuntimeConfig* config) override;
void MakeDecision(AudioEncoderRuntimeConfig* config) override;
private:
bool FecEnablingDecision() const;

View File

@ -113,7 +113,7 @@ void UpdateNetworkMetrics(
void CheckDecision(FecControllerRplrBased* controller,
bool expected_enable_fec,
float expected_uplink_packet_loss_fraction) {
AudioNetworkAdaptor::EncoderRuntimeConfig config;
AudioEncoderRuntimeConfig config;
controller->MakeDecision(&config);
// Less compact than comparing optionals, but yields more readable errors.

View File

@ -65,8 +65,7 @@ void FrameLengthController::UpdateNetworkMetrics(
overhead_bytes_per_packet_ = network_metrics.overhead_bytes_per_packet;
}
void FrameLengthController::MakeDecision(
AudioNetworkAdaptor::EncoderRuntimeConfig* config) {
void FrameLengthController::MakeDecision(AudioEncoderRuntimeConfig* config) {
// Decision on |frame_length_ms| should not have been made.
RTC_DCHECK(!config->frame_length_ms);
@ -92,7 +91,7 @@ bool FrameLengthController::Config::FrameLengthChange::operator<(
}
bool FrameLengthController::FrameLengthIncreasingDecision(
const AudioNetworkAdaptor::EncoderRuntimeConfig& config) const {
const AudioEncoderRuntimeConfig& config) const {
// Increase frame length if
// 1. |uplink_bandwidth_bps| is known to be smaller or equal than
// |min_encoder_bitrate_bps| plus |prevent_overuse_margin_bps| plus the
@ -129,7 +128,7 @@ bool FrameLengthController::FrameLengthIncreasingDecision(
}
bool FrameLengthController::FrameLengthDecreasingDecision(
const AudioNetworkAdaptor::EncoderRuntimeConfig& config) const {
const AudioEncoderRuntimeConfig& config) const {
// Decrease frame length if
// 1. shorter frame length is available AND
// 2. |uplink_bandwidth_bps| is known to be bigger than

View File

@ -54,14 +54,14 @@ class FrameLengthController final : public Controller {
void UpdateNetworkMetrics(const NetworkMetrics& network_metrics) override;
void MakeDecision(AudioNetworkAdaptor::EncoderRuntimeConfig* config) override;
void MakeDecision(AudioEncoderRuntimeConfig* config) override;
private:
bool FrameLengthIncreasingDecision(
const AudioNetworkAdaptor::EncoderRuntimeConfig& config) const;
const AudioEncoderRuntimeConfig& config) const;
bool FrameLengthDecreasingDecision(
const AudioNetworkAdaptor::EncoderRuntimeConfig& config) const;
const AudioEncoderRuntimeConfig& config) const;
const Config config_;

View File

@ -101,7 +101,7 @@ void UpdateNetworkMetrics(
void CheckDecision(FrameLengthController* controller,
const rtc::Optional<bool>& enable_fec,
int expected_frame_length_ms) {
AudioNetworkAdaptor::EncoderRuntimeConfig config;
AudioEncoderRuntimeConfig config;
config.enable_fec = enable_fec;
controller->MakeDecision(&config);
EXPECT_EQ(rtc::Optional<int>(expected_frame_length_ms),

View File

@ -15,28 +15,29 @@
namespace webrtc {
struct AudioEncoderRuntimeConfig {
AudioEncoderRuntimeConfig();
AudioEncoderRuntimeConfig(const AudioEncoderRuntimeConfig& other);
~AudioEncoderRuntimeConfig();
rtc::Optional<int> bitrate_bps;
rtc::Optional<int> frame_length_ms;
// Note: This is what we tell the encoder. It doesn't have to reflect
// the actual NetworkMetrics; it's subject to our decision.
rtc::Optional<float> uplink_packet_loss_fraction;
rtc::Optional<bool> enable_fec;
rtc::Optional<bool> enable_dtx;
// Some encoders can encode fewer channels than the actual input to make
// better use of the bandwidth. |num_channels| sets the number of channels
// to encode.
rtc::Optional<size_t> num_channels;
};
// An AudioNetworkAdaptor optimizes the audio experience by suggesting a
// suitable runtime configuration (bit rate, frame length, FEC, etc.) to the
// encoder based on network metrics.
class AudioNetworkAdaptor {
public:
struct EncoderRuntimeConfig {
EncoderRuntimeConfig();
EncoderRuntimeConfig(const EncoderRuntimeConfig& other);
~EncoderRuntimeConfig();
rtc::Optional<int> bitrate_bps;
rtc::Optional<int> frame_length_ms;
// Note: This is what we tell the encoder. It doesn't have to reflect
// the actual NetworkMetrics; it's subject to our decision.
rtc::Optional<float> uplink_packet_loss_fraction;
rtc::Optional<bool> enable_fec;
rtc::Optional<bool> enable_dtx;
// Some encoders can encode fewer channels than the actual input to make
// better use of the bandwidth. |num_channels| sets the number of channels
// to encode.
rtc::Optional<size_t> num_channels;
};
virtual ~AudioNetworkAdaptor() = default;
@ -54,7 +55,7 @@ class AudioNetworkAdaptor {
virtual void SetOverhead(size_t overhead_bytes_per_packet) = 0;
virtual EncoderRuntimeConfig GetEncoderRuntimeConfig() = 0;
virtual AudioEncoderRuntimeConfig GetEncoderRuntimeConfig() = 0;
virtual void StartDebugDump(FILE* file_handle) = 0;

View File

@ -35,7 +35,7 @@ class MockAudioNetworkAdaptor : public AudioNetworkAdaptor {
MOCK_METHOD1(SetOverhead, void(size_t overhead_bytes_per_packet));
MOCK_METHOD0(GetEncoderRuntimeConfig, EncoderRuntimeConfig());
MOCK_METHOD0(GetEncoderRuntimeConfig, AudioEncoderRuntimeConfig());
MOCK_METHOD1(StartDebugDump, void(FILE* file_handle));

View File

@ -22,8 +22,7 @@ class MockController : public Controller {
MOCK_METHOD0(Die, void());
MOCK_METHOD1(UpdateNetworkMetrics,
void(const NetworkMetrics& network_metrics));
MOCK_METHOD1(MakeDecision,
void(AudioNetworkAdaptor::EncoderRuntimeConfig* config));
MOCK_METHOD1(MakeDecision, void(AudioEncoderRuntimeConfig* config));
};
} // namespace webrtc

View File

@ -22,7 +22,7 @@ class MockDebugDumpWriter : public DebugDumpWriter {
MOCK_METHOD0(Die, void());
MOCK_METHOD2(DumpEncoderRuntimeConfig,
void(const AudioNetworkAdaptor::EncoderRuntimeConfig& config,
void(const AudioEncoderRuntimeConfig& config,
int64_t timestamp));
MOCK_METHOD2(DumpNetworkMetrics,
void(const Controller::NetworkMetrics& metrics,

View File

@ -87,14 +87,14 @@ AudioEncoderOpusStates CreateCodec(size_t num_channels) {
return states;
}
AudioNetworkAdaptor::EncoderRuntimeConfig CreateEncoderRuntimeConfig() {
AudioEncoderRuntimeConfig CreateEncoderRuntimeConfig() {
constexpr int kBitrate = 40000;
constexpr int kFrameLength = 60;
constexpr bool kEnableFec = true;
constexpr bool kEnableDtx = false;
constexpr size_t kNumChannels = 1;
constexpr float kPacketLossFraction = 0.1f;
AudioNetworkAdaptor::EncoderRuntimeConfig config;
AudioEncoderRuntimeConfig config;
config.bitrate_bps = rtc::Optional<int>(kBitrate);
config.frame_length_ms = rtc::Optional<int>(kFrameLength);
config.enable_fec = rtc::Optional<bool>(kEnableFec);
@ -105,9 +105,8 @@ AudioNetworkAdaptor::EncoderRuntimeConfig CreateEncoderRuntimeConfig() {
return config;
}
void CheckEncoderRuntimeConfig(
const AudioEncoderOpus* encoder,
const AudioNetworkAdaptor::EncoderRuntimeConfig& config) {
void CheckEncoderRuntimeConfig(const AudioEncoderOpus* encoder,
const AudioEncoderRuntimeConfig& config) {
EXPECT_EQ(*config.bitrate_bps, encoder->GetTargetBitrate());
EXPECT_EQ(*config.frame_length_ms, encoder->next_frame_length_ms());
EXPECT_EQ(*config.enable_fec, encoder->fec_enabled());
@ -472,7 +471,7 @@ TEST(AudioEncoderOpusTest, EmptyConfigDoesNotAffectEncoderSettings) {
states.encoder->EnableAudioNetworkAdaptor("", nullptr, nullptr);
auto config = CreateEncoderRuntimeConfig();
AudioNetworkAdaptor::EncoderRuntimeConfig empty_config;
AudioEncoderRuntimeConfig empty_config;
EXPECT_CALL(**states.mock_audio_network_adaptor, GetEncoderRuntimeConfig())
.WillOnce(Return(config))