Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(746)

Unified Diff: webrtc/modules/audio_mixer/audio_mixer_impl.cc

Issue 2408683002: Cleanup of the mixer interface. (Closed)
Patch Set: Added checks in Add/Remove source, double add or nonexistent remove is an error. Created 4 years, 2 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
Index: webrtc/modules/audio_mixer/audio_mixer_impl.cc
diff --git a/webrtc/modules/audio_mixer/audio_mixer_impl.cc b/webrtc/modules/audio_mixer/audio_mixer_impl.cc
index 21f753984ac442ea682173c5fc9cc7b4ad7a290d..9fe54576ff956ec7ed73e6fe28b9109c8dd89f49 100644
--- a/webrtc/modules/audio_mixer/audio_mixer_impl.cc
+++ b/webrtc/modules/audio_mixer/audio_mixer_impl.cc
@@ -137,13 +137,8 @@ AudioMixerImpl::SourceStatusList::iterator FindSourceInList(
} // 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<AudioProcessing> limiter)
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 @@ std::unique_ptr<AudioMixerImpl> AudioMixerImpl::Create() {
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,
@@ -202,11 +197,9 @@ void AudioMixerImpl::Mix(int sample_rate,
}
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) {
@@ -219,7 +212,7 @@ void AudioMixerImpl::Mix(int sample_rate,
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_);
@@ -247,39 +240,26 @@ int AudioMixerImpl::OutputFrequency() const {
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::CritScope lock(&crit_);
+ RTC_DCHECK(audio_source);
the sun 2016/10/10 14:16:27 nit: check arguments before claiming lock
aleloi 2016/10/10 14:50:35 Done.
+ RTC_DCHECK(FindSourceInList(audio_source, &audio_source_list_) ==
the sun 2016/10/10 14:16:27 Use the _EQ, _NE etc versions of the macros as you
aleloi 2016/10/10 14:50:35 RTC_DCHECK_EQ seems to be implemented with templat
the sun 2016/10/10 19:24:48 Ah, of course, I guess it is stringstream::operato
+ 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::CritScope lock(&crit_);
+ RTC_DCHECK(audio_source);
the sun 2016/10/10 14:16:27 same nits as above
aleloi 2016/10/10 14:50:35 Done.
+ 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;
@@ -331,24 +311,6 @@ AudioFrameList AudioMixerImpl::GetNonAnonymousAudio() {
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_);
« no previous file with comments | « webrtc/modules/audio_mixer/audio_mixer_impl.h ('k') | webrtc/modules/audio_mixer/test/audio_mixer_unittest.cc » ('j') | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698