Added audio mixer and removed audio device module in AudioState::Config.

webrtc/api/call/audio_state.h

 /*
  *  Copyright (c) 2015 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.
  */
 #ifndef WEBRTC_API_CALL_AUDIO_STATE_H_
 #define WEBRTC_API_CALL_AUDIO_STATE_H_
 
+#include "webrtc/api/audio/audio_mixer.h"
 #include "webrtc/base/refcount.h"
 #include "webrtc/base/scoped_ref_ptr.h"
 
 namespace webrtc {
 
 class AudioDeviceModule;
 class VoiceEngine;
 
 // WORK IN PROGRESS
 // This class is under development and is not yet intended for for use outside
 // of WebRtc/Libjingle. Please use the VoiceEngine API instead.
 // See: https://bugs.chromium.org/p/webrtc/issues/detail?id=4690
 
 // AudioState holds the state which must be shared between multiple instances of
 // webrtc::Call for audio processing purposes.
 class AudioState : public rtc::RefCountInterface {
  public:
   struct Config {
     // VoiceEngine used for audio streams and audio/video synchronization.
     // AudioState will tickle the VoE refcount to keep it alive for as long as
     // the AudioState itself.
     VoiceEngine* voice_engine = nullptr;
 
-    // The AudioDeviceModule associated with the Calls.
-    AudioDeviceModule* audio_device_module = nullptr;

[aleloi, 2016/10/31 11:45:01]

This field is currently unused, and AudioState gets the audio_device_module from
|voice_engine|. When voice engine no longer has an audio_device_module it can be
added back. I think it's more confusing if I comment it out rather than deleting
it.

[kwiberg-webrtc, 2016/10/31 12:31:06]

Acknowledged.

+    // The audio mixer connected to active receive streams. One per
+    // AudioState.
+    rtc::scoped_refptr<AudioMixer> audio_mixer = nullptr;

[kwiberg-webrtc, 2016/10/31 12:31:06]

You don't need to explicitly initialize scoped_refptr to null. (AFAIK this is
true of all smart pointers---they tend to be tricky to implement otherwise, and
it's good for usability.)

Why is this reference counted rather than uniquely owned? And what consequences
does that have? For example, is the user allowed to use the mixer for something
else while it's being used for this AudioState?

[aleloi, 2016/10/31 12:44:01]

Thanks, removed default initializer (it is called a default initializer,
right?).

The motivation for having a mixer refcounted is because we want to allow a user
to provide their own mixer. Then we don't know who owns it and don't want to
destroy it by mistake. It's not needed for the current mixer implementation. As
for consequences, the mixer is written in a way to allow multi-threaded access
of the public API methods.

Since we can't control what a user does with the mixer, we don't forbid anything
except that only one thread should call Mix at a time (but that is a clear
mis-use; if that were allowed one listener would get fragments of the audio).

[kwiberg-webrtc, 2016/10/31 13:30:33]

I think the term is "default member initializer". It's probably better to not
omit the "member" part, since otherwise it's very similar to "default
initialization", which is an entirely different thing.

If you have an hour or three that you don't know what to do with, see
http://en.cppreference.com/w/cpp/language/initialization.
So let me propose an alternative solution: passing the mixer in a unique_ptr.
That way you know it's not being used by anyone else, which makes life very
simple for everyone. No need for the mixer to do its own locking, for example.

Will the restrictions brought about by using unique_ptr be in the way of some
reasonable use case?

[aleloi, 2016/10/31 14:56:40]

Avoiding shared pointers is always good! I've asked what constraints there are
on external mixers. Depending on the outcome, I'll add a CL before this that
removes RefCountInterface inheritance from the mixer. I think we can wait a few
days.

[kwiberg-webrtc, 2016/11/01 00:45:59]

Excellent. It's not always possible to avoid shared ownership, but it has a very
real cost in increased code complexity, so we should only use it if we have a
good reason.

[ossu, 2016/11/01 16:06:36]

Agreed. unique_ptr sounds good to me!

I'd like to add that we also shouldn't make something reference counted just
because we don't know how it's going to be owned/used. Shared ownership is not a
replacement for actually figuring out ownership yourself. I used it like that in
a previous project and it wasn't long before we had cyclical ownership and
nothing would get destructed. Took a while to untangle.

   };
 
   // TODO(solenberg): Replace scoped_refptr with shared_ptr once we can use it.
   static rtc::scoped_refptr<AudioState> Create(
       const AudioState::Config& config);
 
   virtual ~AudioState() {}
 };
 }  // namespace webrtc
 
 #endif  // WEBRTC_API_CALL_AUDIO_STATE_H_