Restoring behavior where PeerConnection tracks changes to MediaStreams.

talk/app/webrtc/peerconnection.cc

 /*
  * libjingle
  * Copyright 2012 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.
  */
 
 #include "talk/app/webrtc/peerconnection.h"
 
+#include <algorithm>
 #include <vector>
 #include <cctype>  // for isdigit
 
 #include "talk/app/webrtc/audiotrack.h"
 #include "talk/app/webrtc/dtmfsender.h"
 #include "talk/app/webrtc/jsepicecandidate.h"
 #include "talk/app/webrtc/jsepsessiondescription.h"
 #include "talk/app/webrtc/mediaconstraintsinterface.h"
 #include "talk/app/webrtc/mediastream.h"
+#include "talk/app/webrtc/mediastreamobserver.h"
 #include "talk/app/webrtc/mediastreamproxy.h"
 #include "talk/app/webrtc/mediastreamtrackproxy.h"
 #include "talk/app/webrtc/remoteaudiosource.h"
 #include "talk/app/webrtc/remotevideocapturer.h"
 #include "talk/app/webrtc/rtpreceiver.h"
 #include "talk/app/webrtc/rtpsender.h"
 #include "talk/app/webrtc/streamcollection.h"
 #include "talk/app/webrtc/videosource.h"
 #include "talk/app/webrtc/videotrack.h"
 #include "talk/media/sctp/sctpdataengine.h"
@@ skipping 677 matching lines @@
 
 bool PeerConnection::AddStream(MediaStreamInterface* local_stream) {
   if (IsClosed()) {
     return false;
   }
   if (!CanAddLocalMediaStream(local_streams_, local_stream)) {
     return false;
   }
 
   local_streams_->AddStream(local_stream);
+  MediaStreamObserver* observer = new MediaStreamObserver(local_stream);
+  observer->SignalAudioTrackAdded.connect(this,
+                                          &PeerConnection::OnAudioTrackAdded);
+  observer->SignalAudioTrackRemoved.connect(
+      this, &PeerConnection::OnAudioTrackRemoved);
+  observer->SignalVideoTrackAdded.connect(this,
+                                          &PeerConnection::OnVideoTrackAdded);
+  observer->SignalVideoTrackRemoved.connect(
+      this, &PeerConnection::OnVideoTrackRemoved);
+  stream_observers_.push_back(rtc::scoped_ptr<MediaStreamObserver>(observer));
 
   for (const auto& track : local_stream->GetAudioTracks()) {
-    auto sender = FindSenderForTrack(track.get());
-    if (sender == senders_.end()) {
-      // Normal case; we've never seen this track before.
-      AudioRtpSender* new_sender = new AudioRtpSender(
-          track.get(), local_stream->label(), session_.get(), stats_.get());
-      senders_.push_back(new_sender);
-      // If the sender has already been configured in SDP, we call SetSsrc,
-      // which will connect the sender to the underlying transport. This can
-      // occur if a local session description that contains the ID of the sender
-      // is set before AddStream is called. It can also occur if the local
-      // session description is not changed and RemoveStream is called, and
-      // later AddStream is called again with the same stream.
-      const TrackInfo* track_info = FindTrackInfo(
-          local_audio_tracks_, local_stream->label(), track->id());
-      if (track_info) {
-        new_sender->SetSsrc(track_info->ssrc);
-      }
-    } else {
-      // We already have a sender for this track, so just change the stream_id
-      // so that it's correct in the next call to CreateOffer.
-      (*sender)->set_stream_id(local_stream->label());
-    }
+    OnAudioTrackAdded(track.get(), local_stream);
   }
   for (const auto& track : local_stream->GetVideoTracks()) {
-    auto sender = FindSenderForTrack(track.get());
-    if (sender == senders_.end()) {
-      // Normal case; we've never seen this track before.
-      VideoRtpSender* new_sender = new VideoRtpSender(
-          track.get(), local_stream->label(), session_.get());
-      senders_.push_back(new_sender);
-      const TrackInfo* track_info = FindTrackInfo(
-          local_video_tracks_, local_stream->label(), track->id());
-      if (track_info) {
-        new_sender->SetSsrc(track_info->ssrc);
-      }
-    } else {
-      // We already have a sender for this track, so just change the stream_id
-      // so that it's correct in the next call to CreateOffer.
-      (*sender)->set_stream_id(local_stream->label());
-    }
+    OnVideoTrackAdded(track.get(), local_stream);
   }
 
   stats_->AddStream(local_stream);
   observer_->OnRenegotiationNeeded();
   return true;
 }
 
-// TODO(deadbeef): Don't destroy RtpSenders here; they should be kept around
-// indefinitely, when we have unified plan SDP.
 void PeerConnection::RemoveStream(MediaStreamInterface* local_stream) {
   for (const auto& track : local_stream->GetAudioTracks()) {
-    auto sender = FindSenderForTrack(track.get());
-    if (sender == senders_.end()) {
-      LOG(LS_WARNING) << "RtpSender for track with id " << track->id()
-                      << " doesn't exist.";
-      continue;
-    }
-    (*sender)->Stop();
-    senders_.erase(sender);
+    OnAudioTrackRemoved(track.get(), local_stream);
   }
   for (const auto& track : local_stream->GetVideoTracks()) {
-    auto sender = FindSenderForTrack(track.get());
-    if (sender == senders_.end()) {
-      LOG(LS_WARNING) << "RtpSender for track with id " << track->id()
-                      << " doesn't exist.";
-      continue;
-    }
-    (*sender)->Stop();
-    senders_.erase(sender);
+    OnVideoTrackRemoved(track.get(), local_stream);
   }
 
   local_streams_->RemoveStream(local_stream);
+  auto it = std::find_if(
+      stream_observers_.begin(), stream_observers_.end(),
+      [local_stream](
+          const rtc::scoped_ptr<MediaStreamObserver>& observer) -> bool {

[pthatcher1, 2015/12/08 21:31:05]

I think you can leave off the "-> bool" (it can be inferred).

[Taylor Brandstetter, 2015/12/09 00:36:02]

Done.

+        return observer->stream()->label().compare(local_stream->label()) == 0;
+      });
+  if (it != stream_observers_.end()) {
+    stream_observers_.erase(it);
+  }

[pthatcher1, 2015/12/08 21:31:05]

I think you can use std::remove_if to save a few lines and a few cycles.

[Taylor Brandstetter, 2015/12/09 00:36:02]

Done.

 
   if (IsClosed()) {
     return;
   }
   observer_->OnRenegotiationNeeded();
 }
 
 rtc::scoped_refptr<DtmfSenderInterface> PeerConnection::CreateDtmfSender(
     AudioTrackInterface* track) {
   if (!track) {
@@ skipping 582 matching lines @@
     observer_->OnIceConnectionChange(ice_connection_state_);
     if (ice_gathering_state_ != kIceGatheringComplete) {
       ice_gathering_state_ = kIceGatheringComplete;
       observer_->OnIceGatheringChange(ice_gathering_state_);
     }
   }
   observer_->OnSignalingChange(signaling_state_);
   observer_->OnStateChange(PeerConnectionObserver::kSignalingState);
 }
 
+void PeerConnection::OnAudioTrackAdded(AudioTrackInterface* track,
+                                       MediaStreamInterface* stream) {
+  auto sender = FindSenderForTrack(track);
+  if (sender == senders_.end()) {

[pthatcher1, 2015/12/08 21:31:05]

Might as well use an early return, here and in OnVideoTrackEnded.

if (sender != senders_.end()) {
  // We already have a sender for this track, so just change the stream_id
  // so that it's correct in the next call to CreateOffer.
  (*sender)->set_stream_id(stream->label());
  return;
}

// Normal case; we've never seen this track before.
...

[Taylor Brandstetter, 2015/12/09 00:36:02]

Done.

+    // Normal case; we've never seen this track before.
+    AudioRtpSender* new_sender = new AudioRtpSender(
+        track, stream->label(), session_.get(), stats_.get());
+    senders_.push_back(new_sender);
+    // If the sender has already been configured in SDP, we call SetSsrc,
+    // which will connect the sender to the underlying transport. This can
+    // occur if a local session description that contains the ID of the sender
+    // is set before AddStream is called. It can also occur if the local
+    // session description is not changed and RemoveStream is called, and
+    // later AddStream is called again with the same stream.
+    const TrackInfo* track_info =
+        FindTrackInfo(local_audio_tracks_, stream->label(), track->id());
+    if (track_info) {
+      new_sender->SetSsrc(track_info->ssrc);
+    }
+  } else {
+    // We already have a sender for this track, so just change the stream_id
+    // so that it's correct in the next call to CreateOffer.
+    (*sender)->set_stream_id(stream->label());
+  }
+}
+
+// TODO(deadbeef): Don't destroy RtpSenders here; they should be kept around
+// indefinitely, when we have unified plan SDP.
+void PeerConnection::OnAudioTrackRemoved(AudioTrackInterface* track,
+                                         MediaStreamInterface* stream) {
+  auto sender = FindSenderForTrack(track);
+  if (sender == senders_.end()) {
+    LOG(LS_WARNING) << "RtpSender for track with id " << track->id()
+                    << " doesn't exist.";
+    return;
+  }
+  (*sender)->Stop();
+  senders_.erase(sender);
+}
+
+void PeerConnection::OnVideoTrackAdded(VideoTrackInterface* track,
+                                       MediaStreamInterface* stream) {
+  auto sender = FindSenderForTrack(track);
+  if (sender == senders_.end()) {
+    // Normal case; we've never seen this track before.
+    VideoRtpSender* new_sender =
+        new VideoRtpSender(track, stream->label(), session_.get());
+    senders_.push_back(new_sender);
+    const TrackInfo* track_info =
+        FindTrackInfo(local_video_tracks_, stream->label(), track->id());
+    if (track_info) {
+      new_sender->SetSsrc(track_info->ssrc);
+    }
+  } else {
+    // We already have a sender for this track, so just change the stream_id
+    // so that it's correct in the next call to CreateOffer.
+    (*sender)->set_stream_id(stream->label());
+  }
+}
+
+void PeerConnection::OnVideoTrackRemoved(VideoTrackInterface* track,
+                                         MediaStreamInterface* stream) {
+  auto sender = FindSenderForTrack(track);
+  if (sender == senders_.end()) {
+    LOG(LS_WARNING) << "RtpSender for track with id " << track->id()
+                    << " doesn't exist.";
+    return;
+  }
+  (*sender)->Stop();
+  senders_.erase(sender);
+}
+
 void PeerConnection::PostSetSessionDescriptionFailure(
     SetSessionDescriptionObserver* observer,
     const std::string& error) {
   SetSessionDescriptionMsg* msg = new SetSessionDescriptionMsg(observer);
   msg->error = error;
   signaling_thread()->Post(this, MSG_SET_SESSIONDESCRIPTION_FAILED, msg);
 }
 
 void PeerConnection::PostCreateSessionDescriptionFailure(
     CreateSessionDescriptionObserver* observer,
@@ skipping 587 matching lines @@
 DataChannel* PeerConnection::FindDataChannelBySid(int sid) const {
   for (const auto& channel : sctp_data_channels_) {
     if (channel->id() == sid) {
       return channel;
     }
   }
   return nullptr;
 }
 
 }  // namespace webrtc