From 041ecb87f57ea5a7489de0a936e64756ee23a7ea Mon Sep 17 00:00:00 2001 From: Harald Alvestrand Date: Mon, 20 Mar 2023 14:13:42 +0000 Subject: [PATCH] New PeerConnectionFactory::CreateVideoTrack with refcounted source MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Bug: webrtc:15017 Change-Id: I04c794d8959583bb4cc5c3898f4175783ec49f16 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/249363 Reviewed-by: Henrik Boström Reviewed-by: Tomas Gunnarsson Commit-Queue: Harald Alvestrand Cr-Commit-Position: refs/heads/main@{#39635} --- api/peer_connection_interface.h | 11 ++++++++++- api/test/mock_peer_connection_factory_interface.h | 5 +++++ examples/androidnativeapi/jni/android_call_client.cc | 4 ++-- examples/objcnativeapi/objc/objc_call_client.mm | 2 +- examples/peerconnection/client/conductor.cc | 3 +-- examples/unityplugin/simple_peer_connection.cc | 7 +++---- pc/peer_connection_adaptation_integrationtest.cc | 2 +- pc/peer_connection_factory.cc | 9 ++++----- pc/peer_connection_factory.h | 4 ++-- pc/peer_connection_factory_proxy.h | 4 ++-- pc/peer_connection_factory_unittest.cc | 2 +- pc/peer_connection_field_trial_tests.cc | 3 +-- pc/peer_connection_interface_unittest.cc | 3 +-- pc/peer_connection_rampup_tests.cc | 4 ++-- pc/peer_connection_wrapper.cc | 3 +-- pc/test/integration_test_helpers.h | 6 +++--- pc/test/peer_connection_test_wrapper.cc | 3 +-- sdk/android/src/jni/pc/peer_connection_factory.cc | 5 +++-- sdk/objc/api/peerconnection/RTCVideoTrack.mm | 2 +- test/pc/e2e/media/media_helper.cc | 4 ++-- test/peer_scenario/peer_scenario_client.cc | 2 +- 21 files changed, 48 insertions(+), 40 deletions(-) diff --git a/api/peer_connection_interface.h b/api/peer_connection_interface.h index 1097a1639c..58e2d3396f 100644 --- a/api/peer_connection_interface.h +++ b/api/peer_connection_interface.h @@ -1552,9 +1552,18 @@ class RTC_EXPORT PeerConnectionFactoryInterface // Creates a new local VideoTrack. The same `source` can be used in several // tracks. + virtual rtc::scoped_refptr CreateVideoTrack( + rtc::scoped_refptr source, + absl::string_view label) = 0; + // TODO(bugs.webrtc.org/15017): Deprecate this function once Chrome + // has been updated - it can't land as deprecated. + // ABSL_DEPRECATED("Use version with scoped_refptr") virtual rtc::scoped_refptr CreateVideoTrack( const std::string& label, - VideoTrackSourceInterface* source) = 0; + VideoTrackSourceInterface* source) { + return CreateVideoTrack( + rtc::scoped_refptr(source), label); + } // Creates an new AudioTrack. At the moment `source` can be null. virtual rtc::scoped_refptr CreateAudioTrack( diff --git a/api/test/mock_peer_connection_factory_interface.h b/api/test/mock_peer_connection_factory_interface.h index ae1fbfbbb7..67a67b8e06 100644 --- a/api/test/mock_peer_connection_factory_interface.h +++ b/api/test/mock_peer_connection_factory_interface.h @@ -65,6 +65,11 @@ class MockPeerConnectionFactoryInterface CreateVideoTrack, (const std::string&, VideoTrackSourceInterface*), (override)); + MOCK_METHOD(rtc::scoped_refptr, + CreateVideoTrack, + (rtc::scoped_refptr, + absl::string_view), + (override)); MOCK_METHOD(rtc::scoped_refptr, CreateAudioTrack, (const std::string&, AudioSourceInterface*), diff --git a/examples/androidnativeapi/jni/android_call_client.cc b/examples/androidnativeapi/jni/android_call_client.cc index 7da56e6e60..ae0a40b9ba 100644 --- a/examples/androidnativeapi/jni/android_call_client.cc +++ b/examples/androidnativeapi/jni/android_call_client.cc @@ -186,8 +186,8 @@ void AndroidCallClient::CreatePeerConnection() { RTC_LOG(LS_INFO) << "PeerConnection created: " << pc_.get(); - rtc::scoped_refptr local_video_track( - pcf_->CreateVideoTrack("video", video_source_.get())); + rtc::scoped_refptr local_video_track = + pcf_->CreateVideoTrack(video_source_, "video"); local_video_track->AddOrUpdateSink(local_sink_.get(), rtc::VideoSinkWants()); pc_->AddTransceiver(local_video_track); RTC_LOG(LS_INFO) << "Local video sink set up: " << local_video_track.get(); diff --git a/examples/objcnativeapi/objc/objc_call_client.mm b/examples/objcnativeapi/objc/objc_call_client.mm index 7dd57b499b..90bcfcc35b 100644 --- a/examples/objcnativeapi/objc/objc_call_client.mm +++ b/examples/objcnativeapi/objc/objc_call_client.mm @@ -149,7 +149,7 @@ void ObjCCallClient::CreatePeerConnection() { RTC_LOG(LS_INFO) << "PeerConnection created: " << pc_.get(); rtc::scoped_refptr local_video_track = - pcf_->CreateVideoTrack("video", video_source_.get()); + pcf_->CreateVideoTrack(video_source_, "video"); pc_->AddTransceiver(local_video_track); RTC_LOG(LS_INFO) << "Local video sink set up: " << local_video_track.get(); diff --git a/examples/peerconnection/client/conductor.cc b/examples/peerconnection/client/conductor.cc index 965525abff..052d417d7f 100644 --- a/examples/peerconnection/client/conductor.cc +++ b/examples/peerconnection/client/conductor.cc @@ -468,8 +468,7 @@ void Conductor::AddTracks() { CapturerTrackSource::Create(); if (video_device) { rtc::scoped_refptr video_track_( - peer_connection_factory_->CreateVideoTrack(kVideoLabel, - video_device.get())); + peer_connection_factory_->CreateVideoTrack(video_device, kVideoLabel)); main_wnd_->StartLocalRenderer(video_track_.get()); result_or_error = peer_connection_->AddTrack(video_track_, {kStreamId}); diff --git a/examples/unityplugin/simple_peer_connection.cc b/examples/unityplugin/simple_peer_connection.cc index 861b22f29c..de49d5cd07 100644 --- a/examples/unityplugin/simple_peer_connection.cc +++ b/examples/unityplugin/simple_peer_connection.cc @@ -460,16 +460,15 @@ void SimplePeerConnection::AddStreams(bool audio_only) { g_camera = (jobject)env->NewGlobalRef(camera_tmp); rtc::scoped_refptr video_track( - g_peer_connection_factory->CreateVideoTrack(kVideoLabel, - source.release())); + g_peer_connection_factory->CreateVideoTrack(source, kVideoLabel)); stream->AddTrack(video_track); #else rtc::scoped_refptr video_device = CapturerTrackSource::Create(); if (video_device) { rtc::scoped_refptr video_track( - g_peer_connection_factory->CreateVideoTrack(kVideoLabel, - video_device.get())); + g_peer_connection_factory->CreateVideoTrack(video_device, + kVideoLabel)); stream->AddTrack(video_track); } diff --git a/pc/peer_connection_adaptation_integrationtest.cc b/pc/peer_connection_adaptation_integrationtest.cc index 5922e15034..882fa36a57 100644 --- a/pc/peer_connection_adaptation_integrationtest.cc +++ b/pc/peer_connection_adaptation_integrationtest.cc @@ -64,7 +64,7 @@ TrackWithPeriodicSource CreateTrackWithPeriodicSource( periodic_track_source_config, /* remote */ false); TrackWithPeriodicSource track_with_source; track_with_source.track = - factory->CreateVideoTrack("PeriodicTrack", periodic_track_source.get()); + factory->CreateVideoTrack(periodic_track_source, "PeriodicTrack"); track_with_source.periodic_track_source = periodic_track_source; return track_with_source; } diff --git a/pc/peer_connection_factory.cc b/pc/peer_connection_factory.cc index afebdd79a5..eaa69d3e7c 100644 --- a/pc/peer_connection_factory.cc +++ b/pc/peer_connection_factory.cc @@ -273,12 +273,11 @@ PeerConnectionFactory::CreateLocalMediaStream(const std::string& stream_id) { } rtc::scoped_refptr PeerConnectionFactory::CreateVideoTrack( - const std::string& id, - VideoTrackSourceInterface* source) { + rtc::scoped_refptr source, + absl::string_view id) { RTC_DCHECK(signaling_thread()->IsCurrent()); - rtc::scoped_refptr track = VideoTrack::Create( - id, rtc::scoped_refptr(source), - worker_thread()); + rtc::scoped_refptr track = + VideoTrack::Create(id, source, worker_thread()); return VideoTrackProxy::Create(signaling_thread(), worker_thread(), track); } diff --git a/pc/peer_connection_factory.h b/pc/peer_connection_factory.h index dac3702e37..f55d09f6d8 100644 --- a/pc/peer_connection_factory.h +++ b/pc/peer_connection_factory.h @@ -85,8 +85,8 @@ class PeerConnectionFactory : public PeerConnectionFactoryInterface { const cricket::AudioOptions& options) override; rtc::scoped_refptr CreateVideoTrack( - const std::string& id, - VideoTrackSourceInterface* video_source) override; + rtc::scoped_refptr video_source, + absl::string_view id) override; rtc::scoped_refptr CreateAudioTrack( const std::string& id, diff --git a/pc/peer_connection_factory_proxy.h b/pc/peer_connection_factory_proxy.h index 59e373db7b..4781497642 100644 --- a/pc/peer_connection_factory_proxy.h +++ b/pc/peer_connection_factory_proxy.h @@ -43,8 +43,8 @@ PROXY_METHOD1(rtc::scoped_refptr, const cricket::AudioOptions&) PROXY_METHOD2(rtc::scoped_refptr, CreateVideoTrack, - const std::string&, - VideoTrackSourceInterface*) + rtc::scoped_refptr, + absl::string_view) PROXY_METHOD2(rtc::scoped_refptr, CreateAudioTrack, const std::string&, diff --git a/pc/peer_connection_factory_unittest.cc b/pc/peer_connection_factory_unittest.cc index 6aa7f49079..b7fb726cc3 100644 --- a/pc/peer_connection_factory_unittest.cc +++ b/pc/peer_connection_factory_unittest.cc @@ -567,7 +567,7 @@ TEST_F(PeerConnectionFactoryTest, LocalRendering) { ASSERT_TRUE(source.get() != NULL); rtc::scoped_refptr track( - factory_->CreateVideoTrack("testlabel", source.get())); + factory_->CreateVideoTrack(source, "testlabel")); ASSERT_TRUE(track.get() != NULL); FakeVideoTrackRenderer local_renderer(track.get()); diff --git a/pc/peer_connection_field_trial_tests.cc b/pc/peer_connection_field_trial_tests.cc index 9220623e60..c3b3a2db7f 100644 --- a/pc/peer_connection_field_trial_tests.cc +++ b/pc/peer_connection_field_trial_tests.cc @@ -237,8 +237,7 @@ TEST_F(PeerConnectionFieldTrialTest, ApplyFakeNetworkConfig) { auto video_track_source = rtc::make_ref_counted( config, clock_, /*is_screencast=*/false); - caller->AddTrack( - pc_factory_->CreateVideoTrack("v", video_track_source.get())); + caller->AddTrack(pc_factory_->CreateVideoTrack(video_track_source, "v")); WrapperPtr callee = CreatePeerConnection(); ASSERT_TRUE(callee->SetRemoteDescription(caller->CreateOfferAndSetAsLocal())); diff --git a/pc/peer_connection_interface_unittest.cc b/pc/peer_connection_interface_unittest.cc index f7f408bcc8..afbfcea6bf 100644 --- a/pc/peer_connection_interface_unittest.cc +++ b/pc/peer_connection_interface_unittest.cc @@ -814,8 +814,7 @@ class PeerConnectionInterfaceBaseTest : public ::testing::Test { rtc::scoped_refptr CreateVideoTrack( const std::string& label) { - return pc_factory_->CreateVideoTrack(label, - FakeVideoTrackSource::Create().get()); + return pc_factory_->CreateVideoTrack(FakeVideoTrackSource::Create(), label); } void AddVideoTrack(const std::string& track_label, diff --git a/pc/peer_connection_rampup_tests.cc b/pc/peer_connection_rampup_tests.cc index 47f463e968..0fbbe7502f 100644 --- a/pc/peer_connection_rampup_tests.cc +++ b/pc/peer_connection_rampup_tests.cc @@ -127,8 +127,8 @@ class PeerConnectionWrapperForRampUpTest : public PeerConnectionWrapper { config, clock, /*is_screencast=*/false)); video_track_sources_.back()->Start(); return rtc::scoped_refptr( - pc_factory()->CreateVideoTrack(rtc::CreateRandomUuid(), - video_track_sources_.back().get())); + pc_factory()->CreateVideoTrack(video_track_sources_.back(), + rtc::CreateRandomUuid())); } rtc::scoped_refptr CreateLocalAudioTrack( diff --git a/pc/peer_connection_wrapper.cc b/pc/peer_connection_wrapper.cc index 2fbca1dd07..44f4256b10 100644 --- a/pc/peer_connection_wrapper.cc +++ b/pc/peer_connection_wrapper.cc @@ -277,8 +277,7 @@ rtc::scoped_refptr PeerConnectionWrapper::CreateAudioTrack( rtc::scoped_refptr PeerConnectionWrapper::CreateVideoTrack( const std::string& label) { - return pc_factory()->CreateVideoTrack(label, - FakeVideoTrackSource::Create().get()); + return pc_factory()->CreateVideoTrack(FakeVideoTrackSource::Create(), label); } rtc::scoped_refptr PeerConnectionWrapper::AddTrack( diff --git a/pc/test/integration_test_helpers.h b/pc/test/integration_test_helpers.h index c2fc7993aa..200a025ca5 100644 --- a/pc/test/integration_test_helpers.h +++ b/pc/test/integration_test_helpers.h @@ -868,9 +868,9 @@ class PeerConnectionIntegrationWrapper : public webrtc::PeerConnectionObserver, video_track_sources_.emplace_back( rtc::make_ref_counted( config, false /* remote */)); - rtc::scoped_refptr track( - peer_connection_factory_->CreateVideoTrack( - rtc::CreateRandomUuid(), video_track_sources_.back().get())); + rtc::scoped_refptr track = + peer_connection_factory_->CreateVideoTrack(video_track_sources_.back(), + rtc::CreateRandomUuid()); if (!local_video_renderer_) { local_video_renderer_.reset( new webrtc::FakeVideoTrackRenderer(track.get())); diff --git a/pc/test/peer_connection_test_wrapper.cc b/pc/test/peer_connection_test_wrapper.cc index c35628eae3..84a01f4438 100644 --- a/pc/test/peer_connection_test_wrapper.cc +++ b/pc/test/peer_connection_test_wrapper.cc @@ -350,8 +350,7 @@ PeerConnectionTestWrapper::GetUserMedia( std::string videotrack_label = stream_id + kVideoTrackLabelBase; rtc::scoped_refptr video_track( - peer_connection_factory_->CreateVideoTrack(videotrack_label, - source.get())); + peer_connection_factory_->CreateVideoTrack(source, videotrack_label)); stream->AddTrack(video_track); } diff --git a/sdk/android/src/jni/pc/peer_connection_factory.cc b/sdk/android/src/jni/pc/peer_connection_factory.cc index c2950b31cf..045a2b9d84 100644 --- a/sdk/android/src/jni/pc/peer_connection_factory.cc +++ b/sdk/android/src/jni/pc/peer_connection_factory.cc @@ -487,8 +487,9 @@ static jlong JNI_PeerConnectionFactory_CreateVideoTrack( rtc::scoped_refptr track = PeerConnectionFactoryFromJava(native_factory) ->CreateVideoTrack( - JavaToStdString(jni, id), - reinterpret_cast(native_source)); + rtc::scoped_refptr( + reinterpret_cast(native_source)), + JavaToStdString(jni, id)); return jlongFromPointer(track.release()); } diff --git a/sdk/objc/api/peerconnection/RTCVideoTrack.mm b/sdk/objc/api/peerconnection/RTCVideoTrack.mm index d4862e3748..d3296f6279 100644 --- a/sdk/objc/api/peerconnection/RTCVideoTrack.mm +++ b/sdk/objc/api/peerconnection/RTCVideoTrack.mm @@ -31,7 +31,7 @@ NSParameterAssert(trackId.length); std::string nativeId = [NSString stdStringForString:trackId]; rtc::scoped_refptr track = - factory.nativeFactory->CreateVideoTrack(nativeId, source.nativeVideoSource.get()); + factory.nativeFactory->CreateVideoTrack(source.nativeVideoSource, nativeId); if (self = [self initWithFactory:factory nativeTrack:track type:RTCMediaStreamTrackTypeVideo]) { _source = source; } diff --git a/test/pc/e2e/media/media_helper.cc b/test/pc/e2e/media/media_helper.cc index e945bd4dae..d14dcdd908 100644 --- a/test/pc/e2e/media/media_helper.cc +++ b/test/pc/e2e/media/media_helper.cc @@ -64,8 +64,8 @@ MediaHelper::MaybeAddVideo(TestPeer* peer) { RTC_LOG(LS_INFO) << "Adding video with video_config.stream_label=" << video_config.stream_label.value(); rtc::scoped_refptr track = - peer->pc_factory()->CreateVideoTrack(video_config.stream_label.value(), - source.get()); + peer->pc_factory()->CreateVideoTrack(source, + video_config.stream_label.value()); if (video_config.content_hint.has_value()) { track->set_content_hint(video_config.content_hint.value()); } diff --git a/test/peer_scenario/peer_scenario_client.cc b/test/peer_scenario/peer_scenario_client.cc index 5d77f17561..907bac3c51 100644 --- a/test/peer_scenario/peer_scenario_client.cc +++ b/test/peer_scenario/peer_scenario_client.cc @@ -323,7 +323,7 @@ PeerScenarioClient::VideoSendTrack PeerScenarioClient::CreateVideo( capturer->Init(); res.source = rtc::make_ref_counted( std::move(capturer), config.screencast); - auto track = pc_factory_->CreateVideoTrack(track_id, res.source.get()); + auto track = pc_factory_->CreateVideoTrack(res.source, track_id); res.track = track.get(); res.sender = peer_connection_->AddTrack(track, {kCommonStreamId}).MoveValue().get();