Cleanup of the mixer interface.

webrtc/modules/audio_mixer/audio_mixer.h

 /*
  *  Copyright (c) 2011 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_MODULES_AUDIO_MIXER_AUDIO_MIXER_H_
 #define WEBRTC_MODULES_AUDIO_MIXER_AUDIO_MIXER_H_
 
 #include <memory>
 
+#include "webrtc/base/refcount.h"
 #include "webrtc/modules/include/module_common_types.h"
 
 namespace webrtc {
 
-class AudioMixer {
+class AudioMixer : public rtc::RefCountInterface {
  public:
-  static const int kMaximumAmountOfMixedAudioSources = 3;
   // A callback class that all mixer participants must inherit from/implement.
   class Source {
    public:
     enum class AudioFrameInfo {
       kNormal,  // The samples in audio_frame are valid and should be used.
       kMuted,   // The samples in audio_frame should not be used, but should be
       // implicitly interpreted as zero. Other fields in audio_frame
       // may be read and should contain meaningful values.
       kError  // audio_frame will not be used.
     };
 
     struct AudioFrameWithMuted {
       AudioFrame* audio_frame;
       AudioFrameInfo audio_frame_info;
     };
 
     // The implementation of GetAudioFrameWithMuted 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.
     virtual AudioFrameWithMuted GetAudioFrameWithMuted(int sample_rate_hz) = 0;
 
+    // A way for a mixer implementation do distinguish participants.
+    virtual int ssrc() = 0;
+
    protected:
     virtual ~Source() {}
   };
 
-  // Factory method. Constructor disabled.
-  static std::unique_ptr<AudioMixer> Create();
-  virtual ~AudioMixer() {}
+  ~AudioMixer() override {}
 
-  // Add/remove audio sources as candidates for mixing.
-  virtual int32_t SetMixabilityStatus(Source* audio_source, bool mixable) = 0;
+  // Returns true if adding/removing was successful. Adding the same
+  // source twice is forbidden, as is removing a source that is not

[hlundin-webrtc, 2016/10/11 11:50:21]

Does it have to be "forbidden" in all implementations of this interface? I would
suggest a re-write to something like
"Returns true if adding/removing was successful. Adding the same source twice or
removing a source that is not present would make a typical implementation return
false."
WDYT?

[the sun, 2016/10/11 12:10:55]

It's important to not mix up the interface and the framework. We own the
framework, and the canonical implementation. That's why the latter should not be
defensive and return false if a source is added twice. Instead it should blow up
(assert) as quickly as possible. This way we can check our framework
implementation.

In the *interface* however, we need to provide the possibility for the
implementer to return false, because they may depend on some resource which may
or may not be available. We can state in the comment that the method will never
be called multiple times for the same source. But to say "forbidden" in this
context, to me, is mixing up the viewpoints.

[hlundin-webrtc, 2016/10/11 12:49:54]

OK, solenberg, your turn to have a go at formulating the comment. :)

[aleloi, 2016/10/12 10:20:53]

We want to point out that Add / remove mustn't be called twice and
maybe that the default implementation ensures that by asserting.

We want to allow an implementation to return a 'success' flag, but not
to reserve the flag for when the condition above is violated.

Does this correctly summarizes the discussion so far?

+  // present.
+  virtual bool AddSource(Source* audio_source) = 0;
+  virtual bool RemoveSource(Source* audio_source) = 0;
 
   // Performs mixing by asking registered audio sources for audio. The
   // mixed result is placed in the provided AudioFrame. Will only be
   // called from a single thread. The rate and channels arguments
   // specify the rate and number of channels of the mix result.
   virtual void Mix(int sample_rate_hz,
                    size_t number_of_channels,
                    AudioFrame* audio_frame_for_mixing) = 0;
 };
 }  // namespace webrtc
 
 #endif  // WEBRTC_MODULES_AUDIO_MIXER_AUDIO_MIXER_H_