Move audio frame memory handling inside AudioMixer.

webrtc/api/audio/audio_mixer.h

Index: webrtc/api/audio/audio_mixer.h
diff --git a/webrtc/api/audio/audio_mixer.h b/webrtc/api/audio/audio_mixer.h
index 960adbbd430b8cb24403a5736aa9f0d039a43a6d..f8d1d61ab7af2559579b49ff4cef8adda9d8e4e6 100644
--- a/webrtc/api/audio/audio_mixer.h
+++ b/webrtc/api/audio/audio_mixer.h
@@ -35,33 +35,18 @@ class AudioMixer : public rtc::RefCountInterface {
       kError,   // The audio_frame will not be used.
     };
 
-    struct AudioFrameWithInfo {
-      AudioFrame* audio_frame;
-      AudioFrameInfo audio_frame_info;
-    };
-
-    // The implementation of GetAudioFrameWithInfo should update
-    // audio_frame with new audio every time it's called. Implementing
-    // classes are allowed to return the same AudioFrame pointer on
-    // different calls. The pointer must stay valid until the next
-    // mixing call or until this audio source is disconnected from the
-    // mixer. The mixer may modify the contents of the passed
-    // AudioFrame pointer at any time until the next call to
-    // GetAudioFrameWithInfo, or until the source is removed from the
-    // mixer.
-    virtual AudioFrameWithInfo GetAudioFrameWithInfo(int sample_rate_hz) = 0;
+    // Overwrites |audio_frame|. The data_ field is overwritten with

[hlundin-webrtc, 2016/10/18 22:16:13]

So, the data_ field is overwritten, but what other fields in audio_frame must
the implementation update?

[aleloi, 2016/10/19 09:56:46]

That is a good question. The simple answer is 'all of them except for id_'.

These fields *must always* be updated. There are asserts and checks for them:

 samples_per_channel_
 number_of_channels_
 num_channels_

These fields are read in the AudioTransport function that asks for the mixed
sound. When I connected the mixer, I found tests for at least one of them as
well (can't recall which now, but will look it up when we connect the mixer for
real). They are only filled in correctly if there is a single audio source.
Otherwise they are set to -1.

 elapsed_time_ms_
 ntp_time_ms_

These fields are filled in by our default mixer implementation, but are
(currently) never read. At least I think they are never read: maybe they are in
AudioProcessing during echo detection or limiting.

 vad_activity_ (set to kVadPassive if no frames are mixed or kVadActive if at
least one is)
 speech_type_ (set to kNormalSpeech)
 timestamp_ (ticks up by sample_size_ starting from 0 in the mixer).


I am reluctant and a bit afraid to specify too much in the interface, at least
before the new mixer is connected. It seems that most of the above could change.

The slightly longer answer is thus that 'nothing should break if an impl ignores
all but the rate and channel fields'.

[aleloi, 2016/10/19 11:33:40]

I just realized that I answered the wrong question. The above answer explains
what the *mixer* does with the AudioFrame provided in Mix and not what is
expected by GetAudioFrameWithInfo.

For GetAudioFrameWithInfo, *every* field except for 'id_' must be filled in.
vad_activity_ and speech_type_ is used to determine whether the frame will be
mixed. elapsed_time and ntp_time are copied to the mixed result if there is only
one source.

+    // 10 ms of new audio (either 1 or 2 interleaved channels) at
+    // |sample_rate_hz|.
+    virtual AudioFrameInfo GetAudioFrameWithInfo(int sample_rate_hz,
+                                                 AudioFrame* audio_frame) = 0;
 
     // A way for a mixer implementation to distinguish participants.
     virtual int Ssrc() = 0;
 
-   protected:
     virtual ~Source() {}
   };
 
-  // Since the mixer is reference counted, the destructor may be
-  // called from any thread.
-  ~AudioMixer() override {}
-
   // Returns true if adding/removing was successful. A source is never
   // added twice and removal is never attempted if a source has not
   // been successfully added to the mixer. Addition and removal can
@@ -76,6 +61,11 @@ class AudioMixer : public rtc::RefCountInterface {
   virtual void Mix(int sample_rate_hz,
                    size_t number_of_channels,
                    AudioFrame* audio_frame_for_mixing) = 0;
+
+ protected:
+  // Since the mixer is reference counted, the destructor may be
+  // called from any thread.
+  ~AudioMixer() override {}
 };
 }  // namespace webrtc