Move audio frame memory handling inside AudioMixer.

webrtc/modules/audio_mixer/audio_mixer_impl.h

Index: webrtc/modules/audio_mixer/audio_mixer_impl.h
diff --git a/webrtc/modules/audio_mixer/audio_mixer_impl.h b/webrtc/modules/audio_mixer/audio_mixer_impl.h
index 500bb7862e5defe2b9fd906c057c6cf60fec3087..df14b3b469e31b4049806cd540b72cc87662bb9f 100644
--- a/webrtc/modules/audio_mixer/audio_mixer_impl.h
+++ b/webrtc/modules/audio_mixer/audio_mixer_impl.h
@@ -29,15 +29,22 @@ typedef std::vector<AudioFrame*> AudioFrameList;
 
 class AudioMixerImpl : public AudioMixer {
  public:
-  struct SourceStatus {
-    SourceStatus(Source* audio_source, bool is_mixed, float gain)
-        : audio_source(audio_source), is_mixed(is_mixed), gain(gain) {}
+  struct SourceStatusWithFrame {
+    SourceStatusWithFrame(Source* audio_source, bool is_mixed, float gain)

[kwiberg-webrtc, 2016/10/14 19:59:33]

I don't think you need to change the name. That we have an audio frame here is
an implementation detail.

[aleloi, 2016/10/17 11:06:39]

OK, I've changed it back.

+        : audio_source(audio_source),
+          is_mixed(is_mixed),
+          gain(gain),
+          audio_frame() {}

[kwiberg-webrtc, 2016/10/14 19:59:33]

You don't need to mention audio_frame here; since if you don't, it'll default to
using the default member initializer on line 43.

[aleloi, 2016/10/17 11:06:39]

Thanks! I mis-applied a guideline from Scott Meyers here: 'always initialize
every field during construction' turned into 'always write initialization lists
for every field'.

     Source* audio_source = nullptr;
     bool is_mixed = false;
     float gain = 0.0f;
+
+    // A frame that will be passed to audio_source->GetAudioFrameWithInfo.
+    AudioFrame audio_frame{};
   };
 
-  typedef std::vector<SourceStatus> SourceStatusList;
+  typedef std::vector<std::unique_ptr<SourceStatusWithFrame>>
+      SourceStatusWithFrameList;

[kwiberg-webrtc, 2016/10/14 19:59:33]

Did you start using unique_ptr here because you can't (or don't want to) move or
swap AudioFrames? (I'm asking because I'm hoping that that can be fixed,
allowing us to eliminate a level of indirection.)

[kwiberg-webrtc, 2016/10/14 19:59:33]

Prefer to use "using" instead of "typedef". It has much better syntax.

[aleloi, 2016/10/17 11:06:39]

Done.

[aleloi, 2016/10/17 11:06:39]

Yes, exactly. We could probably add move constructors and move assignments to
AudioFrame, but I am not sure about what implications that would have.

[kwiberg-webrtc, 2016/10/17 12:08:32]

I don't think anything bad would happen. But since it has a big array embedded
in the object rather than on the heap, efficient move will take some
refactoring; it's certainly out of scope for this CL.

 
   // AudioProcessing only accepts 10 ms frames.
   static const int kFrameDurationInMs = 10;
@@ -77,9 +84,10 @@ class AudioMixerImpl : public AudioMixer {
   // Add/remove the MixerAudioSource to the specified
   // MixerAudioSource list.
   bool AddAudioSourceToList(Source* audio_source,
-                            SourceStatusList* audio_source_list) const;
-  bool RemoveAudioSourceFromList(Source* remove_audio_source,
-                                 SourceStatusList* audio_source_list) const;
+                            SourceStatusWithFrameList* audio_source_list) const;
+  bool RemoveAudioSourceFromList(
+      Source* remove_audio_source,
+      SourceStatusWithFrameList* audio_source_list) const;
 
   bool LimitMixedAudio(AudioFrame* mixed_audio) const;
 
@@ -91,7 +99,8 @@ class AudioMixerImpl : public AudioMixer {
   size_t sample_size_ ACCESS_ON(&thread_checker_);
 
   // List of all audio sources. Note all lists are disjunct
-  SourceStatusList audio_source_list_ GUARDED_BY(crit_);  // May be mixed.
+  SourceStatusWithFrameList audio_source_list_
+      GUARDED_BY(crit_);  // May be mixed.
 
   // Determines if we will use a limiter for clipping protection during
   // mixing.