From 13b8bad23543c59e57428a642aa31a24f0923d76 Mon Sep 17 00:00:00 2001 From: Seth Hampson Date: Tue, 13 Mar 2018 16:05:28 -0700 Subject: [PATCH] Final name changing of MediaStreamInterface.label() to id(). Downstreams have been updated, and this now updates all uses of label() to id() within WebRTC code. This change also makes id() pure virtual and removes label(). Bug: webrtc:8977 Change-Id: Ib045ea4fabba6f14447c64875c7aeba87dc2be24 Reviewed-on: https://webrtc-review.googlesource.com/60382 Reviewed-by: Per Kjellander Reviewed-by: Taylor Brandstetter Commit-Queue: Seth Hampson Cr-Commit-Position: refs/heads/master@{#22431} --- api/mediastreaminterface.cc | 11 ---------- api/mediastreaminterface.h | 5 +---- api/mediastreamproxy.h | 1 - examples/peerconnection/client/conductor.cc | 6 +++--- .../unityplugin/simple_peer_connection.cc | 4 ++-- pc/mediastream.h | 1 - pc/peerconnection.cc | 21 +++++++++---------- pc/peerconnection_jsep_unittest.cc | 2 +- pc/peerconnection_rtp_unittest.cc | 4 ++-- pc/peerconnectioninterface_unittest.cc | 6 +++--- pc/rtcstatscollector.cc | 2 +- pc/rtcstatscollector_unittest.cc | 12 +++++------ pc/rtpreceiver.cc | 8 +++---- pc/rtpsenderreceiver_unittest.cc | 10 ++++----- pc/streamcollection.h | 8 +++---- pc/test/peerconnectiontestwrapper.cc | 6 ++---- 16 files changed, 44 insertions(+), 63 deletions(-) diff --git a/api/mediastreaminterface.cc b/api/mediastreaminterface.cc index 3d4eb66789..6f08a0caf1 100644 --- a/api/mediastreaminterface.cc +++ b/api/mediastreaminterface.cc @@ -58,15 +58,4 @@ AudioTrackInterface::GetAudioProcessor() { return nullptr; } -// TODO(shampson): Remove this once downstreams are using id(). -std::string MediaStreamInterface::label() const { - return id(); -} - -// TODO(shampson): Remove this default implementation once downstreams have -// implemented. -std::string MediaStreamInterface::id() const { - return std::string(); -} - } // namespace webrtc diff --git a/api/mediastreaminterface.h b/api/mediastreaminterface.h index e2412deec3..2e2cff00f1 100644 --- a/api/mediastreaminterface.h +++ b/api/mediastreaminterface.h @@ -313,10 +313,7 @@ typedef std::vector > class MediaStreamInterface : public rtc::RefCountInterface, public NotifierInterface { public: - // TODO(shampson): Remove once downstreams are updated to use id(). - virtual std::string label() const; - // TODO(shampson): Make pure virtual once downstreams have implemented. - virtual std::string id() const; + virtual std::string id() const = 0; virtual AudioTrackVector GetAudioTracks() = 0; virtual VideoTrackVector GetVideoTracks() = 0; diff --git a/api/mediastreamproxy.h b/api/mediastreamproxy.h index 848b75b27b..3f261db0a8 100644 --- a/api/mediastreamproxy.h +++ b/api/mediastreamproxy.h @@ -22,7 +22,6 @@ namespace webrtc { // are called on is an implementation detail. BEGIN_SIGNALING_PROXY_MAP(MediaStream) PROXY_SIGNALING_THREAD_DESTRUCTOR() - PROXY_CONSTMETHOD0(std::string, label) PROXY_CONSTMETHOD0(std::string, id) PROXY_METHOD0(AudioTrackVector, GetAudioTracks) PROXY_METHOD0(VideoTrackVector, GetVideoTracks) diff --git a/examples/peerconnection/client/conductor.cc b/examples/peerconnection/client/conductor.cc index 16dc6be20d..5e56592cf2 100644 --- a/examples/peerconnection/client/conductor.cc +++ b/examples/peerconnection/client/conductor.cc @@ -162,13 +162,13 @@ void Conductor::EnsureStreamingUI() { // Called when a remote stream is added void Conductor::OnAddStream( rtc::scoped_refptr stream) { - RTC_LOG(INFO) << __FUNCTION__ << " " << stream->label(); + RTC_LOG(INFO) << __FUNCTION__ << " " << stream->id(); main_wnd_->QueueUIThreadCallback(NEW_STREAM_ADDED, stream.release()); } void Conductor::OnRemoveStream( rtc::scoped_refptr stream) { - RTC_LOG(INFO) << __FUNCTION__ << " " << stream->label(); + RTC_LOG(INFO) << __FUNCTION__ << " " << stream->id(); main_wnd_->QueueUIThreadCallback(STREAM_REMOVED, stream.release()); } @@ -441,7 +441,7 @@ void Conductor::AddStreams() { typedef std::pair > MediaStreamPair; - active_streams_.insert(MediaStreamPair(stream->label(), stream)); + active_streams_.insert(MediaStreamPair(stream->id(), stream)); main_wnd_->SwitchToStreamingUI(); } diff --git a/examples/unityplugin/simple_peer_connection.cc b/examples/unityplugin/simple_peer_connection.cc index a830f91b37..2b8ef93a5d 100644 --- a/examples/unityplugin/simple_peer_connection.cc +++ b/examples/unityplugin/simple_peer_connection.cc @@ -378,7 +378,7 @@ void SimplePeerConnection::SetAudioControl() { void SimplePeerConnection::OnAddStream( rtc::scoped_refptr stream) { - RTC_LOG(INFO) << __FUNCTION__ << " " << stream->label(); + RTC_LOG(INFO) << __FUNCTION__ << " " << stream->id(); remote_stream_ = stream; if (remote_video_observer_ && !remote_stream_->GetVideoTracks().empty()) { remote_stream_->GetVideoTracks()[0]->AddOrUpdateSink( @@ -491,7 +491,7 @@ void SimplePeerConnection::AddStreams(bool audio_only) { typedef std::pair> MediaStreamPair; - active_streams_.insert(MediaStreamPair(stream->label(), stream)); + active_streams_.insert(MediaStreamPair(stream->id(), stream)); } bool SimplePeerConnection::CreateDataChannel() { diff --git a/pc/mediastream.h b/pc/mediastream.h index 708b13d8d0..7f5d8e81be 100644 --- a/pc/mediastream.h +++ b/pc/mediastream.h @@ -25,7 +25,6 @@ class MediaStream : public Notifier { public: static rtc::scoped_refptr Create(const std::string& label); - std::string label() const override { return id_; } std::string id() const override { return id_; } bool AddTrack(AudioTrackInterface* track) override; diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index 7f1ddef78a..17977ad31b 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -142,8 +142,8 @@ bool CanAddLocalMediaStream(webrtc::StreamCollectionInterface* current_streams, if (!new_stream || !current_streams) { return false; } - if (current_streams->find(new_stream->label()) != nullptr) { - RTC_LOG(LS_ERROR) << "MediaStream with label " << new_stream->label() + if (current_streams->find(new_stream->id()) != nullptr) { + RTC_LOG(LS_ERROR) << "MediaStream with ID " << new_stream->id() << " is already added."; return false; } @@ -1091,8 +1091,7 @@ void PeerConnection::RemoveStream(MediaStreamInterface* local_stream) { std::remove_if( stream_observers_.begin(), stream_observers_.end(), [local_stream](const std::unique_ptr& observer) { - return observer->stream()->label().compare(local_stream->label()) == - 0; + return observer->stream()->id().compare(local_stream->id()) == 0; }), stream_observers_.end()); @@ -1112,7 +1111,7 @@ rtc::scoped_refptr PeerConnection::AddTrack( RTC_LOG(LS_ERROR) << "Stream list has null element."; return nullptr; } - stream_ids.push_back(stream->label()); + stream_ids.push_back(stream->id()); } auto sender_or_error = AddTrack(track, stream_ids); if (!sender_or_error.ok()) { @@ -3233,13 +3232,13 @@ void PeerConnection::AddAudioTrack(AudioTrackInterface* track, if (sender) { // 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->internal()->set_stream_id(stream->label()); + sender->internal()->set_stream_id(stream->id()); return; } // Normal case; we've never seen this track before. auto new_sender = - CreateSender(cricket::MEDIA_TYPE_AUDIO, track, {stream->label()}); + CreateSender(cricket::MEDIA_TYPE_AUDIO, track, {stream->id()}); new_sender->internal()->SetVoiceMediaChannel(voice_media_channel()); GetAudioTransceiver()->internal()->AddSender(new_sender); // If the sender has already been configured in SDP, we call SetSsrc, @@ -3249,7 +3248,7 @@ void PeerConnection::AddAudioTrack(AudioTrackInterface* track, // session description is not changed and RemoveStream is called, and // later AddStream is called again with the same stream. const RtpSenderInfo* sender_info = - FindSenderInfo(local_audio_sender_infos_, stream->label(), track->id()); + FindSenderInfo(local_audio_sender_infos_, stream->id(), track->id()); if (sender_info) { new_sender->internal()->SetSsrc(sender_info->first_ssrc); } @@ -3276,17 +3275,17 @@ void PeerConnection::AddVideoTrack(VideoTrackInterface* track, if (sender) { // 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->internal()->set_stream_id(stream->label()); + sender->internal()->set_stream_id(stream->id()); return; } // Normal case; we've never seen this track before. auto new_sender = - CreateSender(cricket::MEDIA_TYPE_VIDEO, track, {stream->label()}); + CreateSender(cricket::MEDIA_TYPE_VIDEO, track, {stream->id()}); new_sender->internal()->SetVideoMediaChannel(video_media_channel()); GetVideoTransceiver()->internal()->AddSender(new_sender); const RtpSenderInfo* sender_info = - FindSenderInfo(local_video_sender_infos_, stream->label(), track->id()); + FindSenderInfo(local_video_sender_infos_, stream->id(), track->id()); if (sender_info) { new_sender->internal()->SetSsrc(sender_info->first_ssrc); } diff --git a/pc/peerconnection_jsep_unittest.cc b/pc/peerconnection_jsep_unittest.cc index 6d3cf76b38..84bd060d6f 100644 --- a/pc/peerconnection_jsep_unittest.cc +++ b/pc/peerconnection_jsep_unittest.cc @@ -1187,7 +1187,7 @@ TEST_F(PeerConnectionJsepTest, const auto& event = track_events[0]; ASSERT_EQ(1u, event.streams.size()); auto stream = event.streams[0]; - EXPECT_EQ(kStreamId, stream->label()); + EXPECT_EQ(kStreamId, stream->id()); EXPECT_THAT(track_events[0].snapshotted_stream_tracks.at(stream), ElementsAre(event.receiver->track())); EXPECT_EQ(event.receiver->streams(), track_events[0].streams); diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc index 83a185836b..69f329cc1e 100644 --- a/pc/peerconnection_rtp_unittest.cc +++ b/pc/peerconnection_rtp_unittest.cc @@ -136,7 +136,7 @@ TEST_F(PeerConnectionRtpCallbacksTest, AddTrackWithStreamFiresOnAddTrack) { ASSERT_EQ(callee->observer()->add_track_events_.size(), 1u); auto& add_track_event = callee->observer()->add_track_events_[0]; ASSERT_EQ(add_track_event.streams.size(), 1u); - EXPECT_EQ("audio_stream", add_track_event.streams[0]->label()); + EXPECT_EQ("audio_stream", add_track_event.streams[0]->id()); EXPECT_TRUE(add_track_event.streams[0]->FindAudioTrack("audio_track")); EXPECT_EQ(add_track_event.streams, add_track_event.receiver->streams()); } @@ -355,7 +355,7 @@ TEST_F(PeerConnectionRtpObserverTest, AddSenderWithStreamAddsReceiver) { auto receiver_added = callee->pc()->GetReceivers()[0]; EXPECT_EQ("audio_track", receiver_added->track()->id()); EXPECT_EQ(receiver_added->streams().size(), 1u); - EXPECT_EQ("audio_stream", receiver_added->streams()[0]->label()); + EXPECT_EQ("audio_stream", receiver_added->streams()[0]->id()); EXPECT_TRUE(receiver_added->streams()[0]->FindAudioTrack("audio_track")); } diff --git a/pc/peerconnectioninterface_unittest.cc b/pc/peerconnectioninterface_unittest.cc index 198e1e55b1..62fd3bfbb5 100644 --- a/pc/peerconnectioninterface_unittest.cc +++ b/pc/peerconnectioninterface_unittest.cc @@ -569,7 +569,7 @@ bool CompareStreamCollections(StreamCollectionInterface* s1, } for (size_t i = 0; i != s1->count(); ++i) { - if (s1->at(i)->label() != s2->at(i)->label()) { + if (s1->at(i)->id() != s2->at(i)->id()) { return false; } webrtc::AudioTrackVector audio_tracks1 = s1->at(i)->GetAudioTracks(); @@ -2930,7 +2930,7 @@ TEST_F(PeerConnectionInterfaceTestPlanB, SdpWithoutMsidCreatesDefaultStream) { EXPECT_EQ(1u, remote_stream->GetAudioTracks().size()); EXPECT_EQ(0u, remote_stream->GetVideoTracks().size()); - EXPECT_EQ("default", remote_stream->label()); + EXPECT_EQ("default", remote_stream->id()); CreateAndSetRemoteOffer(kSdpStringWithoutStreams); ASSERT_EQ(1u, observer_.remote_streams()->count()); @@ -2960,7 +2960,7 @@ TEST_F(PeerConnectionInterfaceTestPlanB, EXPECT_EQ(1u, remote_stream->GetAudioTracks().size()); EXPECT_EQ(1u, remote_stream->GetVideoTracks().size()); - EXPECT_EQ("default", remote_stream->label()); + EXPECT_EQ("default", remote_stream->id()); } // This tests that it won't crash when PeerConnection tries to remove diff --git a/pc/rtcstatscollector.cc b/pc/rtcstatscollector.cc index e442ffe78c..e9e9f1c253 100644 --- a/pc/rtcstatscollector.cc +++ b/pc/rtcstatscollector.cc @@ -987,7 +987,7 @@ void RTCStatsCollector::ProduceMediaStreamStats_s( RTCMediaStreamTrackStatsIDFromDirectionAndAttachment( kReceiver, receiver->internal()->AttachmentId()); for (auto& stream : receiver->streams()) { - track_ids[stream->label()].push_back(track_id); + track_ids[stream->id()].push_back(track_id); } } } diff --git a/pc/rtcstatscollector_unittest.cc b/pc/rtcstatscollector_unittest.cc index 42b42dca0d..8ec0929fd1 100644 --- a/pc/rtcstatscollector_unittest.cc +++ b/pc/rtcstatscollector_unittest.cc @@ -1216,13 +1216,13 @@ TEST_F(RTCStatsCollectorTest, stats_->CreateMockRtpSendersReceiversAndChannels( {std::make_pair(local_audio_track.get(), voice_sender_info_ssrc1)}, {}, - {}, {}, {local_stream->label()}, {}); + {}, {}, {local_stream->id()}, {}); rtc::scoped_refptr report = stats_->GetStatsReport(); RTCMediaStreamStats expected_local_stream( IdForType(report), report->timestamp_us()); - expected_local_stream.stream_identifier = local_stream->label(); + expected_local_stream.stream_identifier = local_stream->id(); expected_local_stream.track_ids = { IdForType(report)}; ASSERT_TRUE(report->Get(expected_local_stream.id())) @@ -1284,7 +1284,7 @@ TEST_F(RTCStatsCollectorTest, RTCMediaStreamStats expected_remote_stream( IdForType(report), report->timestamp_us()); - expected_remote_stream.stream_identifier = remote_stream->label(); + expected_remote_stream.stream_identifier = remote_stream->id(); expected_remote_stream.track_ids = std::vector({IdForType(report)}); ASSERT_TRUE(report->Get(expected_remote_stream.id())) @@ -1338,7 +1338,7 @@ TEST_F(RTCStatsCollectorTest, stats_->CreateMockRtpSendersReceiversAndChannels( {}, {}, {std::make_pair(local_video_track.get(), video_sender_info_ssrc1)}, {}, - {local_stream->label()}, {}); + {local_stream->id()}, {}); rtc::scoped_refptr report = stats_->GetStatsReport(); @@ -1350,7 +1350,7 @@ TEST_F(RTCStatsCollectorTest, RTCMediaStreamStats expected_local_stream(stats_of_my_type[0]->id(), report->timestamp_us()); - expected_local_stream.stream_identifier = local_stream->label(); + expected_local_stream.stream_identifier = local_stream->id(); expected_local_stream.track_ids = std::vector({stats_of_track_type[0]->id()}); ASSERT_TRUE(report->Get(expected_local_stream.id())); @@ -1414,7 +1414,7 @@ TEST_F(RTCStatsCollectorTest, RTCMediaStreamStats expected_remote_stream(stats_of_my_type[0]->id(), report->timestamp_us()); - expected_remote_stream.stream_identifier = remote_stream->label(); + expected_remote_stream.stream_identifier = remote_stream->id(); expected_remote_stream.track_ids = std::vector({stats_of_track_type[0]->id()}); ASSERT_TRUE(report->Get(expected_remote_stream.id())); diff --git a/pc/rtpreceiver.cc b/pc/rtpreceiver.cc index bf0b1d68da..ee45c4e547 100644 --- a/pc/rtpreceiver.cc +++ b/pc/rtpreceiver.cc @@ -147,7 +147,7 @@ void AudioRtpReceiver::SetStreams( for (auto existing_stream : streams_) { bool removed = true; for (auto stream : streams) { - if (existing_stream->label() == stream->label()) { + if (existing_stream->id() == stream->id()) { RTC_DCHECK_EQ(existing_stream.get(), stream.get()); removed = false; break; @@ -161,7 +161,7 @@ void AudioRtpReceiver::SetStreams( for (auto stream : streams) { bool added = true; for (auto existing_stream : streams_) { - if (stream->label() == existing_stream->label()) { + if (stream->id() == existing_stream->id()) { RTC_DCHECK_EQ(stream.get(), existing_stream.get()); added = false; break; @@ -302,7 +302,7 @@ void VideoRtpReceiver::SetStreams( for (auto existing_stream : streams_) { bool removed = true; for (auto stream : streams) { - if (existing_stream->label() == stream->label()) { + if (existing_stream->id() == stream->id()) { RTC_DCHECK_EQ(existing_stream.get(), stream.get()); removed = false; break; @@ -316,7 +316,7 @@ void VideoRtpReceiver::SetStreams( for (auto stream : streams) { bool added = true; for (auto existing_stream : streams_) { - if (stream->label() == existing_stream->label()) { + if (stream->id() == existing_stream->id()) { RTC_DCHECK_EQ(stream.get(), existing_stream.get()); added = false; break; diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc index f856248595..279aba27af 100644 --- a/pc/rtpsenderreceiver_unittest.cc +++ b/pc/rtpsenderreceiver_unittest.cc @@ -138,7 +138,7 @@ class RtpSenderReceiverTest : public testing::Test, EXPECT_TRUE(local_stream_->AddTrack(audio_track_)); audio_rtp_sender_ = new AudioRtpSender(worker_thread_, local_stream_->GetAudioTracks()[0], - {local_stream_->label()}, nullptr); + {local_stream_->id()}, nullptr); audio_rtp_sender_->SetVoiceMediaChannel(voice_media_channel_); audio_rtp_sender_->SetSsrc(kAudioSsrc); audio_rtp_sender_->GetOnDestroyedSignal()->connect( @@ -159,7 +159,7 @@ class RtpSenderReceiverTest : public testing::Test, AddVideoTrack(is_screencast); video_rtp_sender_ = new VideoRtpSender(worker_thread_, local_stream_->GetVideoTracks()[0], - {local_stream_->label()}); + {local_stream_->id()}); video_rtp_sender_->SetVideoMediaChannel(video_media_channel_); video_rtp_sender_->SetSsrc(kVideoSsrc); VerifyVideoChannelInput(); @@ -780,9 +780,9 @@ TEST_F(RtpSenderReceiverTest, // 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(worker_thread_, - local_stream_->GetVideoTracks()[0], - {local_stream_->label()}); + video_rtp_sender_ = + new VideoRtpSender(worker_thread_, local_stream_->GetVideoTracks()[0], + {local_stream_->id()}); video_rtp_sender_->SetVideoMediaChannel(video_media_channel_); video_track_->set_enabled(true); diff --git a/pc/streamcollection.h b/pc/streamcollection.h index 19ddca991f..4c6f0b19c6 100644 --- a/pc/streamcollection.h +++ b/pc/streamcollection.h @@ -42,10 +42,10 @@ class StreamCollection : public StreamCollectionInterface { return media_streams_.at(index); } - virtual MediaStreamInterface* find(const std::string& label) { + virtual MediaStreamInterface* find(const std::string& id) { for (StreamVector::iterator it = media_streams_.begin(); it != media_streams_.end(); ++it) { - if ((*it)->label().compare(label) == 0) { + if ((*it)->id().compare(id) == 0) { return (*it); } } @@ -77,7 +77,7 @@ class StreamCollection : public StreamCollectionInterface { void AddStream(MediaStreamInterface* stream) { for (StreamVector::iterator it = media_streams_.begin(); it != media_streams_.end(); ++it) { - if ((*it)->label().compare(stream->label()) == 0) + if ((*it)->id().compare(stream->id()) == 0) return; } media_streams_.push_back(stream); @@ -86,7 +86,7 @@ class StreamCollection : public StreamCollectionInterface { void RemoveStream(MediaStreamInterface* remove_stream) { for (StreamVector::iterator it = media_streams_.begin(); it != media_streams_.end(); ++it) { - if ((*it)->label().compare(remove_stream->label()) == 0) { + if ((*it)->id().compare(remove_stream->id()) == 0) { media_streams_.erase(it); break; } diff --git a/pc/test/peerconnectiontestwrapper.cc b/pc/test/peerconnectiontestwrapper.cc index dfcbb85e87..1b872089b0 100644 --- a/pc/test/peerconnectiontestwrapper.cc +++ b/pc/test/peerconnectiontestwrapper.cc @@ -250,12 +250,10 @@ void PeerConnectionTestWrapper::GetAndAddUserMedia( rtc::scoped_refptr stream = GetUserMedia(audio, audio_constraints, video, video_constraints); for (auto audio_track : stream->GetAudioTracks()) { - EXPECT_TRUE( - peer_connection_->AddTrack(audio_track, {stream->label()}).ok()); + EXPECT_TRUE(peer_connection_->AddTrack(audio_track, {stream->id()}).ok()); } for (auto video_track : stream->GetVideoTracks()) { - EXPECT_TRUE( - peer_connection_->AddTrack(video_track, {stream->label()}).ok()); + EXPECT_TRUE(peer_connection_->AddTrack(video_track, {stream->id()}).ok()); } }