Less lock acquisitions for AudioMixer.

webrtc/modules/audio_mixer/audio_mixer_impl.h

 /*
  *  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 12 matching lines @@
 #include "webrtc/voice_engine/level_indicator.h"
 
 namespace webrtc {
 class AudioProcessing;
 class CriticalSectionWrapper;
 
 struct FrameAndMuteInfo {
   FrameAndMuteInfo(AudioFrame* f, bool m) : frame(f), muted(m) {}
   AudioFrame* frame;
   bool muted;
 };

[kwiberg-webrtc, 2016/08/30 16:29:45]

Is this struct still used somewhere, or can you remove it?

 
-typedef std::list<FrameAndMuteInfo> AudioFrameList;
+typedef std::list<AudioFrame*> AudioFrameList;

[aleloi, 2016/08/30 15:13:34]

Mixing structure is a little simplified. Mute information is only used
inside of Get*AnonymousAudio. Muted frames are filtered out one step
earlier. Therefore a |muted| field is no longer needed here.

 typedef std::list<MixerAudioSource*> MixerAudioSourceList;
 
 // Cheshire cat implementation of MixerAudioSource's non virtual functions.
 class NewMixHistory {
  public:
   NewMixHistory();
   ~NewMixHistory();
 
   // Returns true if the audio source is being mixed.
   bool IsMixed() const;
@@ skipping 35 matching lines @@
   bool AnonymousMixabilityStatus(
       const MixerAudioSource& audio_source) const override;
 
  private:
   // Set/get mix frequency
   int32_t SetOutputFrequency(const Frequency& frequency);
   Frequency OutputFrequency() const;
 
   // Compute what audio sources to mix from audio_source_list_. Ramp in
   // and out. Update mixed status. maxAudioFrameCounter specifies how
   // many participants are allowed to be mixed.

[kwiberg-webrtc, 2016/08/30 16:29:45]

This comment needs updating.

-  AudioFrameList UpdateToMix(size_t maxAudioFrameCounter) const;
+  AudioFrameList GetNonAnonymousAudio() const;
 
-  // Return the lowest mixing frequency that can be used without having to
-  // downsample any audio.
-  int32_t GetLowestMixingFrequency() const;
-  int32_t GetLowestMixingFrequencyFromList(
-      const MixerAudioSourceList& mixList) const;

[aleloi, 2016/08/30 15:13:34]

These two have not been used for the last 3-4 mixer changes.

-
-  // Return the AudioFrames that should be mixed anonymously.
-  void GetAdditionalAudio(AudioFrameList* additionalFramesList) const;
+  // Return the AudioFrames that should be mixed anonymously. Ramp in
+  // and out. Update mixed status.
+  AudioFrameList GetAnonymousAudio() const;

[aleloi, 2016/08/30 15:13:34]

Made naming and signature more consistent between the Get*Audio
methods.

 
   // This function returns true if it finds the MixerAudioSource in the
   // specified list of MixerAudioSources.
   bool IsAudioSourceInList(const MixerAudioSource& audio_source,
                            const MixerAudioSourceList& audioSourceList) const;
 
   // Add/remove the MixerAudioSource to the specified
   // MixerAudioSource list.
   bool AddAudioSourceToList(MixerAudioSource* audio_source,
                             MixerAudioSourceList* audioSourceList) const;
   bool RemoveAudioSourceFromList(MixerAudioSource* removeAudioSource,
                                  MixerAudioSourceList* audioSourceList) const;
 
   // Mix the AudioFrames stored in audioFrameList into mixedAudio.
   static int32_t MixFromList(AudioFrame* mixedAudio,
                              const AudioFrameList& audioFrameList,
                              int32_t id,
                              bool use_limiter);
 
-  // Mix the AudioFrames stored in audioFrameList into mixedAudio. No
-  // record will be kept of this mix (e.g. the corresponding MixerAudioSources
-  // will not be marked as IsMixed()
-  int32_t MixAnonomouslyFromList(AudioFrame* mixedAudio,
-                                 const AudioFrameList& audioFrameList) const;
-

[aleloi, 2016/08/30 15:13:34]

Merged with MixFromList

   bool LimitMixedAudio(AudioFrame* mixedAudio) const;
 
   // Output level functions for VoEVolumeControl.
   int GetOutputAudioLevel() override;
 
   int GetOutputAudioLevelFullRange() override;
 
   std::unique_ptr<CriticalSectionWrapper> crit_;
-  std::unique_ptr<CriticalSectionWrapper> cb_crit_;

[aleloi, 2016/08/30 15:13:34]

I think only one lock is needed. Adding/removing/querying status of
participants can happen on different threads. In addition the mixing
is done on another thread.

The public adding/removing/querying methods are all guarded by cb_crit_.

Getting audio from participants and updating participants mixing
history is all done guarded by cb_crit_. The other parts of mixing do
not hold locks. AFAIK this is enough, since the mixing is
done on a single thread.

 
   int32_t id_;
 
   // The current sample frequency and sample size when mixing.
   Frequency output_frequency_;
   size_t sample_size_;
 
   // List of all audio sources. Note all lists are disjunct
   MixerAudioSourceList audio_source_list_;  // May be mixed.
 
-  // Always mixed, anonomously.
+  // Always mixed, anonymously.
   MixerAudioSourceList additional_audio_source_list_;
 
   size_t num_mixed_audio_sources_;
   // Determines if we will use a limiter for clipping protection during
   // mixing.
   bool use_limiter_;
 
   uint32_t time_stamp_;
 
   // Ensures that Mix is called from the same thread.
   rtc::ThreadChecker thread_checker_;
 
   // Used for inhibiting saturation in mixing.
   std::unique_ptr<AudioProcessing> limiter_;
 
   // Measures audio level for the combined signal.
   voe::AudioLevel audio_level_;
+
+  RTC_DISALLOW_COPY_AND_ASSIGN(AudioMixerImpl);
 };
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_AUDIO_MIXER_AUDIO_MIXER_IMPL_H_