MediaController/Call instantiation.

talk/app/webrtc/mediacontroller.h

 /*
  * libjingle
- * Copyright 2013 Google Inc.
+ * Copyright 2015 Google Inc.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions are met:
  *
  *  1. Redistributions of source code must retain the above copyright notice,
  *     this list of conditions and the following disclaimer.
  *  2. Redistributions in binary form must reproduce the above copyright notice,
  *     this list of conditions and the following disclaimer in the documentation
  *     and/or other materials provided with the distribution.
  *  3. The name of the author may not be used to endorse or promote products
  *     derived from this software without specific prior written permission.
  *
  * THIS SOFTWARE IS PROVIDED BY THE AUTHOR ``AS IS'' AND ANY EXPRESS OR IMPLIED
  * WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED WARRANTIES OF
  * MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE DISCLAIMED. IN NO
  * EVENT SHALL THE AUTHOR BE LIABLE FOR ANY DIRECT, INDIRECT, INCIDENTAL,
  * SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES (INCLUDING, BUT NOT LIMITED TO,
  * PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES; LOSS OF USE, DATA, OR PROFITS;
  * OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON ANY THEORY OF LIABILITY,
  * WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT (INCLUDING NEGLIGENCE OR
  * OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS SOFTWARE, EVEN IF
  * ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
  */
 
-#ifndef TALK_APP_WEBRTC_AUDIOTRACKRENDERER_H_
-#define TALK_APP_WEBRTC_AUDIOTRACKRENDERER_H_
+#ifndef TALK_APP_WEBRTC_MEDIACONTROLLER_H_
+#define TALK_APP_WEBRTC_MEDIACONTROLLER_H_
 
-#include "talk/media/base/audiorenderer.h"
+#include "webrtc/base/criticalsection.h"
 #include "webrtc/base/thread.h"
+#include "webrtc/call.h"
+
+namespace cricket {
+class VideoChannel;
+}  // namespace cricket
 
 namespace webrtc {
+class VoiceEngine;
 
-// Class used for AudioTrack to get the ID of WebRtc voice channel that
-// the AudioTrack is connecting to.
-// Each AudioTrack owns a AudioTrackRenderer instance.
-// AddChannel() will be called when an AudioTrack is added to a MediaStream.
-// RemoveChannel will be called when the AudioTrack or WebRtc VoE channel is
-// going away.
-// This implementation only supports one channel, and it is only used by
-// Chrome for remote audio tracks."
-class AudioTrackRenderer : public cricket::AudioRenderer {
+// MediaController owns state shared between multiple concurrent media streams.

[pthatcher1, 2015/09/04 04:52:31]

What does "concurrent" mean?  
And what does "media stream" mean?

Perhaps for now we should just say "currently owns shared state between media
channels, but in the future will create and own RtpSenders and RtpReceivers", or
something like that.

[the sun, 2015/09/04 10:18:47]

"media stream" was referring to the actual stream objects that share the state
through the Call instance.
That's slightly better, agreed, although "future" is kind of relative. Doesn't
mean much without a date attached to it. :)

+class MediaController : public webrtc::LoadObserver {
  public:
-  AudioTrackRenderer();
-  ~AudioTrackRenderer();
+  MediaController(rtc::Thread* worker_thread,
+                  webrtc::VoiceEngine* voice_engine);
+  ~MediaController() override;
 
-  // Implements cricket::AudioRenderer.
-  void AddChannel(int channel_id) override;
-  void RemoveChannel(int channel_id) override;
+  webrtc::Call* call() { return call_.get(); }

[pthatcher1, 2015/09/04 04:52:31]

This still needs clear thread safety.

[the sun, 2015/09/04 10:18:47]

Let me first say that thread safety is a very big concern for me. However, I
don't see how this construct is any more dangerous than what is already going on
with e.g. BaseSession* being passed to ChannelManager::CreateNnChannel(). In
this case, the life span of the session, MediaController and Call is close to
identical.

If your objective is that we should fetch the Call* on the worker thread, I can
see a few ways to accomplish that:

1. Forward the MediaController* instead of webrtc::Call* to
ChannelManager::CreateNnChannel(), pass it down to the _w functions and pull out
the Call* there; we could rename this getter call_w() then.

2. Add a "virtual MediaController* BaseSession::media_controller_w() = 0"; now
we can get the Call* just when the mediachannel is created, on the worker
thread.

3. Let ChannelManager keep a map BaseSession* -> Call* so we don't need the MC
at all, right now. Mapping is implicit and lookup/create/destroy Call* all
happens on the worker thread.

I'd pick #2 because that allows the ChannelManager interface to be simpler, or
leave as it is right now.

[pthatcher1, 2015/09/04 22:58:32]

That's exactly why we're refactoring ChannelManager so we don't pass in Session,
and why in the replacement (TranpsortController) we're being very careful about
which methods can be called on which threads.  If all goes well, that CL is
going in today.

Out of those, I think #1 makes a lot of sense.  I'd rather not see anything more
go into BaseSession (#2).  We're trying to get rid of BaseSession.


Another option is to do a thread hop from call() to call_w().  But I think I
like #1 better.  

+  void set_video_channel(cricket::VideoChannel* video_channel);
+
+  // Implements webrtc::LoadObserver.
+  void OnLoadUpdate(Load load) override;
 
  private:
-  int channel_id_;
+  void Construct_w(webrtc::VoiceEngine* voice_engine);
+  void Destruct_w();
+
+  rtc::Thread* worker_thread_;
+  rtc::scoped_ptr<webrtc::Call> call_;
+  rtc::CriticalSection critical_section_;
+  cricket::VideoChannel* video_channel_ GUARDED_BY(critical_section_);
+
+  DISALLOW_IMPLICIT_CONSTRUCTORS(MediaController);
 };
-
 }  // namespace webrtc
 
-#endif  // TALK_APP_WEBRTC_AUDIOTRACKRENDERER_H_
+#endif  // TALK_APP_WEBRTC_MEDIACONTROLLER_H_