Hi,

Removed OnLocalStreamInitialized callback from the PeerConnection callback list. After adding OnAddStream trigger at the originator this callback was redundant. Also other modification is to provide same stream label in OnAddStream callback at the originator which provided in AddStream API.
Review URL: http://webrtc-codereview.appspot.com/138002

git-svn-id: http://webrtc.googlecode.com/svn/trunk@490 4adac7df-926f-26a2-2b94-8c16560cd09d

peerconnection/samples/client/conductor.cc

@@ -115,10 +115,6 @@ void Conductor::StartCaptureDevice() {
 // PeerConnectionObserver implementation.
 //
 
-void Conductor::OnInitialized() {
-  main_wnd_->QueueUIThreadCallback(PEER_CONNECTION_ADDSTREAMS, NULL);
-}
-
 void Conductor::OnError() {
   LOG(LS_ERROR) << __FUNCTION__;
   main_wnd_->QueueUIThreadCallback(PEER_CONNECTION_ERROR, NULL);
@@ -131,36 +127,10 @@ void Conductor::OnSignalingMessage(const std::string& msg) {
   main_wnd_->QueueUIThreadCallback(SEND_MESSAGE_TO_PEER, msg_copy);
 }
 
-// Called when a local stream is added and initialized
-void Conductor::OnLocalStreamInitialized(const std::string& stream_id,
-                                         bool video) {
-  LOG(INFO) << __FUNCTION__ << " " << stream_id;
-  bool send_notification = (waiting_for_video_ || waiting_for_audio_);
-  if (video) {
-    ASSERT(video_channel_.empty());
-    video_channel_ = stream_id;
-    waiting_for_video_ = false;
-    LOG(INFO) << "Setting video renderer for stream: " << stream_id;
-    bool ok = peer_connection_->SetVideoRenderer(stream_id,
-                                                 main_wnd_->remote_renderer());
-    ASSERT(ok);
-  } else {
-    ASSERT(audio_channel_.empty());
-    audio_channel_ = stream_id;
-    waiting_for_audio_ = false;
-  }
-
-  if (send_notification && !waiting_for_audio_ && !waiting_for_video_)
-    main_wnd_->QueueUIThreadCallback(MEDIA_CHANNELS_INITIALIZED, NULL);
-
-  if (!waiting_for_audio_ && !waiting_for_video_)
-    main_wnd_->QueueUIThreadCallback(PEER_CONNECTION_CONNECT, NULL);
-}
-
 // Called when a remote stream is added
 void Conductor::OnAddStream(const std::string& stream_id, bool video) {
   LOG(INFO) << __FUNCTION__ << " " << stream_id;
-  bool send_notification = (waiting_for_video_ || waiting_for_audio_);
+
   if (video) {
     // ASSERT(video_channel_.empty());
     video_channel_ = stream_id;
@@ -177,11 +147,8 @@ void Conductor::OnAddStream(const std::string& stream_id, bool video) {
     waiting_for_audio_ = false;
   }
 
-  if (send_notification && !waiting_for_audio_ && !waiting_for_video_)
+  if (!waiting_for_video_)
     main_wnd_->QueueUIThreadCallback(MEDIA_CHANNELS_INITIALIZED, NULL);
-
-  if (!waiting_for_audio_ && !waiting_for_video_)
-    main_wnd_->QueueUIThreadCallback(PEER_CONNECTION_CONNECT, NULL);
 }
 
 void Conductor::OnRemoveStream(const std::string& stream_id, bool video) {
@@ -307,7 +274,7 @@ void Conductor::ConnectToPeer(int peer_id) {
   if (InitializePeerConnection()) {
     peer_id_ = peer_id;
     main_wnd_->SwitchToStreamingUI();
-    OnInitialized();  // TODO(tommi): Figure out why we don't get this callback.
+    AddStreams();
   } else {
     main_wnd_->MessageBox("Error", "Failed to initialize PeerConnection", true);
   }
@@ -325,6 +292,15 @@ void Conductor::AddStreams() {
 
   if (!peer_connection_->AddStream(kAudioLabel, false))
     waiting_for_audio_ = false;
+
+  // At the initiator of the call, after adding streams we need
+  // kick start the ICE candidates discovery process, which
+  // is done by the Connect method. Earlier this was done after
+  // getting the OnLocalStreamInitialized callback which is removed
+  // now. Connect will trigger OnSignalingMessage callback when
+  // ICE candidates are available.
+  if (waiting_for_audio_ || waiting_for_video_)
+    peer_connection_->Connect();
 }
 
 void Conductor::DisconnectFromCurrentPeer() {
@@ -340,8 +316,6 @@ void Conductor::DisconnectFromCurrentPeer() {
 
 void Conductor::UIThreadCallback(int msg_id, void* data) {
   if (msg_id == MEDIA_CHANNELS_INITIALIZED) {
-    bool ok = peer_connection_->Connect();
-    ASSERT(ok);
     StartCaptureDevice();
     // When we get an OnSignalingMessage notification, we'll send our
     // json encoded signaling message to the peer, which is the first step
@@ -388,8 +362,6 @@ void Conductor::UIThreadCallback(int msg_id, void* data) {
     }
   } else if (msg_id == PEER_CONNECTION_ADDSTREAMS) {
     AddStreams();
-  } else if (msg_id == PEER_CONNECTION_CONNECT) {
-    peer_connection_->Connect();
   } else if (msg_id == PEER_CONNECTION_ERROR) {
     main_wnd_->MessageBox("Error", "an unknown error occurred", true);
   }

peerconnection/samples/client/conductor.h

@@ -35,7 +35,6 @@ class Conductor
     PEER_CONNECTION_CLOSED,
     SEND_MESSAGE_TO_PEER,
     PEER_CONNECTION_ADDSTREAMS,
-    PEER_CONNECTION_CONNECT,
     PEER_CONNECTION_ERROR,
   };
 
@@ -55,15 +54,9 @@ class Conductor
   //
   // PeerConnectionObserver implementation.
   //
-
-  virtual void OnInitialized();
   virtual void OnError();
   virtual void OnSignalingMessage(const std::string& msg);
 
-  // Called when a local stream is added and initialized
-  virtual void OnLocalStreamInitialized(const std::string& stream_id,
-                                        bool video);
-
   // Called when a remote stream is added
   virtual void OnAddStream(const std::string& stream_id, bool video);
 

third_party_mods/libjingle/source/talk/app/webrtc/peerconnection.h

@@ -48,16 +48,11 @@ namespace webrtc {
 
 class PeerConnectionObserver {
  public:
-  virtual void OnInitialized() = 0;
   virtual void OnError() = 0;
 
   // serialized signaling message
   virtual void OnSignalingMessage(const std::string& msg) = 0;
 
-  // Triggered when a local stream has been added and initialized
-  virtual void OnLocalStreamInitialized(const std::string& stream_id,
-      bool video) = 0;
-
   // Triggered when a remote peer accepts a media connection.
   virtual void OnAddStream(const std::string& stream_id, bool video) = 0;
 

third_party_mods/libjingle/source/talk/app/webrtc/peerconnection_impl.cc

@@ -56,8 +56,8 @@ PeerConnectionImpl::~PeerConnectionImpl() {
 bool PeerConnectionImpl::Init() {
   std::string sid;
   talk_base::CreateRandomString(8, &sid);
-  const bool incoming = false;
-  session_.reset(CreateMediaSession(sid, incoming)); // default outgoing direction
+  const bool incoming = false;  // default outgoing direction
+  session_.reset(CreateMediaSession(sid, incoming));
   if (session_.get() == NULL) {
     ASSERT(false && "failed to initialize a session");
     return false;
@@ -107,9 +107,6 @@ WebRtcSession* PeerConnectionImpl::CreateMediaSession(
     session->SignalRemoveStream.connect(
         this,
         &PeerConnectionImpl::OnRemoveStream);
-    session->SignalRtcMediaChannelCreated.connect(
-        this,
-        &PeerConnectionImpl::OnRtcMediaChannelCreated);
     session->SignalLocalDescription.connect(
         this,
         &PeerConnectionImpl::OnLocalDescription);
@@ -209,13 +206,6 @@ void PeerConnectionImpl::OnRemoveStream(const std::string& stream_id,
   }
 }
 
-void PeerConnectionImpl::OnRtcMediaChannelCreated(const std::string& stream_id,
-                                                  bool video) {
-  if (event_callback_) {
-    event_callback_->OnLocalStreamInitialized(stream_id, video);
-  }
-}
-
 PeerConnectionImpl::ReadyState PeerConnectionImpl::GetReadyState() {
   ReadyState ready_state;
   cricket::BaseSession::State state = session_->state();

third_party_mods/libjingle/source/talk/app/webrtc/peerconnection_impl.h

@@ -80,8 +80,6 @@ class PeerConnectionImpl : public PeerConnection,
       const cricket::SessionDescription* desc,
       const std::vector<cricket::Candidate>& candidates);
   void OnFailedCall();
-  void OnRtcMediaChannelCreated(const std::string& stream_id,
-                                bool video);
   bool Init();
 
  private:

third_party_mods/libjingle/source/talk/app/webrtc/webrtcsession.cc

@@ -131,10 +131,6 @@ bool WebRtcSession::CreateVoiceChannel(const std::string& stream_id) {
       channel_manager_->CreateVoiceChannel(this, stream_id, true);
   ASSERT(voice_channel != NULL);
   stream_info->channel = voice_channel;
-
-  if (!incoming()) {
-    SignalRtcMediaChannelCreated(stream_id, false);
-  }
   return true;
 }
 
@@ -148,10 +144,6 @@ bool WebRtcSession::CreateVideoChannel(const std::string& stream_id) {
       channel_manager_->CreateVideoChannel(this, stream_id, true, NULL);
   ASSERT(video_channel != NULL);
   stream_info->channel = video_channel;
-
-  if (!incoming()) {
-    SignalRtcMediaChannelCreated(stream_id, true);
-  }
   return true;
 }
 
@@ -451,6 +443,7 @@ bool WebRtcSession::OnRemoteDescription(
     transport_->OnRemoteCandidates(candidates);
     return true;
   }
+
   // Session description is always accepted.
   set_remote_description(desc);
   SetState(STATE_RECEIVEDACCEPT);
@@ -460,17 +453,32 @@ bool WebRtcSession::OnRemoteDescription(
   if (!incoming()) {
     // Trigger OnAddStream callback at the initiator
     const cricket::ContentInfo* video_content = GetFirstVideoContent(desc);
-    if (video_content) {
-      SignalAddStream(video_content->name, true);
+    if (video_content && !SendSignalAddStream(true)) {
+      LOG(LERROR) << "failed to find video stream in map";
+      ASSERT(false);
     } else {
       const cricket::ContentInfo* audio_content = GetFirstAudioContent(desc);
-      if (audio_content)
-        SignalAddStream(audio_content->name, false);
+      if (audio_content && !SendSignalAddStream(false)) {
+        LOG(LERROR) << "failed to find audio stream in map";
+        ASSERT(false);
+      }
     }
   }
   return true;
 }
 
+bool WebRtcSession::SendSignalAddStream(bool video) {
+  StreamMap::const_iterator iter;
+  for (iter = streams_.begin(); iter != streams_.end(); ++iter) {
+    StreamInfo* sinfo = (*iter);
+    if (sinfo->video == video) {
+      SignalAddStream(sinfo->stream_id, video);
+      return true;
+    }
+  }
+  return false;
+}
+
 cricket::SessionDescription* WebRtcSession::CreateOffer() {
   cricket::SessionDescription* offer = new cricket::SessionDescription();
   StreamMap::iterator iter;

third_party_mods/libjingle/source/talk/app/webrtc/webrtcsession.h

@@ -106,10 +106,6 @@ class WebRtcSession : public cricket::BaseSession {
   // message from the remote peer with the candidates port equal to 0.
   sigslot::signal2<const std::string&, bool> SignalRemoveStream;
 
-  // This signal occurs when audio/video channel has been created for the
-  // new added stream.
-  sigslot::signal2<const std::string&, bool> SignalRtcMediaChannelCreated;
-
   // This signal occurs when the local candidate is ready
   sigslot::signal2<const cricket::SessionDescription*,
       const std::vector<cricket::Candidate>&> SignalLocalDescription;
@@ -200,6 +196,7 @@ class WebRtcSession : public cricket::BaseSession {
 
   bool SetVideoCapture(bool capture);
   void EnableAllStreams();
+  bool SendSignalAddStream(bool video);
 
   cricket::Transport* transport_;
   cricket::ChannelManager* channel_manager_;