From 9e6fd2bd4799e9abb8561eb552c2d87ea3e1ef32 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Henrik=20Bostr=C3=B6m?= Date: Tue, 21 Nov 2017 13:41:51 +0100 Subject: [PATCH] Add streams() to RtpReceiverInterface and implementations. MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This makes the receiver know about its associated set of streams, the equivalent of the [[AssociatedRemoteMediaStreams]] slot in the spec, https://w3c.github.io/webrtc-pc/#dfn-x%5B%5Bassociatedremotemediastreams%5D%5D This does not change layers below peerconnection.cc. The streams are set upon the receiver's construction and is not modified for the duration of its lifetime. When we support modifying the associated set of streams of a receiver the receiver needs to know about it. The receiver's streams() should be used in all places where a receiver's streams need to be known. Bug: webrtc:8473 Change-Id: I31202973aed98e61fa9b6a78b52e815227b6c17d Reviewed-on: https://webrtc-review.googlesource.com/22922 Reviewed-by: Peter Thatcher Reviewed-by: Magnus Jedvert Reviewed-by: Steve Anton Commit-Queue: Henrik Boström Cr-Commit-Position: refs/heads/master@{#20825} --- api/rtpreceiverinterface.h | 10 ++++++++ ortc/ortcrtpreceiveradapter.cc | 9 ++++---- pc/peerconnection.cc | 19 +++++++-------- pc/peerconnection_rtp_unittest.cc | 19 ++++++++------- pc/rtpreceiver.cc | 23 ++++++++++++------- pc/rtpreceiver.h | 28 +++++++++++++++++------ pc/rtpsenderreceiver_unittest.cc | 23 +++++++++++++++---- sdk/android/src/jni/pc/mediastream_jni.cc | 2 +- 8 files changed, 89 insertions(+), 44 deletions(-) diff --git a/api/rtpreceiverinterface.h b/api/rtpreceiverinterface.h index afb737b71e..311971d951 100644 --- a/api/rtpreceiverinterface.h +++ b/api/rtpreceiverinterface.h @@ -94,6 +94,14 @@ class RtpReceiverObserverInterface { class RtpReceiverInterface : public rtc::RefCountInterface { public: virtual rtc::scoped_refptr track() const = 0; + // The list of streams that |track| is associated with. This is the same as + // the [[AssociatedRemoteMediaStreams]] internal slot in the spec. + // https://w3c.github.io/webrtc-pc/#dfn-x%5B%5Bassociatedremotemediastreams%5D%5D + // TODO(hbos): Make pure virtual as soon as Chromium's mock implements this. + virtual std::vector> streams() + const { + return std::vector>(); + } // Audio or video receiver? virtual cricket::MediaType media_type() const = 0; @@ -130,6 +138,8 @@ class RtpReceiverInterface : public rtc::RefCountInterface { BEGIN_SIGNALING_PROXY_MAP(RtpReceiver) PROXY_SIGNALING_THREAD_DESTRUCTOR() PROXY_CONSTMETHOD0(rtc::scoped_refptr, track) + PROXY_CONSTMETHOD0(std::vector>, + streams) PROXY_CONSTMETHOD0(cricket::MediaType, media_type) PROXY_CONSTMETHOD0(std::string, id) PROXY_CONSTMETHOD0(RtpParameters, GetParameters); diff --git a/ortc/ortcrtpreceiveradapter.cc b/ortc/ortcrtpreceiveradapter.cc index b30f63c515..ffa9387b54 100644 --- a/ortc/ortcrtpreceiveradapter.cc +++ b/ortc/ortcrtpreceiveradapter.cc @@ -152,13 +152,14 @@ void OrtcRtpReceiverAdapter::MaybeRecreateInternalReceiver() { switch (kind_) { case cricket::MEDIA_TYPE_AUDIO: internal_receiver_ = - new AudioRtpReceiver(rtc::CreateRandomUuid(), ssrc, + new AudioRtpReceiver(rtc::CreateRandomUuid(), {}, ssrc, rtp_transport_controller_->voice_channel()); break; case cricket::MEDIA_TYPE_VIDEO: - internal_receiver_ = new VideoRtpReceiver( - rtc::CreateRandomUuid(), rtp_transport_controller_->worker_thread(), - ssrc, rtp_transport_controller_->video_channel()); + internal_receiver_ = + new VideoRtpReceiver(rtc::CreateRandomUuid(), {}, + rtp_transport_controller_->worker_thread(), ssrc, + rtp_transport_controller_->video_channel()); break; case cricket::MEDIA_TYPE_DATA: RTC_NOTREACHED(); diff --git a/pc/peerconnection.cc b/pc/peerconnection.cc index e4864c9795..b9091c7a33 100644 --- a/pc/peerconnection.cc +++ b/pc/peerconnection.cc @@ -2079,33 +2079,34 @@ void PeerConnection::OnMessage(rtc::Message* msg) { void PeerConnection::CreateAudioReceiver( MediaStreamInterface* stream, const RtpSenderInfo& remote_sender_info) { + std::vector> streams; + streams.push_back(rtc::scoped_refptr(stream)); rtc::scoped_refptr> receiver = RtpReceiverProxyWithInternal::Create( signaling_thread(), - new AudioRtpReceiver(remote_sender_info.sender_id, + new AudioRtpReceiver(remote_sender_info.sender_id, streams, remote_sender_info.first_ssrc, voice_channel())); stream->AddTrack( static_cast(receiver->internal()->track().get())); GetAudioTransceiver()->internal()->AddReceiver(receiver); - std::vector> streams; - streams.push_back(rtc::scoped_refptr(stream)); - observer_->OnAddTrack(receiver, streams); + observer_->OnAddTrack(receiver, std::move(streams)); } void PeerConnection::CreateVideoReceiver( MediaStreamInterface* stream, const RtpSenderInfo& remote_sender_info) { + std::vector> streams; + streams.push_back(rtc::scoped_refptr(stream)); rtc::scoped_refptr> receiver = RtpReceiverProxyWithInternal::Create( signaling_thread(), - new VideoRtpReceiver(remote_sender_info.sender_id, worker_thread(), - remote_sender_info.first_ssrc, video_channel())); + new VideoRtpReceiver(remote_sender_info.sender_id, streams, + worker_thread(), remote_sender_info.first_ssrc, + video_channel())); stream->AddTrack( static_cast(receiver->internal()->track().get())); GetVideoTransceiver()->internal()->AddReceiver(receiver); - std::vector> streams; - streams.push_back(rtc::scoped_refptr(stream)); - observer_->OnAddTrack(receiver, streams); + observer_->OnAddTrack(receiver, std::move(streams)); } // TODO(deadbeef): Keep RtpReceivers around even if track goes away in remote diff --git a/pc/peerconnection_rtp_unittest.cc b/pc/peerconnection_rtp_unittest.cc index 0261b1a36f..860c6dc50e 100644 --- a/pc/peerconnection_rtp_unittest.cc +++ b/pc/peerconnection_rtp_unittest.cc @@ -72,10 +72,10 @@ TEST_F(PeerConnectionRtpTest, AddTrackWithoutStreamFiresOnAddTrack) { ASSERT_EQ(1u, callee->observer()->add_track_events_.size()); // TODO(deadbeef): When no stream is handled correctly we would expect // |add_track_events_[0].streams| to be empty. https://crbug.com/webrtc/7933 - ASSERT_EQ(1u, callee->observer()->add_track_events_[0].streams.size()); - EXPECT_TRUE( - callee->observer()->add_track_events_[0].streams[0]->FindAudioTrack( - "audio_track")); + auto& add_track_event = callee->observer()->add_track_events_[0]; + ASSERT_EQ(1u, add_track_event.streams.size()); + EXPECT_TRUE(add_track_event.streams[0]->FindAudioTrack("audio_track")); + EXPECT_EQ(add_track_event.streams, add_track_event.receiver->streams()); } TEST_F(PeerConnectionRtpTest, AddTrackWithStreamFiresOnAddTrack) { @@ -89,12 +89,11 @@ TEST_F(PeerConnectionRtpTest, AddTrackWithStreamFiresOnAddTrack) { ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); ASSERT_EQ(1u, callee->observer()->add_track_events_.size()); - ASSERT_EQ(1u, callee->observer()->add_track_events_[0].streams.size()); - EXPECT_EQ("audio_stream", - callee->observer()->add_track_events_[0].streams[0]->label()); - EXPECT_TRUE( - callee->observer()->add_track_events_[0].streams[0]->FindAudioTrack( - "audio_track")); + auto& add_track_event = callee->observer()->add_track_events_[0]; + ASSERT_EQ(1u, add_track_event.streams.size()); + EXPECT_EQ("audio_stream", add_track_event.streams[0]->label()); + EXPECT_TRUE(add_track_event.streams[0]->FindAudioTrack("audio_track")); + EXPECT_EQ(add_track_event.streams, add_track_event.receiver->streams()); } TEST_F(PeerConnectionRtpTest, RemoveTrackWithoutStreamFiresOnRemoveTrack) { diff --git a/pc/rtpreceiver.cc b/pc/rtpreceiver.cc index 4d69aa20f3..da7e64213f 100644 --- a/pc/rtpreceiver.cc +++ b/pc/rtpreceiver.cc @@ -10,6 +10,7 @@ #include "pc/rtpreceiver.h" +#include #include #include "api/mediastreamtrackproxy.h" @@ -20,9 +21,11 @@ namespace webrtc { -AudioRtpReceiver::AudioRtpReceiver(const std::string& track_id, - uint32_t ssrc, - cricket::VoiceChannel* channel) +AudioRtpReceiver::AudioRtpReceiver( + const std::string& track_id, + std::vector> streams, + uint32_t ssrc, + cricket::VoiceChannel* channel) : id_(track_id), ssrc_(ssrc), channel_(channel), @@ -30,6 +33,7 @@ AudioRtpReceiver::AudioRtpReceiver(const std::string& track_id, rtc::Thread::Current(), AudioTrack::Create(track_id, RemoteAudioSource::Create(ssrc, channel)))), + streams_(std::move(streams)), cached_track_enabled_(track_->enabled()) { RTC_DCHECK(track_->GetSource()->remote()); track_->RegisterObserver(this); @@ -144,10 +148,12 @@ void AudioRtpReceiver::OnFirstPacketReceived(cricket::BaseChannel* channel) { received_first_packet_ = true; } -VideoRtpReceiver::VideoRtpReceiver(const std::string& track_id, - rtc::Thread* worker_thread, - uint32_t ssrc, - cricket::VideoChannel* channel) +VideoRtpReceiver::VideoRtpReceiver( + const std::string& track_id, + std::vector> streams, + rtc::Thread* worker_thread, + uint32_t ssrc, + cricket::VideoChannel* channel) : id_(track_id), ssrc_(ssrc), channel_(channel), @@ -161,7 +167,8 @@ VideoRtpReceiver::VideoRtpReceiver(const std::string& track_id, VideoTrackSourceProxy::Create(rtc::Thread::Current(), worker_thread, source_), - worker_thread))) { + worker_thread))), + streams_(std::move(streams)) { source_->SetState(MediaSourceInterface::kLive); if (!channel_) { RTC_LOG(LS_ERROR) diff --git a/pc/rtpreceiver.h b/pc/rtpreceiver.h index d3f0f2679e..a1af13cead 100644 --- a/pc/rtpreceiver.h +++ b/pc/rtpreceiver.h @@ -49,9 +49,11 @@ class AudioRtpReceiver : public ObserverInterface, // sees. // TODO(deadbeef): Use rtc::Optional, or have another constructor that // doesn't take an SSRC, and make this one DCHECK(ssrc != 0). - AudioRtpReceiver(const std::string& track_id, - uint32_t ssrc, - cricket::VoiceChannel* channel); + AudioRtpReceiver( + const std::string& track_id, + std::vector> streams, + uint32_t ssrc, + cricket::VoiceChannel* channel); virtual ~AudioRtpReceiver(); @@ -69,6 +71,10 @@ class AudioRtpReceiver : public ObserverInterface, rtc::scoped_refptr track() const override { return track_.get(); } + std::vector> streams() + const override { + return streams_; + } cricket::MediaType media_type() const override { return cricket::MEDIA_TYPE_AUDIO; @@ -99,6 +105,7 @@ class AudioRtpReceiver : public ObserverInterface, const uint32_t ssrc_; cricket::VoiceChannel* channel_; const rtc::scoped_refptr track_; + std::vector> streams_; bool cached_track_enabled_; double cached_volume_ = 1; bool stopped_ = false; @@ -111,10 +118,12 @@ class VideoRtpReceiver : public rtc::RefCountedObject, public: // An SSRC of 0 will create a receiver that will match the first SSRC it // sees. - VideoRtpReceiver(const std::string& track_id, - rtc::Thread* worker_thread, - uint32_t ssrc, - cricket::VideoChannel* channel); + VideoRtpReceiver( + const std::string& track_id, + std::vector> streams, + rtc::Thread* worker_thread, + uint32_t ssrc, + cricket::VideoChannel* channel); virtual ~VideoRtpReceiver(); @@ -126,6 +135,10 @@ class VideoRtpReceiver : public rtc::RefCountedObject, rtc::scoped_refptr track() const override { return track_.get(); } + std::vector> streams() + const override { + return streams_; + } cricket::MediaType media_type() const override { return cricket::MEDIA_TYPE_VIDEO; @@ -160,6 +173,7 @@ class VideoRtpReceiver : public rtc::RefCountedObject, // the VideoRtpReceiver is stopped. rtc::scoped_refptr source_; rtc::scoped_refptr track_; + std::vector> streams_; bool stopped_ = false; RtpReceiverObserverInterface* observer_ = nullptr; bool received_first_packet_ = false; diff --git a/pc/rtpsenderreceiver_unittest.cc b/pc/rtpsenderreceiver_unittest.cc index f5b06d11cb..03bb84b656 100644 --- a/pc/rtpsenderreceiver_unittest.cc +++ b/pc/rtpsenderreceiver_unittest.cc @@ -165,16 +165,19 @@ class RtpSenderReceiverTest : public testing::Test, VerifyVideoChannelNoInput(); } - void CreateAudioRtpReceiver() { - audio_rtp_receiver_ = - new AudioRtpReceiver(kAudioTrackId, kAudioSsrc, voice_channel_); + void CreateAudioRtpReceiver( + std::vector> streams = {}) { + audio_rtp_receiver_ = new AudioRtpReceiver( + kAudioTrackId, std::move(streams), kAudioSsrc, voice_channel_); audio_track_ = audio_rtp_receiver_->audio_track(); VerifyVoiceChannelOutput(); } - void CreateVideoRtpReceiver() { + void CreateVideoRtpReceiver( + std::vector> streams = {}) { video_rtp_receiver_ = new VideoRtpReceiver( - kVideoTrackId, rtc::Thread::Current(), kVideoSsrc, video_channel_); + kVideoTrackId, std::move(streams), rtc::Thread::Current(), kVideoSsrc, + video_channel_); video_track_ = video_rtp_receiver_->video_track(); VerifyVideoChannelOutput(); } @@ -292,6 +295,16 @@ TEST_F(RtpSenderReceiverTest, AddAndDestroyVideoRtpReceiver) { DestroyVideoRtpReceiver(); } +TEST_F(RtpSenderReceiverTest, AddAndDestroyAudioRtpReceiverWithStreams) { + CreateAudioRtpReceiver({local_stream_}); + DestroyAudioRtpReceiver(); +} + +TEST_F(RtpSenderReceiverTest, AddAndDestroyVideoRtpReceiverWithStreams) { + CreateVideoRtpReceiver({local_stream_}); + DestroyVideoRtpReceiver(); +} + // Test that the AudioRtpSender applies options from the local audio source. TEST_F(RtpSenderReceiverTest, LocalAudioSourceOptionsApplied) { cricket::AudioOptions options; diff --git a/sdk/android/src/jni/pc/mediastream_jni.cc b/sdk/android/src/jni/pc/mediastream_jni.cc index a56802ada8..e9c7309e71 100644 --- a/sdk/android/src/jni/pc/mediastream_jni.cc +++ b/sdk/android/src/jni/pc/mediastream_jni.cc @@ -64,7 +64,7 @@ JNI_FUNCTION_DECLARATION(jstring, } JNI_FUNCTION_DECLARATION(void, MediaStream_free, JNIEnv*, jclass, jlong j_p) { - CHECK_RELEASE(reinterpret_cast(j_p)); + reinterpret_cast(j_p)->Release(); } } // namespace jni