From 51c3a3df658594fd7fb6c384363e6df3d6eb83d7 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 25 Jun 2018 15:03:19 -0600 Subject: [PATCH 1/7] update to latest webrtc artifact --- Carthage | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Carthage b/Carthage index bd80dc48af..cf52d8e963 160000 --- a/Carthage +++ b/Carthage @@ -1 +1 @@ -Subproject commit bd80dc48af6706442a77c33854e427c289a9399f +Subproject commit cf52d8e963e990d2a386c17266ec1e2d7810f317 From 064035f3f4faf8e0be4def407943505cb0110217 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 25 Jun 2018 12:07:48 -0600 Subject: [PATCH 2/7] WIP M67 - plumb through AVCaptureSession TODO: -[x] plumb through AVCaptureSession -[] get AVCaptureSession from PeerConnectionClient -[] RTCDataChannel not unwrapped -[] no member avFoundationSource -[] no member "back camera" --- .../ViewControllers/CallViewController.swift | 16 +++++---- Signal/src/call/CallService.swift | 34 ++++++++++++++----- Signal/src/call/PeerConnectionClient.swift | 2 +- .../call/UserInterface/CallUIAdapter.swift | 1 + 4 files changed, 37 insertions(+), 16 deletions(-) diff --git a/Signal/src/ViewControllers/CallViewController.swift b/Signal/src/ViewControllers/CallViewController.swift index 7dc23929d8..4aff7ff130 100644 --- a/Signal/src/ViewControllers/CallViewController.swift +++ b/Signal/src/ViewControllers/CallViewController.swift @@ -73,6 +73,7 @@ class CallViewController: OWSViewController, CallObserver, CallServiceObserver, var localVideoView: RTCCameraPreviewView! var hasShownLocalVideo = false weak var localVideoTrack: RTCVideoTrack? + weak var localCaptureSession: AVCaptureSession? weak var remoteVideoTrack: RTCVideoTrack? override public var canBecomeFirstResponder: Bool { @@ -1001,18 +1002,19 @@ class CallViewController: OWSViewController, CallObserver, CallServiceObserver, // MARK: - Video - internal func updateLocalVideoTrack(localVideoTrack: RTCVideoTrack?) { + // MJK TODO remove localVideoTrack? + internal func updateLocalVideoTrack(localVideoTrack: RTCVideoTrack?, + captureSession: AVCaptureSession?) { + SwiftAssertIsOnMainThread(#function) guard self.localVideoTrack != localVideoTrack else { return } self.localVideoTrack = localVideoTrack + localVideoView.captureSession = captureSession + let isHidden = captureSession == nil - let source = localVideoTrack?.source as? RTCAVFoundationVideoSource - - localVideoView.captureSession = source?.captureSession - let isHidden = source == nil Logger.info("\(TAG) \(#function) isHidden: \(isHidden)") localVideoView.isHidden = isHidden @@ -1117,12 +1119,14 @@ class CallViewController: OWSViewController, CallObserver, CallServiceObserver, // Do nothing. } + // TODO remove localCaptureSession: internal func didUpdateVideoTracks(call: SignalCall?, localVideoTrack: RTCVideoTrack?, + localCaptureSession: AVCaptureSession?, remoteVideoTrack: RTCVideoTrack?) { SwiftAssertIsOnMainThread(#function) - updateLocalVideoTrack(localVideoTrack: localVideoTrack) + updateLocalVideoTrack(localVideoTrack: localVideoTrack, captureSession: localCaptureSession) updateRemoteVideoTrack(remoteVideoTrack: remoteVideoTrack) } } diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index 7195b250fc..b38ce6ec1c 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -93,8 +93,10 @@ protocol CallServiceObserver: class { /** * Fired whenever the local or remote video track become active or inactive. */ + // TODO remove localCaptureSession: func didUpdateVideoTracks(call: SignalCall?, localVideoTrack: RTCVideoTrack?, + localCaptureSession: AVCaptureSession?, remoteVideoTrack: RTCVideoTrack?) } @@ -125,6 +127,14 @@ private class SignalCallData: NSObject { } } + weak var localCaptureSession: AVCaptureSession? { + didSet { + SwiftAssertIsOnMainThread(#function) + + Logger.info("\(self.logTag) \(#function)") + } + } + weak var remoteVideoTrack: RTCVideoTrack? { didSet { SwiftAssertIsOnMainThread(#function) @@ -282,6 +292,15 @@ private class SignalCallData: NSObject { return callData?.localVideoTrack } } + + weak var localCaptureSession: AVCaptureSession? { + get { + SwiftAssertIsOnMainThread(#function) + + return callData?.localCaptureSession + } + } + var remoteVideoTrack: RTCVideoTrack? { get { SwiftAssertIsOnMainThread(#function) @@ -1622,11 +1641,10 @@ private class SignalCallData: NSObject { observers.append(Weak(value: observer)) // Synchronize observer with current call state - let call = self.call - let localVideoTrack = self.localVideoTrack let remoteVideoTrack = self.isRemoteVideoEnabled ? self.remoteVideoTrack : nil - observer.didUpdateVideoTracks(call: call, - localVideoTrack: localVideoTrack, + observer.didUpdateVideoTracks(call: self.call, + localVideoTrack: self.localVideoTrack, + localCaptureSession: self.localCaptureSession, remoteVideoTrack: remoteVideoTrack) } @@ -1649,13 +1667,11 @@ private class SignalCallData: NSObject { private func fireDidUpdateVideoTracks() { SwiftAssertIsOnMainThread(#function) - let call = self.call - let localVideoTrack = self.localVideoTrack let remoteVideoTrack = self.isRemoteVideoEnabled ? self.remoteVideoTrack : nil - for observer in observers { - observer.value?.didUpdateVideoTracks(call: call, - localVideoTrack: localVideoTrack, + observer.value?.didUpdateVideoTracks(call: self.call, + localVideoTrack: self.localVideoTrack, + localCaptureSession: self.localCaptureSession, remoteVideoTrack: remoteVideoTrack) } } diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index ed39d674ab..326122a220 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -235,7 +235,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD private var videoCaptureSession: AVCaptureSession? private var videoSender: RTCRtpSender? private var localVideoTrack: RTCVideoTrack? - private var localVideoSource: RTCAVFoundationVideoSource? + private var localVideoSource: RTCVideoSource? // RTCVideoTrack is fragile and prone to throwing exceptions and/or // causing deadlock in its destructor. Therefore we take great care diff --git a/Signal/src/call/UserInterface/CallUIAdapter.swift b/Signal/src/call/UserInterface/CallUIAdapter.swift index 1437bf34bb..bfbf45d09e 100644 --- a/Signal/src/call/UserInterface/CallUIAdapter.swift +++ b/Signal/src/call/UserInterface/CallUIAdapter.swift @@ -273,6 +273,7 @@ extension CallUIAdaptee { internal func didUpdateVideoTracks(call: SignalCall?, localVideoTrack: RTCVideoTrack?, + localCaptureSession: AVCaptureSession?, remoteVideoTrack: RTCVideoTrack?) { SwiftAssertIsOnMainThread(#function) From 0cd1cb80cc059ae199d4ce27965432a0a61cce41 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 25 Jun 2018 14:45:56 -0600 Subject: [PATCH 3/7] Compiling, but video sending not working. --- .../ViewControllers/CallViewController.swift | 7 +- Signal/src/call/CallService.swift | 10 +- Signal/src/call/PeerConnectionClient.swift | 142 +++++++++++++++--- .../call/UserInterface/CallUIAdapter.swift | 4 +- 4 files changed, 126 insertions(+), 37 deletions(-) diff --git a/Signal/src/ViewControllers/CallViewController.swift b/Signal/src/ViewControllers/CallViewController.swift index 4aff7ff130..6a592e362c 100644 --- a/Signal/src/ViewControllers/CallViewController.swift +++ b/Signal/src/ViewControllers/CallViewController.swift @@ -883,13 +883,12 @@ class CallViewController: OWSViewController, CallObserver, CallServiceObserver, } @objc func didPressFlipCamera(sender: UIButton) { - // toggle value sender.isSelected = !sender.isSelected - let useBackCamera = sender.isSelected - Logger.info("\(TAG) in \(#function) with useBackCamera: \(useBackCamera)") + let isUsingFrontCamera = !sender.isSelected + Logger.info("\(TAG) in \(#function) with isUsingFrontCamera: \(isUsingFrontCamera)") - callUIAdapter.setCameraSource(call: call, useBackCamera: useBackCamera) + callUIAdapter.setCameraSource(call: call, isUsingFrontCamera: isUsingFrontCamera) } /** diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index b38ce6ec1c..ae39f6f5b3 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -1278,20 +1278,14 @@ private class SignalCallData: NSObject { self.setHasLocalVideo(hasLocalVideo: true) } - func setCameraSource(call: SignalCall, useBackCamera: Bool) { + func setCameraSource(call: SignalCall, isUsingFrontCamera: Bool) { SwiftAssertIsOnMainThread(#function) - guard call == self.call else { - owsFail("\(logTag) in \(#function) for non-current call.") - return - } - guard let peerConnectionClient = self.peerConnectionClient else { - owsFail("\(logTag) in \(#function) peerConnectionClient was unexpectedly nil") return } - peerConnectionClient.setCameraSource(useBackCamera: useBackCamera) + peerConnectionClient.setCameraSource(isUsingFrontCamera: isUsingFrontCamera) } /** diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index 326122a220..cb141ce300 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -192,7 +192,7 @@ class PeerConnectionProxy: NSObject, RTCPeerConnectionDelegate, RTCDataChannelDe * It is primarily a wrapper around `RTCPeerConnection`, which is responsible for sending and receiving our call data * including audio, video, and some post-connected signaling (hangup, add video) */ -class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelDelegate { +class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelDelegate, VideoCaptureSettingsDelegate { enum Identifiers: String { case mediaStream = "ARDAMS", @@ -232,6 +232,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD // Video + private var videoCaptureController: VideoCaptureController? private var videoCaptureSession: AVCaptureSession? private var videoSender: RTCRtpSender? private var localVideoTrack: RTCVideoTrack? @@ -307,15 +308,21 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD let configuration = RTCDataChannelConfiguration() // Insist upon an "ordered" TCP data channel for delivery reliability. configuration.isOrdered = true - let dataChannel = peerConnection.dataChannel(forLabel: Identifiers.dataChannelSignaling.rawValue, - configuration: configuration) + + guard let dataChannel = peerConnection.dataChannel(forLabel: Identifiers.dataChannelSignaling.rawValue, + configuration: configuration) else { + + // TODO fail outgoing call? + owsFail("dataChannel was unexpectedly nil") + return + } dataChannel.delegate = proxy assert(self.dataChannel == nil) self.dataChannel = dataChannel } - // MARK: Video + // MARK: - Video fileprivate func createVideoSender() { SwiftAssertIsOnMainThread(#function) @@ -331,20 +338,18 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD return } - // TODO: We could cap the maximum video size. - let cameraConstraints = RTCMediaConstraints(mandatoryConstraints: nil, - optionalConstraints: nil) + let videoSource = factory.videoSource() - // TODO: Revisit the cameraConstraints. - let videoSource = factory.avFoundationVideoSource(with: cameraConstraints) + // TODO - MJK I don't think anyone cares about videoSource, just the capturer. Remove it? self.localVideoSource = videoSource - - self.videoCaptureSession = videoSource.captureSession - videoSource.useBackCamera = false + let capturer = RTCCameraVideoCapturer(delegate: videoSource) + self.videoCaptureSession = capturer.captureSession let localVideoTrack = factory.videoTrack(with: videoSource, trackId: Identifiers.videoTrack.rawValue) self.localVideoTrack = localVideoTrack + self.videoCaptureController = VideoCaptureController(capturer: capturer, settingsDelegate: self) + // Disable by default until call is connected. // FIXME - do we require mic permissions at this point? // if so maybe it would be better to not even add the track until the call is connected @@ -356,24 +361,20 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD self.videoSender = videoSender } - public func setCameraSource(useBackCamera: Bool) { + public func setCameraSource(isUsingFrontCamera: Bool) { SwiftAssertIsOnMainThread(#function) let proxyCopy = self.proxy PeerConnectionClient.signalingQueue.async { guard let strongSelf = proxyCopy.get() else { return } - guard let localVideoSource = strongSelf.localVideoSource else { - Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") + + guard let captureController = strongSelf.videoCaptureController else { + owsFail("\(self.logTag) in \(#function) captureController was unexpectedly nil") return } - // certain devices, e.g. 16GB iPod touch don't have a back camera - guard localVideoSource.canUseBackCamera else { - owsFail("\(strongSelf.logTag) in \(#function) canUseBackCamera was unexpectedly false") - return - } - - localVideoSource.useBackCamera = useBackCamera + captureController.switchCamera(isUsingFrontCamera: isUsingFrontCamera) + captureController.startCapture() } } @@ -416,7 +417,18 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD } } - // MARK: Audio + // MARK: VideoCaptureSettingsDelegate + + // MJK: fixme + var videoWidth: Int32 { + return 400 + } + + var videoHeight: Int32 { + return 400 + } + + // MARK: - Audio fileprivate func createAudioSender() { SwiftAssertIsOnMainThread(#function) @@ -1092,6 +1104,90 @@ class HardenedRTCSessionDescription { } } +protocol VideoCaptureSettingsDelegate: class { + var videoWidth: Int32 { get } + var videoHeight: Int32 { get } +} + +class VideoCaptureController { + + let capturer: RTCCameraVideoCapturer + weak var settingsDelegate: VideoCaptureSettingsDelegate? + var isUsingFrontCamera: Bool = true + + public init(capturer: RTCCameraVideoCapturer, settingsDelegate: VideoCaptureSettingsDelegate) { + self.capturer = capturer + self.settingsDelegate = settingsDelegate + } + + public func startCapture() { + let position: AVCaptureDevice.Position = isUsingFrontCamera ? .front : .back + guard let device: AVCaptureDevice = self.device(position: position) else { + owsFail("unable to find captureDevice") + return + } + + guard let format: AVCaptureDevice.Format = self.format(device: device) else { + owsFail("unable to find captureDevice") + return + } + + let fps = self.framesPerSecond(format: format) + + capturer.startCapture(with: device, format: format, fps: fps) + } + + public func stopCapture() { + self.capturer.stopCapture() + } + + public func switchCamera(isUsingFrontCamera: Bool) { + self.isUsingFrontCamera = isUsingFrontCamera + self.startCapture() + } + + private func device(position: AVCaptureDevice.Position) -> AVCaptureDevice? { + let captureDevices = RTCCameraVideoCapturer.captureDevices() + guard let device = (captureDevices.first { $0.position == position }) else { + Logger.debug("unable to find desired position: \(position)") + return captureDevices.first + } + + return device + } + + private func format(device: AVCaptureDevice) -> AVCaptureDevice.Format? { + let formats = RTCCameraVideoCapturer.supportedFormats(for: device) + let targetWidth = settingsDelegate?.videoWidth ?? 0 + let targetHeight = settingsDelegate?.videoHeight ?? 0 + + var selectedFormat: AVCaptureDevice.Format? + var currentDiff: Int32 = Int32.max + + for format in formats { + let dimension = CMVideoFormatDescriptionGetDimensions(format.formatDescription) + let diff = abs(targetWidth - dimension.width) + abs(targetHeight - dimension.height) + if diff < currentDiff { + selectedFormat = format + currentDiff = diff + } + } + + assert(selectedFormat != nil) + + return selectedFormat + } + + private func framesPerSecond(format: AVCaptureDevice.Format) -> Int { + var maxFrameRate: Float64 = 0 + for range in format.videoSupportedFrameRateRanges { + maxFrameRate = max(maxFrameRate, range.maxFrameRate) + } + + return Int(maxFrameRate) + } +} + // Mark: Pretty Print Objc enums. fileprivate extension RTCSignalingState { diff --git a/Signal/src/call/UserInterface/CallUIAdapter.swift b/Signal/src/call/UserInterface/CallUIAdapter.swift index bfbf45d09e..f521b86385 100644 --- a/Signal/src/call/UserInterface/CallUIAdapter.swift +++ b/Signal/src/call/UserInterface/CallUIAdapter.swift @@ -250,10 +250,10 @@ extension CallUIAdaptee { call.audioSource = audioSource } - internal func setCameraSource(call: SignalCall, useBackCamera: Bool) { + internal func setCameraSource(call: SignalCall, isUsingFrontCamera: Bool) { SwiftAssertIsOnMainThread(#function) - callService.setCameraSource(call: call, useBackCamera: useBackCamera) + callService.setCameraSource(call: call, isUsingFrontCamera: isUsingFrontCamera) } // CallKit handles ringing state on it's own. But for non-call kit we trigger ringing start/stop manually. From afa385feae53e1325c66806d070cd4fe2e67c7dc Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 25 Jun 2018 15:02:52 -0600 Subject: [PATCH 4/7] adapt to capturer abstraction --- Signal/src/call/CallService.swift | 6 +++-- Signal/src/call/PeerConnectionClient.swift | 29 ++++++++++++++++------ 2 files changed, 25 insertions(+), 10 deletions(-) diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index ae39f6f5b3..83d6d92e95 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -1422,7 +1422,7 @@ private class SignalCallData: NSObject { self.handleDataChannelMessage(dataChannelMessage) } - internal func peerConnectionClient(_ peerConnectionClient: PeerConnectionClient, didUpdateLocal videoTrack: RTCVideoTrack?) { + internal func peerConnectionClient(_ peerConnectionClient: PeerConnectionClient, didUpdateLocalVideoTrack videoTrack: RTCVideoTrack?, captureSession: AVCaptureSession?) { SwiftAssertIsOnMainThread(#function) guard peerConnectionClient == self.peerConnectionClient else { @@ -1434,11 +1434,13 @@ private class SignalCallData: NSObject { return } + // MJK TODO remove localVideo Track? callData.localVideoTrack = videoTrack + callData.localCaptureSession = captureSession fireDidUpdateVideoTracks() } - internal func peerConnectionClient(_ peerConnectionClient: PeerConnectionClient, didUpdateRemote videoTrack: RTCVideoTrack?) { + internal func peerConnectionClient(_ peerConnectionClient: PeerConnectionClient, didUpdateRemoteVideoTrack videoTrack: RTCVideoTrack?) { SwiftAssertIsOnMainThread(#function) guard peerConnectionClient == self.peerConnectionClient else { diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index cb141ce300..c74e5369d1 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -58,12 +58,12 @@ protocol PeerConnectionClientDelegate: class { /** * Fired whenever the local video track become active or inactive. */ - func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, didUpdateLocal videoTrack: RTCVideoTrack?) + func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, didUpdateLocalVideoTrack videoTrack: RTCVideoTrack?, captureSession: AVCaptureSession?) /** * Fired whenever the remote video track become active or inactive. */ - func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, didUpdateRemote videoTrack: RTCVideoTrack?) + func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, didUpdateRemoteVideoTrack videoTrack: RTCVideoTrack?) } // In Swift (at least in Swift v3.3), weak variables aren't thread safe. It @@ -383,9 +383,16 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD let proxyCopy = self.proxy let completion = { guard let strongSelf = proxyCopy.get() else { return } + + // Should these really be guards? Don't we want to pass nil when it's been disabled? guard let localVideoTrack = strongSelf.localVideoTrack else { return } + guard let videoCaptureSession = strongSelf.videoCaptureSession else { return } guard let strongDelegate = strongSelf.delegate else { return } - strongDelegate.peerConnectionClient(strongSelf, didUpdateLocal: enabled ? localVideoTrack : nil) + + let videoTrack = enabled ? localVideoTrack : nil + let captureSession = enabled ? videoCaptureSession : nil + + strongDelegate.peerConnectionClient(strongSelf, didUpdateLocalVideoTrack: videoTrack, captureSession: captureSession) } PeerConnectionClient.signalingQueue.async { @@ -398,19 +405,25 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } - guard let videoCaptureSession = strongSelf.videoCaptureSession else { +// guard let videoCaptureSession = strongSelf.videoCaptureSession else { +// Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") +// return +// } + + guard let videoCaptureController = strongSelf.videoCaptureController else { Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } - localVideoTrack.isEnabled = enabled if enabled { Logger.debug("\(strongSelf.logTag) in \(#function) starting videoCaptureSession") - videoCaptureSession.startRunning() + videoCaptureController.startCapture() +// videoCaptureSession.startRunning() } else { Logger.debug("\(strongSelf.logTag) in \(#function) stopping videoCaptureSession") - videoCaptureSession.stopRunning() + videoCaptureController.stopCapture() +// videoCaptureSession.stopRunning() } DispatchQueue.main.async(execute: completion) @@ -872,7 +885,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD // TODO: Consider checking for termination here. - strongDelegate.peerConnectionClient(strongSelf, didUpdateRemote: remoteVideoTrack) + strongDelegate.peerConnectionClient(strongSelf, didUpdateRemoteVideoTrack: remoteVideoTrack) } PeerConnectionClient.signalingQueue.async { From 61156656aa4ca57508a865c4e4ae321c16fcacbf Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 25 Jun 2018 15:17:23 -0600 Subject: [PATCH 5/7] Only PCC needs to know about the local RTCTrack --- .../ViewControllers/CallViewController.swift | 13 ++-------- Signal/src/call/CallService.swift | 23 +----------------- Signal/src/call/PeerConnectionClient.swift | 24 +++++++------------ .../call/UserInterface/CallUIAdapter.swift | 1 - 4 files changed, 11 insertions(+), 50 deletions(-) diff --git a/Signal/src/ViewControllers/CallViewController.swift b/Signal/src/ViewControllers/CallViewController.swift index 6a592e362c..617d6012ee 100644 --- a/Signal/src/ViewControllers/CallViewController.swift +++ b/Signal/src/ViewControllers/CallViewController.swift @@ -72,7 +72,6 @@ class CallViewController: OWSViewController, CallObserver, CallServiceObserver, var remoteVideoView: RemoteVideoView! var localVideoView: RTCCameraPreviewView! var hasShownLocalVideo = false - weak var localVideoTrack: RTCVideoTrack? weak var localCaptureSession: AVCaptureSession? weak var remoteVideoTrack: RTCVideoTrack? @@ -1001,16 +1000,10 @@ class CallViewController: OWSViewController, CallObserver, CallServiceObserver, // MARK: - Video - // MJK TODO remove localVideoTrack? - internal func updateLocalVideoTrack(localVideoTrack: RTCVideoTrack?, - captureSession: AVCaptureSession?) { + internal func updateLocalVideo(captureSession: AVCaptureSession?) { SwiftAssertIsOnMainThread(#function) - guard self.localVideoTrack != localVideoTrack else { - return - } - self.localVideoTrack = localVideoTrack localVideoView.captureSession = captureSession let isHidden = captureSession == nil @@ -1118,14 +1111,12 @@ class CallViewController: OWSViewController, CallObserver, CallServiceObserver, // Do nothing. } - // TODO remove localCaptureSession: internal func didUpdateVideoTracks(call: SignalCall?, - localVideoTrack: RTCVideoTrack?, localCaptureSession: AVCaptureSession?, remoteVideoTrack: RTCVideoTrack?) { SwiftAssertIsOnMainThread(#function) - updateLocalVideoTrack(localVideoTrack: localVideoTrack, captureSession: localCaptureSession) + updateLocalVideo(captureSession: localCaptureSession) updateRemoteVideoTrack(remoteVideoTrack: remoteVideoTrack) } } diff --git a/Signal/src/call/CallService.swift b/Signal/src/call/CallService.swift index 83d6d92e95..f4bff4cc3e 100644 --- a/Signal/src/call/CallService.swift +++ b/Signal/src/call/CallService.swift @@ -93,9 +93,7 @@ protocol CallServiceObserver: class { /** * Fired whenever the local or remote video track become active or inactive. */ - // TODO remove localCaptureSession: func didUpdateVideoTracks(call: SignalCall?, - localVideoTrack: RTCVideoTrack?, localCaptureSession: AVCaptureSession?, remoteVideoTrack: RTCVideoTrack?) } @@ -119,14 +117,6 @@ private class SignalCallData: NSObject { let rejectReadyToSendIceUpdatesPromise: ((Error) -> Void) let readyToSendIceUpdatesPromise: Promise - weak var localVideoTrack: RTCVideoTrack? { - didSet { - SwiftAssertIsOnMainThread(#function) - - Logger.info("\(self.logTag) \(#function)") - } - } - weak var localCaptureSession: AVCaptureSession? { didSet { SwiftAssertIsOnMainThread(#function) @@ -285,13 +275,6 @@ private class SignalCallData: NSObject { return callData?.peerConnectionClient } } - var localVideoTrack: RTCVideoTrack? { - get { - SwiftAssertIsOnMainThread(#function) - - return callData?.localVideoTrack - } - } weak var localCaptureSession: AVCaptureSession? { get { @@ -1422,7 +1405,7 @@ private class SignalCallData: NSObject { self.handleDataChannelMessage(dataChannelMessage) } - internal func peerConnectionClient(_ peerConnectionClient: PeerConnectionClient, didUpdateLocalVideoTrack videoTrack: RTCVideoTrack?, captureSession: AVCaptureSession?) { + internal func peerConnectionClient(_ peerConnectionClient: PeerConnectionClient, didUpdateLocalVideoCaptureSession captureSession: AVCaptureSession?) { SwiftAssertIsOnMainThread(#function) guard peerConnectionClient == self.peerConnectionClient else { @@ -1434,8 +1417,6 @@ private class SignalCallData: NSObject { return } - // MJK TODO remove localVideo Track? - callData.localVideoTrack = videoTrack callData.localCaptureSession = captureSession fireDidUpdateVideoTracks() } @@ -1639,7 +1620,6 @@ private class SignalCallData: NSObject { // Synchronize observer with current call state let remoteVideoTrack = self.isRemoteVideoEnabled ? self.remoteVideoTrack : nil observer.didUpdateVideoTracks(call: self.call, - localVideoTrack: self.localVideoTrack, localCaptureSession: self.localCaptureSession, remoteVideoTrack: remoteVideoTrack) } @@ -1666,7 +1646,6 @@ private class SignalCallData: NSObject { let remoteVideoTrack = self.isRemoteVideoEnabled ? self.remoteVideoTrack : nil for observer in observers { observer.value?.didUpdateVideoTracks(call: self.call, - localVideoTrack: self.localVideoTrack, localCaptureSession: self.localCaptureSession, remoteVideoTrack: remoteVideoTrack) } diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index c74e5369d1..9a7f062b67 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -58,7 +58,7 @@ protocol PeerConnectionClientDelegate: class { /** * Fired whenever the local video track become active or inactive. */ - func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, didUpdateLocalVideoTrack videoTrack: RTCVideoTrack?, captureSession: AVCaptureSession?) + func peerConnectionClient(_ peerconnectionClient: PeerConnectionClient, didUpdateLocalVideoCaptureSession captureSession: AVCaptureSession?) /** * Fired whenever the remote video track become active or inactive. @@ -235,12 +235,12 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD private var videoCaptureController: VideoCaptureController? private var videoCaptureSession: AVCaptureSession? private var videoSender: RTCRtpSender? - private var localVideoTrack: RTCVideoTrack? private var localVideoSource: RTCVideoSource? // RTCVideoTrack is fragile and prone to throwing exceptions and/or // causing deadlock in its destructor. Therefore we take great care // with this property. + private var localVideoTrack: RTCVideoTrack? private var remoteVideoTrack: RTCVideoTrack? private var cameraConstraints: RTCMediaConstraints @@ -347,7 +347,6 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD let localVideoTrack = factory.videoTrack(with: videoSource, trackId: Identifiers.videoTrack.rawValue) self.localVideoTrack = localVideoTrack - self.videoCaptureController = VideoCaptureController(capturer: capturer, settingsDelegate: self) // Disable by default until call is connected. @@ -385,14 +384,12 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD guard let strongSelf = proxyCopy.get() else { return } // Should these really be guards? Don't we want to pass nil when it's been disabled? - guard let localVideoTrack = strongSelf.localVideoTrack else { return } guard let videoCaptureSession = strongSelf.videoCaptureSession else { return } guard let strongDelegate = strongSelf.delegate else { return } - let videoTrack = enabled ? localVideoTrack : nil let captureSession = enabled ? videoCaptureSession : nil - strongDelegate.peerConnectionClient(strongSelf, didUpdateLocalVideoTrack: videoTrack, captureSession: captureSession) + strongDelegate.peerConnectionClient(strongSelf, didUpdateLocalVideoCaptureSession: captureSession) } PeerConnectionClient.signalingQueue.async { @@ -401,29 +398,24 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } - guard let localVideoTrack = strongSelf.localVideoTrack else { + + guard let videoCaptureController = strongSelf.videoCaptureController else { Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } -// guard let videoCaptureSession = strongSelf.videoCaptureSession else { -// Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") -// return -// } - guard let videoCaptureController = strongSelf.videoCaptureController else { + guard let localVideoTrack = strongSelf.localVideoTrack else { Logger.debug("\(strongSelf.logTag) \(#function) Ignoring obsolete event in terminated client") return } localVideoTrack.isEnabled = enabled if enabled { - Logger.debug("\(strongSelf.logTag) in \(#function) starting videoCaptureSession") + Logger.debug("\(strongSelf.logTag) in \(#function) starting video capture") videoCaptureController.startCapture() -// videoCaptureSession.startRunning() } else { - Logger.debug("\(strongSelf.logTag) in \(#function) stopping videoCaptureSession") + Logger.debug("\(strongSelf.logTag) in \(#function) stopping video capture") videoCaptureController.stopCapture() -// videoCaptureSession.stopRunning() } DispatchQueue.main.async(execute: completion) diff --git a/Signal/src/call/UserInterface/CallUIAdapter.swift b/Signal/src/call/UserInterface/CallUIAdapter.swift index f521b86385..0031fd2236 100644 --- a/Signal/src/call/UserInterface/CallUIAdapter.swift +++ b/Signal/src/call/UserInterface/CallUIAdapter.swift @@ -272,7 +272,6 @@ extension CallUIAdaptee { } internal func didUpdateVideoTracks(call: SignalCall?, - localVideoTrack: RTCVideoTrack?, localCaptureSession: AVCaptureSession?, remoteVideoTrack: RTCVideoTrack?) { SwiftAssertIsOnMainThread(#function) From af603e53c7c25889e293549a9a0d20a807b7af15 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 25 Jun 2018 15:44:57 -0600 Subject: [PATCH 6/7] remove more unused state from PCC --- Signal/src/call/PeerConnectionClient.swift | 29 +++++++++++----------- 1 file changed, 15 insertions(+), 14 deletions(-) diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index 9a7f062b67..0400b28161 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -233,9 +233,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD // Video private var videoCaptureController: VideoCaptureController? - private var videoCaptureSession: AVCaptureSession? private var videoSender: RTCRtpSender? - private var localVideoSource: RTCVideoSource? // RTCVideoTrack is fragile and prone to throwing exceptions and/or // causing deadlock in its destructor. Therefore we take great care @@ -340,21 +338,17 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD let videoSource = factory.videoSource() - // TODO - MJK I don't think anyone cares about videoSource, just the capturer. Remove it? - self.localVideoSource = videoSource - let capturer = RTCCameraVideoCapturer(delegate: videoSource) - self.videoCaptureSession = capturer.captureSession - let localVideoTrack = factory.videoTrack(with: videoSource, trackId: Identifiers.videoTrack.rawValue) self.localVideoTrack = localVideoTrack - self.videoCaptureController = VideoCaptureController(capturer: capturer, settingsDelegate: self) - // Disable by default until call is connected. // FIXME - do we require mic permissions at this point? // if so maybe it would be better to not even add the track until the call is connected // instead of creating it and disabling it. localVideoTrack.isEnabled = false + let capturer = RTCCameraVideoCapturer(delegate: videoSource) + self.videoCaptureController = VideoCaptureController(capturer: capturer, settingsDelegate: self) + let videoSender = peerConnection.sender(withKind: kVideoTrackType, streamId: Identifiers.mediaStream.rawValue) videoSender.track = localVideoTrack self.videoSender = videoSender @@ -382,12 +376,20 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD let proxyCopy = self.proxy let completion = { guard let strongSelf = proxyCopy.get() else { return } - - // Should these really be guards? Don't we want to pass nil when it's been disabled? - guard let videoCaptureSession = strongSelf.videoCaptureSession else { return } guard let strongDelegate = strongSelf.delegate else { return } - let captureSession = enabled ? videoCaptureSession : nil + let captureSession: AVCaptureSession? = { + guard enabled else { + return nil + } + + guard let captureController = strongSelf.videoCaptureController else { + owsFail("\(self.logTag) in \(#function) videoCaptureController was unexpectedly nil") + return nil + } + + return captureController.capturer.captureSession + }() strongDelegate.peerConnectionClient(strongSelf, didUpdateLocalVideoCaptureSession: captureSession) } @@ -760,7 +762,6 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD audioSender = nil audioTrack = nil videoSender = nil - localVideoSource = nil localVideoTrack = nil remoteVideoTrack = nil From 38ee3653f78f5f63ea8fd7cd20eed11406b2f3b7 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 25 Jun 2018 16:44:29 -0600 Subject: [PATCH 7/7] synchronize access to CaptureController state // FREEBIE --- Signal/src/call/PeerConnectionClient.swift | 42 ++++++++++++++++++---- 1 file changed, 36 insertions(+), 6 deletions(-) diff --git a/Signal/src/call/PeerConnectionClient.swift b/Signal/src/call/PeerConnectionClient.swift index 0400b28161..5f6e162257 100644 --- a/Signal/src/call/PeerConnectionClient.swift +++ b/Signal/src/call/PeerConnectionClient.swift @@ -367,7 +367,6 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD } captureController.switchCamera(isUsingFrontCamera: isUsingFrontCamera) - captureController.startCapture() } } @@ -426,7 +425,6 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD // MARK: VideoCaptureSettingsDelegate - // MJK: fixme var videoWidth: Int32 { return 400 } @@ -764,6 +762,7 @@ class PeerConnectionClient: NSObject, RTCPeerConnectionDelegate, RTCDataChannelD videoSender = nil localVideoTrack = nil remoteVideoTrack = nil + videoCaptureController = nil if let peerConnection = peerConnection { peerConnection.delegate = nil @@ -1117,16 +1116,35 @@ protocol VideoCaptureSettingsDelegate: class { class VideoCaptureController { + let serialQueue = DispatchQueue(label: "org.signal.videoCaptureController") let capturer: RTCCameraVideoCapturer weak var settingsDelegate: VideoCaptureSettingsDelegate? var isUsingFrontCamera: Bool = true + func assertIsOnSerialQueue() { + if _isDebugAssertConfiguration(), #available(iOS 10.0, *) { + assertOnQueue(serialQueue) + } + } + public init(capturer: RTCCameraVideoCapturer, settingsDelegate: VideoCaptureSettingsDelegate) { self.capturer = capturer self.settingsDelegate = settingsDelegate } public func startCapture() { + serialQueue.sync { [weak self] in + guard let strongSelf = self else { + return + } + + strongSelf.startCaptureSync() + } + } + + private func startCaptureSync() { + assertIsOnSerialQueue() + let position: AVCaptureDevice.Position = isUsingFrontCamera ? .front : .back guard let device: AVCaptureDevice = self.device(position: position) else { owsFail("unable to find captureDevice") @@ -1139,17 +1157,24 @@ class VideoCaptureController { } let fps = self.framesPerSecond(format: format) - capturer.startCapture(with: device, format: format, fps: fps) } public func stopCapture() { - self.capturer.stopCapture() + serialQueue.sync { [weak self] in + guard let strongSelf = self else { + return + } + + strongSelf.capturer.stopCapture() + } } public func switchCamera(isUsingFrontCamera: Bool) { - self.isUsingFrontCamera = isUsingFrontCamera - self.startCapture() + serialQueue.sync { + self.isUsingFrontCamera = isUsingFrontCamera + self.startCaptureSync() + } } private func device(position: AVCaptureDevice.Position) -> AVCaptureDevice? { @@ -1179,6 +1204,11 @@ class VideoCaptureController { } } + if _isDebugAssertConfiguration(), let selectedFormat = selectedFormat { + let dimension = CMVideoFormatDescriptionGetDimensions(selectedFormat.formatDescription) + Logger.debug("in \(#function) selected format width: \(dimension.width) height: \(dimension.height)") + } + assert(selectedFormat != nil) return selectedFormat