Replace AudioConferenceMixer with AudioMixer.

webrtc/audio/audio_receive_stream.cc

Index: webrtc/audio/audio_receive_stream.cc
diff --git a/webrtc/audio/audio_receive_stream.cc b/webrtc/audio/audio_receive_stream.cc
index cc30939b92134af837ce91d76ec350d5ce570258..a2083ebc3b69b2bfbc7eb9f451bec6910766d1b9 100644
--- a/webrtc/audio/audio_receive_stream.cc
+++ b/webrtc/audio/audio_receive_stream.cc
@@ -14,7 +14,6 @@
 #include <utility>
 
 #include "webrtc/api/call/audio_sink.h"
-#include "webrtc/audio/audio_state.h"
 #include "webrtc/audio/conversion.h"
 #include "webrtc/base/checks.h"
 #include "webrtc/base/logging.h"
@@ -142,7 +141,9 @@ AudioReceiveStream::AudioReceiveStream(
 AudioReceiveStream::~AudioReceiveStream() {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   LOG(LS_INFO) << "~AudioReceiveStream: " << config_.ToString();
-  Stop();
+  if (playing_) {
+    Stop();
+  }
   channel_proxy_->DeRegisterExternalTransport();
   channel_proxy_->ResetCongestionControlObjects();
   channel_proxy_->SetRtcEventLog(nullptr);
@@ -152,16 +153,41 @@ AudioReceiveStream::~AudioReceiveStream() {
 }
 
 void AudioReceiveStream::Start() {
-  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  RTC_DCHECK_RUN_ON(&thread_checker_);

[the sun, 2016/11/14 20:03:46]

I don't like that some methods in this class use
RTC_DCHECK_RUN_ON(&thread_checker_);
and some use
RTC_DCHECK(thread_checker_.CalledOnValidThread());

Please make it consistent.

[aleloi, 2016/11/15 16:56:54]

Done.

+  if (playing_) {
+    return;
+  }
+
   ScopedVoEInterface<VoEBase> base(voice_engine());
   int error = base->StartPlayout(config_.voe_channel_id);
   if (error != 0) {
-    LOG(LS_ERROR) << "AudioReceiveStream::Start failed with error: " << error;
+    RTC_NOTREACHED() << "AudioReceiveStream::Start failed with error: "

[the sun, 2016/11/14 20:03:46]

Why did you change this? We can conceivably end up here at run time, so this
should still be logging, not "NOTREACHED"

[aleloi, 2016/11/15 16:56:54]

Point, that was wrong.

+                     << error;
+    return;
+  }
+
+  playing_ = true;

[the sun, 2016/11/14 20:03:46]

Note that you're violating the AudioMixer promises of never calling
RemoveSource() unless the source was successfully added. You need to sort out
the logic and and the missing test case.

[aleloi, 2016/11/15 16:56:54]

Acknowledged.

+
+  auto* const the_audio_state = audio_state();
+
+  if (!the_audio_state->mixer()->AddSource(this)) {
+    LOG(LS_ERROR) << "Failed to add source to mixer.";
   }
 }
 
 void AudioReceiveStream::Stop() {
-  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  RTC_DCHECK_RUN_ON(&thread_checker_);
+
+  if (!playing_) {
+    return;
+  }
+  playing_ = false;
+
+  auto* const the_audio_state = audio_state();
+  if (!the_audio_state->mixer()->RemoveSource(this)) {
+    RTC_NOTREACHED() << "Failed to remove stream from mixer.";

[the sun, 2016/11/14 20:03:46]

If the return code is *supposed* to always be true, what's the point of it.
Change the interface.

[aleloi, 2016/11/15 16:56:54]

For AudioMixerImpl it's supposed to always be true. What about other
implementations? I thought an AddSource() return code of 'false' allowed  other
impls that way have more complex initialization to signal failure. 

But then asserting that it will never happen here is wrong... I'll ask the
interested parties about changing the interface. We shouldn't land this before
we've sorted it out of course.

+  }
+
   ScopedVoEInterface<VoEBase> base(voice_engine());
   base->StopPlayout(config_.voe_channel_id);
 }
@@ -284,10 +310,13 @@ int AudioReceiveStream::Ssrc() const {
   return config_.rtp.local_ssrc;
 }
 
+internal::AudioState* AudioReceiveStream::audio_state() const {
+  return static_cast<internal::AudioState*>(audio_state_.get());

[the sun, 2016/11/14 20:03:46]

Pull into a local variable and DCHECK it's not null before cast+return.

[aleloi, 2016/11/15 16:56:54]

Done.

+}
+
 VoiceEngine* AudioReceiveStream::voice_engine() const {
-  internal::AudioState* audio_state =
-      static_cast<internal::AudioState*>(audio_state_.get());
-  VoiceEngine* voice_engine = audio_state->voice_engine();
+  internal::AudioState* the_audio_state = audio_state();

[the sun, 2016/11/14 20:03:46]

Make that a one-liner instead

[aleloi, 2016/11/15 16:56:54]

Done.

+  VoiceEngine* voice_engine = the_audio_state->voice_engine();
   RTC_DCHECK(voice_engine);
   return voice_engine;
 }