Remove usage of VoEBase::AssociateSendChannel() from WVoMC.

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..840352d9c2e8536dcefdc1a220b665392f4b9434 100644
--- a/webrtc/audio/audio_receive_stream.cc
+++ b/webrtc/audio/audio_receive_stream.cc
@@ -14,6 +14,7 @@
 #include <utility>
 
 #include "webrtc/api/call/audio_sink.h"
+#include "webrtc/audio/audio_send_stream.h"
 #include "webrtc/audio/audio_state.h"
 #include "webrtc/audio/conversion.h"
 #include "webrtc/base/checks.h"
@@ -143,6 +144,7 @@ AudioReceiveStream::~AudioReceiveStream() {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
   LOG(LS_INFO) << "~AudioReceiveStream: " << config_.ToString();
   Stop();
+  AssociateSendStream(nullptr);

[kwiberg-webrtc, 2016/11/11 13:47:39]

Or just channel_proxy_->DisassociateSendChannel();, so that it's easier to see
what it does.

[the sun, 2016/11/11 20:03:39]

Yes, thanks.

   channel_proxy_->DeRegisterExternalTransport();
   channel_proxy_->ResetCongestionControlObjects();
   channel_proxy_->SetRtcEventLog(nullptr);
@@ -230,6 +232,18 @@ const webrtc::AudioReceiveStream::Config& AudioReceiveStream::config() const {
   return config_;
 }
 
+void AudioReceiveStream::AssociateSendStream(AudioSendStream* send_stream) {
+  RTC_DCHECK(thread_checker_.CalledOnValidThread());
+  if (send_stream) {
+    VoiceEngineImpl* voe_impl = static_cast<VoiceEngineImpl*>(voice_engine());

[kwiberg-webrtc, 2016/11/11 13:47:39]

Is this static_cast guaranteed to be safe? And so, would it be possible to
convince the type system of that fact so that we could avoid the cast?

[the sun, 2016/11/11 20:03:39]

I don't see how, without exposing VoiceEngineImpl in AudioState. We need to cast
because of the GetChannelProxy() method, which is on VoiceEngineImpl but not on
VoiceEngine. What we do know is that a VoiceEngine* will always point to an
instance of VoiceEngineImpl so the cast is safe (the same pattern is used in the
ctor for both AudioSendStream and AudioReceiveStream.

Do you have any persuasive arguments I could throw on the type system?

[kwiberg-webrtc, 2016/11/13 20:17:02]

The only way to make this statically safe (since you can't verify that a class
has only one subclass) is to not do down casts, which means you can't do up
casts either---in other words, you'd need to pass it around as the most specific
type you need. Meaning either you'd need to expose the Impl class, or move
GetChannelProxy to VoiceEngine. (The latter sounds more appealing.)

This makes me a bit suspicious, though---if you depend on VoiceEngineImpl being
the only subclass of the abstract class VoiceEngine, why not just turn
VoiceEngine into a concrete, final class?

I realize that whatever the answers are to my questions, getting rid of the down
cast is going to be outside the scope of this CL...

[the sun, 2016/11/14 18:58:06]

Well, no, we don't want to expose that method in the public API.
Not only that, but we also depend on VoiceEngineImpl to inherit from all the
VoENnn interface classes. See e.g. VoEBase::GetInterface(). I consider this part
of the legacy API and will not change it now.
Indeed. :)

+    std::unique_ptr<voe::ChannelProxy> send_channel_proxy =
+        voe_impl->GetChannelProxy(send_stream->config().voe_channel_id);
+    channel_proxy_->AssociateSendChannel(*send_channel_proxy.get());
+  } else {
+    channel_proxy_->DisassociateSendChannel();
+  }
+}
+
 void AudioReceiveStream::SignalNetworkState(NetworkState state) {
   RTC_DCHECK(thread_checker_.CalledOnValidThread());
 }