Enabling usage of native PeerConnection v2 in libjingle.

content/renderer/media/media_stream_impl.cc

Index: content/renderer/media/media_stream_impl.cc
diff --git a/content/renderer/media/media_stream_impl.cc b/content/renderer/media/media_stream_impl.cc
index ab48883cbd1eee4aaede86848f823cc6a537496a..fd698115561ae92f8b2cc5a9e05a2b12f4c7e57f 100644
--- a/content/renderer/media/media_stream_impl.cc
+++ b/content/renderer/media/media_stream_impl.cc
@@ -23,9 +23,6 @@
 #include "content/renderer/p2p/socket_dispatcher.h"
 #include "jingle/glue/thread_wrapper.h"
 #include "media/base/message_loop_factory.h"
-#include "third_party/libjingle/source/talk/p2p/client/httpportallocator.h"
-#include "third_party/libjingle/source/talk/session/phone/dummydevicemanager.h"
-#include "third_party/libjingle/source/talk/session/phone/webrtcmediaengine.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebMediaStreamDescriptor.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/WebMediaStreamRegistry.h"
 #include "third_party/WebKit/Source/WebKit/chromium/public/platform/WebMediaStreamSource.h"
@@ -41,6 +38,9 @@ static const int kVideoCaptureFramePerSecond = 30;
 
 int MediaStreamImpl::next_request_id_ = 0;
 
+// TODO(grunell): BEFORE COMMIT. Add DCHECK(CalledOnValidThread()) in all
+// functions.
+
 MediaStreamImpl::MediaStreamImpl(
     MediaStreamDispatcher* media_stream_dispatcher,
     content::P2PSocketDispatcher* p2p_socket_dispatcher,
@@ -48,7 +48,6 @@ MediaStreamImpl::MediaStreamImpl(
     MediaStreamDependencyFactory* dependency_factory)
     : dependency_factory_(dependency_factory),
       media_stream_dispatcher_(media_stream_dispatcher),
-      media_engine_(NULL),
       p2p_socket_dispatcher_(p2p_socket_dispatcher),
       network_manager_(NULL),
       vc_manager_(vc_manager),
@@ -56,14 +55,13 @@ MediaStreamImpl::MediaStreamImpl(
       message_loop_proxy_(base::MessageLoopProxy::current()),
       signaling_thread_(NULL),
       worker_thread_(NULL),
-      chrome_worker_thread_("Chrome_libJingle_WorkerThread"),
-      vcm_created_(false) {
+      chrome_worker_thread_("Chrome_libJingle_WorkerThread") {
 }
 
 MediaStreamImpl::~MediaStreamImpl() {
   DCHECK(!peer_connection_handler_);
   if (dependency_factory_.get())
-    dependency_factory_->DeletePeerConnectionFactory();
+    dependency_factory_->ReleasePeerConnectionFactory();
   if (network_manager_) {
     // The network manager needs to free its resources on the thread they were
     // created, which is the worked thread.
@@ -84,87 +82,34 @@ WebKit::WebPeerConnectionHandler* MediaStreamImpl::CreatePeerConnectionHandler(
     DVLOG(1) << "A PeerConnection already exists";
     return NULL;
   }
-
-  if (!media_engine_) {
-    media_engine_ = dependency_factory_->CreateWebRtcMediaEngine();
-  }
-
-  if (!signaling_thread_) {
-    jingle_glue::JingleThreadWrapper::EnsureForCurrentThread();
-    jingle_glue::JingleThreadWrapper::current()->set_send_allowed(true);
-    signaling_thread_ = jingle_glue::JingleThreadWrapper::current();
-  }
-
-  if (!worker_thread_) {
-    if (!chrome_worker_thread_.IsRunning()) {
-      if (!chrome_worker_thread_.Start()) {
-        LOG(ERROR) << "Could not start worker thread";
-        delete media_engine_;
-        media_engine_ = NULL;
-        signaling_thread_ = NULL;
-        return NULL;
-      }
-    }
-    base::WaitableEvent event(true, false);
-    chrome_worker_thread_.message_loop()->PostTask(
-        FROM_HERE,
-        base::Bind(&MediaStreamImpl::InitializeWorkerThread, this,
-                   &worker_thread_, &event));
-    event.Wait();
-    DCHECK(worker_thread_);
-  }
-
-  if (!network_manager_)
-    network_manager_ = new content::IpcNetworkManager(p2p_socket_dispatcher_);
-
-  if (!socket_factory_.get()) {
-    socket_factory_.reset(
-        new content::IpcPacketSocketFactory(p2p_socket_dispatcher_));
-  }
-
-  if (!dependency_factory_->PeerConnectionFactoryCreated()) {
-    if (!dependency_factory_->CreatePeerConnectionFactory(media_engine_,
-                                                          worker_thread_)) {
-      LOG(ERROR) << "Could not initialize PeerConnection factory";
-      return NULL;
-    }
-  }
+  EnsurePeerConnectionFactory();
 
   peer_connection_handler_ = new PeerConnectionHandler(
       client,
       this,
-      dependency_factory_.get(),
-      signaling_thread_,
-      p2p_socket_dispatcher_,
-      network_manager_,
-      socket_factory_.get());
+      dependency_factory_.get());
 
   return peer_connection_handler_;
 }
 
 void MediaStreamImpl::ClosePeerConnection() {
   DCHECK(CalledOnValidThread());
-  rtc_video_decoder_ = NULL;
-  media_engine_->SetVideoCaptureModule(NULL);
-  vcm_created_ = false;
+  video_renderer_ = NULL;
   peer_connection_handler_ = NULL;
+  // TODO(grunell): This is a temporary workaround for an error in native
+  // PeerConnection where added live tracks are not seen on the remote side.
+  MediaStreamTrackPtrMap::const_iterator it = local_tracks_.begin();
+  for (; it != local_tracks_.end(); ++it)
+    it->second->set_state(webrtc::MediaStreamTrackInterface::kEnded);
 }
 
-bool MediaStreamImpl::SetVideoCaptureModule(const std::string& label) {
-  DCHECK(CalledOnValidThread());
-  if (vcm_created_)
-    return true;
-  // Set the capture device.
-  // TODO(grunell): Instead of using the first track, the selected track
-  // should be used.
-  int id = media_stream_dispatcher_->video_session_id(label, 0);
-  if (id == media_stream::StreamDeviceInfo::kNoId)
-    return false;
-  webrtc::VideoCaptureModule* vcm =
-      new VideoCaptureModuleImpl(id, vc_manager_.get());
-  vcm_created_ = true;
-  media_engine_->SetVideoCaptureModule(vcm);
-  return true;
+webrtc::MediaStreamTrackInterface* MediaStreamImpl::GetLocalMediaStreamTrack(
+    std::string label) {
+  MediaStreamTrackPtrMap::iterator it = local_tracks_.find(label);
+  if (it == local_tracks_.end())
+    return NULL;
+  MediaStreamTrackPtr stream = it->second;
+  return stream.get();
 }
 
 void MediaStreamImpl::requestUserMedia(
@@ -226,22 +171,33 @@ scoped_refptr<media::VideoDecoder> MediaStreamImpl::GetVideoDecoder(
       WebKit::WebMediaStreamRegistry::lookupMediaStreamDescriptor(url));
   if (descriptor.isNull())
     return NULL;  // This is not a valid stream.
+
+  // We must find out if this is a local or remote stream. The tracks in a local
+  // stream have IDs that are in the form
+  // <MediaStreamManager-label>#{audio,video}-<session-ID>. We extract the

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

can we move the code that does this parsing out to a separate function?  I think
there's detail here that would be good to separate from this function as it does
more things.

[Henrik Grunell, 2012/01/26 13:03:16]

Broke out the generation and parsing of the track label.

+  // MSM-label and if found in the dispatcher we have a local stream, otherwise
+  // we have a remote stream. There will be changes soon so that we don't have
+  // to bother about the type of stream here. Hence this solution is OK for now.
   WebKit::WebVector<WebKit::WebMediaStreamSource> source_vector;
   descriptor.sources(source_vector);
-  std::string label;
+  std::string msm_label;
   for (size_t i = 0; i < source_vector.size(); ++i) {
     if (source_vector[i].type() == WebKit::WebMediaStreamSource::TypeVideo) {
-      label = UTF16ToUTF8(source_vector[i].id());
+      // We assume there is one video track only.
+      msm_label = UTF16ToUTF8(source_vector[i].id());
+      size_t pos = msm_label.rfind("#");
+      msm_label = msm_label.substr(0, pos);

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

what if pos is npos?

[Henrik Grunell, 2012/01/26 13:03:16]

Commented on that in the new functions. (Doesn't matter if npos.)

       break;
     }
   }
-  if (label.empty())
+  if (msm_label.empty())
     return NULL;
 
   scoped_refptr<media::VideoDecoder> decoder;
-  if (media_stream_dispatcher_->IsStream(label)) {
+  if (media_stream_dispatcher_->IsStream(msm_label)) {
     // It's a local stream.
-    int video_session_id = media_stream_dispatcher_->video_session_id(label, 0);
+    int video_session_id =
+        media_stream_dispatcher_->video_session_id(msm_label, 0);
     media::VideoCapture::VideoCaptureCapability capability;
     capability.width = kVideoCaptureWidth;
     capability.height = kVideoCaptureHeight;
@@ -256,20 +212,27 @@ scoped_refptr<media::VideoDecoder> MediaStreamImpl::GetVideoDecoder(
         capability);
   } else {
     // It's a remote stream.
-    size_t found = label.rfind("-remote");
-    if (found != std::string::npos)
-      label = label.substr(0, found);
-    if (rtc_video_decoder_.get()) {
+    if (!video_renderer_.get())
+      video_renderer_ = new talk_base::RefCountedObject<VideoRendererWrapper>();
+    if (video_renderer_->renderer()) {
       // The renderer is used by PeerConnection, release it first.
-      if (peer_connection_handler_)
-        peer_connection_handler_->SetVideoRenderer(label, NULL);
+      if (peer_connection_handler_) {
+        peer_connection_handler_->SetVideoRenderer(
+            UTF16ToUTF8(descriptor.label()),
+            NULL);
+      }
+      video_renderer_->SetVideoDecoder(NULL);
     }
-    rtc_video_decoder_ = new RTCVideoDecoder(
+    RTCVideoDecoder* rtc_video_decoder = new RTCVideoDecoder(
         message_loop_factory->GetMessageLoop("RtcVideoDecoderThread"),
         url.spec());
-    decoder = rtc_video_decoder_;
-    if (peer_connection_handler_)
-      peer_connection_handler_->SetVideoRenderer(label, rtc_video_decoder_);
+    decoder = rtc_video_decoder;
+    video_renderer_->SetVideoDecoder(rtc_video_decoder);
+    if (peer_connection_handler_) {
+      peer_connection_handler_->SetVideoRenderer(
+          UTF16ToUTF8(descriptor.label()),
+          video_renderer_);
+    }
   }
   return decoder;
 }
@@ -280,32 +243,57 @@ void MediaStreamImpl::OnStreamGenerated(
     const media_stream::StreamDeviceInfoArray& audio_array,
     const media_stream::StreamDeviceInfoArray& video_array) {
   DCHECK(CalledOnValidThread());
+  EnsurePeerConnectionFactory();
 
-  // We only support max one audio track and one video track. If the UI
-  // for selecting device starts to allow several devices, we must implement
-  // handling for this.
-  DCHECK_LE(audio_array.size(), 1u);
-  DCHECK_LE(video_array.size(), 1u);
   WebKit::WebVector<WebKit::WebMediaStreamSource> source_vector(
       audio_array.size() + video_array.size());
 
-  WebKit::WebString track_label_audio(UTF8ToUTF16("AudioDevice"));
-  WebKit::WebString track_label_video(UTF8ToUTF16("VideoCapture"));
-  size_t track_num = source_vector.size();
-  while (track_num--) {
-    if (track_num < audio_array.size()) {
-      source_vector[track_num].initialize(
-          UTF8ToUTF16(label),
+  // The label for a stream is globally unique. The track session id is globally
+  // unique for the set of audio tracks and video tracks respectively. An audio
+  // track and a video track can have the same session id (without being
+  // related). Hence we create a unique track label from the stream label, track
+  // type and track session id.
+
+  // Add audio tracks.
+  size_t i = 0;

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

unless you use 'i' outside the loops, just declare it inside the for()
statement.

[Henrik Grunell, 2012/01/26 13:03:16]

Relic from earlier stage of local dev. Fixed.

+  std::string track_label;
+  for (; i < audio_array.size(); ++i) {
+    track_label = label;
+    track_label += "#audio-";
+    track_label +=  audio_array[i].session_id;
+    talk_base::scoped_refptr<webrtc::LocalAudioTrackInterface> audio_track(
+        dependency_factory_->CreateLocalAudioTrack(audio_array[i].name, NULL));
+    local_tracks_.insert(
+        std::pair<std::string, MediaStreamTrackPtr>(track_label, audio_track));
+    source_vector[i].initialize(
+          UTF8ToUTF16(track_label),
           WebKit::WebMediaStreamSource::TypeAudio,
-          track_label_audio);
-    } else {
-      source_vector[track_num].initialize(
-          UTF8ToUTF16(label),
+          UTF8ToUTF16(audio_array[i].name));
+  }
+
+  // Add video tracks.
+  for (i = 0; i < video_array.size(); ++i) {
+    track_label = label;
+    track_label += "#video-";
+    track_label +=  video_array[i].session_id;
+    webrtc::VideoCaptureModule* vcm =
+        new VideoCaptureModuleImpl(video_array[i].session_id,
+                                   vc_manager_.get());
+    talk_base::scoped_refptr<webrtc::LocalVideoTrackInterface> video_track(

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

should video_track be of type MediaStreamTrackPtr?

[Henrik Grunell, 2012/01/26 13:03:16]

Yes, that's better. Changed it and for audio_track.

+        dependency_factory_->CreateLocalVideoTrack(
+            video_array[i].name,
+            webrtc::CreateVideoCapturer(vcm)));

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

add a comment about the ownership of vcm?

[Henrik Grunell, 2012/01/26 13:03:16]

Done.

+    local_tracks_.insert(
+        std::pair<std::string, MediaStreamTrackPtr>(track_label, video_track));
+    source_vector[audio_array.size() + i].initialize(
+          UTF8ToUTF16(track_label),
           WebKit::WebMediaStreamSource::TypeVideo,
-          track_label_video);
-    }
+          UTF8ToUTF16(video_array[i].name));
   }
 
+  // TODO(grunell): Remove tracks from the map when support to stop is
+  // added in WebKit.
+
   MediaRequestMap::iterator it = user_media_requests_.find(request_id);
   if (it == user_media_requests_.end()) {
     DVLOG(1) << "Request ID not found";
@@ -313,7 +301,6 @@ void MediaStreamImpl::OnStreamGenerated(
   }
   WebKit::WebUserMediaRequest user_media_request = it->second;
   user_media_requests_.erase(it);
-  stream_labels_.push_back(label);
 
   user_media_request.requestSucceeded(source_vector);
 }
@@ -364,3 +351,65 @@ void MediaStreamImpl::DeleteIpcNetworkManager() {
   delete network_manager_;
   network_manager_ = NULL;
 }
+
+bool MediaStreamImpl::EnsurePeerConnectionFactory() {
+  DCHECK(CalledOnValidThread());
+  if (!signaling_thread_) {
+    jingle_glue::JingleThreadWrapper::EnsureForCurrentThread();
+    jingle_glue::JingleThreadWrapper::current()->set_send_allowed(true);
+    signaling_thread_ = jingle_glue::JingleThreadWrapper::current();
+  }
+
+  if (!worker_thread_) {
+    if (!chrome_worker_thread_.IsRunning()) {
+      if (!chrome_worker_thread_.Start()) {
+        LOG(ERROR) << "Could not start worker thread";
+        signaling_thread_ = NULL;
+        return false;
+      }
+    }
+    base::WaitableEvent event(true, false);
+    chrome_worker_thread_.message_loop()->PostTask(FROM_HERE, base::Bind(
+        &MediaStreamImpl::InitializeWorkerThread,
+        this,
+        &worker_thread_,
+        &event));
+    event.Wait();

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

is there a specific reason why we need to do this operation synchronously? 
Looks like we don't care if it succeeds or not.  I'm assuming that all future
tasks will be posted to the same message queue and if so, then all tasks are
serialized and guaranteed to execute after InitializeWorkerThread has run.

[Henrik Grunell, 2012/01/26 13:03:16]

Yes there's a reason. worker_thread_ must be set before we can continue, it's
used below when creating the PC factory.

+    DCHECK(worker_thread_);
+  }
+
+  if (!network_manager_)
+    network_manager_ = new content::IpcNetworkManager(p2p_socket_dispatcher_);
+
+  if (!socket_factory_.get()) {
+    socket_factory_.reset(
+        new content::IpcPacketSocketFactory(p2p_socket_dispatcher_));
+  }
+
+  if (!dependency_factory_->PeerConnectionFactoryCreated()) {
+    if (!dependency_factory_->CreatePeerConnectionFactory(
+        worker_thread_,

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

it feels weird to me to see parameters indented to the same indent as the method
name.  I would rather expect this:
if (!SomeLongFunctionName(
        param1,
        param2)) {

[Henrik Grunell, 2012/01/26 13:03:16]

That seems better. Fixed.

+        signaling_thread_,
+        p2p_socket_dispatcher_,
+        network_manager_,
+        socket_factory_.get())) {
+      LOG(ERROR) << "Could not initialize PeerConnection factory";
+      return false;
+    }
+  }
+
+  return true;
+}
+
+MediaStreamImpl::VideoRendererWrapper::VideoRendererWrapper() {}
+
+MediaStreamImpl::VideoRendererWrapper::~VideoRendererWrapper() {}
+
+cricket::VideoRenderer* MediaStreamImpl::VideoRendererWrapper::renderer() {

[tommi (sloooow) - chröme, 2012/01/24 15:18:56]

nit: one liners like this one (all lower case) usually have their implementation
inline in the header.

[Henrik Grunell, 2012/01/26 13:03:16]

Done.

+  return rtc_video_decoder_.get();
+}
+
+void MediaStreamImpl::VideoRendererWrapper::SetVideoDecoder(
+    RTCVideoDecoder* decoder) {
+  rtc_video_decoder_ = decoder;
+}