Cleanup of the mixer interface.

webrtc/modules/audio_mixer/audio_mixer.h

Index: webrtc/modules/audio_mixer/audio_mixer.h
diff --git a/webrtc/modules/audio_mixer/audio_mixer.h b/webrtc/modules/audio_mixer/audio_mixer.h
index 258011a330f0b761955bebc35886a1808236c01c..e62e5fd0f1ba1f265431e72e7a12444c30c6035a 100644
--- a/webrtc/modules/audio_mixer/audio_mixer.h
+++ b/webrtc/modules/audio_mixer/audio_mixer.h
@@ -13,13 +13,13 @@
 
 #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:
@@ -44,16 +44,20 @@ class AudioMixer {
     // 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