Skip to content

Commit

Permalink
Remove voe::TransmitMixer
Browse files Browse the repository at this point in the history 
TransmitMixer's functionality is moved into the AudioTransportProxy
owned by AudioState. This removes the need for an AudioTransport
implementation in VoEBaseImpl, which means that the proxy is no longer
a proxy, hence AudioTransportProxy is renamed to AudioTransportImpl.

In the short term, AudioState needs to know which AudioDeviceModule is
used, so it is added in AudioState::Config. AudioTransportImpl needs
to know which AudioSendStream:s are currently enabled to send, so
AudioState maintains a map of them, which is reduced into a simple
vector for AudioTransportImpl.

To encode and transmit audio,
AudioSendStream::OnAudioData(std::unique_ptr<AudioFrame> audio_frame)
is introduced, which is used in both the Chromium and standalone use
cases. This removes the need for two different instances of
voe::Channel::ProcessAndEncodeAudio(), so there is now only one,
taking an AudioFrame as argument. Callers need to allocate their own
AudioFrame:s, which is wasteful but not a regression since this was
already happening in the voe::Channel functions.

Most of the logic changed resides in
AudioTransportImpl::RecordedDataIsAvailable(), where two strange
things were found:

  1. The clock drift parameter was ineffective since
     apm->echo_cancellation()->enable_drift_compensation(false) is
     called during initialization.

  2. The output parameter 'new_mic_volume' was never set - instead it
     was returned as a result, causing the ADM to never update the
     analog mic gain
     (https://cs.chromium.org/chromium/src/third_party/webrtc/voice_engine/voe_base_impl.cc?q=voe_base_impl.cc&dr&l=100).

Besides this, tests are updated, and some dead code is removed which
was found in the process.

Bug: webrtc:4690, webrtc:8591
Change-Id: I789d5296bf5efb7299a5ee05a4f3ce6abf9124b2
Reviewed-on: https://webrtc-review.googlesource.com/26681
Commit-Queue: Fredrik Solenberg <[email protected]>
Reviewed-by: Oskar Sundbom <[email protected]>
Reviewed-by: Karl Wiberg <[email protected]>
Cr-Commit-Position: refs/heads/master@{#21301}
  • Loading branch information
Fredrik Solenberg authored and Commit Bot committed Dec 15, 2017
1 parent 3af4791 commit 2a87797
Show file tree
Hide file tree
Showing 46 changed files with 929 additions and 1,135 deletions.
7 changes: 5 additions & 2 deletions audio/BUILD.gn
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,8 @@ rtc_static_library("audio") {
"audio_send_stream.h",
"audio_state.cc",
"audio_state.h",
"audio_transport_proxy.cc",
"audio_transport_proxy.h",
"audio_transport_impl.cc",
"audio_transport_impl.h",
"conversion.h",
"null_audio_poller.cc",
"null_audio_poller.h",
Expand Down Expand Up @@ -61,6 +61,8 @@ rtc_static_library("audio") {
"../system_wrappers",
"../system_wrappers:field_trial_api",
"../voice_engine",
"../voice_engine:audio_level",
"utility:audio_frame_operations",
]
}
if (rtc_include_tests) {
Expand Down Expand Up @@ -99,6 +101,7 @@ if (rtc_include_tests) {
":audio",
":audio_end_to_end_test",
"../api:mock_audio_mixer",
"../call:mock_call_interfaces",
"../call:mock_rtp_interfaces",
"../call:rtp_interfaces",
"../call:rtp_receiver",
Expand Down
9 changes: 3 additions & 6 deletions audio/DEPS
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,10 +19,7 @@ specific_include_rules = {
"audio_send_stream.cc": [
"+modules/audio_coding/codecs/cng/audio_encoder_cng.h",
],
# TODO(ossu): Remove this exception when builtin_audio_encoder_factory.h
# has moved to api/, or when the proper mocks have been made.
"audio_send_stream_unittest.cc": [
"+modules/audio_coding/codecs/builtin_audio_encoder_factory.h",
],
"audio_transport_impl.h": [
"+modules/audio_processing/typing_detection.h",
]
}

5 changes: 3 additions & 2 deletions audio/audio_receive_stream_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
#include "audio/conversion.h"
#include "call/rtp_stream_receiver_controller.h"
#include "logging/rtc_event_log/mock/mock_rtc_event_log.h"
#include "modules/audio_device/include/mock_audio_device.h"
#include "modules/audio_processing/include/mock_audio_processing.h"
#include "modules/bitrate_controller/include/mock/mock_bitrate_controller.h"
#include "modules/pacing/packet_router.h"
Expand Down Expand Up @@ -75,12 +76,12 @@ struct ConfigHelper {
audio_mixer_(new rtc::RefCountedObject<MockAudioMixer>()) {
using testing::Invoke;

EXPECT_CALL(voice_engine_, audio_transport());

AudioState::Config config;
config.voice_engine = &voice_engine_;
config.audio_mixer = audio_mixer_;
config.audio_processing = new rtc::RefCountedObject<MockAudioProcessing>();
config.audio_device_module =
new rtc::RefCountedObject<MockAudioDeviceModule>();
audio_state_ = AudioState::Create(config);

EXPECT_CALL(voice_engine_, ChannelProxyFactory(kChannelId))
Expand Down
63 changes: 52 additions & 11 deletions audio/audio_send_stream.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,6 @@
#include "system_wrappers/include/field_trial.h"
#include "voice_engine/channel_proxy.h"
#include "voice_engine/include/voe_base.h"
#include "voice_engine/transmit_mixer.h"
#include "voice_engine/voice_engine_impl.h"

namespace webrtc {
Expand Down Expand Up @@ -121,6 +120,7 @@ AudioSendStream::AudioSendStream(
AudioSendStream::~AudioSendStream() {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
RTC_LOG(LS_INFO) << "~AudioSendStream: " << config_.ToString();
RTC_DCHECK(!sending_);
transport_->send_side_cc()->DeRegisterPacketFeedbackObserver(this);
channel_proxy_->RegisterTransport(nullptr);
channel_proxy_->ResetSenderCongestionControlObjects();
Expand All @@ -135,6 +135,7 @@ const webrtc::AudioSendStream::Config& AudioSendStream::GetConfig() const {

void AudioSendStream::Reconfigure(
const webrtc::AudioSendStream::Config& new_config) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
ConfigureStream(this, new_config, false);
}

Expand Down Expand Up @@ -232,6 +233,10 @@ void AudioSendStream::ConfigureStream(

void AudioSendStream::Start() {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
if (sending_) {
return;
}

if (config_.min_bitrate_bps != -1 && config_.max_bitrate_bps != -1 &&
(FindExtensionIds(config_.rtp.extensions).transport_sequence_number !=
0 ||
Expand All @@ -246,17 +251,31 @@ void AudioSendStream::Start() {
if (error != 0) {
RTC_LOG(LS_ERROR) << "AudioSendStream::Start failed with error: " << error;
}
sending_ = true;
audio_state()->AddSendingStream(this, encoder_sample_rate_hz_,
encoder_num_channels_);
}

void AudioSendStream::Stop() {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
if (!sending_) {
return;
}

RemoveBitrateObserver();

ScopedVoEInterface<VoEBase> base(voice_engine());
int error = base->StopSend(config_.voe_channel_id);
if (error != 0) {
RTC_LOG(LS_ERROR) << "AudioSendStream::Stop failed with error: " << error;
}
sending_ = false;
audio_state()->RemoveSendingStream(this);
}

void AudioSendStream::SendAudioData(std::unique_ptr<AudioFrame> audio_frame) {
RTC_CHECK_RUNS_SERIALIZED(&audio_capture_race_checker_);
channel_proxy_->ProcessAndEncodeAudio(std::move(audio_frame));
}

bool AudioSendStream::SendTelephoneEvent(int payload_type,
Expand Down Expand Up @@ -313,17 +332,12 @@ webrtc::AudioSendStream::Stats AudioSendStream::GetStats(
}
}

ScopedVoEInterface<VoEBase> base(voice_engine());
RTC_DCHECK(base->transmit_mixer());
stats.audio_level = base->transmit_mixer()->AudioLevelFullRange();
RTC_DCHECK_LE(0, stats.audio_level);

stats.total_input_energy = base->transmit_mixer()->GetTotalInputEnergy();
stats.total_input_duration = base->transmit_mixer()->GetTotalInputDuration();
AudioState::Stats input_stats = audio_state()->GetAudioInputStats();
stats.audio_level = input_stats.audio_level;
stats.total_input_energy = input_stats.total_energy;
stats.total_input_duration = input_stats.total_duration;

internal::AudioState* audio_state =
static_cast<internal::AudioState*>(audio_state_.get());
stats.typing_noise_detected = audio_state->typing_noise_detected();
stats.typing_noise_detected = audio_state()->typing_noise_detected();
stats.ana_statistics = channel_proxy_->GetANAStatistics();
RTC_DCHECK(audio_state_->audio_processing());
stats.apm_statistics =
Expand Down Expand Up @@ -418,6 +432,20 @@ const TimeInterval& AudioSendStream::GetActiveLifetime() const {
return active_lifetime_;
}

internal::AudioState* AudioSendStream::audio_state() {
internal::AudioState* audio_state =
static_cast<internal::AudioState*>(audio_state_.get());
RTC_DCHECK(audio_state);
return audio_state;
}

const internal::AudioState* AudioSendStream::audio_state() const {
internal::AudioState* audio_state =
static_cast<internal::AudioState*>(audio_state_.get());
RTC_DCHECK(audio_state);
return audio_state;
}

VoiceEngine* AudioSendStream::voice_engine() const {
internal::AudioState* audio_state =
static_cast<internal::AudioState*>(audio_state_.get());
Expand All @@ -426,6 +454,17 @@ VoiceEngine* AudioSendStream::voice_engine() const {
return voice_engine;
}

void AudioSendStream::StoreEncoderProperties(int sample_rate_hz,
size_t num_channels) {
RTC_DCHECK(worker_thread_checker_.CalledOnValidThread());
encoder_sample_rate_hz_ = sample_rate_hz;
encoder_num_channels_ = num_channels;
if (sending_) {
// Update AudioState's information about the stream.
audio_state()->AddSendingStream(this, sample_rate_hz, num_channels);
}
}

// Apply current codec settings to a single voe::Channel used for sending.
bool AudioSendStream::SetupSendCodec(AudioSendStream* stream,
const Config& new_config) {
Expand Down Expand Up @@ -472,6 +511,8 @@ bool AudioSendStream::SetupSendCodec(AudioSendStream* stream,
new_config.send_codec_spec->format.clockrate_hz);
}

stream->StoreEncoderProperties(encoder->SampleRateHz(),
encoder->NumChannels());
stream->channel_proxy_->SetEncoder(new_config.send_codec_spec->payload_type,
std::move(encoder));
return true;
Expand Down
13 changes: 13 additions & 0 deletions audio/audio_send_stream.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
#include "call/bitrate_allocator.h"
#include "modules/rtp_rtcp/include/rtp_rtcp.h"
#include "rtc_base/constructormagic.h"
#include "rtc_base/race_checker.h"
#include "rtc_base/thread_checker.h"
#include "voice_engine/transport_feedback_packet_loss_tracker.h"

Expand All @@ -35,6 +36,8 @@ class ChannelProxy;
} // namespace voe

namespace internal {
class AudioState;

class AudioSendStream final : public webrtc::AudioSendStream,
public webrtc::BitrateAllocatorObserver,
public webrtc::PacketFeedbackObserver {
Expand All @@ -54,6 +57,7 @@ class AudioSendStream final : public webrtc::AudioSendStream,
void Reconfigure(const webrtc::AudioSendStream::Config& config) override;
void Start() override;
void Stop() override;
void SendAudioData(std::unique_ptr<AudioFrame> audio_frame) override;
bool SendTelephoneEvent(int payload_type, int payload_frequency, int event,
int duration_ms) override;
void SetMuted(bool muted) override;
Expand Down Expand Up @@ -83,8 +87,12 @@ class AudioSendStream final : public webrtc::AudioSendStream,
private:
class TimedTransport;

internal::AudioState* audio_state();
const internal::AudioState* audio_state() const;
VoiceEngine* voice_engine() const;

void StoreEncoderProperties(int sample_rate_hz, size_t num_channels);

// These are all static to make it less likely that (the old) config_ is
// accessed unintentionally.
static void ConfigureStream(AudioSendStream* stream,
Expand All @@ -105,12 +113,17 @@ class AudioSendStream final : public webrtc::AudioSendStream,

rtc::ThreadChecker worker_thread_checker_;
rtc::ThreadChecker pacer_thread_checker_;
rtc::RaceChecker audio_capture_race_checker_;
rtc::TaskQueue* worker_queue_;
webrtc::AudioSendStream::Config config_;
rtc::scoped_refptr<webrtc::AudioState> audio_state_;
std::unique_ptr<voe::ChannelProxy> channel_proxy_;
RtcEventLog* const event_log_;

int encoder_sample_rate_hz_ = 0;
size_t encoder_num_channels_ = 0;
bool sending_ = false;

BitrateAllocator* const bitrate_allocator_;
RtpTransportControllerSendInterface* const transport_;

Expand Down
39 changes: 7 additions & 32 deletions audio/audio_send_stream_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
#include "call/fake_rtp_transport_controller_send.h"
#include "call/rtp_transport_controller_send_interface.h"
#include "logging/rtc_event_log/mock/mock_rtc_event_log.h"
#include "modules/audio_device/include/mock_audio_device.h"
#include "modules/audio_mixer/audio_mixer_impl.h"
#include "modules/audio_processing/include/audio_processing_statistics.h"
#include "modules/audio_processing/include/mock_audio_processing.h"
Expand All @@ -33,7 +34,6 @@
#include "test/mock_audio_encoder_factory.h"
#include "test/mock_voe_channel_proxy.h"
#include "test/mock_voice_engine.h"
#include "voice_engine/transmit_mixer.h"

namespace webrtc {
namespace test {
Expand All @@ -58,9 +58,6 @@ const double kEchoReturnLoss = -65;
const double kEchoReturnLossEnhancement = 101;
const double kResidualEchoLikelihood = -1.0f;
const double kResidualEchoLikelihoodMax = 23.0f;
const int32_t kSpeechInputLevel = 96;
const double kTotalInputEnergy = 0.25;
const double kTotalInputDuration = 0.5;
const CallStatistics kCallStats = {
1345, 1678, 1901, 1234, 112, 13456, 17890, 1567, -1890, -1123};
const ReportBlock kReportBlock = {456, 780, 123, 567, 890, 132, 143, 13354};
Expand All @@ -85,14 +82,6 @@ class MockLimitObserver : public BitrateAllocator::LimitObserver {
uint32_t max_padding_bitrate_bps));
};

class MockTransmitMixer : public voe::TransmitMixer {
public:
MOCK_CONST_METHOD0(AudioLevelFullRange, int16_t());
MOCK_CONST_METHOD0(GetTotalInputEnergy, double());
MOCK_CONST_METHOD0(GetTotalInputDuration, double());
MOCK_CONST_METHOD0(typing_noise_detected, bool());
};

std::unique_ptr<MockAudioEncoder> SetupAudioEncoderMock(
int payload_type,
const SdpAudioFormat& format) {
Expand Down Expand Up @@ -151,12 +140,12 @@ struct ConfigHelper {
audio_encoder_(nullptr) {
using testing::Invoke;

EXPECT_CALL(voice_engine_, audio_transport());

AudioState::Config config;
config.voice_engine = &voice_engine_;
config.audio_mixer = AudioMixerImpl::Create();
config.audio_processing = audio_processing_;
config.audio_device_module =
new rtc::RefCountedObject<MockAudioDeviceModule>();
audio_state_ = AudioState::Create(config);

SetupDefaultChannelProxy(audio_bwe_enabled);
Expand Down Expand Up @@ -301,17 +290,6 @@ struct ConfigHelper {
.WillRepeatedly(Return(report_blocks));
EXPECT_CALL(*channel_proxy_, GetANAStatistics())
.WillRepeatedly(Return(ANAStats()));
EXPECT_CALL(voice_engine_, transmit_mixer())
.WillRepeatedly(Return(&transmit_mixer_));

EXPECT_CALL(transmit_mixer_, AudioLevelFullRange())
.WillRepeatedly(Return(kSpeechInputLevel));
EXPECT_CALL(transmit_mixer_, GetTotalInputEnergy())
.WillRepeatedly(Return(kTotalInputEnergy));
EXPECT_CALL(transmit_mixer_, GetTotalInputDuration())
.WillRepeatedly(Return(kTotalInputDuration));
EXPECT_CALL(transmit_mixer_, typing_noise_detected())
.WillRepeatedly(Return(true));

audio_processing_stats_.echo_return_loss = kEchoReturnLoss;
audio_processing_stats_.echo_return_loss_enhancement =
Expand All @@ -334,7 +312,6 @@ struct ConfigHelper {
AudioSendStream::Config stream_config_;
testing::StrictMock<MockVoEChannelProxy>* channel_proxy_ = nullptr;
rtc::scoped_refptr<MockAudioProcessing> audio_processing_;
MockTransmitMixer transmit_mixer_;
AudioProcessingStats audio_processing_stats_;
SimulatedClock simulated_clock_;
PacketRouter packet_router_;
Expand Down Expand Up @@ -447,9 +424,9 @@ TEST(AudioSendStreamTest, GetStats) {
(kIsacCodec.plfreq / 1000)),
stats.jitter_ms);
EXPECT_EQ(kCallStats.rttMs, stats.rtt_ms);
EXPECT_EQ(static_cast<int32_t>(kSpeechInputLevel), stats.audio_level);
EXPECT_EQ(kTotalInputEnergy, stats.total_input_energy);
EXPECT_EQ(kTotalInputDuration, stats.total_input_duration);
EXPECT_EQ(0, stats.audio_level);
EXPECT_EQ(0, stats.total_input_energy);
EXPECT_EQ(0, stats.total_input_duration);
EXPECT_EQ(kEchoDelayMedian, stats.apm_statistics.delay_median_ms);
EXPECT_EQ(kEchoDelayStdDev, stats.apm_statistics.delay_standard_deviation_ms);
EXPECT_EQ(kEchoReturnLoss, stats.apm_statistics.echo_return_loss);
Expand All @@ -461,7 +438,7 @@ TEST(AudioSendStreamTest, GetStats) {
stats.apm_statistics.residual_echo_likelihood);
EXPECT_EQ(kResidualEchoLikelihoodMax,
stats.apm_statistics.residual_echo_likelihood_recent_max);
EXPECT_TRUE(stats.typing_noise_detected);
EXPECT_FALSE(stats.typing_noise_detected);
}

TEST(AudioSendStreamTest, SendCodecAppliesAudioNetworkAdaptor) {
Expand Down Expand Up @@ -594,7 +571,5 @@ TEST(AudioSendStreamTest, ReconfigureTransportCcResetsFirst) {
}
send_stream.Reconfigure(new_config);
}


} // namespace test
} // namespace webrtc
Loading

0 comments on commit 2a87797

Please sign in to comment.