Switch to using AddTrack with stream labels
Bug: webrtc:8587
Change-Id: I8d4a3a225e6f6a6ae59def972ecae3255c0f2bda
Reviewed-on: https://webrtc-review.googlesource.com/37547
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Peter Thatcher <pthatcher@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21509}
diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index 78fcf1d..020f3c3 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -1009,18 +1009,17 @@
}
stream_labels.push_back(stream->label());
}
- auto sender_or_error = AddTrackWithStreamLabels(track, stream_labels);
+ auto sender_or_error = AddTrack(track, stream_labels);
if (!sender_or_error.ok()) {
return nullptr;
}
return sender_or_error.MoveValue();
}
-RTCErrorOr<rtc::scoped_refptr<RtpSenderInterface>>
-PeerConnection::AddTrackWithStreamLabels(
+RTCErrorOr<rtc::scoped_refptr<RtpSenderInterface>> PeerConnection::AddTrack(
rtc::scoped_refptr<MediaStreamTrackInterface> track,
const std::vector<std::string>& stream_labels) {
- TRACE_EVENT0("webrtc", "PeerConnection::AddTrackWithStreamLabels");
+ TRACE_EVENT0("webrtc", "PeerConnection::AddTrack");
if (!track) {
LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER, "Track is null.");
}
diff --git a/pc/peerconnection.h b/pc/peerconnection.h
index a3797f4..3fd944c 100644
--- a/pc/peerconnection.h
+++ b/pc/peerconnection.h
@@ -88,7 +88,7 @@
bool AddStream(MediaStreamInterface* local_stream) override;
void RemoveStream(MediaStreamInterface* local_stream) override;
- RTCErrorOr<rtc::scoped_refptr<RtpSenderInterface>> AddTrackWithStreamLabels(
+ RTCErrorOr<rtc::scoped_refptr<RtpSenderInterface>> AddTrack(
rtc::scoped_refptr<MediaStreamTrackInterface> track,
const std::vector<std::string>& stream_labels) override;
rtc::scoped_refptr<RtpSenderInterface> AddTrack(
diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc
index 1e16728..78e18c3 100644
--- a/pc/peerconnection_rtp_unittest.cc
+++ b/pc/peerconnection_rtp_unittest.cc
@@ -101,9 +101,7 @@
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
- rtc::scoped_refptr<AudioTrackInterface> audio_track(
- pc_factory_->CreateAudioTrack("audio_track", nullptr));
- EXPECT_TRUE(caller->pc()->AddTrack(audio_track.get(), {}));
+ ASSERT_TRUE(caller->AddTrack(caller->CreateAudioTrack("audio_track")));
ASSERT_TRUE(
callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
static_cast<webrtc::RTCError*>(nullptr)));
@@ -121,10 +119,8 @@
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
- rtc::scoped_refptr<AudioTrackInterface> audio_track(
- pc_factory_->CreateAudioTrack("audio_track", nullptr));
- auto stream = MediaStream::Create("audio_stream");
- EXPECT_TRUE(caller->pc()->AddTrack(audio_track.get(), {stream.get()}));
+ ASSERT_TRUE(caller->AddTrack(caller->CreateAudioTrack("audio_track"),
+ {"audio_stream"}));
ASSERT_TRUE(
callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
static_cast<webrtc::RTCError*>(nullptr)));
@@ -142,9 +138,7 @@
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
- rtc::scoped_refptr<AudioTrackInterface> audio_track(
- pc_factory_->CreateAudioTrack("audio_track", nullptr));
- auto sender = caller->pc()->AddTrack(audio_track.get(), {});
+ auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"), {});
ASSERT_TRUE(
callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
static_cast<webrtc::RTCError*>(nullptr)));
@@ -164,10 +158,8 @@
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
- rtc::scoped_refptr<AudioTrackInterface> audio_track(
- pc_factory_->CreateAudioTrack("audio_track", nullptr));
- auto stream = MediaStream::Create("audio_stream");
- auto sender = caller->pc()->AddTrack(audio_track.get(), {stream.get()});
+ auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"),
+ {"audio_stream"});
ASSERT_TRUE(
callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
static_cast<webrtc::RTCError*>(nullptr)));
@@ -187,14 +179,11 @@
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
- rtc::scoped_refptr<AudioTrackInterface> audio_track1(
- pc_factory_->CreateAudioTrack("audio_track1", nullptr));
- rtc::scoped_refptr<AudioTrackInterface> audio_track2(
- pc_factory_->CreateAudioTrack("audio_track2", nullptr));
- auto stream = MediaStream::Create("shared_audio_stream");
- std::vector<MediaStreamInterface*> streams{stream.get()};
- auto sender1 = caller->pc()->AddTrack(audio_track1.get(), streams);
- auto sender2 = caller->pc()->AddTrack(audio_track2.get(), streams);
+ const char kSharedStreamLabel[] = "shared_audio_stream";
+ auto sender1 = caller->AddTrack(caller->CreateAudioTrack("audio_track1"),
+ {kSharedStreamLabel});
+ auto sender2 = caller->AddTrack(caller->CreateAudioTrack("audio_track2"),
+ {kSharedStreamLabel});
ASSERT_TRUE(
callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
static_cast<webrtc::RTCError*>(nullptr)));
@@ -230,9 +219,7 @@
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
- rtc::scoped_refptr<webrtc::AudioTrackInterface> audio_track(
- pc_factory_->CreateAudioTrack("audio_track", nullptr));
- EXPECT_TRUE(caller->pc()->AddTrack(audio_track.get(), {}));
+ ASSERT_TRUE(caller->AddTrack(caller->CreateAudioTrack("audio_track"), {}));
ASSERT_TRUE(
callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
static_cast<webrtc::RTCError*>(nullptr)));
@@ -250,10 +237,8 @@
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
- rtc::scoped_refptr<webrtc::AudioTrackInterface> audio_track(
- pc_factory_->CreateAudioTrack("audio_track", nullptr));
- auto stream = webrtc::MediaStream::Create("audio_stream");
- EXPECT_TRUE(caller->pc()->AddTrack(audio_track.get(), {stream}));
+ ASSERT_TRUE(caller->AddTrack(caller->CreateAudioTrack("audio_track"),
+ {"audio_stream"}));
ASSERT_TRUE(
callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
static_cast<webrtc::RTCError*>(nullptr)));
@@ -271,9 +256,7 @@
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
- rtc::scoped_refptr<webrtc::AudioTrackInterface> audio_track(
- pc_factory_->CreateAudioTrack("audio_track", nullptr));
- auto sender = caller->pc()->AddTrack(audio_track.get(), {});
+ auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"), {});
ASSERT_TRUE(sender);
ASSERT_TRUE(
callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
@@ -294,10 +277,8 @@
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
- rtc::scoped_refptr<webrtc::AudioTrackInterface> audio_track(
- pc_factory_->CreateAudioTrack("audio_track", nullptr));
- auto stream = webrtc::MediaStream::Create("audio_stream");
- auto sender = caller->pc()->AddTrack(audio_track.get(), {stream});
+ auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"),
+ {"audio_stream"});
ASSERT_TRUE(sender);
ASSERT_TRUE(
callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
@@ -319,14 +300,11 @@
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
- rtc::scoped_refptr<webrtc::AudioTrackInterface> audio_track1(
- pc_factory_->CreateAudioTrack("audio_track1", nullptr));
- rtc::scoped_refptr<webrtc::AudioTrackInterface> audio_track2(
- pc_factory_->CreateAudioTrack("audio_track2", nullptr));
- auto stream = webrtc::MediaStream::Create("shared_audio_stream");
- std::vector<webrtc::MediaStreamInterface*> streams{stream.get()};
- auto sender1 = caller->pc()->AddTrack(audio_track1.get(), streams);
- auto sender2 = caller->pc()->AddTrack(audio_track2.get(), streams);
+ const char kSharedStreamLabel[] = "shared_audio_stream";
+ auto sender1 = caller->AddTrack(caller->CreateAudioTrack("audio_track1"),
+ {kSharedStreamLabel});
+ auto sender2 = caller->AddTrack(caller->CreateAudioTrack("audio_track2"),
+ {kSharedStreamLabel});
ASSERT_TRUE(
callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal(),
static_cast<webrtc::RTCError*>(nullptr)));
@@ -375,11 +353,9 @@
auto caller = CreatePeerConnection();
auto callee = CreatePeerConnection();
- rtc::scoped_refptr<webrtc::AudioTrackInterface> audio_track(
- pc_factory_->CreateAudioTrack("audio_track", nullptr));
// Create SDP for adding a track and for removing it. This will be used in the
// first and second SetRemoteDescription() calls.
- auto sender = caller->pc()->AddTrack(audio_track.get(), {});
+ auto sender = caller->AddTrack(caller->CreateAudioTrack("audio_track"), {});
auto srd1_sdp = caller->CreateOfferAndSetAsLocal();
EXPECT_TRUE(caller->pc()->RemoveTrack(sender));
auto srd2_sdp = caller->CreateOfferAndSetAsLocal();
@@ -612,7 +588,7 @@
auto caller = CreatePeerConnectionWithUnifiedPlan();
auto audio_track = caller->CreateAudioTrack("a");
- auto sender = caller->pc()->AddTrack(audio_track, {});
+ auto sender = caller->AddTrack(audio_track);
ASSERT_TRUE(sender);
EXPECT_EQ(cricket::MEDIA_TYPE_AUDIO, sender->media_type());
@@ -625,7 +601,7 @@
auto caller = CreatePeerConnectionWithUnifiedPlan();
auto video_track = caller->CreateVideoTrack("a");
- auto sender = caller->pc()->AddTrack(video_track, {});
+ auto sender = caller->AddTrack(video_track);
ASSERT_TRUE(sender);
EXPECT_EQ(cricket::MEDIA_TYPE_VIDEO, sender->media_type());
@@ -654,7 +630,7 @@
auto transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
auto audio_track = caller->CreateAudioTrack("a");
- auto sender = caller->pc()->AddTrack(audio_track, {});
+ auto sender = caller->AddTrack(audio_track);
ASSERT_TRUE(sender);
auto transceivers = caller->pc()->GetTransceivers();
@@ -769,7 +745,9 @@
caller->pc()->Close();
caller->observer()->clear_negotiation_needed();
- EXPECT_FALSE(caller->pc()->AddTrack(audio_track, {}));
+ auto result = caller->pc()
+ ->AddTrack(audio_track, std::vector<std::string>());
+ EXPECT_EQ(RTCErrorType::INVALID_STATE, result.error().type());
EXPECT_FALSE(caller->observer()->negotiation_needed());
}
@@ -777,10 +755,12 @@
auto caller = CreatePeerConnectionWithUnifiedPlan();
auto audio_track = caller->CreateAudioTrack("a");
- ASSERT_TRUE(caller->pc()->AddTrack(audio_track, {}));
+ ASSERT_TRUE(caller->AddTrack(audio_track));
caller->observer()->clear_negotiation_needed();
- EXPECT_FALSE(caller->pc()->AddTrack(audio_track, {}));
+ auto result = caller->pc()
+ ->AddTrack(audio_track, std::vector<std::string>());
+ EXPECT_EQ(RTCErrorType::INVALID_PARAMETER, result.error().type());
EXPECT_FALSE(caller->observer()->negotiation_needed());
}
diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc
index 56c12e9..d8e1761 100644
--- a/pc/peerconnectioninterface_unittest.cc
+++ b/pc/peerconnectioninterface_unittest.cc
@@ -1409,11 +1409,6 @@
// Also tests that RemoveTrack removes the tracks from subsequent offers.
TEST_F(PeerConnectionInterfaceTest, AddTrackRemoveTrack) {
CreatePeerConnectionWithoutDtls();
- // Create a dummy stream, so tracks share a stream label.
- rtc::scoped_refptr<MediaStreamInterface> stream(
- pc_factory_->CreateLocalMediaStream(kStreamLabel1));
- std::vector<MediaStreamInterface*> stream_list;
- stream_list.push_back(stream.get());
rtc::scoped_refptr<AudioTrackInterface> audio_track(
pc_factory_->CreateAudioTrack("audio_track", nullptr));
rtc::scoped_refptr<VideoTrackInterface> video_track(
@@ -1421,8 +1416,8 @@
"video_track", pc_factory_->CreateVideoSource(
std::unique_ptr<cricket::VideoCapturer>(
new cricket::FakeVideoCapturer()))));
- auto audio_sender = pc_->AddTrack(audio_track, stream_list);
- auto video_sender = pc_->AddTrack(video_track, stream_list);
+ auto audio_sender = pc_->AddTrack(audio_track, {kStreamLabel1}).MoveValue();
+ auto video_sender = pc_->AddTrack(video_track, {kStreamLabel1}).MoveValue();
EXPECT_EQ(1UL, audio_sender->stream_ids().size());
EXPECT_EQ(kStreamLabel1, audio_sender->stream_ids()[0]);
EXPECT_EQ("audio_track", audio_sender->id());
@@ -1483,9 +1478,9 @@
std::unique_ptr<cricket::VideoCapturer>(
new cricket::FakeVideoCapturer()))));
auto audio_sender =
- pc_->AddTrack(audio_track, std::vector<MediaStreamInterface*>());
+ pc_->AddTrack(audio_track, std::vector<std::string>()).MoveValue();
auto video_sender =
- pc_->AddTrack(video_track, std::vector<MediaStreamInterface*>());
+ pc_->AddTrack(video_track, std::vector<std::string>()).MoveValue();
EXPECT_EQ("audio_track", audio_sender->id());
EXPECT_EQ(audio_track, audio_sender->track());
EXPECT_EQ("video_track", video_sender->id());
@@ -1506,10 +1501,8 @@
"video_track", pc_factory_->CreateVideoSource(
std::unique_ptr<cricket::VideoCapturer>(
new cricket::FakeVideoCapturer()))));
- auto audio_sender =
- pc_->AddTrack(audio_track, std::vector<MediaStreamInterface*>());
- auto video_sender =
- pc_->AddTrack(video_track, std::vector<MediaStreamInterface*>());
+ auto audio_sender = pc_->AddTrack(audio_track, std::vector<std::string>());
+ auto video_sender = pc_->AddTrack(video_track, std::vector<std::string>());
EXPECT_TRUE(DoGetStats(nullptr));
}
diff --git a/pc/peerconnectionwrapper.cc b/pc/peerconnectionwrapper.cc
index 5d804a8..35aebcc 100644
--- a/pc/peerconnectionwrapper.cc
+++ b/pc/peerconnectionwrapper.cc
@@ -266,6 +266,15 @@
return pc_factory()->CreateVideoTrack(label, video_source);
}
+rtc::scoped_refptr<RtpSenderInterface> PeerConnectionWrapper::AddTrack(
+ rtc::scoped_refptr<MediaStreamTrackInterface> track,
+ const std::vector<std::string>& stream_labels) {
+ RTCErrorOr<rtc::scoped_refptr<RtpSenderInterface>> result =
+ pc()->AddTrack(track, stream_labels);
+ EXPECT_EQ(RTCErrorType::NONE, result.error().type());
+ return result.MoveValue();
+}
+
rtc::scoped_refptr<RtpSenderInterface> PeerConnectionWrapper::AddAudioTrack(
const std::string& track_label,
std::vector<MediaStreamInterface*> streams) {
diff --git a/pc/peerconnectionwrapper.h b/pc/peerconnectionwrapper.h
index 0830b55..c20c1f2 100644
--- a/pc/peerconnectionwrapper.h
+++ b/pc/peerconnectionwrapper.h
@@ -130,6 +130,12 @@
rtc::scoped_refptr<VideoTrackInterface> CreateVideoTrack(
const std::string& label);
+ // Wrapper for the underlying PeerConnection's AddTrack method. DCHECKs if
+ // AddTrack fails.
+ rtc::scoped_refptr<RtpSenderInterface> AddTrack(
+ rtc::scoped_refptr<MediaStreamTrackInterface> track,
+ const std::vector<std::string>& stream_labels = {});
+
// Calls the underlying PeerConnection's AddTrack method with an audio media
// stream track not bound to any source.
rtc::scoped_refptr<RtpSenderInterface> AddAudioTrack(