Stylizing AudioConferenceMixer.

webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc

Index: webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc
diff --git a/webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc b/webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc
index 490fe58e9979d1e88dc031f7a77beb35cd55b527..45a45400d68a47a22bacb417f614a0e1e76161e6 100644
--- a/webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc
+++ b/webrtc/modules/audio_conference_mixer/source/audio_conference_mixer_impl.cc
@@ -70,7 +70,7 @@ MixerParticipant::~MixerParticipant() {
     delete _mixHistory;
 }
 
-int32_t MixerParticipant::IsMixed(bool& mixed) const {
+int32_t MixerParticipant::IsMixed(bool* mixed) const {
     return _mixHistory->IsMixed(mixed);
 }
 
@@ -81,12 +81,12 @@ MixHistory::MixHistory()
 MixHistory::~MixHistory() {
 }
 
-int32_t MixHistory::IsMixed(bool& mixed) const {
-    mixed = _isMixed;
+int32_t MixHistory::IsMixed(bool* mixed) const {
+    *mixed = _isMixed;
     return 0;
 }
 
-int32_t MixHistory::WasMixed(bool& wasMixed) const {
+int32_t MixHistory::WasMixed(bool* wasMixed) const {
     // Was mixed is the same as is mixed depending on perspective. This function
     // is for the perspective of AudioConferenceMixerImpl.
     return IsMixed(wasMixed);
@@ -256,7 +256,7 @@ int32_t AudioConferenceMixerImpl::Process() {
         }
 
         UpdateToMix(&mixList, &rampOutList, &mixedParticipantsMap,
-                    remainingParticipantsAllowedToMix);
+                    &remainingParticipantsAllowedToMix);
 
         GetAdditionalAudio(&additionalFramesList);
         UpdateMixedStatus(mixedParticipantsMap);
@@ -294,9 +294,9 @@ int32_t AudioConferenceMixerImpl::Process() {
         use_limiter_ = _numMixedParticipants > 1 &&
                        _outputFrequency <= kAudioProcMaxNativeSampleRateHz;
 
-        MixFromList(*mixedAudio, &mixList);
-        MixAnonomouslyFromList(*mixedAudio, &additionalFramesList);
-        MixAnonomouslyFromList(*mixedAudio, &rampOutList);
+        MixFromList(mixedAudio, &mixList);
+        MixAnonomouslyFromList(mixedAudio, &additionalFramesList);
+        MixAnonomouslyFromList(mixedAudio, &rampOutList);
 
         if(mixedAudio->samples_per_channel_ == 0) {
             // Nothing was mixed, set the audio samples to silence.
@@ -304,7 +304,7 @@ int32_t AudioConferenceMixerImpl::Process() {
             mixedAudio->Mute();
         } else {
             // Only call the limiter if we have something to mix.
-            if(!LimitMixedAudio(*mixedAudio))
+            if(!LimitMixedAudio(mixedAudio))
                 retval = -1;
         }
     }
@@ -334,12 +334,12 @@ int32_t AudioConferenceMixerImpl::Process() {
 }
 
 int32_t AudioConferenceMixerImpl::RegisterMixedStreamCallback(
-    AudioMixerOutputReceiver& mixReceiver) {
+    AudioMixerOutputReceiver* mixReceiver) {
     CriticalSectionScoped cs(_cbCrit.get());
     if(_mixReceiver != NULL) {
         return -1;
     }
-    _mixReceiver = &mixReceiver;
+    _mixReceiver = mixReceiver;
     return 0;
 }
 
@@ -353,7 +353,7 @@ int32_t AudioConferenceMixerImpl::UnRegisterMixedStreamCallback() {
 }
 
 int32_t AudioConferenceMixerImpl::SetOutputFrequency(
-    const Frequency frequency) {
+    const Frequency& frequency) {
     CriticalSectionScoped cs(_crit.get());
 
     _outputFrequency = frequency;
@@ -370,8 +370,7 @@ AudioConferenceMixerImpl::OutputFrequency() const {
 }
 
 int32_t AudioConferenceMixerImpl::SetMixabilityStatus(
-    MixerParticipant& participant,
-    bool mixable) {
+    MixerParticipant* participant, bool mixable) {
     if (!mixable) {
         // Anonymous participants are in a separate list. Make sure that the
         // participant is in the _participantList if it is being mixed.
@@ -381,7 +380,7 @@ int32_t AudioConferenceMixerImpl::SetMixabilityStatus(
     {
         CriticalSectionScoped cs(_cbCrit.get());
         const bool isMixed =
-            IsParticipantInList(participant, &_participantList);
+            IsParticipantInList(*participant, &_participantList);
         // API must be called with a new state.
         if(!(mixable ^ isMixed)) {
             WEBRTC_TRACE(kTraceWarning, kTraceAudioMixerServer, _id,
@@ -419,17 +418,16 @@ int32_t AudioConferenceMixerImpl::SetMixabilityStatus(
 }
 
 int32_t AudioConferenceMixerImpl::MixabilityStatus(
-    MixerParticipant& participant,
-    bool& mixable) {
+    const MixerParticipant& participant, bool* mixable) const {
     CriticalSectionScoped cs(_cbCrit.get());
-    mixable = IsParticipantInList(participant, &_participantList);
+    *mixable = IsParticipantInList(participant, &_participantList);
     return 0;
 }
 
 int32_t AudioConferenceMixerImpl::SetAnonymousMixabilityStatus(
-    MixerParticipant& participant, const bool anonymous) {
+    MixerParticipant* participant, bool anonymous) {
     CriticalSectionScoped cs(_cbCrit.get());
-    if(IsParticipantInList(participant, &_additionalParticipantList)) {
+    if(IsParticipantInList(*participant, &_additionalParticipantList)) {
         if(anonymous) {
             return 0;
         }
@@ -462,10 +460,10 @@ int32_t AudioConferenceMixerImpl::SetAnonymousMixabilityStatus(
 }
 
 int32_t AudioConferenceMixerImpl::AnonymousMixabilityStatus(
-    MixerParticipant& participant, bool& mixable) {
+    const MixerParticipant& participant, bool* mixable) const {
     CriticalSectionScoped cs(_cbCrit.get());
-    mixable = IsParticipantInList(participant,
-                                  &_additionalParticipantList);
+    *mixable = IsParticipantInList(participant,
+                                   &_additionalParticipantList);
     return 0;
 }
 
@@ -493,7 +491,7 @@ int32_t AudioConferenceMixerImpl::SetMinimumMixingFrequency(
 
 // Check all AudioFrames that are to be mixed. The highest sampling frequency
 // found is the lowest that can be used without losing information.
-int32_t AudioConferenceMixerImpl::GetLowestMixingFrequency() {
+int32_t AudioConferenceMixerImpl::GetLowestMixingFrequency() const {
     const int participantListFrequency =
         GetLowestMixingFrequencyFromList(&_participantList);
     const int anonymousListFrequency =
@@ -511,9 +509,9 @@ int32_t AudioConferenceMixerImpl::GetLowestMixingFrequency() {
 }
 
 int32_t AudioConferenceMixerImpl::GetLowestMixingFrequencyFromList(
-    MixerParticipantList* mixList) {
+    const MixerParticipantList* mixList) const {
     int32_t highestFreq = 8000;
-    for (MixerParticipantList::iterator iter = mixList->begin();
+    for (MixerParticipantList::const_iterator iter = mixList->begin();

[Andrew MacDonald, 2015/08/26 15:56:01]

You should be able to use range based for loops in several places in this file:
for (const auto& participant : mixList) ...

Feel free to change in a separate CL or not at all.

[Andrew MacDonald, 2015/08/28 18:33:20]

No change necessary, just wanted to make sure you saw this comment.

          iter != mixList->end();
          ++iter) {
         const int32_t neededFrequency = (*iter)->NeededFrequency(_id);
@@ -528,28 +526,28 @@ void AudioConferenceMixerImpl::UpdateToMix(
     AudioFrameList* mixList,
     AudioFrameList* rampOutList,
     std::map<int, MixerParticipant*>* mixParticipantList,
-    size_t& maxAudioFrameCounter) {
+    size_t* maxAudioFrameCounter) const {
     WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, _id,
                  "UpdateToMix(mixList,rampOutList,mixParticipantList,%d)",
-                 maxAudioFrameCounter);
+                 *maxAudioFrameCounter);
     const size_t mixListStartSize = mixList->size();
     AudioFrameList activeList;
     // Struct needed by the passive lists to keep track of which AudioFrame
     // belongs to which MixerParticipant.
     ParticipantFramePairList passiveWasNotMixedList;
     ParticipantFramePairList passiveWasMixedList;
-    for (MixerParticipantList::iterator participant = _participantList.begin();
-         participant != _participantList.end();
+    for (MixerParticipantList::const_iterator participant =
+        _participantList.begin(); participant != _participantList.end();
          ++participant) {
         // Stop keeping track of passive participants if there are already
         // enough participants available (they wont be mixed anyway).
-        bool mustAddToPassiveList = (maxAudioFrameCounter >
+        bool mustAddToPassiveList = (*maxAudioFrameCounter >
                                     (activeList.size() +
                                      passiveWasMixedList.size() +
                                      passiveWasNotMixedList.size()));
 
         bool wasMixed = false;
-        (*participant)->_mixHistory->WasMixed(wasMixed);
+        (*participant)->_mixHistory->WasMixed(&wasMixed);
         AudioFrame* audioFrame = NULL;
         if(_audioFramePool->PopMemory(audioFrame) == -1) {
             WEBRTC_TRACE(kTraceMemory, kTraceAudioMixerServer, _id,
@@ -559,7 +557,7 @@ void AudioConferenceMixerImpl::UpdateToMix(
         }
         audioFrame->sample_rate_hz_ = _outputFrequency;
 
-        if((*participant)->GetAudioFrame(_id,*audioFrame) != 0) {
+        if((*participant)->GetAudioFrame(_id, audioFrame) != 0) {
             WEBRTC_TRACE(kTraceWarning, kTraceAudioMixerServer, _id,
                          "failed to GetAudioFrame() from participant");
             _audioFramePool->PushMemory(audioFrame);
@@ -584,7 +582,7 @@ void AudioConferenceMixerImpl::UpdateToMix(
                 RampIn(*audioFrame);
             }
 
-            if(activeList.size() >= maxAudioFrameCounter) {
+            if(activeList.size() >= *maxAudioFrameCounter) {
                 // There are already more active participants than should be
                 // mixed. Only keep the ones with the highest energy.
                 AudioFrameList::iterator replaceItem;
@@ -606,14 +604,14 @@ void AudioConferenceMixerImpl::UpdateToMix(
                     AudioFrame* replaceFrame = *replaceItem;
 
                     bool replaceWasMixed = false;
-                    std::map<int, MixerParticipant*>::iterator it =
+                    std::map<int, MixerParticipant*>::const_iterator it =
                         mixParticipantList->find(replaceFrame->id_);
 
                     // When a frame is pushed to |activeList| it is also pushed
                     // to mixParticipantList with the frame's id. This means
                     // that the Find call above should never fail.
                     assert(it != mixParticipantList->end());
-                    it->second->_mixHistory->WasMixed(replaceWasMixed);
+                    it->second->_mixHistory->WasMixed(&replaceWasMixed);
 
                     mixParticipantList->erase(replaceFrame->id_);
                     activeList.erase(replaceItem);
@@ -664,10 +662,10 @@ void AudioConferenceMixerImpl::UpdateToMix(
             }
         }
     }
-    assert(activeList.size() <= maxAudioFrameCounter);
+    assert(activeList.size() <= *maxAudioFrameCounter);
     // At this point it is known which participants should be mixed. Transfer
     // this information to this functions output parameters.
-    for (AudioFrameList::iterator iter = activeList.begin();
+    for (AudioFrameList::const_iterator iter = activeList.begin();
          iter != activeList.end();
          ++iter) {
         mixList->push_back(*iter);
@@ -676,10 +674,10 @@ void AudioConferenceMixerImpl::UpdateToMix(
     // Always mix a constant number of AudioFrames. If there aren't enough
     // active participants mix passive ones. Starting with those that was mixed
     // last iteration.
-    for (ParticipantFramePairList::iterator iter = passiveWasMixedList.begin();
-         iter != passiveWasMixedList.end();
+    for (ParticipantFramePairList::const_iterator
+        iter = passiveWasMixedList.begin(); iter != passiveWasMixedList.end();
          ++iter) {
-        if(mixList->size() < maxAudioFrameCounter + mixListStartSize) {
+        if(mixList->size() < *maxAudioFrameCounter + mixListStartSize) {
             mixList->push_back((*iter)->audioFrame);
             (*mixParticipantList)[(*iter)->audioFrame->id_] =
                 (*iter)->participant;
@@ -691,11 +689,11 @@ void AudioConferenceMixerImpl::UpdateToMix(
         delete *iter;
     }
     // And finally the ones that have not been mixed for a while.
-    for (ParticipantFramePairList::iterator iter =
+    for (ParticipantFramePairList::const_iterator iter =
              passiveWasNotMixedList.begin();
          iter != passiveWasNotMixedList.end();
          ++iter) {
-        if(mixList->size() <  maxAudioFrameCounter + mixListStartSize) {
+        if(mixList->size() <  *maxAudioFrameCounter + mixListStartSize) {
           mixList->push_back((*iter)->audioFrame);
             (*mixParticipantList)[(*iter)->audioFrame->id_] =
                 (*iter)->participant;
@@ -706,12 +704,12 @@ void AudioConferenceMixerImpl::UpdateToMix(
         }
         delete *iter;
     }
-    assert(maxAudioFrameCounter + mixListStartSize >= mixList->size());
-    maxAudioFrameCounter += mixListStartSize - mixList->size();
+    assert(*maxAudioFrameCounter + mixListStartSize >= mixList->size());
+    *maxAudioFrameCounter += mixListStartSize - mixList->size();
 }
 
 void AudioConferenceMixerImpl::GetAdditionalAudio(
-    AudioFrameList* additionalFramesList) {
+    AudioFrameList* additionalFramesList) const {
     WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, _id,
                  "GetAdditionalAudio(additionalFramesList)");
     // The GetAudioFrame() callback may result in the participant being removed
@@ -723,7 +721,7 @@ void AudioConferenceMixerImpl::GetAdditionalAudio(
                                      _additionalParticipantList.begin(),
                                      _additionalParticipantList.end());
 
-    for (MixerParticipantList::iterator participant =
+    for (MixerParticipantList::const_iterator participant =
              additionalParticipantList.begin();
          participant != additionalParticipantList.end();
          ++participant) {
@@ -735,7 +733,7 @@ void AudioConferenceMixerImpl::GetAdditionalAudio(
             return;
         }
         audioFrame->sample_rate_hz_ = _outputFrequency;
-        if((*participant)->GetAudioFrame(_id, *audioFrame) != 0) {
+        if((*participant)->GetAudioFrame(_id, audioFrame) != 0) {
             WEBRTC_TRACE(kTraceWarning, kTraceAudioMixerServer, _id,
                          "failed to GetAudioFrame() from participant");
             _audioFramePool->PushMemory(audioFrame);
@@ -751,18 +749,19 @@ void AudioConferenceMixerImpl::GetAdditionalAudio(
 }
 
 void AudioConferenceMixerImpl::UpdateMixedStatus(
-    std::map<int, MixerParticipant*>& mixedParticipantsMap) {
+    const std::map<int, MixerParticipant*>& mixedParticipantsMap) const {
     WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, _id,
                  "UpdateMixedStatus(mixedParticipantsMap)");
     assert(mixedParticipantsMap.size() <= kMaximumAmountOfMixedParticipants);
 
     // Loop through all participants. If they are in the mix map they
     // were mixed.
-    for (MixerParticipantList::iterator participant = _participantList.begin();
-         participant != _participantList.end();
+    for (MixerParticipantList::const_iterator
+        participant =_participantList.begin();
+        participant != _participantList.end();
          ++participant) {
         bool isMixed = false;
-        for (std::map<int, MixerParticipant*>::iterator it =
+        for (std::map<int, MixerParticipant*>::const_iterator it =
                  mixedParticipantsMap.begin();
              it != mixedParticipantsMap.end();
              ++it) {
@@ -776,7 +775,7 @@ void AudioConferenceMixerImpl::UpdateMixedStatus(
 }
 
 void AudioConferenceMixerImpl::ClearAudioFrameList(
-    AudioFrameList* audioFrameList) {
+    AudioFrameList* audioFrameList) const {
     WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, _id,
                  "ClearAudioFrameList(audioFrameList)");
     for (AudioFrameList::iterator iter = audioFrameList->begin();
@@ -788,11 +787,11 @@ void AudioConferenceMixerImpl::ClearAudioFrameList(
 }
 
 void AudioConferenceMixerImpl::UpdateVADPositiveParticipants(
-    AudioFrameList* mixList) {
+    AudioFrameList* mixList) const {
     WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, _id,
                  "UpdateVADPositiveParticipants(mixList)");
 
-    for (AudioFrameList::iterator iter = mixList->begin();
+    for (AudioFrameList::const_iterator iter = mixList->begin();
          iter != mixList->end();
          ++iter) {
         CalculateEnergy(**iter);
@@ -800,8 +799,8 @@ void AudioConferenceMixerImpl::UpdateVADPositiveParticipants(
 }
 
 bool AudioConferenceMixerImpl::IsParticipantInList(
-    MixerParticipant& participant,
-    MixerParticipantList* participantList) const {
+    const MixerParticipant& participant,
+    const MixerParticipantList* participantList) const {
     WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, _id,
                  "IsParticipantInList(participant,participantList)");
     for (MixerParticipantList::const_iterator iter = participantList->begin();
@@ -815,28 +814,28 @@ bool AudioConferenceMixerImpl::IsParticipantInList(
 }
 
 bool AudioConferenceMixerImpl::AddParticipantToList(
-    MixerParticipant& participant,
-    MixerParticipantList* participantList) {
+    MixerParticipant* participant,
+    MixerParticipantList* participantList) const {
     WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, _id,
                  "AddParticipantToList(participant, participantList)");
-    participantList->push_back(&participant);
+    participantList->push_back(participant);
     // Make sure that the mixed status is correct for new MixerParticipant.
-    participant._mixHistory->ResetMixedStatus();
+    participant->_mixHistory->ResetMixedStatus();
     return true;
 }
 
 bool AudioConferenceMixerImpl::RemoveParticipantFromList(
-    MixerParticipant& participant,
-    MixerParticipantList* participantList) {
+    MixerParticipant* participant,
+    MixerParticipantList* participantList) const {
     WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, _id,
                  "RemoveParticipantFromList(participant, participantList)");
     for (MixerParticipantList::iterator iter = participantList->begin();
          iter != participantList->end();
          ++iter) {
-        if(*iter == &participant) {
+        if(*iter == participant) {
             participantList->erase(iter);
             // Participant is no longer mixed, reset to default.
-            participant._mixHistory->ResetMixedStatus();
+            participant->_mixHistory->ResetMixedStatus();
             return true;
         }
     }
@@ -844,8 +843,8 @@ bool AudioConferenceMixerImpl::RemoveParticipantFromList(
 }
 
 int32_t AudioConferenceMixerImpl::MixFromList(
-    AudioFrame& mixedAudio,
-    const AudioFrameList* audioFrameList) {
+    AudioFrame* mixedAudio,
+    const AudioFrameList* audioFrameList) const {
     WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, _id,
                  "MixFromList(mixedAudio, audioFrameList)");
     if(audioFrameList->empty()) return 0;
@@ -853,13 +852,13 @@ int32_t AudioConferenceMixerImpl::MixFromList(
     uint32_t position = 0;
 
     if (_numMixedParticipants == 1) {
-      mixedAudio.timestamp_ = audioFrameList->front()->timestamp_;
-      mixedAudio.elapsed_time_ms_ = audioFrameList->front()->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;
+      mixedAudio->timestamp_ = 0;
+      mixedAudio->elapsed_time_ms_ = -1;
     }
 
     for (AudioFrameList::const_iterator iter = audioFrameList->begin();
@@ -876,7 +875,7 @@ int32_t AudioConferenceMixerImpl::MixFromList(
             assert(false);
             position = 0;
         }
-        MixFrames(&mixedAudio, (*iter), use_limiter_);
+        MixFrames(mixedAudio, (*iter), use_limiter_);
 
         position++;
     }
@@ -886,8 +885,8 @@ int32_t AudioConferenceMixerImpl::MixFromList(
 
 // TODO(andrew): consolidate this function with MixFromList.
 int32_t AudioConferenceMixerImpl::MixAnonomouslyFromList(
-    AudioFrame& mixedAudio,
-    const AudioFrameList* audioFrameList) {
+    AudioFrame* mixedAudio,
+    const AudioFrameList* audioFrameList) const {
     WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, _id,
                  "MixAnonomouslyFromList(mixedAudio, audioFrameList)");
 
@@ -896,18 +895,18 @@ int32_t AudioConferenceMixerImpl::MixAnonomouslyFromList(
     for (AudioFrameList::const_iterator iter = audioFrameList->begin();
          iter != audioFrameList->end();
          ++iter) {
-        MixFrames(&mixedAudio, *iter, use_limiter_);
+        MixFrames(mixedAudio, *iter, use_limiter_);
     }
     return 0;
 }
 
-bool AudioConferenceMixerImpl::LimitMixedAudio(AudioFrame& mixedAudio) {
+bool AudioConferenceMixerImpl::LimitMixedAudio(AudioFrame* mixedAudio) const {

[Andrew MacDonald, 2015/08/26 15:56:01]

Even though you can, I don't think you should mark this const. It's modifying
the object pointed at by _limiter. 

[minyue-webrtc, 2015/08/27 11:31:15]

const means that the class members are unaltered. _limiter as pointer is not
altered. and therefore it function can be const.

I do not think making it a const function brings confusion.

[Andrew MacDonald, 2015/08/28 18:33:20]

Right, I know what const methods are. There's the concept of physical and
logical constness:
https://isocpp.org/wiki/faq/const-correctness#logical-vs-physical-const

Since this changes the visible state of the object I would argue it should not
be const, but I don't insist.

     if (!use_limiter_) {
       return true;
     }
 
     // Smoothly limit the mixed frame.
-    const int error = _limiter->ProcessStream(&mixedAudio);
+    const int error = _limiter->ProcessStream(mixedAudio);
 
     // And now we can safely restore the level. This procedure results in
     // some loss of resolution, deemed acceptable.
@@ -919,7 +918,7 @@ bool AudioConferenceMixerImpl::LimitMixedAudio(AudioFrame& mixedAudio) {
     //
     // Instead we double the frame (with addition since left-shifting a
     // negative value is undefined).
-    mixedAudio += mixedAudio;
+    *mixedAudio += *mixedAudio;
 
     if(error != _limiter->kNoError) {
         WEBRTC_TRACE(kTraceError, kTraceAudioMixerServer, _id,