Signal track ID correctly when Unified Plan semantics selected
This change corrects PeerConnection behavior under Unified
Plan semantics to:
- Set the RtpSender id to be the track ID if created with AddTrack.
- Put the RtpSender id in the SDP as part of the MSID.
- Set the RtpReceiver id to be the track part of the MSID
when created via SetRemoteDescription.
Also, the RtpSender constructors have been simplified to defer
mutable state (in this case, setting BaseChannels) to method calls.
Bug: webrtc:8721
Change-Id: Idc80965e2df7a803b8bbeec1d96de9ad95391cce
Reviewed-on: https://webrtc-review.googlesource.com/38480
Commit-Queue: Steve Anton <steveanton@webrtc.org>
Reviewed-by: Taylor Brandstetter <deadbeef@webrtc.org>
Cr-Commit-Position: refs/heads/master@{#21563}diff --git a/ortc/ortcrtpsenderadapter.cc b/ortc/ortcrtpsenderadapter.cc
index 869a7cd..306db64 100644
--- a/ortc/ortcrtpsenderadapter.cc
+++ b/ortc/ortcrtpsenderadapter.cc
@@ -155,14 +155,18 @@
void OrtcRtpSenderAdapter::CreateInternalSender() {
switch (kind_) {
- case cricket::MEDIA_TYPE_AUDIO:
- internal_sender_ = new AudioRtpSender(
- rtp_transport_controller_->voice_channel(), nullptr);
+ case cricket::MEDIA_TYPE_AUDIO: {
+ auto* audio_sender = new AudioRtpSender(nullptr);
+ audio_sender->SetChannel(rtp_transport_controller_->voice_channel());
+ internal_sender_ = audio_sender;
break;
- case cricket::MEDIA_TYPE_VIDEO:
- internal_sender_ =
- new VideoRtpSender(rtp_transport_controller_->video_channel());
+ }
+ case cricket::MEDIA_TYPE_VIDEO: {
+ auto* video_sender = new VideoRtpSender();
+ video_sender->SetChannel(rtp_transport_controller_->video_channel());
+ internal_sender_ = video_sender;
break;
+ }
case cricket::MEDIA_TYPE_DATA:
RTC_NOTREACHED();
}
diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc
index 3bab2e1d..d6b4414 100644
--- a/pc/peerconnection.cc
+++ b/pc/peerconnection.cc
@@ -1066,36 +1066,34 @@
PeerConnection::AddTrackPlanB(
rtc::scoped_refptr<MediaStreamTrackInterface> track,
const std::vector<std::string>& stream_labels) {
+ cricket::MediaType media_type =
+ (track->kind() == MediaStreamTrackInterface::kAudioKind
+ ? cricket::MEDIA_TYPE_AUDIO
+ : cricket::MEDIA_TYPE_VIDEO);
+ auto new_sender = CreateSender(media_type, track, stream_labels);
if (track->kind() == MediaStreamTrackInterface::kAudioKind) {
- auto new_sender = RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
- signaling_thread(),
- new AudioRtpSender(static_cast<AudioTrackInterface*>(track.get()),
- voice_channel(), stats_.get()));
+ static_cast<AudioRtpSender*>(new_sender->internal())
+ ->SetChannel(voice_channel());
GetAudioTransceiver()->internal()->AddSender(new_sender);
- new_sender->internal()->set_stream_ids(stream_labels);
const RtpSenderInfo* sender_info =
FindSenderInfo(local_audio_sender_infos_,
new_sender->internal()->stream_id(), track->id());
if (sender_info) {
new_sender->internal()->SetSsrc(sender_info->first_ssrc);
}
- return rtc::scoped_refptr<RtpSenderInterface>(new_sender);
} else {
RTC_DCHECK_EQ(MediaStreamTrackInterface::kVideoKind, track->kind());
- auto new_sender = RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
- signaling_thread(),
- new VideoRtpSender(static_cast<VideoTrackInterface*>(track.get()),
- video_channel()));
+ static_cast<VideoRtpSender*>(new_sender->internal())
+ ->SetChannel(video_channel());
GetVideoTransceiver()->internal()->AddSender(new_sender);
- new_sender->internal()->set_stream_ids(stream_labels);
const RtpSenderInfo* sender_info =
FindSenderInfo(local_video_sender_infos_,
new_sender->internal()->stream_id(), track->id());
if (sender_info) {
new_sender->internal()->SetSsrc(sender_info->first_ssrc);
}
- return rtc::scoped_refptr<RtpSenderInterface>(new_sender);
}
+ return rtc::scoped_refptr<RtpSenderInterface>(new_sender);
}
RTCErrorOr<rtc::scoped_refptr<RtpSenderInterface>>
@@ -1109,17 +1107,19 @@
} else if (transceiver->direction() == RtpTransceiverDirection::kInactive) {
transceiver->SetDirection(RtpTransceiverDirection::kSendOnly);
}
+ transceiver->sender()->SetTrack(track);
+ transceiver->internal()->sender_internal()->set_stream_ids(stream_labels);
} else {
cricket::MediaType media_type =
(track->kind() == MediaStreamTrackInterface::kAudioKind
? cricket::MEDIA_TYPE_AUDIO
: cricket::MEDIA_TYPE_VIDEO);
- transceiver = CreateTransceiver(media_type);
+ auto sender = CreateSender(media_type, track, stream_labels);
+ auto receiver = CreateReceiver(media_type, rtc::CreateRandomUuid());
+ transceiver = CreateAndAddTransceiver(sender, receiver);
transceiver->internal()->set_created_by_addtrack(true);
transceiver->SetDirection(RtpTransceiverDirection::kSendRecv);
}
- transceiver->sender()->SetTrack(track);
- transceiver->internal()->sender_internal()->set_stream_ids(stream_labels);
return transceiver->sender();
}
@@ -1259,43 +1259,76 @@
// TODO(bugs.webrtc.org/7600): Verify init.
- auto transceiver = CreateTransceiver(media_type);
- transceiver->SetDirection(init.direction);
- if (track) {
- transceiver->sender()->SetTrack(track);
+ if (init.stream_labels.size() > 1u) {
+ LOG_AND_RETURN_ERROR(RTCErrorType::UNSUPPORTED_PARAMETER,
+ "More than one stream is not yet supported.");
}
+ std::vector<std::string> stream_labels = {!init.stream_labels.empty()
+ ? init.stream_labels[0]
+ : rtc::CreateRandomUuid()};
+
+ auto sender = CreateSender(media_type, track, stream_labels);
+ auto receiver = CreateReceiver(media_type, rtc::CreateRandomUuid());
+ auto transceiver = CreateAndAddTransceiver(sender, receiver);
+ transceiver->internal()->set_direction(init.direction);
+
observer_->OnRenegotiationNeeded();
return rtc::scoped_refptr<RtpTransceiverInterface>(transceiver);
}
-rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>
-PeerConnection::CreateTransceiver(cricket::MediaType media_type) {
+rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>>
+PeerConnection::CreateSender(
+ cricket::MediaType media_type,
+ rtc::scoped_refptr<MediaStreamTrackInterface> track,
+ const std::vector<std::string>& stream_labels) {
rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> sender;
+ if (media_type == cricket::MEDIA_TYPE_AUDIO) {
+ RTC_DCHECK(!track ||
+ (track->kind() == MediaStreamTrackInterface::kAudioKind));
+ sender = RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
+ signaling_thread(),
+ new AudioRtpSender(static_cast<AudioTrackInterface*>(track.get()),
+ stream_labels, stats_.get()));
+ } else {
+ RTC_DCHECK_EQ(media_type, cricket::MEDIA_TYPE_VIDEO);
+ RTC_DCHECK(!track ||
+ (track->kind() == MediaStreamTrackInterface::kVideoKind));
+ sender = RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
+ signaling_thread(),
+ new VideoRtpSender(static_cast<VideoTrackInterface*>(track.get()),
+ stream_labels));
+ }
+ sender->internal()->set_stream_ids(stream_labels);
+ return sender;
+}
+
+rtc::scoped_refptr<RtpReceiverProxyWithInternal<RtpReceiverInternal>>
+PeerConnection::CreateReceiver(cricket::MediaType media_type,
+ const std::string& receiver_id) {
rtc::scoped_refptr<RtpReceiverProxyWithInternal<RtpReceiverInternal>>
receiver;
- std::string receiver_id = rtc::CreateRandomUuid();
- // TODO(bugs.webrtc.org/7600): Initializing the sender/receiver with a null
- // channel prevents users from calling SetParameters on them, which is needed
- // to be in compliance with the spec.
if (media_type == cricket::MEDIA_TYPE_AUDIO) {
- sender = RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
- signaling_thread(), new AudioRtpSender(nullptr, stats_.get()));
receiver = RtpReceiverProxyWithInternal<RtpReceiverInternal>::Create(
signaling_thread(),
new AudioRtpReceiver(worker_thread(), receiver_id, {}, 0, nullptr));
} else {
- RTC_DCHECK_EQ(cricket::MEDIA_TYPE_VIDEO, media_type);
- sender = RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
- signaling_thread(), new VideoRtpSender(nullptr));
+ RTC_DCHECK_EQ(media_type, cricket::MEDIA_TYPE_VIDEO);
receiver = RtpReceiverProxyWithInternal<RtpReceiverInternal>::Create(
signaling_thread(),
new VideoRtpReceiver(worker_thread(), receiver_id, {}, 0, nullptr));
}
- rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>
- transceiver = RtpTransceiverProxyWithInternal<RtpTransceiver>::Create(
- signaling_thread(), new RtpTransceiver(sender, receiver));
+ return receiver;
+}
+
+rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>
+PeerConnection::CreateAndAddTransceiver(
+ rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> sender,
+ rtc::scoped_refptr<RtpReceiverProxyWithInternal<RtpReceiverInternal>>
+ receiver) {
+ auto transceiver = RtpTransceiverProxyWithInternal<RtpTransceiver>::Create(
+ signaling_thread(), new RtpTransceiver(sender, receiver));
transceivers_.push_back(transceiver);
return transceiver;
}
@@ -1327,25 +1360,31 @@
return nullptr;
}
+ std::vector<std::string> stream_labels;
+ if (!stream_id.empty()) {
+ stream_labels.push_back(stream_id);
+ }
+
// TODO(steveanton): Move construction of the RtpSenders to RtpTransceiver.
rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> new_sender;
if (kind == MediaStreamTrackInterface::kAudioKind) {
+ auto* audio_sender =
+ new AudioRtpSender(nullptr, stream_labels, stats_.get());
+ audio_sender->SetChannel(voice_channel());
new_sender = RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
- signaling_thread(), new AudioRtpSender(voice_channel(), stats_.get()));
+ signaling_thread(), audio_sender);
GetAudioTransceiver()->internal()->AddSender(new_sender);
} else if (kind == MediaStreamTrackInterface::kVideoKind) {
+ auto* video_sender = new VideoRtpSender(nullptr, stream_labels);
+ video_sender->SetChannel(video_channel());
new_sender = RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
- signaling_thread(), new VideoRtpSender(video_channel()));
+ signaling_thread(), video_sender);
GetVideoTransceiver()->internal()->AddSender(new_sender);
} else {
RTC_LOG(LS_ERROR) << "CreateSender called with invalid kind: " << kind;
return nullptr;
}
- if (!stream_id.empty()) {
- new_sender->internal()->set_stream_id(stream_id);
- }
-
return new_sender;
}
@@ -2223,7 +2262,11 @@
// If no RtpTransceiver was found in the previous step, create one with a
// recvonly direction.
if (!transceiver) {
- transceiver = CreateTransceiver(media_desc->type());
+ auto sender =
+ CreateSender(media_desc->type(), nullptr, {rtc::CreateRandomUuid()});
+ auto receiver =
+ CreateReceiver(media_desc->type(), media_desc->streams()[0].id);
+ transceiver = CreateAndAddTransceiver(sender, receiver);
transceiver->internal()->set_direction(
RtpTransceiverDirection::kRecvOnly);
}
@@ -2770,11 +2813,10 @@
}
// Normal case; we've never seen this track before.
- rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> new_sender =
- RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
- signaling_thread(),
- new AudioRtpSender(track, {stream->label()}, voice_channel(),
- stats_.get()));
+ auto new_sender =
+ CreateSender(cricket::MEDIA_TYPE_AUDIO, track, {stream->label()});
+ static_cast<AudioRtpSender*>(new_sender->internal())
+ ->SetChannel(voice_channel());
GetAudioTransceiver()->internal()->AddSender(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
@@ -2815,10 +2857,10 @@
}
// Normal case; we've never seen this track before.
- rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> new_sender =
- RtpSenderProxyWithInternal<RtpSenderInternal>::Create(
- signaling_thread(),
- new VideoRtpSender(track, {stream->label()}, video_channel()));
+ auto new_sender =
+ CreateSender(cricket::MEDIA_TYPE_VIDEO, track, {stream->label()});
+ static_cast<VideoRtpSender*>(new_sender->internal())
+ ->SetChannel(video_channel());
GetVideoTransceiver()->internal()->AddSender(new_sender);
const RtpSenderInfo* sender_info =
FindSenderInfo(local_video_sender_infos_, stream->label(), track->id());
@@ -5277,6 +5319,30 @@
}
}
+ // Unified Plan SDP should have exactly one stream per m= section for audio
+ // and video.
+ if (IsUnifiedPlan()) {
+ for (const ContentInfo& content : sdesc->description()->contents()) {
+ if (content.rejected) {
+ continue;
+ }
+ if (content.media_description()) {
+ cricket::MediaType media_type = content.media_description()->type();
+ if (!(media_type == cricket::MEDIA_TYPE_AUDIO ||
+ media_type == cricket::MEDIA_TYPE_VIDEO)) {
+ continue;
+ }
+ const cricket::StreamParamsVec& streams =
+ content.media_description()->streams();
+ if (streams.size() != 1u) {
+ LOG_AND_RETURN_ERROR(RTCErrorType::INVALID_PARAMETER,
+ "Unified Plan SDP should have exactly one "
+ "track per media section for audio and video.");
+ }
+ }
+ }
+ }
+
return RTCError::OK();
}
diff --git a/pc/peerconnection.h b/pc/peerconnection.h
index 4c7973b..f2f833c 100644
--- a/pc/peerconnection.h
+++ b/pc/peerconnection.h
@@ -379,10 +379,21 @@
rtc::scoped_refptr<MediaStreamTrackInterface> track,
const RtpTransceiverInit& init);
+ rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>>
+ CreateSender(cricket::MediaType media_type,
+ rtc::scoped_refptr<MediaStreamTrackInterface> track,
+ const std::vector<std::string>& stream_labels);
+
+ rtc::scoped_refptr<RtpReceiverProxyWithInternal<RtpReceiverInternal>>
+ CreateReceiver(cricket::MediaType media_type, const std::string& receiver_id);
+
// Create a new RtpTransceiver of the given type and add it to the list of
// transceivers.
rtc::scoped_refptr<RtpTransceiverProxyWithInternal<RtpTransceiver>>
- CreateTransceiver(cricket::MediaType media_type);
+ CreateAndAddTransceiver(
+ rtc::scoped_refptr<RtpSenderProxyWithInternal<RtpSenderInternal>> sender,
+ rtc::scoped_refptr<RtpReceiverProxyWithInternal<RtpReceiverInternal>>
+ receiver);
void SetIceConnectionState(IceConnectionState new_state);
// Called any time the IceGatheringState changes
diff --git a/pc/peerconnection_jsep_unittest.cc b/pc/peerconnection_jsep_unittest.cc
index 3ab8b5a..edab082 100644
--- a/pc/peerconnection_jsep_unittest.cc
+++ b/pc/peerconnection_jsep_unittest.cc
@@ -242,6 +242,18 @@
EXPECT_EQ(video_mid, video_transceiver->mid());
}
+TEST_F(PeerConnectionJsepTest, SetLocalOfferFailsWithNoTrackInMediaSection) {
+ auto caller = CreatePeerConnection();
+ caller->AddAudioTrack("audio");
+
+ auto offer = caller->CreateOffer();
+ auto& contents = offer->description()->contents();
+ ASSERT_EQ(1u, contents.size());
+ contents[0].description->mutable_streams().clear();
+
+ ASSERT_FALSE(caller->SetLocalDescription(std::move(offer)));
+}
+
// Tests for JSEP SetRemoteDescription with a remote offer.
// Test that setting a remote offer with sendrecv audio and video creates two
@@ -876,4 +888,77 @@
EXPECT_EQ(2u, callee->pc()->GetTransceivers().size());
}
+// Tests for MSID properties.
+
+// Test that adding a track with AddTrack results in an offer that signals the
+// track's ID.
+TEST_F(PeerConnectionJsepTest, AddingTrackWithAddTrackSpecifiesTrackId) {
+ const std::string kTrackId = "audio_track";
+
+ auto caller = CreatePeerConnection();
+ caller->AddAudioTrack(kTrackId);
+
+ auto offer = caller->CreateOffer();
+ auto contents = offer->description()->contents();
+ ASSERT_EQ(1u, contents.size());
+ auto streams = contents[0].media_description()->streams();
+ ASSERT_EQ(1u, streams.size());
+ EXPECT_EQ(kTrackId, streams[0].id);
+}
+
+// Test that adding a track by calling AddTransceiver then SetTrack results in
+// an offer that does not signal the track's ID and signals a random ID.
+TEST_F(PeerConnectionJsepTest,
+ AddingTrackWithAddTransceiverSpecifiesRandomTrackId) {
+ const std::string kTrackId = "audio_track";
+
+ auto caller = CreatePeerConnection();
+ auto transceiver = caller->AddTransceiver(cricket::MEDIA_TYPE_AUDIO);
+ transceiver->sender()->SetTrack(caller->CreateAudioTrack(kTrackId));
+
+ auto offer = caller->CreateOffer();
+ auto contents = offer->description()->contents();
+ ASSERT_EQ(1u, contents.size());
+ auto streams = contents[0].media_description()->streams();
+ ASSERT_EQ(1u, streams.size());
+ EXPECT_NE(kTrackId, streams[0].id);
+}
+
+// Test that the callee RtpReceiver created by a call to SetRemoteDescription
+// has its ID set to the signaled track ID.
+TEST_F(PeerConnectionJsepTest,
+ RtpReceiverCreatedBySetRemoteDescriptionHasSignaledTrackId) {
+ const std::string kTrackId = "audio_track";
+
+ auto caller = CreatePeerConnection();
+ auto callee = CreatePeerConnection();
+ caller->AddAudioTrack(kTrackId);
+
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
+
+ ASSERT_EQ(1u, callee->pc()->GetReceivers().size());
+ auto receiver = callee->pc()->GetReceivers()[0];
+ EXPECT_EQ(kTrackId, receiver->id());
+}
+
+// Test that if the callee RtpReceiver is reused by a call to
+// SetRemoteDescription, its ID does not change.
+TEST_F(PeerConnectionJsepTest,
+ RtpReceiverCreatedBeforeSetRemoteDescriptionKeepsId) {
+ const std::string kTrackId = "audio_track";
+
+ auto caller = CreatePeerConnection();
+ auto callee = CreatePeerConnection();
+ caller->AddAudioTrack(kTrackId);
+ callee->AddAudioTrack("dummy_track");
+
+ ASSERT_EQ(1u, callee->pc()->GetReceivers().size());
+ auto receiver = callee->pc()->GetReceivers()[0];
+ std::string receiver_id = receiver->id();
+
+ ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal()));
+
+ EXPECT_EQ(receiver_id, receiver->id());
+}
+
} // namespace webrtc
diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc
index 78e18c3..f418f7e 100644
--- a/pc/peerconnection_rtp_unittest.cc
+++ b/pc/peerconnection_rtp_unittest.cc
@@ -736,6 +736,17 @@
EXPECT_EQ(RtpTransceiverDirection::kSendRecv, transceiver->direction());
}
+TEST_F(PeerConnectionRtpUnifiedPlanTest, AddTrackCreatesSenderWithTrackId) {
+ const std::string kTrackId = "audio_track";
+
+ auto caller = CreatePeerConnectionWithUnifiedPlan();
+
+ auto audio_track = caller->CreateAudioTrack(kTrackId);
+ auto sender = caller->AddTrack(audio_track);
+
+ EXPECT_EQ(kTrackId, sender->id());
+}
+
// Unified Plan AddTrack error handling.
TEST_F(PeerConnectionRtpUnifiedPlanTest, AddTrackErrorIfClosed) {
diff --git a/pc/rtpsender.cc b/pc/rtpsender.cc
index 9c1d3b6..91e84d7 100644
--- a/pc/rtpsender.cc
+++ b/pc/rtpsender.cc
@@ -46,50 +46,28 @@
sink_ = sink;
}
-AudioRtpSender::AudioRtpSender(rtc::scoped_refptr<AudioTrackInterface> track,
- const std::vector<std::string>& stream_ids,
- cricket::VoiceChannel* channel,
- StatsCollector* stats)
- : id_(track->id()),
- stream_ids_(stream_ids),
- channel_(channel),
- stats_(stats),
- track_(track),
- cached_track_enabled_(track->enabled()),
- sink_adapter_(new LocalAudioSinkAdapter()) {
- // TODO(steveanton): Relax this constraint once more Unified Plan work is
- // done.
- RTC_CHECK(stream_ids_.size() == 1U);
- track_->RegisterObserver(this);
- track_->AddSink(sink_adapter_.get());
- CreateDtmfSender();
-}
+AudioRtpSender::AudioRtpSender(StatsCollector* stats)
+ : AudioRtpSender(nullptr, {rtc::CreateRandomUuid()}, stats) {}
AudioRtpSender::AudioRtpSender(rtc::scoped_refptr<AudioTrackInterface> track,
- cricket::VoiceChannel* channel,
+ const std::vector<std::string>& stream_labels,
StatsCollector* stats)
- : id_(track->id()),
- // TODO(steveanton): With Unified Plan this should be empty.
- stream_ids_({rtc::CreateRandomUuid()}),
- channel_(channel),
+ : id_(track ? track->id() : rtc::CreateRandomUuid()),
+ stream_ids_(stream_labels),
stats_(stats),
track_(track),
- cached_track_enabled_(track->enabled()),
+ dtmf_sender_proxy_(DtmfSenderProxy::Create(
+ rtc::Thread::Current(),
+ DtmfSender::Create(track_, rtc::Thread::Current(), this))),
+ cached_track_enabled_(track ? track->enabled() : false),
sink_adapter_(new LocalAudioSinkAdapter()) {
- track_->RegisterObserver(this);
- track_->AddSink(sink_adapter_.get());
- CreateDtmfSender();
-}
-
-AudioRtpSender::AudioRtpSender(cricket::VoiceChannel* channel,
- StatsCollector* stats)
- : id_(rtc::CreateRandomUuid()),
- // TODO(steveanton): With Unified Plan this should be empty.
- stream_ids_({rtc::CreateRandomUuid()}),
- channel_(channel),
- stats_(stats),
- sink_adapter_(new LocalAudioSinkAdapter()) {
- CreateDtmfSender();
+ // TODO(bugs.webrtc.org/7932): Remove once zero or multiple streams are
+ // supported.
+ RTC_DCHECK_EQ(stream_labels.size(), 1u);
+ if (track_) {
+ track_->RegisterObserver(this);
+ track_->AddSink(sink_adapter_.get());
+ }
}
AudioRtpSender::~AudioRtpSender() {
@@ -289,53 +267,26 @@
}
}
-void AudioRtpSender::CreateDtmfSender() {
- // Should be on signaling thread.
- // TODO(deadbeef): Add thread checking to RtpSender/RtpReceiver
- // implementations.
- rtc::scoped_refptr<DtmfSenderInterface> sender(
- DtmfSender::Create(track_, rtc::Thread::Current(), this));
- if (!sender.get()) {
- RTC_LOG(LS_ERROR) << "CreateDtmfSender failed on DtmfSender::Create.";
- RTC_NOTREACHED();
+VideoRtpSender::VideoRtpSender()
+ : VideoRtpSender(nullptr, {rtc::CreateRandomUuid()}) {}
+
+VideoRtpSender::VideoRtpSender(rtc::scoped_refptr<VideoTrackInterface> track,
+ const std::vector<std::string>& stream_labels)
+ : id_(track ? track->id() : rtc::CreateRandomUuid()),
+ stream_ids_(stream_labels),
+ track_(track),
+ cached_track_enabled_(track ? track->enabled() : false),
+ cached_track_content_hint_(
+ track ? track->content_hint()
+ : VideoTrackInterface::ContentHint::kNone) {
+ // TODO(bugs.webrtc.org/7932): Remove once zero or multiple streams are
+ // supported.
+ RTC_DCHECK_EQ(stream_labels.size(), 1u);
+ if (track_) {
+ track_->RegisterObserver(this);
}
- dtmf_sender_proxy_ =
- DtmfSenderProxy::Create(rtc::Thread::Current(), sender.get());
}
-VideoRtpSender::VideoRtpSender(rtc::scoped_refptr<VideoTrackInterface> track,
- const std::vector<std::string>& stream_ids,
- cricket::VideoChannel* channel)
- : id_(track->id()),
- stream_ids_({stream_ids}),
- channel_(channel),
- track_(track),
- cached_track_enabled_(track->enabled()),
- cached_track_content_hint_(track->content_hint()) {
- // TODO(steveanton): Relax this constraint once more Unified Plan work is
- // done.
- RTC_CHECK(stream_ids_.size() == 1U);
- track_->RegisterObserver(this);
-}
-
-VideoRtpSender::VideoRtpSender(rtc::scoped_refptr<VideoTrackInterface> track,
- cricket::VideoChannel* channel)
- : id_(track->id()),
- // TODO(steveanton): With Unified Plan this should be empty.
- stream_ids_({rtc::CreateRandomUuid()}),
- channel_(channel),
- track_(track),
- cached_track_enabled_(track->enabled()),
- cached_track_content_hint_(track->content_hint()) {
- track_->RegisterObserver(this);
-}
-
-VideoRtpSender::VideoRtpSender(cricket::VideoChannel* channel)
- : id_(rtc::CreateRandomUuid()),
- // TODO(steveanton): With Unified Plan this should be empty.
- stream_ids_({rtc::CreateRandomUuid()}),
- channel_(channel) {}
-
VideoRtpSender::~VideoRtpSender() {
Stop();
}
diff --git a/pc/rtpsender.h b/pc/rtpsender.h
index 4eeb4d5..a0c759a 100644
--- a/pc/rtpsender.h
+++ b/pc/rtpsender.h
@@ -82,21 +82,16 @@
public:
// StatsCollector provided so that Add/RemoveLocalAudioTrack can be called
// at the appropriate times.
- // |channel| can be null if one does not exist yet.
- AudioRtpSender(rtc::scoped_refptr<AudioTrackInterface> track,
- const std::vector<std::string>& stream_id,
- cricket::VoiceChannel* channel,
- StatsCollector* stats);
- // Randomly generates stream_id.
- // |channel| can be null if one does not exist yet.
- AudioRtpSender(rtc::scoped_refptr<AudioTrackInterface> track,
- cricket::VoiceChannel* channel,
- StatsCollector* stats);
+ // Construct an AudioRtpSender with a null track, a single, randomly generated
+ // stream label, and a randomly generated ID.
+ explicit AudioRtpSender(StatsCollector* stats);
- // Randomly generates id and stream_id.
- // |channel| can be null if one does not exist yet.
- AudioRtpSender(cricket::VoiceChannel* channel, StatsCollector* stats);
+ // Construct an AudioRtpSender with the given track and stream labels. The
+ // sender ID will be set to the track's ID.
+ AudioRtpSender(rtc::scoped_refptr<AudioTrackInterface> track,
+ const std::vector<std::string>& stream_labels,
+ StatsCollector* stats);
virtual ~AudioRtpSender();
@@ -156,11 +151,9 @@
// Helper function to call SetAudioSend with "stop sending" parameters.
void ClearAudioSend();
- void CreateDtmfSender();
-
sigslot::signal0<> SignalDestroyed;
- std::string id_;
+ const std::string id_;
// TODO(steveanton): Until more Unified Plan work is done, this can only have
// exactly one element.
std::vector<std::string> stream_ids_;
@@ -180,19 +173,14 @@
class VideoRtpSender : public ObserverInterface,
public rtc::RefCountedObject<RtpSenderInternal> {
public:
- // |channel| can be null if one does not exist yet.
- VideoRtpSender(rtc::scoped_refptr<VideoTrackInterface> track,
- const std::vector<std::string>& stream_id,
- cricket::VideoChannel* channel);
+ // Construct a VideoRtpSender with a null track, a single, randomly generated
+ // stream label, and a randomly generated ID.
+ VideoRtpSender();
- // Randomly generates stream_id.
- // |channel| can be null if one does not exist yet.
+ // Construct a VideoRtpSender with the given track and stream labels. The
+ // sender ID will be set to the track's ID.
VideoRtpSender(rtc::scoped_refptr<VideoTrackInterface> track,
- cricket::VideoChannel* channel);
-
- // Randomly generates id and stream_id.
- // |channel| can be null if one does not exist yet.
- explicit VideoRtpSender(cricket::VideoChannel* channel);
+ const std::vector<std::string>& stream_labels);
virtual ~VideoRtpSender();
@@ -245,7 +233,7 @@
// Helper function to call SetVideoSend with "stop sending" parameters.
void ClearVideoSend();
- std::string id_;
+ const std::string id_;
// TODO(steveanton): Until more Unified Plan work is done, this can only have
// exactly one element.
std::vector<std::string> stream_ids_;
diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc
index f1ba6d8..9ccbe95 100644
--- a/pc/rtpsenderreceiver_unittest.cc
+++ b/pc/rtpsenderreceiver_unittest.cc
@@ -134,28 +134,38 @@
const rtc::scoped_refptr<LocalAudioSource>& source) {
audio_track_ = AudioTrack::Create(kAudioTrackId, source);
EXPECT_TRUE(local_stream_->AddTrack(audio_track_));
- audio_rtp_sender_ =
- new AudioRtpSender(local_stream_->GetAudioTracks()[0],
- {local_stream_->label()}, voice_channel_, nullptr);
+ audio_rtp_sender_ = new AudioRtpSender(local_stream_->GetAudioTracks()[0],
+ {local_stream_->label()}, nullptr);
+ audio_rtp_sender_->SetChannel(voice_channel_);
audio_rtp_sender_->SetSsrc(kAudioSsrc);
audio_rtp_sender_->GetOnDestroyedSignal()->connect(
this, &RtpSenderReceiverTest::OnAudioSenderDestroyed);
VerifyVoiceChannelInput();
}
+ void CreateAudioRtpSenderWithNoTrack() {
+ audio_rtp_sender_ = new AudioRtpSender(nullptr);
+ audio_rtp_sender_->SetChannel(voice_channel_);
+ }
+
void OnAudioSenderDestroyed() { audio_sender_destroyed_signal_fired_ = true; }
void CreateVideoRtpSender() { CreateVideoRtpSender(false); }
void CreateVideoRtpSender(bool is_screencast) {
AddVideoTrack(is_screencast);
- video_rtp_sender_ =
- new VideoRtpSender(local_stream_->GetVideoTracks()[0],
- {local_stream_->label()}, video_channel_);
+ video_rtp_sender_ = new VideoRtpSender(local_stream_->GetVideoTracks()[0],
+ {local_stream_->label()});
+ video_rtp_sender_->SetChannel(video_channel_);
video_rtp_sender_->SetSsrc(kVideoSsrc);
VerifyVideoChannelInput();
}
+ void CreateVideoRtpSenderWithNoTrack() {
+ video_rtp_sender_ = new VideoRtpSender();
+ video_rtp_sender_->SetChannel(video_channel_);
+ }
+
void DestroyAudioRtpSender() {
audio_rtp_sender_ = nullptr;
VerifyVoiceChannelNoInput();
@@ -425,7 +435,7 @@
// Test that the media channel isn't enabled for sending if the audio sender
// doesn't have both a track and SSRC.
TEST_F(RtpSenderReceiverTest, AudioSenderWithoutTrackAndSsrc) {
- audio_rtp_sender_ = new AudioRtpSender(voice_channel_, nullptr);
+ CreateAudioRtpSenderWithNoTrack();
rtc::scoped_refptr<AudioTrackInterface> track =
AudioTrack::Create(kAudioTrackId, nullptr);
@@ -442,7 +452,7 @@
// Test that the media channel isn't enabled for sending if the video sender
// doesn't have both a track and SSRC.
TEST_F(RtpSenderReceiverTest, VideoSenderWithoutTrackAndSsrc) {
- video_rtp_sender_ = new VideoRtpSender(video_channel_);
+ CreateVideoRtpSenderWithNoTrack();
// Track but no SSRC.
EXPECT_TRUE(video_rtp_sender_->SetTrack(video_track_));
@@ -457,7 +467,7 @@
// Test that the media channel is enabled for sending when the audio sender
// has a track and SSRC, when the SSRC is set first.
TEST_F(RtpSenderReceiverTest, AudioSenderEarlyWarmupSsrcThenTrack) {
- audio_rtp_sender_ = new AudioRtpSender(voice_channel_, nullptr);
+ CreateAudioRtpSenderWithNoTrack();
rtc::scoped_refptr<AudioTrackInterface> track =
AudioTrack::Create(kAudioTrackId, nullptr);
audio_rtp_sender_->SetSsrc(kAudioSsrc);
@@ -470,7 +480,7 @@
// Test that the media channel is enabled for sending when the audio sender
// has a track and SSRC, when the SSRC is set last.
TEST_F(RtpSenderReceiverTest, AudioSenderEarlyWarmupTrackThenSsrc) {
- audio_rtp_sender_ = new AudioRtpSender(voice_channel_, nullptr);
+ CreateAudioRtpSenderWithNoTrack();
rtc::scoped_refptr<AudioTrackInterface> track =
AudioTrack::Create(kAudioTrackId, nullptr);
audio_rtp_sender_->SetTrack(track);
@@ -484,7 +494,7 @@
// has a track and SSRC, when the SSRC is set first.
TEST_F(RtpSenderReceiverTest, VideoSenderEarlyWarmupSsrcThenTrack) {
AddVideoTrack();
- video_rtp_sender_ = new VideoRtpSender(video_channel_);
+ CreateVideoRtpSenderWithNoTrack();
video_rtp_sender_->SetSsrc(kVideoSsrc);
video_rtp_sender_->SetTrack(video_track_);
VerifyVideoChannelInput();
@@ -496,7 +506,7 @@
// has a track and SSRC, when the SSRC is set last.
TEST_F(RtpSenderReceiverTest, VideoSenderEarlyWarmupTrackThenSsrc) {
AddVideoTrack();
- video_rtp_sender_ = new VideoRtpSender(video_channel_);
+ CreateVideoRtpSenderWithNoTrack();
video_rtp_sender_->SetTrack(video_track_);
video_rtp_sender_->SetSsrc(kVideoSsrc);
VerifyVideoChannelInput();
@@ -762,9 +772,9 @@
// Setting detailed overrides the default non-screencast mode. This should be
// applied even if the track is set on construction.
video_track_->set_content_hint(VideoTrackInterface::ContentHint::kDetailed);
- video_rtp_sender_ =
- new VideoRtpSender(local_stream_->GetVideoTracks()[0],
- {local_stream_->label()}, video_channel_);
+ video_rtp_sender_ = new VideoRtpSender(local_stream_->GetVideoTracks()[0],
+ {local_stream_->label()});
+ video_rtp_sender_->SetChannel(video_channel_);
video_track_->set_enabled(true);
// Sender is not ready to send (no SSRC) so no option should have been set.
