Implemented further mixer interface change suggestions from https://codereview.webrtc.org/2386383003/
Changed mixability status into AddSource/RemoveSource. Added 'ssrc()'
method to the MixerSource interface. Removed unnecessary member 'num_audio_sources_' and made the mixer be refcounted.
BUG=webrtc:6346
NOTRY=True
Review-Url: https://codereview.webrtc.org/2408683002
Cr-Commit-Position: refs/heads/master@{#14612}
diff --git a/webrtc/modules/BUILD.gn b/webrtc/modules/BUILD.gn
index a44f006..ff0918e 100644
--- a/webrtc/modules/BUILD.gn
+++ b/webrtc/modules/BUILD.gn
@@ -337,7 +337,7 @@
"audio_conference_mixer/test/audio_conference_mixer_unittest.cc",
"audio_device/fine_audio_buffer_unittest.cc",
"audio_mixer/audio_frame_manipulator_unittest.cc",
- "audio_mixer/test/audio_mixer_unittest.cc",
+ "audio_mixer/audio_mixer_impl_unittest.cc",
"audio_processing/aec/echo_cancellation_unittest.cc",
"audio_processing/aec/system_delay_unittest.cc",
"audio_processing/agc/agc_manager_direct_unittest.cc",
diff --git a/webrtc/modules/audio_mixer/audio_mixer.h b/webrtc/modules/audio_mixer/audio_mixer.h
index 0c71469..7e58a8d 100644
--- a/webrtc/modules/audio_mixer/audio_mixer.h
+++ b/webrtc/modules/audio_mixer/audio_mixer.h
@@ -13,13 +13,13 @@
#include <memory>
+#include "webrtc/base/refcount.h"
#include "webrtc/modules/include/module_common_types.h"
namespace webrtc {
-class AudioMixer {
+class AudioMixer : public rtc::RefCountInterface {
public:
- static const int kMaximumAmountOfMixedAudioSources = 3;
// A callback class that all mixer participants must inherit from/implement.
class Source {
public:
@@ -47,16 +47,23 @@
// mixer.
virtual AudioFrameWithInfo GetAudioFrameWithInfo(int sample_rate_hz) = 0;
+ // A way for a mixer implementation do distinguish participants.
+ virtual int ssrc() = 0;
+
protected:
virtual ~Source() {}
};
- // Factory method. Constructor disabled.
- static std::unique_ptr<AudioMixer> Create();
- virtual ~AudioMixer() {}
+ // Since the mixer is reference counted, the destructor may be
+ // called from any thread.
+ ~AudioMixer() override {}
- // Add/remove audio sources as candidates for mixing.
- virtual int32_t SetMixabilityStatus(Source* audio_source, bool mixable) = 0;
+ // Returns true if adding/removing was successful. A source is never
+ // added twice and removal is never attempted if a source has not
+ // been successfully added to the mixer. Addition and removal can
+ // happen on different threads.
+ virtual bool AddSource(Source* audio_source) = 0;
+ virtual bool RemoveSource(Source* audio_source) = 0;
// Performs mixing by asking registered audio sources for audio. The
// mixed result is placed in the provided AudioFrame. Will only be
diff --git a/webrtc/modules/audio_mixer/audio_mixer_impl.cc b/webrtc/modules/audio_mixer/audio_mixer_impl.cc
index e570e22..57fae8c 100644
--- a/webrtc/modules/audio_mixer/audio_mixer_impl.cc
+++ b/webrtc/modules/audio_mixer/audio_mixer_impl.cc
@@ -137,13 +137,8 @@
} // namespace
-std::unique_ptr<AudioMixer> AudioMixer::Create() {
- return AudioMixerImpl::Create();
-}
-
AudioMixerImpl::AudioMixerImpl(std::unique_ptr<AudioProcessing> limiter)
: audio_source_list_(),
- num_mixed_audio_sources_(0),
use_limiter_(true),
time_stamp_(0),
limiter_(std::move(limiter)) {
@@ -153,7 +148,7 @@
AudioMixerImpl::~AudioMixerImpl() {}
-std::unique_ptr<AudioMixerImpl> AudioMixerImpl::Create() {
+rtc::scoped_refptr<AudioMixerImpl> AudioMixerImpl::Create() {
Config config;
config.Set<ExperimentalAgc>(new ExperimentalAgc(false));
std::unique_ptr<AudioProcessing> limiter(AudioProcessing::Create(config));
@@ -186,8 +181,8 @@
return nullptr;
}
- return std::unique_ptr<AudioMixerImpl>(
- new AudioMixerImpl(std::move(limiter)));
+ return rtc::scoped_refptr<AudioMixerImpl>(
+ new rtc::RefCountedObject<AudioMixerImpl>(std::move(limiter)));
}
void AudioMixerImpl::Mix(int sample_rate,
@@ -201,11 +196,9 @@
}
AudioFrameList mix_list;
- size_t num_mixed_audio_sources;
{
rtc::CritScope lock(&crit_);
- mix_list = GetNonAnonymousAudio();
- num_mixed_audio_sources = num_mixed_audio_sources_;
+ mix_list = GetAudioFromSources();
}
for (const auto& frame : mix_list) {
@@ -218,7 +211,7 @@
time_stamp_ += static_cast<uint32_t>(sample_size_);
- use_limiter_ = num_mixed_audio_sources > 1;
+ use_limiter_ = mix_list.size() > 1;
// We only use the limiter if we're actually mixing multiple streams.
MixFromList(audio_frame_for_mixing, mix_list, use_limiter_);
@@ -246,39 +239,26 @@
return output_frequency_;
}
-int32_t AudioMixerImpl::SetMixabilityStatus(Source* audio_source,
- bool mixable) {
- {
- rtc::CritScope lock(&crit_);
- const bool is_mixed = FindSourceInList(audio_source, &audio_source_list_) !=
- audio_source_list_.end();
- // API must be called with a new state.
- if (!(mixable ^ is_mixed)) {
- return -1;
- }
- bool success = false;
- if (mixable) {
- success = AddAudioSourceToList(audio_source, &audio_source_list_);
- } else {
- success = RemoveAudioSourceFromList(audio_source, &audio_source_list_);
- }
- if (!success) {
- RTC_NOTREACHED();
- return -1;
- }
-
- size_t num_mixed_non_anonymous = audio_source_list_.size();
- if (num_mixed_non_anonymous > kMaximumAmountOfMixedAudioSources) {
- num_mixed_non_anonymous = kMaximumAmountOfMixedAudioSources;
- }
- num_mixed_audio_sources_ = num_mixed_non_anonymous;
- }
- return 0;
+bool AudioMixerImpl::AddSource(Source* audio_source) {
+ RTC_DCHECK(audio_source);
+ rtc::CritScope lock(&crit_);
+ RTC_DCHECK(FindSourceInList(audio_source, &audio_source_list_) ==
+ audio_source_list_.end())
+ << "Source already added to mixer";
+ audio_source_list_.emplace_back(audio_source, false, 0);
+ return true;
}
+bool AudioMixerImpl::RemoveSource(Source* audio_source) {
+ RTC_DCHECK(audio_source);
+ rtc::CritScope lock(&crit_);
+ const auto iter = FindSourceInList(audio_source, &audio_source_list_);
+ RTC_DCHECK(iter != audio_source_list_.end()) << "Source not present in mixer";
+ audio_source_list_.erase(iter);
+ return true;
+}
-
-AudioFrameList AudioMixerImpl::GetNonAnonymousAudio() {
+AudioFrameList AudioMixerImpl::GetAudioFromSources() {
RTC_DCHECK_RUN_ON(&thread_checker_);
AudioFrameList result;
std::vector<SourceFrame> audio_source_mixing_data_list;
@@ -330,24 +310,6 @@
return result;
}
-bool AudioMixerImpl::AddAudioSourceToList(
- Source* audio_source,
- SourceStatusList* audio_source_list) const {
- audio_source_list->emplace_back(audio_source, false, 0);
- return true;
-}
-
-bool AudioMixerImpl::RemoveAudioSourceFromList(
- Source* audio_source,
- SourceStatusList* audio_source_list) const {
- const auto iter = FindSourceInList(audio_source, audio_source_list);
- if (iter != audio_source_list->end()) {
- audio_source_list->erase(iter);
- return true;
- } else {
- return false;
- }
-}
bool AudioMixerImpl::LimitMixedAudio(AudioFrame* mixed_audio) const {
RTC_DCHECK_RUN_ON(&thread_checker_);
diff --git a/webrtc/modules/audio_mixer/audio_mixer_impl.h b/webrtc/modules/audio_mixer/audio_mixer_impl.h
index 9048775..87541ee 100644
--- a/webrtc/modules/audio_mixer/audio_mixer_impl.h
+++ b/webrtc/modules/audio_mixer/audio_mixer_impl.h
@@ -14,6 +14,7 @@
#include <memory>
#include <vector>
+#include "webrtc/base/scoped_ref_ptr.h"
#include "webrtc/base/thread_annotations.h"
#include "webrtc/base/thread_checker.h"
#include "webrtc/modules/audio_mixer/audio_mixer.h"
@@ -41,15 +42,18 @@
// AudioProcessing only accepts 10 ms frames.
static const int kFrameDurationInMs = 10;
+ static const int kMaximumAmountOfMixedAudioSources = 3;
static const int kDefaultFrequency = 48000;
- static std::unique_ptr<AudioMixerImpl> Create();
+ static rtc::scoped_refptr<AudioMixerImpl> Create();
~AudioMixerImpl() override;
// AudioMixer functions
- int32_t SetMixabilityStatus(Source* audio_source, bool mixable) override;
- void Mix(int sample_rate,
+ bool AddSource(Source* audio_source) override;
+ bool RemoveSource(Source* audio_source) override;
+
+ void Mix(int sample_rate_hz,
size_t number_of_channels,
AudioFrame* audio_frame_for_mixing) override;
@@ -58,9 +62,10 @@
// mixer.
bool GetAudioSourceMixabilityStatusForTest(Source* audio_source) const;
- private:
+ protected:
explicit AudioMixerImpl(std::unique_ptr<AudioProcessing> limiter);
+ private:
// Set/get mix frequency
void SetOutputFrequency(int frequency);
int OutputFrequency() const;
@@ -68,8 +73,7 @@
// Compute what audio sources to mix from audio_source_list_. Ramp
// in and out. Update mixed status. Mixes up to
// kMaximumAmountOfMixedAudioSources audio sources.
- AudioFrameList GetNonAnonymousAudio() EXCLUSIVE_LOCKS_REQUIRED(crit_);
-
+ AudioFrameList GetAudioFromSources() EXCLUSIVE_LOCKS_REQUIRED(crit_);
// Add/remove the MixerAudioSource to the specified
// MixerAudioSource list.
@@ -90,7 +94,6 @@
// List of all audio sources. Note all lists are disjunct
SourceStatusList audio_source_list_ GUARDED_BY(crit_); // May be mixed.
- size_t num_mixed_audio_sources_ GUARDED_BY(crit_);
// Determines if we will use a limiter for clipping protection during
// mixing.
bool use_limiter_ ACCESS_ON(&thread_checker_);
diff --git a/webrtc/modules/audio_mixer/test/audio_mixer_unittest.cc b/webrtc/modules/audio_mixer/audio_mixer_impl_unittest.cc
similarity index 86%
rename from webrtc/modules/audio_mixer/test/audio_mixer_unittest.cc
rename to webrtc/modules/audio_mixer/audio_mixer_impl_unittest.cc
index 2226fe9..1e5f1fe 100644
--- a/webrtc/modules/audio_mixer/test/audio_mixer_unittest.cc
+++ b/webrtc/modules/audio_mixer/audio_mixer_impl_unittest.cc
@@ -10,6 +10,7 @@
#include <string.h>
+#include <limits>
#include <memory>
#include <utility>
@@ -59,6 +60,8 @@
MOCK_METHOD1(GetAudioFrameWithInfo, AudioFrameWithInfo(int sample_rate_hz));
+ MOCK_METHOD0(ssrc, int());
+
AudioFrame* fake_frame() { return &fake_frame_; }
AudioFrameInfo fake_info() { return fake_audio_frame_info_; }
void set_fake_info(const AudioFrameInfo audio_frame_info) {
@@ -87,7 +90,7 @@
RTC_DCHECK(frames.size() == frame_info.size());
RTC_DCHECK(frame_info.size() == expected_status.size());
- const std::unique_ptr<AudioMixerImpl> mixer(AudioMixerImpl::Create());
+ const auto mixer = AudioMixerImpl::Create();
std::vector<MockMixerAudioSource> participants(num_audio_sources);
for (int i = 0; i < num_audio_sources; i++) {
@@ -96,7 +99,7 @@
}
for (int i = 0; i < num_audio_sources; i++) {
- EXPECT_EQ(0, mixer->SetMixabilityStatus(&participants[i], true));
+ EXPECT_TRUE(mixer->AddSource(&participants[i]));
EXPECT_CALL(participants[i], GetAudioFrameWithInfo(kDefaultSampleRateHz))
.Times(Exactly(1));
}
@@ -112,9 +115,9 @@
TEST(AudioMixer, LargestEnergyVadActiveMixed) {
constexpr int kAudioSources =
- AudioMixer::kMaximumAmountOfMixedAudioSources + 3;
+ AudioMixerImpl::kMaximumAmountOfMixedAudioSources + 3;
- const std::unique_ptr<AudioMixerImpl> mixer(AudioMixerImpl::Create());
+ const auto mixer = AudioMixerImpl::Create();
MockMixerAudioSource participants[kAudioSources];
@@ -125,7 +128,7 @@
// modified by a ramped-in window.
participants[i].fake_frame()->data_[80] = i;
- EXPECT_EQ(0, mixer->SetMixabilityStatus(&participants[i], true));
+ EXPECT_TRUE(mixer->AddSource(&participants[i]));
EXPECT_CALL(participants[i], GetAudioFrameWithInfo(_)).Times(Exactly(1));
}
@@ -143,7 +146,8 @@
bool is_mixed =
mixer->GetAudioSourceMixabilityStatusForTest(&participants[i]);
if (i == kAudioSources - 1 ||
- i < kAudioSources - 1 - AudioMixer::kMaximumAmountOfMixedAudioSources) {
+ i < kAudioSources - 1 -
+ AudioMixerImpl::kMaximumAmountOfMixedAudioSources) {
EXPECT_FALSE(is_mixed) << "Mixing status of AudioSource #" << i
<< " wrong.";
} else {
@@ -154,7 +158,7 @@
}
TEST(AudioMixer, FrameNotModifiedForSingleParticipant) {
- const std::unique_ptr<AudioMixer> mixer(AudioMixer::Create());
+ const auto mixer = AudioMixerImpl::Create();
MockMixerAudioSource participant;
@@ -166,7 +170,7 @@
participant.fake_frame()->data_[j] = j;
}
- EXPECT_EQ(0, mixer->SetMixabilityStatus(&participant, true));
+ EXPECT_TRUE(mixer->AddSource(&participant));
EXPECT_CALL(participant, GetAudioFrameWithInfo(_)).Times(Exactly(2));
AudioFrame audio_frame;
@@ -182,12 +186,12 @@
}
TEST(AudioMixer, ParticipantSampleRate) {
- const std::unique_ptr<AudioMixer> mixer(AudioMixer::Create());
+ const auto mixer = AudioMixerImpl::Create();
MockMixerAudioSource participant;
ResetFrame(participant.fake_frame());
- EXPECT_EQ(0, mixer->SetMixabilityStatus(&participant, true));
+ EXPECT_TRUE(mixer->AddSource(&participant));
for (auto frequency : {8000, 16000, 32000, 48000}) {
EXPECT_CALL(participant, GetAudioFrameWithInfo(frequency))
.Times(Exactly(1));
@@ -199,12 +203,12 @@
}
TEST(AudioMixer, ParticipantNumberOfChannels) {
- const std::unique_ptr<AudioMixer> mixer(AudioMixer::Create());
+ const auto mixer = AudioMixerImpl::Create();
MockMixerAudioSource participant;
ResetFrame(participant.fake_frame());
- EXPECT_EQ(0, mixer->SetMixabilityStatus(&participant, true));
+ EXPECT_TRUE(mixer->AddSource(&participant));
for (size_t number_of_channels : {1, 2}) {
EXPECT_CALL(participant, GetAudioFrameWithInfo(kDefaultSampleRateHz))
.Times(Exactly(1));
@@ -217,9 +221,9 @@
// another participant with higher energy is added.
TEST(AudioMixer, RampedOutSourcesShouldNotBeMarkedMixed) {
constexpr int kAudioSources =
- AudioMixer::kMaximumAmountOfMixedAudioSources + 1;
+ AudioMixerImpl::kMaximumAmountOfMixedAudioSources + 1;
- const std::unique_ptr<AudioMixerImpl> mixer(AudioMixerImpl::Create());
+ const auto mixer = AudioMixerImpl::Create();
MockMixerAudioSource participants[kAudioSources];
for (int i = 0; i < kAudioSources; i++) {
@@ -231,7 +235,7 @@
// Add all participants but the loudest for mixing.
for (int i = 0; i < kAudioSources - 1; i++) {
- EXPECT_EQ(0, mixer->SetMixabilityStatus(&participants[i], true));
+ EXPECT_TRUE(mixer->AddSource(&participants[i]));
EXPECT_CALL(participants[i], GetAudioFrameWithInfo(kDefaultSampleRateHz))
.Times(Exactly(1));
}
@@ -246,8 +250,7 @@
}
// Add new participant with higher energy.
- EXPECT_EQ(0,
- mixer->SetMixabilityStatus(&participants[kAudioSources - 1], true));
+ EXPECT_TRUE(mixer->AddSource(&participants[kAudioSources - 1]));
for (int i = 0; i < kAudioSources; i++) {
EXPECT_CALL(participants[i], GetAudioFrameWithInfo(kDefaultSampleRateHz))
.Times(Exactly(1));
@@ -273,17 +276,16 @@
std::unique_ptr<rtc::Thread> init_thread = rtc::Thread::Create();
std::unique_ptr<rtc::Thread> participant_thread = rtc::Thread::Create();
init_thread->Start();
- std::unique_ptr<AudioMixer> mixer(
- init_thread->Invoke<std::unique_ptr<AudioMixer>>(
- RTC_FROM_HERE, &AudioMixer::Create));
+ const auto mixer = init_thread->Invoke<rtc::scoped_refptr<AudioMixer>>(
+ RTC_FROM_HERE, &AudioMixerImpl::Create);
MockMixerAudioSource participant;
ResetFrame(participant.fake_frame());
participant_thread->Start();
- EXPECT_EQ(0, participant_thread->Invoke<int>(
- RTC_FROM_HERE, rtc::Bind(&AudioMixer::SetMixabilityStatus,
- mixer.get(), &participant, true)));
+ EXPECT_TRUE(participant_thread->Invoke<int>(
+ RTC_FROM_HERE,
+ rtc::Bind(&AudioMixer::AddSource, mixer.get(), &participant)));
EXPECT_CALL(participant, GetAudioFrameWithInfo(kDefaultSampleRateHz))
.Times(Exactly(1));
@@ -294,7 +296,7 @@
TEST(AudioMixer, MutedShouldMixAfterUnmuted) {
constexpr int kAudioSources =
- AudioMixer::kMaximumAmountOfMixedAudioSources + 1;
+ AudioMixerImpl::kMaximumAmountOfMixedAudioSources + 1;
std::vector<AudioFrame> frames(kAudioSources);
for (auto& frame : frames) {
@@ -312,7 +314,7 @@
TEST(AudioMixer, PassiveShouldMixAfterNormal) {
constexpr int kAudioSources =
- AudioMixer::kMaximumAmountOfMixedAudioSources + 1;
+ AudioMixerImpl::kMaximumAmountOfMixedAudioSources + 1;
std::vector<AudioFrame> frames(kAudioSources);
for (auto& frame : frames) {
@@ -330,7 +332,7 @@
TEST(AudioMixer, ActiveShouldMixBeforeLoud) {
constexpr int kAudioSources =
- AudioMixer::kMaximumAmountOfMixedAudioSources + 1;
+ AudioMixerImpl::kMaximumAmountOfMixedAudioSources + 1;
std::vector<AudioFrame> frames(kAudioSources);
for (auto& frame : frames) {
@@ -350,7 +352,7 @@
TEST(AudioMixer, UnmutedShouldMixBeforeLoud) {
constexpr int kAudioSources =
- AudioMixer::kMaximumAmountOfMixedAudioSources + 1;
+ AudioMixerImpl::kMaximumAmountOfMixedAudioSources + 1;
std::vector<AudioFrame> frames(kAudioSources);
for (auto& frame : frames) {