Simplifications of the mixing algorithm.

webrtc/modules/audio_mixer/audio_mixer_impl.cc

 /*
  *  Copyright (c) 2012 The WebRTC project authors. All Rights Reserved.
  *
  *  Use of this source code is governed by a BSD-style license
  *  that can be found in the LICENSE file in the root of the source
  *  tree. An additional intellectual property rights grant can be found
  *  in the file PATENTS.  All contributing project authors may
  *  be found in the AUTHORS file in the root of the source tree.
  */
 
@@ skipping 18 matching lines @@
   SourceFrame(MixerAudioSource* p, AudioFrame* a, bool m, bool was_mixed_before)
       : audio_source_(p),
         audio_frame_(a),
         muted_(m),
         was_mixed_before_(was_mixed_before) {
     if (!muted_) {
       energy_ = NewMixerCalculateEnergy(*a);
     }
   }
 
+  SourceFrame(MixerAudioSource* p,
+              AudioFrame* a,
+              bool m,
+              bool was_mixed_before,
+              uint32_t energy)
+      : audio_source_(p),
+        audio_frame_(a),
+        muted_(m),
+        energy_(energy),
+        was_mixed_before_(was_mixed_before) {}
+

[aleloi, 2016/08/31 11:34:29]

Differs from the above constructor by not calculating energy. A structure with
source, frame and previous mixed status was needed for |Ramp()|. I reused
|SourceFrame|.

   // a.shouldMixBefore(b) is used to select mixer participants.
   bool shouldMixBefore(const SourceFrame& other) const {
     if (muted_ != other.muted_) {
       return other.muted_;
     }
 
     auto our_activity = audio_frame_->vad_activity_;
     auto other_activity = other.audio_frame_->vad_activity_;
 
     if (our_activity != other_activity) {
@@ skipping 13 matching lines @@
 // Remixes a frame between stereo and mono.
 void RemixFrame(AudioFrame* frame, size_t number_of_channels) {
   RTC_DCHECK(number_of_channels == 1 || number_of_channels == 2);
   if (frame->num_channels_ == 1 && number_of_channels == 2) {
     AudioFrameOperations::MonoToStereo(frame);
   } else if (frame->num_channels_ == 2 && number_of_channels == 1) {
     AudioFrameOperations::StereoToMono(frame);
   }
 }
 
-// Mix |frame| into |mixed_frame|, with saturation protection and upmixing.
-// These effects are applied to |frame| itself prior to mixing. Assumes that
-// |mixed_frame| always has at least as many channels as |frame|. Supports
-// stereo at most.
-//
-void MixFrames(AudioFrame* mixed_frame, AudioFrame* frame, bool use_limiter) {
-  RTC_DCHECK_GE(mixed_frame->num_channels_, frame->num_channels_);
-  if (use_limiter) {
-    // Divide by two to avoid saturation in the mixing.
-    // This is only meaningful if the limiter will be used.
-    *frame >>= 1;

[aleloi, 2016/08/31 11:34:29]

MixFrames is now done in MixFromList.

+void Ramp(const std::vector<SourceFrame>& mixed_sources_and_frames) {
+  for (const auto& source_frame : mixed_sources_and_frames) {
+    // Ramp in previously unmixed.
+    if (!source_frame.was_mixed_before_) {
+      NewMixerRampIn(source_frame.audio_frame_);
+    }
+
+    const bool is_mixed = source_frame.audio_source_->_mixHistory->IsMixed();
+    // Ramp out currently unmixed.
+    if (source_frame.was_mixed_before_ && !is_mixed) {
+      NewMixerRampOut(source_frame.audio_frame_);
+    }

[aleloi, 2016/08/31 11:34:29]

Ramping in/out should be done both for anonymous and ordinary
participants. I broke it out to a function to avoid code duplication.

   }
-  RTC_DCHECK_EQ(frame->num_channels_, mixed_frame->num_channels_);
-  *mixed_frame += *frame;
 }
 
 }  // namespace
 
 MixerAudioSource::MixerAudioSource() : _mixHistory(new NewMixHistory()) {}
 
 MixerAudioSource::~MixerAudioSource() {
   delete _mixHistory;
 }
 
@@ skipping 96 matching lines @@
     WEBRTC_TRACE(kTraceError, kTraceAudioMixerServer, id_,
                  "Invalid frequency: %d", sample_rate);
     RTC_NOTREACHED();
     return;
   }
 
   if (OutputFrequency() != sample_rate) {
     SetOutputFrequency(static_cast<Frequency>(sample_rate));
   }
 
-  AudioFrameList mixList;
-  AudioFrameList additionalFramesList;
+  AudioFrameList mix_list;
+  AudioFrameList anonymous_mix_list;
   {
     CriticalSectionScoped cs(crit_.get());
-    mixList = UpdateToMix(kMaximumAmountOfMixedAudioSources);
-    GetAdditionalAudio(&additionalFramesList);
+    mix_list = GetNonAnonymousAudio();
+    anonymous_mix_list = GetAnonymousAudio();
   }
 
-  for (FrameAndMuteInfo& frame_and_mute : mixList) {
-    RemixFrame(frame_and_mute.frame, number_of_channels);
-  }
-  for (FrameAndMuteInfo& frame_and_mute : additionalFramesList) {
-    RemixFrame(frame_and_mute.frame, number_of_channels);
+  mix_list.insert(mix_list.begin(), anonymous_mix_list.begin(),
+                  anonymous_mix_list.end());
+
+  for (const auto& frame : mix_list) {
+    RemixFrame(frame, number_of_channels);
   }
 
   audio_frame_for_mixing->UpdateFrame(
       -1, time_stamp_, NULL, 0, output_frequency_, AudioFrame::kNormalSpeech,
       AudioFrame::kVadPassive, number_of_channels);
 
   time_stamp_ += static_cast<uint32_t>(sample_size_);
 
   use_limiter_ = num_mixed_audio_sources_ > 1;
 
-  // We only use the limiter if it supports the output sample rate and
-  // we're actually mixing multiple streams.
-  MixFromList(audio_frame_for_mixing, mixList, id_, use_limiter_);
-  MixAnonomouslyFromList(audio_frame_for_mixing, additionalFramesList);
+  // We only use the limiter if we're actually mixing multiple streams.
+  MixFromList(audio_frame_for_mixing, mix_list, id_, use_limiter_);
+
   if (audio_frame_for_mixing->samples_per_channel_ == 0) {
     // Nothing was mixed, set the audio samples to silence.
     audio_frame_for_mixing->samples_per_channel_ = sample_size_;
     audio_frame_for_mixing->Mute();
   } else {
     // Only call the limiter if we have something to mix.
     LimitMixedAudio(audio_frame_for_mixing);
   }
 
   // Pass the final result to the level indicator.
@@ skipping 95 matching lines @@
              ? 0
              : -1;
 }
 
 bool AudioMixerImpl::AnonymousMixabilityStatus(
     const MixerAudioSource& audio_source) const {
   CriticalSectionScoped cs(crit_.get());
   return IsAudioSourceInList(audio_source, additional_audio_source_list_);
 }
 
-AudioFrameList AudioMixerImpl::UpdateToMix(size_t maxAudioFrameCounter) const {
+AudioFrameList AudioMixerImpl::GetNonAnonymousAudio() const {
+  WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, id_,
+               "GetNonAnonymousAudio()");
   AudioFrameList result;
   std::vector<SourceFrame> audioSourceMixingDataList;
+  std::vector<SourceFrame> ramp_list;

[aleloi, 2016/08/31 11:34:29]

See comment about 2:nd SourceFrame constructor.

 
   // Get audio source audio and put it in the struct vector.
   for (MixerAudioSource* audio_source : audio_source_list_) {
     auto audio_frame_with_info = audio_source->GetAudioFrameWithMuted(
         id_, static_cast<int>(output_frequency_));
 
     auto audio_frame_info = audio_frame_with_info.audio_frame_info;
     AudioFrame* audio_source_audio_frame = audio_frame_with_info.audio_frame;
 
     if (audio_frame_info == MixerAudioSource::AudioFrameInfo::kError) {
       WEBRTC_TRACE(kTraceWarning, kTraceAudioMixerServer, id_,
                    "failed to GetAudioFrameWithMuted() from participant");
       continue;
     }
     audioSourceMixingDataList.emplace_back(
         audio_source, audio_source_audio_frame,
         audio_frame_info == MixerAudioSource::AudioFrameInfo::kMuted,
         audio_source->_mixHistory->WasMixed());
   }
 
   // Sort frames by sorting function.
   std::sort(audioSourceMixingDataList.begin(), audioSourceMixingDataList.end(),
             std::mem_fn(&SourceFrame::shouldMixBefore));
 
+  int maxAudioFrameCounter = kMaximumAmountOfMixedAudioSources;
   // Go through list in order and put things in mixList.
   for (SourceFrame& p : audioSourceMixingDataList) {
     // Filter muted.
     if (p.muted_) {
       p.audio_source_->_mixHistory->SetIsMixed(false);
       continue;
     }
 
     // Add frame to result vector for mixing.
     bool is_mixed = false;
     if (maxAudioFrameCounter > 0) {
       --maxAudioFrameCounter;
-      if (!p.was_mixed_before_) {
-        NewMixerRampIn(p.audio_frame_);
-      }
-      result.emplace_back(p.audio_frame_, false);
+      result.push_back(p.audio_frame_);
+      ramp_list.emplace_back(p.audio_source_, p.audio_frame_, false,
+                             p.was_mixed_before_, -1);
       is_mixed = true;
     }
-
-    // Ramp out unmuted.
-    if (p.was_mixed_before_ && !is_mixed) {
-      NewMixerRampOut(p.audio_frame_);
-      result.emplace_back(p.audio_frame_, false);
-    }
-
     p.audio_source_->_mixHistory->SetIsMixed(is_mixed);
   }
+  Ramp(ramp_list);
   return result;
 }
 
-void AudioMixerImpl::GetAdditionalAudio(
-    AudioFrameList* additionalFramesList) const {
+AudioFrameList AudioMixerImpl::GetAnonymousAudio() const {
   WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, id_,
-               "GetAdditionalAudio(additionalFramesList)");
+               "GetAnonymousAudio()");
   // The GetAudioFrameWithMuted() callback may result in the audio source being
   // removed from additionalAudioFramesList_. If that happens it will
   // invalidate any iterators. Create a copy of the audio sources list such
   // that the list of participants can be traversed safely.
+  std::vector<SourceFrame> ramp_list;
   MixerAudioSourceList additionalAudioSourceList;
+  AudioFrameList result;
   additionalAudioSourceList.insert(additionalAudioSourceList.begin(),
                                    additional_audio_source_list_.begin(),
                                    additional_audio_source_list_.end());
 
   for (MixerAudioSourceList::const_iterator audio_source =
            additionalAudioSourceList.begin();
        audio_source != additionalAudioSourceList.end(); ++audio_source) {
     auto audio_frame_with_info =
         (*audio_source)->GetAudioFrameWithMuted(id_, output_frequency_);
     auto ret = audio_frame_with_info.audio_frame_info;
     AudioFrame* audio_frame = audio_frame_with_info.audio_frame;
     if (ret == MixerAudioSource::AudioFrameInfo::kError) {
       WEBRTC_TRACE(kTraceWarning, kTraceAudioMixerServer, id_,
                    "failed to GetAudioFrameWithMuted() from audio_source");
       continue;
     }
-    if (audio_frame->samples_per_channel_ == 0) {
-      // Empty frame. Don't use it.
-      continue;

[aleloi, 2016/08/31 11:34:29]

Replaced this with a RTC_DCHECK_EQ in
MixFromList. Frames should never be empty.

+    if (ret != MixerAudioSource::AudioFrameInfo::kMuted) {
+      result.push_back(audio_frame);
+      ramp_list.emplace_back((*audio_source), audio_frame, false,
+                             (*audio_source)->_mixHistory->IsMixed(), -1);
+      (*audio_source)->_mixHistory->SetIsMixed(true);
     }
-    additionalFramesList->push_back(FrameAndMuteInfo(
-        audio_frame, ret == MixerAudioSource::AudioFrameInfo::kMuted));
   }
+  Ramp(ramp_list);
+  return result;
 }
 
 bool AudioMixerImpl::IsAudioSourceInList(
     const MixerAudioSource& audio_source,
     const MixerAudioSourceList& audioSourceList) const {
   WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, id_,
                "IsAudioSourceInList(audio_source,audioSourceList)");
   return std::find(audioSourceList.begin(), audioSourceList.end(),
                    &audio_source) != audioSourceList.end();
 }
@@ skipping 28 matching lines @@
 
 int32_t AudioMixerImpl::MixFromList(AudioFrame* mixedAudio,
                                     const AudioFrameList& audioFrameList,
                                     int32_t id,
                                     bool use_limiter) {
   WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, id,
                "MixFromList(mixedAudio, audioFrameList)");
   if (audioFrameList.empty())
     return 0;
 
   uint32_t position = 0;

[ivoc, 2016/08/31 15:01:06]

It looks like this is not used anywhere, if so, please remove it.

[aleloi, 2016/08/31 15:25:34]

Removed in dependent CL, https://codereview.webrtc.org/2294263002/

I tried to keep this minimal and only concern the mixing structure changes.

 
   if (audioFrameList.size() == 1) {
-    mixedAudio->timestamp_ = audioFrameList.front().frame->timestamp_;
-    mixedAudio->elapsed_time_ms_ =
-        audioFrameList.front().frame->elapsed_time_ms_;
+    mixedAudio->timestamp_ = audioFrameList.front()->timestamp_;
+    mixedAudio->elapsed_time_ms_ = audioFrameList.front()->elapsed_time_ms_;
   } else {
     // TODO(wu): Issue 3390.
     // Audio frame timestamp is only supported in one channel case.
     mixedAudio->timestamp_ = 0;
     mixedAudio->elapsed_time_ms_ = -1;
   }
 
-  for (AudioFrameList::const_iterator iter = audioFrameList.begin();
-       iter != audioFrameList.end(); ++iter) {
-    if (!iter->muted) {
-      MixFrames(mixedAudio, iter->frame, use_limiter);
+  for (const auto& frame : audioFrameList) {
+    RTC_DCHECK_EQ(mixedAudio->sample_rate_hz_, frame->sample_rate_hz_);
+    RTC_DCHECK_EQ(
+        frame->samples_per_channel_,
+        static_cast<size_t>((mixedAudio->sample_rate_hz_ * kFrameDurationInMs) /
+                            1000));
+
+    // Mix |f.frame| into |mixedAudio|, with saturation protection.
+    // These effect is applied to |f.frame| itself prior to mixing.
+    if (use_limiter) {
+      // Divide by two to avoid saturation in the mixing.
+      // This is only meaningful if the limiter will be used.
+      *frame >>= 1;

[ivoc, 2016/08/31 15:01:06]

Should we try to come up with a better approach here (something less vulnerable
to overflow), or do we postpone that until we switch to float?

[aleloi, 2016/08/31 15:25:34]

I think that was the decision. I am going to take a look at how the limiter
works in the coming weeks, and then probably get back to this.

     }
-
+    RTC_DCHECK_EQ(frame->num_channels_, mixedAudio->num_channels_);
+    *mixedAudio += *frame;
     position++;
   }
-
-  return 0;
-}
-
-// TODO(andrew): consolidate this function with MixFromList.

[aleloi, 2016/08/31 11:34:29]

Done! It took only five years or so:)

-int32_t AudioMixerImpl::MixAnonomouslyFromList(
-    AudioFrame* mixedAudio,
-    const AudioFrameList& audioFrameList) const {
-  WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, id_,
-               "MixAnonomouslyFromList(mixedAudio, audioFrameList)");
-
-  if (audioFrameList.empty())
-    return 0;
-
-  for (AudioFrameList::const_iterator iter = audioFrameList.begin();
-       iter != audioFrameList.end(); ++iter) {
-    if (!iter->muted) {
-      MixFrames(mixedAudio, iter->frame, use_limiter_);
-    }
-  }
   return 0;
 }
 
 bool AudioMixerImpl::LimitMixedAudio(AudioFrame* mixedAudio) const {
   if (!use_limiter_) {
     return true;
   }
 
   // Smoothly limit the mixed frame.
   const int error = limiter_->ProcessStream(mixedAudio);
@@ skipping 26 matching lines @@
   return level;
 }
 
 int AudioMixerImpl::GetOutputAudioLevelFullRange() {
   const int level = audio_level_.LevelFullRange();
   WEBRTC_TRACE(kTraceStateInfo, kTraceAudioMixerServer, id_,
                "GetAudioOutputLevelFullRange() => level=%d", level);
   return level;
 }
 }  // namespace webrtc