Style changes in Audio Mixer

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.
  */
 
 #include "webrtc/modules/audio_mixer/audio_mixer_impl.h"
 
 #include <algorithm>
 #include <functional>
 
-#include "webrtc/base/thread_annotations.h"
 #include "webrtc/modules/audio_mixer/audio_frame_manipulator.h"
-#include "webrtc/modules/audio_mixer/audio_mixer_defines.h"
-#include "webrtc/modules/audio_processing/include/audio_processing.h"
 #include "webrtc/modules/utility/include/audio_frame_operations.h"
-#include "webrtc/system_wrappers/include/critical_section_wrapper.h"
 #include "webrtc/system_wrappers/include/trace.h"
 
 namespace webrtc {
 namespace {
 
 class SourceFrame {
  public:
   SourceFrame(MixerAudioSource* p, AudioFrame* a, bool m, bool was_mixed_before)
       : audio_source_(p),
         audio_frame_(a),
@@ skipping 32 matching lines @@
   }
 
   MixerAudioSource* audio_source_;
   AudioFrame* audio_frame_;
   bool muted_;
   uint32_t energy_;
   bool was_mixed_before_;
 };
 
 // Remixes a frame between stereo and mono.
-void RemixFrame(AudioFrame* frame, size_t number_of_channels) {
+void RemixFrame(AudioFrame* frame, int 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);
   }
 }
 
 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_->mix_history_->IsMixed();
     // Ramp out currently unmixed.
     if (source_frame.was_mixed_before_ && !is_mixed) {
       NewMixerRampOut(source_frame.audio_frame_);
     }
   }
 }
 
-}  // namespace
+// Mix the AudioFrames stored in audioFrameList into mixed_audio.
+int32_t MixFromList(AudioFrame* mixed_audio,
+                    const AudioFrameList& audio_frame_list,
+                    int32_t id,
+                    bool use_limiter) {
+  WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, id,
+               "MixFromList(mixed_audio, audio_frame_list)");
+  if (audio_frame_list.empty())
+    return 0;
 
-MixerAudioSource::MixerAudioSource() : mix_history_(new NewMixHistory()) {}
+  if (audio_frame_list.size() == 1) {
+    mixed_audio->timestamp_ = audio_frame_list.front()->timestamp_;
+    mixed_audio->elapsed_time_ms_ = audio_frame_list.front()->elapsed_time_ms_;
+  } else {
+    // TODO(wu): Issue 3390.
+    // Audio frame timestamp is only supported in one channel case.
+    mixed_audio->timestamp_ = 0;
+    mixed_audio->elapsed_time_ms_ = -1;
+  }
 
-MixerAudioSource::~MixerAudioSource() {
-  delete mix_history_;
-}
+  for (const auto& frame : audio_frame_list) {
+    RTC_DCHECK_EQ(mixed_audio->sample_rate_hz_, frame->sample_rate_hz_);
+    RTC_DCHECK_EQ(
+        frame->samples_per_channel_,
+        static_cast<size_t>((mixed_audio->sample_rate_hz_ *
+                             webrtc::AudioMixerImpl::kFrameDurationInMs) /
+                            1000));
 
-bool MixerAudioSource::IsMixed() const {
-  return mix_history_->IsMixed();
-}
-
-NewMixHistory::NewMixHistory() : is_mixed_(0) {}
-
-NewMixHistory::~NewMixHistory() {}
-
-bool NewMixHistory::IsMixed() const {
-  return is_mixed_;
-}
-
-bool NewMixHistory::WasMixed() const {
-  // Was mixed is the same as is mixed depending on perspective. This function
-  // is for the perspective of NewAudioConferenceMixerImpl.
-  return IsMixed();
-}
-
-int32_t NewMixHistory::SetIsMixed(const bool mixed) {
-  is_mixed_ = mixed;
+    // Mix |f.frame| into |mixed_audio|, 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;
+    }
+    RTC_DCHECK_EQ(frame->num_channels_, mixed_audio->num_channels_);
+    *mixed_audio += *frame;
+  }
   return 0;
 }
 
-void NewMixHistory::ResetMixedStatus() {
-  is_mixed_ = false;
-}
+}  // namespace
 
 std::unique_ptr<AudioMixer> AudioMixer::Create(int id) {
   AudioMixerImpl* mixer = new AudioMixerImpl(id);

[ossu, 2016/09/01 15:42:50]

This is a bit strange. Not sure it's 100% related to this CL, but you should be
able to do:

std::unique_ptr<AudioMixerImpl> mixer(new AudioMixerImpl(id));
if (!mixer->Init()) {
  return nullptr;
}
return mixer;

and avoid manual memory management through delete.

[aleloi, 2016/09/02 11:52:34]

Changed in upstream CL.

   if (!mixer->Init()) {
     delete mixer;
-    return NULL;
+    return std::unique_ptr<AudioMixer>(nullptr);
   }
   return std::unique_ptr<AudioMixer>(mixer);
 }
 
 AudioMixerImpl::AudioMixerImpl(int id)
     : id_(id),
       output_frequency_(kDefaultFrequency),
       sample_size_(0),
       audio_source_list_(),
       additional_audio_source_list_(),
@@ skipping 36 matching lines @@
   if (limiter_->gain_control()->enable_limiter(true) != limiter_->kNoError)
     return false;
 
   if (limiter_->gain_control()->Enable(true) != limiter_->kNoError)
     return false;
 
   return true;
 }
 
 void AudioMixerImpl::Mix(int sample_rate,
-                         size_t number_of_channels,
+                         int number_of_channels,
                          AudioFrame* audio_frame_for_mixing) {
   RTC_DCHECK(number_of_channels == 1 || number_of_channels == 2);
   RTC_DCHECK_RUN_ON(&thread_checker_);
 
   if (sample_rate != kNbInHz && sample_rate != kWbInHz &&
       sample_rate != kSwbInHz && sample_rate != kFbInHz) {
     WEBRTC_TRACE(kTraceError, kTraceAudioMixerServer, id_,
                  "Invalid frequency: %d", sample_rate);
     RTC_NOTREACHED();
     return;
   }
 
   if (OutputFrequency() != sample_rate) {
     SetOutputFrequency(static_cast<Frequency>(sample_rate));
   }
 
   AudioFrameList mix_list;
   AudioFrameList anonymous_mix_list;
   int num_mixed_audio_sources;
   {
     CriticalSectionScoped cs(crit_.get());
     mix_list = GetNonAnonymousAudio();
     anonymous_mix_list = GetAnonymousAudio();
-    num_mixed_audio_sources = static_cast<int>(num_mixed_audio_sources_);
+    num_mixed_audio_sources = num_mixed_audio_sources_;
   }
 
   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(
@@ skipping 18 matching lines @@
 
   // Pass the final result to the level indicator.
   audio_level_.ComputeLevel(*audio_frame_for_mixing);
 
   return;
 }
 
 int32_t AudioMixerImpl::SetOutputFrequency(const Frequency& frequency) {
   RTC_DCHECK_RUN_ON(&thread_checker_);
   output_frequency_ = frequency;
-  sample_size_ =
-      static_cast<size_t>((output_frequency_ * kFrameDurationInMs) / 1000);
+  sample_size_ = (output_frequency_ * kFrameDurationInMs) / 1000;
 
   return 0;
 }
 
 AudioMixer::Frequency AudioMixerImpl::OutputFrequency() const {
   RTC_DCHECK_RUN_ON(&thread_checker_);
   return output_frequency_;
 }
 
 int32_t AudioMixerImpl::SetMixabilityStatus(MixerAudioSource* audio_source,
@@ skipping 19 matching lines @@
     } else {
       success = RemoveAudioSourceFromList(audio_source, &audio_source_list_);
     }
     if (!success) {
       WEBRTC_TRACE(kTraceError, kTraceAudioMixerServer, id_,
                    "failed to %s audio_source", mixable ? "add" : "remove");
       RTC_NOTREACHED();
       return -1;
     }
 
-    size_t num_mixed_non_anonymous = audio_source_list_.size();
+    int num_mixed_non_anonymous = static_cast<int>(audio_source_list_.size());

[ossu, 2016/09/01 15:42:50]

Why not just keep this as size_t? According to the google style guide, size_t is
fine. Maybe the chromium one overrides that?

[aleloi, 2016/09/02 11:52:34]

Thanks! I didn't read it carefully enough. Changing num_mixed_audio_sources_
back to size_t seems more reasonable. It's a count and we avoid two explicit
casts.

     if (num_mixed_non_anonymous > kMaximumAmountOfMixedAudioSources) {
       num_mixed_non_anonymous = kMaximumAmountOfMixedAudioSources;
     }
     num_mixed_audio_sources_ =
-        num_mixed_non_anonymous + additional_audio_source_list_.size();
+        num_mixed_non_anonymous +
+        static_cast<int>(additional_audio_source_list_.size());
   }
   return 0;
 }
 
 bool AudioMixerImpl::MixabilityStatus(
     const MixerAudioSource& audio_source) const {
   CriticalSectionScoped cs(crit_.get());
   return IsAudioSourceInList(audio_source, audio_source_list_);
 }
 
@@ skipping 161 matching lines @@
   if (iter != audio_source_list->end()) {
     audio_source_list->erase(iter);
     // AudioSource is no longer mixed, reset to default.
     audio_source->mix_history_->ResetMixedStatus();
     return true;
   } else {
     return false;
   }
 }
 
-int32_t AudioMixerImpl::MixFromList(AudioFrame* mixed_audio,
-                                    const AudioFrameList& audio_frame_list,
-                                    int32_t id,
-                                    bool use_limiter) {
-  WEBRTC_TRACE(kTraceStream, kTraceAudioMixerServer, id,
-               "MixFromList(mixed_audio, audio_frame_list)");
-  if (audio_frame_list.empty())
-    return 0;
-
-  if (audio_frame_list.size() == 1) {
-    mixed_audio->timestamp_ = audio_frame_list.front()->timestamp_;
-    mixed_audio->elapsed_time_ms_ = audio_frame_list.front()->elapsed_time_ms_;
-  } else {
-    // TODO(wu): Issue 3390.
-    // Audio frame timestamp is only supported in one channel case.
-    mixed_audio->timestamp_ = 0;
-    mixed_audio->elapsed_time_ms_ = -1;
-  }
-
-  for (const auto& frame : audio_frame_list) {
-    RTC_DCHECK_EQ(mixed_audio->sample_rate_hz_, frame->sample_rate_hz_);
-    RTC_DCHECK_EQ(
-        frame->samples_per_channel_,
-        static_cast<size_t>(
-            (mixed_audio->sample_rate_hz_ * kFrameDurationInMs) / 1000));
-
-    // Mix |f.frame| into |mixed_audio|, 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;
-    }
-    RTC_DCHECK_EQ(frame->num_channels_, mixed_audio->num_channels_);
-    *mixed_audio += *frame;
-  }
-  return 0;
-}
-
 bool AudioMixerImpl::LimitMixedAudio(AudioFrame* mixed_audio) const {
   RTC_DCHECK_RUN_ON(&thread_checker_);
   if (!use_limiter_) {
     return true;
   }
 
   // Smoothly limit the mixed frame.
   const int error = limiter_->ProcessStream(mixed_audio);
 
   // And now we can safely restore the level. This procedure results in
@@ skipping 26 matching lines @@
 }
 
 int AudioMixerImpl::GetOutputAudioLevelFullRange() {
   RTC_DCHECK_RUN_ON(&thread_checker_);
   const int level = audio_level_.LevelFullRange();
   WEBRTC_TRACE(kTraceStateInfo, kTraceAudioMixerServer, id_,
                "GetAudioOutputLevelFullRange() => level=%d", level);
   return level;
 }
 }  // namespace webrtc