Skip to content

Commit

Permalink
Voice Engine: Require caller to supply an AudioDecoderFactory
Browse files Browse the repository at this point in the history
So that we don't have to be capable of creating one ourselves, which
requires a dependency on the audio decoders.

In the process, remove the default values for the VoEBase::Init() arguments,
since there isn't a sensible default value for the audio decoder factory
anymore.

BUG=webrtc:8396

Change-Id: Idb433efa49e1a68e8206d369d27b3c255185777a
Reviewed-on: https://webrtc-review.googlesource.com/18200
Commit-Queue: Karl Wiberg <[email protected]>
Reviewed-by: Henrik Andreassson <[email protected]>
Cr-Commit-Position: refs/heads/master@{#20552}
  • Loading branch information
Karl Wiberg authored and Commit Bot committed Nov 2, 2017
1 parent 1803628 commit f3850f6
Show file tree
Hide file tree
Showing 6 changed files with 17 additions and 16 deletions.
1 change: 1 addition & 0 deletions call/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -202,6 +202,7 @@ if (rtc_include_tests) {
"..:webrtc_common",
"../api:array_view",
"../api:mock_audio_mixer",
"../api/audio_codecs:builtin_audio_decoder_factory",
"../logging:rtc_event_log_api",
"../modules/audio_device:mock_audio_device",
"../modules/audio_mixer",
Expand Down
4 changes: 3 additions & 1 deletion call/call_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,7 @@
#include <memory>
#include <utility>

#include "api/audio_codecs/builtin_audio_decoder_factory.h"
#include "api/test/mock_audio_mixer.h"
#include "call/audio_state.h"
#include "call/call.h"
Expand Down Expand Up @@ -456,7 +457,8 @@ TEST(CallTest, RecreatingAudioStreamWithSameSsrcReusesRtpState) {
audio_state_config.voice_engine = voice_engine.voe;
audio_state_config.audio_mixer = mock_mixer;
audio_state_config.audio_processing = AudioProcessing::Create();
voice_engine.base->Init(&mock_adm, audio_state_config.audio_processing.get());
voice_engine.base->Init(&mock_adm, audio_state_config.audio_processing.get(),
CreateBuiltinAudioDecoderFactory());
auto audio_state = AudioState::Create(audio_state_config);

RtcEventLogNullImpl event_log;
Expand Down
3 changes: 1 addition & 2 deletions voice_engine/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,6 @@ rtc_static_library("voice_engine") {
"../api:refcountedbase",
"../api:transport_api",
"../api/audio_codecs:audio_codecs_api",
"../api/audio_codecs:builtin_audio_decoder_factory",
"../api/audio_codecs:builtin_audio_encoder_factory",
"../audio/utility:audio_frame_operations",
"../call:rtp_interfaces",
"../common_audio",
Expand Down Expand Up @@ -101,6 +99,7 @@ if (rtc_include_tests) {
rtc_test("voice_engine_unittests") {
deps = [
":voice_engine",
"../api/audio_codecs:builtin_audio_decoder_factory",
"../common_audio",
"../modules:module_api",
"../modules/audio_coding",
Expand Down
11 changes: 5 additions & 6 deletions voice_engine/include/voe_base.h
Original file line number Diff line number Diff line change
Expand Up @@ -92,13 +92,12 @@ class WEBRTC_DLLEXPORT VoEBase {
// functionality in a separate (reference counted) module.
// - The AudioProcessing module handles capture-side processing.
// - An AudioDecoderFactory - used to create audio decoders.
// If NULL is passed for either of ADM or AudioDecoderFactory, VoiceEngine
// If NULL is passed for ADM, VoiceEngine
// will create its own. Returns -1 in case of an error, 0 otherwise.
// TODO(ajm): Remove default NULLs.
virtual int Init(AudioDeviceModule* external_adm = NULL,
AudioProcessing* external_apm = nullptr,
const rtc::scoped_refptr<AudioDecoderFactory>&
decoder_factory = nullptr) = 0;
virtual int Init(
AudioDeviceModule* external_adm,
AudioProcessing* external_apm,
const rtc::scoped_refptr<AudioDecoderFactory>& decoder_factory) = 0;
// This method is WIP - DO NOT USE!
// Returns NULL before Init() is called.
virtual AudioDeviceModule* audio_device_module() = 0;
Expand Down
7 changes: 2 additions & 5 deletions voice_engine/voe_base_impl.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

#include "voice_engine/voe_base_impl.h"

#include "api/audio_codecs/builtin_audio_decoder_factory.h"
#include "common_audio/signal_processing/include/signal_processing_library.h"
#include "modules/audio_coding/include/audio_coding_module.h"
#include "modules/audio_device/audio_device_impl.h"
Expand Down Expand Up @@ -272,10 +271,8 @@ int VoEBaseImpl::Init(
}
#endif

if (decoder_factory)
decoder_factory_ = decoder_factory;
else
decoder_factory_ = CreateBuiltinAudioDecoderFactory();
RTC_DCHECK(decoder_factory);
decoder_factory_ = decoder_factory;

return 0;
}
Expand Down
7 changes: 5 additions & 2 deletions voice_engine/voe_base_unittest.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@

#include "voice_engine/include/voe_base.h"

#include "api/audio_codecs/builtin_audio_decoder_factory.h"
#include "modules/audio_device/include/fake_audio_device.h"
#include "modules/audio_processing/include/mock_audio_processing.h"
#include "rtc_base/refcountedobject.h"
Expand Down Expand Up @@ -39,11 +40,13 @@ class VoEBaseTest : public ::testing::Test {
};

TEST_F(VoEBaseTest, InitWithExternalAudioDevice) {
EXPECT_EQ(0, base_->Init(&adm_, apm_.get()));
EXPECT_EQ(0,
base_->Init(&adm_, apm_.get(), CreateBuiltinAudioDecoderFactory()));
}

TEST_F(VoEBaseTest, CreateChannelAfterInit) {
EXPECT_EQ(0, base_->Init(&adm_, apm_.get(), nullptr));
EXPECT_EQ(0,
base_->Init(&adm_, apm_.get(), CreateBuiltinAudioDecoderFactory()));
int channelID = base_->CreateChannel();
EXPECT_NE(channelID, -1);
EXPECT_EQ(0, base_->DeleteChannel(channelID));
Expand Down

0 comments on commit f3850f6

Please sign in to comment.