Adding AddTrack/RemoveTrack to native PeerConnection API.

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,
@@ skipping 773 matching lines @@
                    0;
           }),
       stream_observers_.end());
 
   if (IsClosed()) {
     return;
   }
   observer_->OnRenegotiationNeeded();
 }
 
+rtc::scoped_refptr<RtpSenderInterface> PeerConnection::AddTrack(
+    MediaStreamTrackInterface* track,
+    std::vector<MediaStreamInterface*> streams) {

[pthatcher1, 2016/01/11 17:31:51]

Would it make sense to only pass in a std::vector<std::string> with just stream
IDs?  I don't see any reason to take in streams.

Also, should we just return an error/null if 2 or more are passed in?  That's
probably better than silently breaking.

[Taylor Brandstetter, 2016/01/13 00:53:01]

The reason for it to take a stream is that I want this API to match the W3C API
as closely as possible. Maybe right *now* the only thing from the stream used is
its label, but if that changes in the future, I don't want to have to refactor
things all the way up to Chromium to change the type of the argument.

I do agree about returning an error if 2 are passed in, though. That would match
what we do about other things that are in the spec but not supported by us.

+  TRACE_EVENT0("webrtc", "PeerConnection::AddTrack");
+  if (IsClosed()) {
+    return nullptr;
+  }
+  if (FindSenderForTrack(track) != senders_.end()) {
+    LOG(LS_ERROR) << "Sender for track " << track->id() << " already exists.";
+    return nullptr;

[pthatcher1, 2016/01/11 17:31:51]

Will this be supported once we switch to Unified Plan?  It's a silly thing to
do, but I don't think it's disallowed by the standard.  Thus, should we have a
TODO?

[Taylor Brandstetter, 2016/01/13 00:53:01]

I guess I just assumed the spec didn't allow it. I'll add a TODO.

+  }
+
+  // TODO(deadbeef): Support adding a track to multiple streams.
+  rtc::scoped_refptr<RtpSenderInterface> new_sender;
+  if (track->kind() == MediaStreamTrackInterface::kAudioKind) {
+    new_sender = RtpSenderProxy::Create(
+        signaling_thread(),
+        new AudioRtpSender(static_cast<AudioTrackInterface*>(track),
+                           session_.get(), stats_.get()));

[pthatcher1, 2016/01/11 17:31:51]

So later one when we want to do something with an RtpSender within
PeerConnection, do we have to go through the RtpSenderProxy also?  Or will we
store both proxied and non-proxied?

[Taylor Brandstetter, 2016/01/13 00:53:01]

We would use the proxy object. If a method is called while already on the
signaling thread, the proxy will just pass through to the real object, so there
isn't much disadvantage. Sure, it requires a thread check and adds one method to
the call stack, but I think going to extra effort to avoid those things would
qualify as "premature optimization".

+    if (!streams.empty()) {
+      new_sender->set_stream_id(streams[0]->label());
+    }
+    const TrackInfo* track_info = FindTrackInfo(
+        local_audio_tracks_, new_sender->stream_id(), track->id());
+    if (track_info) {
+      new_sender->SetSsrc(track_info->ssrc);
+    }
+  } else if (track->kind() == MediaStreamTrackInterface::kVideoKind) {
+    new_sender = RtpSenderProxy::Create(
+        signaling_thread(),
+        new VideoRtpSender(static_cast<VideoTrackInterface*>(track),
+                           session_.get()));
+    if (!streams.empty()) {
+      new_sender->set_stream_id(streams[0]->label());
+    }
+    const TrackInfo* track_info = FindTrackInfo(
+        local_video_tracks_, new_sender->stream_id(), track->id());
+    if (track_info) {
+      new_sender->SetSsrc(track_info->ssrc);
+    }
+  } else {
+    LOG(LS_ERROR) << "CreateSender called with invalid kind: " << track->kind();
+    return rtc::scoped_refptr<RtpSenderInterface>();
+  }
+
+  senders_.push_back(new_sender);
+  observer_->OnRenegotiationNeeded();
+  return new_sender;
+}
+
+bool PeerConnection::RemoveTrack(RtpSenderInterface* sender) {
+  TRACE_EVENT0("webrtc", "PeerConnection::RemoveTrack");
+
+  auto it = std::find(senders_.begin(), senders_.end(), sender);
+  if (it == senders_.end()) {
+    LOG(LS_ERROR) << "Couldn't find sender " << sender->id() << " to remove.";
+    return false;
+  }
+  senders_.erase(it);
+
+  if (!IsClosed()) {
+    observer_->OnRenegotiationNeeded();
+  }

[pthatcher1, 2016/01/11 17:31:51]

We can RemoveTrack even if IsClosed()?

[Taylor Brandstetter, 2016/01/13 00:53:01]

I was just matching RemoveStream, but it looks like the spec explicitly
disallows this. I'll change it.

+  return true;
+}
+
 rtc::scoped_refptr<DtmfSenderInterface> PeerConnection::CreateDtmfSender(
     AudioTrackInterface* track) {
   TRACE_EVENT0("webrtc", "PeerConnection::CreateDtmfSender");
   if (!track) {
     LOG(LS_ERROR) << "CreateDtmfSender - track is NULL.";
     return NULL;
   }
   if (!local_streams_->FindAudioTrack(track->id())) {
     LOG(LS_ERROR) << "CreateDtmfSender is called with a non local audio track.";
     return NULL;
   }
 
   rtc::scoped_refptr<DtmfSenderInterface> sender(
       DtmfSender::Create(track, signaling_thread(), session_.get()));
   if (!sender.get()) {
     LOG(LS_ERROR) << "CreateDtmfSender failed on DtmfSender::Create.";
     return NULL;
   }
   return DtmfSenderProxy::Create(signaling_thread(), sender.get());
 }
 
 rtc::scoped_refptr<RtpSenderInterface> PeerConnection::CreateSender(
     const std::string& kind,
     const std::string& stream_id) {
   TRACE_EVENT0("webrtc", "PeerConnection::CreateSender");
-  RtpSenderInterface* new_sender;
+  rtc::scoped_refptr<RtpSenderInterface> new_sender;
   if (kind == MediaStreamTrackInterface::kAudioKind) {
-    new_sender = new AudioRtpSender(session_.get(), stats_.get());
+    new_sender = RtpSenderProxy::Create(
+        signaling_thread(), new AudioRtpSender(session_.get(), stats_.get()));
   } else if (kind == MediaStreamTrackInterface::kVideoKind) {
-    new_sender = new VideoRtpSender(session_.get());
+    new_sender = RtpSenderProxy::Create(signaling_thread(),
+                                        new VideoRtpSender(session_.get()));
   } else {
     LOG(LS_ERROR) << "CreateSender called with invalid kind: " << kind;
-    return rtc::scoped_refptr<RtpSenderInterface>();
+    return new_sender;
   }
   if (!stream_id.empty()) {
     new_sender->set_stream_id(stream_id);
   }
   senders_.push_back(new_sender);
-  return RtpSenderProxy::Create(signaling_thread(), new_sender);
+  return new_sender;
 }
 
 std::vector<rtc::scoped_refptr<RtpSenderInterface>> PeerConnection::GetSenders()
     const {
-  std::vector<rtc::scoped_refptr<RtpSenderInterface>> senders;
-  for (const auto& sender : senders_) {
-    senders.push_back(RtpSenderProxy::Create(signaling_thread(), sender.get()));
-  }
-  return senders;
+  return senders_;
 }
 
 std::vector<rtc::scoped_refptr<RtpReceiverInterface>>
 PeerConnection::GetReceivers() const {
-  std::vector<rtc::scoped_refptr<RtpReceiverInterface>> receivers;
-  for (const auto& receiver : receivers_) {
-    receivers.push_back(
-        RtpReceiverProxy::Create(signaling_thread(), receiver.get()));
-  }
-  return receivers;
+  return receivers_;
 }
 
 bool PeerConnection::GetStats(StatsObserver* observer,
                               MediaStreamTrackInterface* track,
                               StatsOutputLevel level) {
   TRACE_EVENT0("webrtc", "PeerConnection::GetStats");
   RTC_DCHECK(signaling_thread()->IsCurrent());
   if (!VERIFY(observer != NULL)) {
     LOG(LS_ERROR) << "GetStats - observer is NULL.";
     return false;
@@ skipping 449 matching lines @@
     }
     default:
       RTC_DCHECK(false && "Not implemented");
       break;
   }
 }
 
 void PeerConnection::CreateAudioReceiver(MediaStreamInterface* stream,
                                          AudioTrackInterface* audio_track,
                                          uint32_t ssrc) {
-  receivers_.push_back(new AudioRtpReceiver(audio_track, ssrc, session_.get()));
+  receivers_.push_back(RtpReceiverProxy::Create(
+      signaling_thread(),
+      new AudioRtpReceiver(audio_track, ssrc, session_.get())));
 }
 
 void PeerConnection::CreateVideoReceiver(MediaStreamInterface* stream,
                                          VideoTrackInterface* video_track,
                                          uint32_t ssrc) {
-  receivers_.push_back(new VideoRtpReceiver(video_track, ssrc, session_.get()));
+  receivers_.push_back(RtpReceiverProxy::Create(
+      signaling_thread(),
+      new VideoRtpReceiver(video_track, ssrc, session_.get())));
 }
 
 // TODO(deadbeef): Keep RtpReceivers around even if track goes away in remote
 // description.
 void PeerConnection::DestroyAudioReceiver(MediaStreamInterface* stream,
                                           AudioTrackInterface* audio_track) {
   auto it = FindReceiverForTrack(audio_track);
   if (it == receivers_.end()) {
     LOG(LS_WARNING) << "RtpReceiver for track with id " << audio_track->id()
                     << " doesn't exist.";
@@ skipping 71 matching lines @@
                                        MediaStreamInterface* stream) {
   auto sender = FindSenderForTrack(track);
   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.
-  AudioRtpSender* new_sender =
-      new AudioRtpSender(track, stream->label(), session_.get(), stats_.get());
+  rtc::scoped_refptr<RtpSenderInterface> new_sender = RtpSenderProxy::Create(
+      signaling_thread(),
+      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) {
@@ skipping 19 matching lines @@
                                        MediaStreamInterface* stream) {
   auto sender = FindSenderForTrack(track);
   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.
-  VideoRtpSender* new_sender =
-      new VideoRtpSender(track, stream->label(), session_.get());
+  rtc::scoped_refptr<RtpSenderInterface> new_sender = RtpSenderProxy::Create(
+      signaling_thread(),
+      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);
   }
 }
 
 void PeerConnection::OnVideoTrackRemoved(VideoTrackInterface* track,
                                          MediaStreamInterface* stream) {
@@ skipping 604 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