From 2b46a5870b46d6d093deecdedc899744ae89f2cf Mon Sep 17 00:00:00 2001 From: Byoungchan Lee Date: Tue, 21 Jun 2022 09:19:34 +0900 Subject: [PATCH] Add proxy access to some methods in Obj-C SDK MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Most calls to C++ PeerConnection and related classes are proxied to internal threads in WebRTC. However, there is no such thing in the Obj-C SDK. It would be nice to proxy methods in the Obj-C SDK as well. RTCMediaStream and RTCVideoTrack have NSMutableArray members, and it can throw NSRangeException when it has race conditions, so that it would be a good starting point. Also, remove some NSAsserts as its condition isn't a fatal error, and it doesn't affect the production already. Bug: None Change-Id: I10b44a9c773d62a5c04c254986733a6b67d51617 Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/262840 Reviewed-by: Mirko Bonadei Commit-Queue: Daniel.L (Byoungchan) Lee Reviewed-by: Kári Helgason Cr-Commit-Position: refs/heads/main@{#37283} --- sdk/BUILD.gn | 1 + sdk/objc/api/peerconnection/RTCMediaStream.mm | 51 ++++++++++++++----- .../RTCPeerConnectionFactory+Private.h | 4 ++ .../RTCPeerConnectionFactory.mm | 8 +++ sdk/objc/api/peerconnection/RTCVideoTrack.mm | 17 ++++++- 5 files changed, 67 insertions(+), 14 deletions(-) diff --git a/sdk/BUILD.gn b/sdk/BUILD.gn index e9a8dbfa36..95221ca256 100644 --- a/sdk/BUILD.gn +++ b/sdk/BUILD.gn @@ -1044,6 +1044,7 @@ if (is_ios || is_mac) { "../rtc_base:network_constants", "../rtc_base:safe_conversions", "../rtc_base:stringutils", + "../rtc_base:threading", "../rtc_base:timeutils", "../stats:rtc_stats", "../system_wrappers:field_trial", diff --git a/sdk/objc/api/peerconnection/RTCMediaStream.mm b/sdk/objc/api/peerconnection/RTCMediaStream.mm index a6292b547c..beb4a7a91b 100644 --- a/sdk/objc/api/peerconnection/RTCMediaStream.mm +++ b/sdk/objc/api/peerconnection/RTCMediaStream.mm @@ -10,8 +10,6 @@ #import "RTCMediaStream+Private.h" -#include - #import "RTCAudioTrack+Private.h" #import "RTCMediaStreamTrack+Private.h" #import "RTCPeerConnectionFactory+Private.h" @@ -20,8 +18,9 @@ @implementation RTC_OBJC_TYPE (RTCMediaStream) { RTC_OBJC_TYPE(RTCPeerConnectionFactory) * _factory; - NSMutableArray *_audioTracks; - NSMutableArray *_videoTracks; + rtc::Thread *_signalingThread; + NSMutableArray *_audioTracks /* accessed on _signalingThread */; + NSMutableArray *_videoTracks /* accessed on _signalingThread */; rtc::scoped_refptr _nativeMediaStream; } @@ -36,10 +35,18 @@ } - (NSArray *)audioTracks { + if (!_signalingThread->IsCurrent()) { + return _signalingThread->Invoke *>( + RTC_FROM_HERE, [self]() { return self.audioTracks; }); + } return [_audioTracks copy]; } - (NSArray *)videoTracks { + if (!_signalingThread->IsCurrent()) { + return _signalingThread->Invoke *>( + RTC_FROM_HERE, [self]() { return self.videoTracks; }); + } return [_videoTracks copy]; } @@ -48,33 +55,52 @@ } - (void)addAudioTrack:(RTC_OBJC_TYPE(RTCAudioTrack) *)audioTrack { + if (!_signalingThread->IsCurrent()) { + return _signalingThread->Invoke( + RTC_FROM_HERE, [audioTrack, self]() { return [self addAudioTrack:audioTrack]; }); + } if (_nativeMediaStream->AddTrack(audioTrack.nativeAudioTrack)) { [_audioTracks addObject:audioTrack]; } } - (void)addVideoTrack:(RTC_OBJC_TYPE(RTCVideoTrack) *)videoTrack { + if (!_signalingThread->IsCurrent()) { + return _signalingThread->Invoke( + RTC_FROM_HERE, [videoTrack, self]() { return [self addVideoTrack:videoTrack]; }); + } if (_nativeMediaStream->AddTrack(videoTrack.nativeVideoTrack)) { [_videoTracks addObject:videoTrack]; } } - (void)removeAudioTrack:(RTC_OBJC_TYPE(RTCAudioTrack) *)audioTrack { + if (!_signalingThread->IsCurrent()) { + return _signalingThread->Invoke( + RTC_FROM_HERE, [audioTrack, self]() { return [self removeAudioTrack:audioTrack]; }); + } NSUInteger index = [_audioTracks indexOfObjectIdenticalTo:audioTrack]; - NSAssert(index != NSNotFound, - @"|removeAudioTrack| called on unexpected RTC_OBJC_TYPE(RTCAudioTrack)"); - if (index != NSNotFound && - _nativeMediaStream->RemoveTrack(audioTrack.nativeAudioTrack)) { + if (index == NSNotFound) { + RTC_LOG(LS_INFO) << "|removeAudioTrack| called on unexpected RTC_OBJC_TYPE(RTCAudioTrack)"; + return; + } + if (_nativeMediaStream->RemoveTrack(audioTrack.nativeAudioTrack)) { [_audioTracks removeObjectAtIndex:index]; } } - (void)removeVideoTrack:(RTC_OBJC_TYPE(RTCVideoTrack) *)videoTrack { + if (!_signalingThread->IsCurrent()) { + return _signalingThread->Invoke( + RTC_FROM_HERE, [videoTrack, self]() { return [self removeVideoTrack:videoTrack]; }); + } NSUInteger index = [_videoTracks indexOfObjectIdenticalTo:videoTrack]; - NSAssert(index != NSNotFound, - @"|removeVideoTrack| called on unexpected RTC_OBJC_TYPE(RTCVideoTrack)"); - if (index != NSNotFound && - _nativeMediaStream->RemoveTrack(videoTrack.nativeVideoTrack)) { + if (index == NSNotFound) { + RTC_LOG(LS_INFO) << "|removeVideoTrack| called on unexpected RTC_OBJC_TYPE(RTCVideoTrack)"; + return; + } + + if (_nativeMediaStream->RemoveTrack(videoTrack.nativeVideoTrack)) { [_videoTracks removeObjectAtIndex:index]; } } @@ -98,6 +124,7 @@ NSParameterAssert(nativeMediaStream); if (self = [super init]) { _factory = factory; + _signalingThread = factory.signalingThread; webrtc::AudioTrackVector audioTracks = nativeMediaStream->GetAudioTracks(); webrtc::VideoTrackVector videoTracks = nativeMediaStream->GetVideoTracks(); diff --git a/sdk/objc/api/peerconnection/RTCPeerConnectionFactory+Private.h b/sdk/objc/api/peerconnection/RTCPeerConnectionFactory+Private.h index ef61c2ed01..9613646270 100644 --- a/sdk/objc/api/peerconnection/RTCPeerConnectionFactory+Private.h +++ b/sdk/objc/api/peerconnection/RTCPeerConnectionFactory+Private.h @@ -12,6 +12,7 @@ #include "api/peer_connection_interface.h" #include "api/scoped_refptr.h" +#include "rtc_base/thread.h" NS_ASSUME_NONNULL_BEGIN @@ -26,6 +27,9 @@ NS_ASSUME_NONNULL_BEGIN @property(nonatomic, readonly) rtc::scoped_refptr nativeFactory; +@property(nonatomic, readonly) rtc::Thread* signalingThread; +@property(nonatomic, readonly) rtc::Thread* workerThread; + @end NS_ASSUME_NONNULL_END diff --git a/sdk/objc/api/peerconnection/RTCPeerConnectionFactory.mm b/sdk/objc/api/peerconnection/RTCPeerConnectionFactory.mm index 4e5aa19750..99b9363d29 100644 --- a/sdk/objc/api/peerconnection/RTCPeerConnectionFactory.mm +++ b/sdk/objc/api/peerconnection/RTCPeerConnectionFactory.mm @@ -331,4 +331,12 @@ _hasStartedAecDump = NO; } +- (rtc::Thread *)signalingThread { + return _signalingThread.get(); +} + +- (rtc::Thread *)workerThread { + return _workerThread.get(); +} + @end diff --git a/sdk/objc/api/peerconnection/RTCVideoTrack.mm b/sdk/objc/api/peerconnection/RTCVideoTrack.mm index 8944d26c13..fb015c6207 100644 --- a/sdk/objc/api/peerconnection/RTCVideoTrack.mm +++ b/sdk/objc/api/peerconnection/RTCVideoTrack.mm @@ -17,7 +17,8 @@ #import "helpers/NSString+StdString.h" @implementation RTC_OBJC_TYPE (RTCVideoTrack) { - NSMutableArray *_adapters; + rtc::Thread *_workerThread; + NSMutableArray *_adapters /* accessed on _workerThread */; } @synthesize source = _source; @@ -46,6 +47,7 @@ NSParameterAssert(type == RTCMediaStreamTrackTypeVideo); if (self = [super initWithFactory:factory nativeTrack:nativeMediaTrack type:type]) { _adapters = [NSMutableArray array]; + _workerThread = factory.workerThread; } return self; } @@ -69,10 +71,15 @@ } - (void)addRenderer:(id)renderer { + if (!_workerThread->IsCurrent()) { + _workerThread->Invoke(RTC_FROM_HERE, [renderer, self] { [self addRenderer:renderer]; }); + return; + } + // Make sure we don't have this renderer yet. for (RTCVideoRendererAdapter *adapter in _adapters) { if (adapter.videoRenderer == renderer) { - NSAssert(NO, @"|renderer| is already attached to this track"); + RTC_LOG(LS_INFO) << "|renderer| is already attached to this track"; return; } } @@ -85,6 +92,11 @@ } - (void)removeRenderer:(id)renderer { + if (!_workerThread->IsCurrent()) { + _workerThread->Invoke(RTC_FROM_HERE, + [renderer, self] { [self removeRenderer:renderer]; }); + return; + } __block NSUInteger indexToRemove = NSNotFound; [_adapters enumerateObjectsUsingBlock:^(RTCVideoRendererAdapter *adapter, NSUInteger idx, @@ -95,6 +107,7 @@ } }]; if (indexToRemove == NSNotFound) { + RTC_LOG(LS_INFO) << "removeRenderer called with a renderer that has not been previously added"; return; } RTCVideoRendererAdapter *adapterToRemove =