From 21fd7b040eed9e5b57010e8e19b4b3d4aaba9aa0 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Sun, 10 Dec 2017 16:17:35 -0500 Subject: [PATCH 01/15] Ensure sent video is mp4 // FREEBIE --- .../attachments/SignalAttachment.swift | 96 ++++++++++++++++++- .../ShareViewController.swift | 1 + 2 files changed, 93 insertions(+), 4 deletions(-) diff --git a/SignalMessaging/attachments/SignalAttachment.swift b/SignalMessaging/attachments/SignalAttachment.swift index 72b0434e5..8cd6c42da 100644 --- a/SignalMessaging/attachments/SignalAttachment.swift +++ b/SignalMessaging/attachments/SignalAttachment.swift @@ -13,6 +13,7 @@ enum SignalAttachmentError: Error { case invalidData case couldNotParseImage case couldNotConvertToJpeg + case couldNotConvertToMpeg4 case invalidFileFormat } @@ -49,6 +50,8 @@ extension SignalAttachmentError: LocalizedError { return NSLocalizedString("ATTACHMENT_ERROR_COULD_NOT_CONVERT_TO_JPEG", comment: "Attachment error message for image attachments which could not be converted to JPEG") case .invalidFileFormat: return NSLocalizedString("ATTACHMENT_ERROR_INVALID_FILE_FORMAT", comment: "Attachment error message for attachments with an invalid file format") + case .couldNotConvertToMpeg4: + return NSLocalizedString("ATTACHMENT_ERROR_COULD_NOT_CONVERT_TO_MP4", comment: "Attachment error message for video attachments which could not be converted to MP4") } } } @@ -353,6 +356,10 @@ public class SignalAttachment: NSObject { return MIMETypeUtil.supportedImageUTITypes().union(animatedImageUTISet) } + private class var outputVideoUTISet: Set { + return Set([kUTTypeMPEG4 as String]) + } + // Returns the set of UTIs that correspond to valid animated image formats // for Signal attachments. private class var animatedImageUTISet: Set { @@ -767,10 +774,91 @@ public class SignalAttachment: NSObject { // NOTE: The attachment returned by this method may not be valid. // Check the attachment's error property. private class func videoAttachment(dataSource: DataSource?, dataUTI: String) -> SignalAttachment { - return newAttachment(dataSource : dataSource, - dataUTI : dataUTI, - validUTISet : videoUTISet, - maxFileSize : kMaxFileSizeVideo) + guard let dataSource = dataSource else { + let dataSource = DataSourceValue.emptyDataSource() + let attachment = SignalAttachment(dataSource:dataSource, dataUTI: dataUTI) + attachment.error = .missingData + return attachment + } + + if isInputVideoValidOutputVideo(dataSource: dataSource, dataUTI: dataUTI) { + return newAttachment(dataSource: dataSource, + dataUTI: dataUTI, + validUTISet: videoUTISet, + maxFileSize: kMaxFileSizeVideo) + } else { + // convert to mp4 + return compressVideoAsMp4(dataSource: dataSource, dataUTI: dataUTI) + } + } + + class var videoTempPath: URL { + let videoDir = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent("video") + OWSFileSystem.ensureDirectoryExists(videoDir.path) + return videoDir + } + + class func compressVideoAsMp4(dataSource: DataSource, dataUTI: String) -> SignalAttachment { + Logger.debug("\(self.TAG) in \(#function)") + + guard let url = dataSource.dataUrl() else { + let attachment = SignalAttachment(dataSource : DataSourceValue.emptyDataSource(), dataUTI: dataUTI) + attachment.error = .missingData + return attachment + } + + let asset = AVAsset(url: url) + + guard let exportSession = AVAssetExportSession(asset: asset, presetName: AVAssetExportPresetMediumQuality) else { + let attachment = SignalAttachment(dataSource : DataSourceValue.emptyDataSource(), dataUTI: dataUTI) + attachment.error = .couldNotConvertToMpeg4 + return attachment + } + + exportSession.shouldOptimizeForNetworkUse = true + exportSession.outputFileType = AVFileTypeMPEG4 + + let exportURL = videoTempPath.appendingPathComponent(UUID().uuidString).appendingPathExtension("mp4") + exportSession.outputURL = exportURL + + Logger.debug("\(self.TAG) starting video export") + let semaphore = DispatchSemaphore(value: 0) + exportSession.exportAsynchronously { + Logger.debug("\(self.TAG) Completed video export") + semaphore.signal() + } + + // FIXME make the API async, return progress. + Logger.debug("\(self.TAG) Waiting for video export") + semaphore.wait() + Logger.debug("\(self.TAG) Done waiting for video export") + + let baseFilename = dataSource.sourceFilename + let mp4Filename = baseFilename?.filenameWithoutExtension.appendingFileExtension("mp4") + + guard let dataSource = DataSourcePath.dataSource(with: exportURL) else { + owsFail("Failed to build data source for exported video URL") + let attachment = SignalAttachment(dataSource : DataSourceValue.emptyDataSource(), dataUTI: dataUTI) + attachment.error = .couldNotConvertToMpeg4 + return attachment + } + dataSource.sourceFilename = mp4Filename + return SignalAttachment(dataSource: dataSource, dataUTI: kUTTypeMPEG4 as String) + } + + class func isInputVideoValidOutputVideo(dataSource: DataSource?, dataUTI: String) -> Bool { + guard let dataSource = dataSource else { + return false + } + + guard SignalAttachment.outputVideoUTISet.contains(dataUTI) else { + return false + } + + if dataSource.dataLength() <= kMaxFileSizeVideo { + return true + } + return false } // MARK: Audio Attachments diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index 55a3a8ddb..7af280978 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -459,6 +459,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE if url.pathExtension.count > 0 { // Determine a more specific utiType based on file extension if let typeExtension = MIMETypeUtil.utiType(forFileExtension: url.pathExtension) { + Logger.debug("\(self.logTag) utiType based on extension: \(typeExtension)") specificUTIType = typeExtension } } From 538b3e5fd56d43815243ed1d08372f5b2d4cc8a4 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Sun, 10 Dec 2017 16:43:24 -0500 Subject: [PATCH 02/15] Async API for video export // FREEBIE --- .../attachments/SignalAttachment.swift | 74 ++++++++++++------- .../ShareViewController.swift | 11 ++- 2 files changed, 54 insertions(+), 31 deletions(-) diff --git a/SignalMessaging/attachments/SignalAttachment.swift b/SignalMessaging/attachments/SignalAttachment.swift index 8cd6c42da..d9af6d74b 100644 --- a/SignalMessaging/attachments/SignalAttachment.swift +++ b/SignalMessaging/attachments/SignalAttachment.swift @@ -5,6 +5,7 @@ import Foundation import MobileCoreServices import SignalServiceKit +import PromiseKit import AVFoundation enum SignalAttachmentError: Error { @@ -781,30 +782,32 @@ public class SignalAttachment: NSObject { return attachment } - if isInputVideoValidOutputVideo(dataSource: dataSource, dataUTI: dataUTI) { - return newAttachment(dataSource: dataSource, - dataUTI: dataUTI, - validUTISet: videoUTISet, - maxFileSize: kMaxFileSizeVideo) - } else { - // convert to mp4 - return compressVideoAsMp4(dataSource: dataSource, dataUTI: dataUTI) + if !isInputVideoValidOutputVideo(dataSource: dataSource, dataUTI: dataUTI) { + // Most people won't hit this because we convert video when picked from the media picker + // But the current API allos sending videos that some Signal clients will not + // be able to view. (e.g. when picked from document picker) + owsFail("building video with invalid output, migrate to async API using compressVideoAsMp4") } + + return newAttachment(dataSource: dataSource, + dataUTI: dataUTI, + validUTISet: videoUTISet, + maxFileSize: kMaxFileSizeVideo) } - class var videoTempPath: URL { + private class var videoTempPath: URL { let videoDir = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent("video") OWSFileSystem.ensureDirectoryExists(videoDir.path) return videoDir } - class func compressVideoAsMp4(dataSource: DataSource, dataUTI: String) -> SignalAttachment { + public class func compressVideoAsMp4(dataSource: DataSource, dataUTI: String) -> (Promise, AVAssetExportSession?) { Logger.debug("\(self.TAG) in \(#function)") guard let url = dataSource.dataUrl() else { let attachment = SignalAttachment(dataSource : DataSourceValue.emptyDataSource(), dataUTI: dataUTI) attachment.error = .missingData - return attachment + return (Promise(value: attachment), nil) } let asset = AVAsset(url: url) @@ -812,7 +815,7 @@ public class SignalAttachment: NSObject { guard let exportSession = AVAssetExportSession(asset: asset, presetName: AVAssetExportPresetMediumQuality) else { let attachment = SignalAttachment(dataSource : DataSourceValue.emptyDataSource(), dataUTI: dataUTI) attachment.error = .couldNotConvertToMpeg4 - return attachment + return (Promise(value: attachment), nil) } exportSession.shouldOptimizeForNetworkUse = true @@ -821,32 +824,47 @@ public class SignalAttachment: NSObject { let exportURL = videoTempPath.appendingPathComponent(UUID().uuidString).appendingPathExtension("mp4") exportSession.outputURL = exportURL + let (promise, fulfill, _) = Promise.pending() + Logger.debug("\(self.TAG) starting video export") - let semaphore = DispatchSemaphore(value: 0) exportSession.exportAsynchronously { Logger.debug("\(self.TAG) Completed video export") - semaphore.signal() + let baseFilename = dataSource.sourceFilename + let mp4Filename = baseFilename?.filenameWithoutExtension.appendingFileExtension("mp4") + + guard let dataSource = DataSourcePath.dataSource(with: exportURL) else { + owsFail("Failed to build data source for exported video URL") + let attachment = SignalAttachment(dataSource : DataSourceValue.emptyDataSource(), dataUTI: dataUTI) + attachment.error = .couldNotConvertToMpeg4 + fulfill(attachment) + return + } + + dataSource.sourceFilename = mp4Filename + + let attachment = SignalAttachment(dataSource: dataSource, dataUTI: kUTTypeMPEG4 as String) + fulfill(attachment) } - // FIXME make the API async, return progress. - Logger.debug("\(self.TAG) Waiting for video export") - semaphore.wait() - Logger.debug("\(self.TAG) Done waiting for video export") + return (promise, exportSession) + } - let baseFilename = dataSource.sourceFilename - let mp4Filename = baseFilename?.filenameWithoutExtension.appendingFileExtension("mp4") + public class func isInvalidVideo(dataSource: DataSource, dataUTI: String) -> Bool { + guard videoUTISet.contains(dataUTI) else { + // not a video + return false + } - guard let dataSource = DataSourcePath.dataSource(with: exportURL) else { - owsFail("Failed to build data source for exported video URL") - let attachment = SignalAttachment(dataSource : DataSourceValue.emptyDataSource(), dataUTI: dataUTI) - attachment.error = .couldNotConvertToMpeg4 - return attachment + guard isInputVideoValidOutputVideo(dataSource: dataSource, dataUTI: dataUTI) else { + // found a video which needs to be converted + return true } - dataSource.sourceFilename = mp4Filename - return SignalAttachment(dataSource: dataSource, dataUTI: kUTTypeMPEG4 as String) + + // It is a video, but it's not invalid + return false } - class func isInputVideoValidOutputVideo(dataSource: DataSource?, dataUTI: String) -> Bool { + private class func isInputVideoValidOutputVideo(dataSource: DataSource?, dataUTI: String) -> Bool { guard let dataSource = dataSource else { return false } diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index 7af280978..687cea9b0 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -448,7 +448,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // TODO accept other data types // TODO whitelist attachment types // TODO coerce when necessary and possible - return promise.then { (url: URL) -> SignalAttachment in + return promise.then { (url: URL) -> Promise in guard let dataSource = DataSourcePath.dataSource(with: url) else { throw ShareViewControllerError.assertionError(description: "Unable to read attachment data") } @@ -464,9 +464,14 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE } } - let attachment = SignalAttachment.attachment(dataSource: dataSource, dataUTI: specificUTIType, imageQuality:.medium) + guard !SignalAttachment.isInvalidVideo(dataSource: dataSource, dataUTI: specificUTIType) else { + let (promise, exportSession) = SignalAttachment.compressVideoAsMp4(dataSource: dataSource, dataUTI: specificUTIType, imageQuality: .medium) + // TODO show progress with exportSession + return promise + } - return attachment + let attachment = SignalAttachment.attachment(dataSource: dataSource, dataUTI: specificUTIType) + return Promise(value: attachment) } } } From b1b6dcfbf004a3609031673563868632cdc826ec Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Sun, 10 Dec 2017 17:48:31 -0500 Subject: [PATCH 03/15] Simplify loading delay, use loading screen as activity indicator for video conversion // FREEBIE --- .../ShareViewController.swift | 58 ++++++++++--------- 1 file changed, 30 insertions(+), 28 deletions(-) diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index 687cea9b0..f13aaf8b3 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -15,21 +15,11 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE private var hasInitialRootViewController = false private var isReadyForAppExtensions = false - var loadViewController: SAELoadViewController! - override open func loadView() { super.loadView() Logger.debug("\(self.logTag) \(#function)") - // We can't show the conversation picker until the DB is set up. - // Normally this will only take a moment, so rather than flickering and then hiding the loading screen - // We start as invisible, and only fade it in if it's going to take a while - self.view.alpha = 0 - UIView.animate(withDuration: 0.1, delay: 0.5, options: [.curveEaseInOut], animations: { - self.view.alpha = 1 - }, completion: nil) - // This should be the first thing we do. let appContext = ShareAppExtensionContext(rootViewController:self) SetCurrentAppContext(appContext) @@ -75,6 +65,20 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE return } + // Don't display load screen immediately, in hopes that we can avoid it altogether. + after(seconds: 2).then { () -> Void in + guard self.presentedViewController == nil else { + Logger.debug("\(self.logTag) setup completed quickly, no need to present load view controller.") + return + } + + Logger.debug("\(self.logTag) setup is slow - showing loading screen") + + let loadViewController = SAELoadViewController(delegate: self) + let navigationController = UINavigationController(rootViewController: loadViewController) + self.present(navigationController, animated: true) + }.retainUntilComplete() + // We shouldn't set up our environment until after we've consulted isReadyForAppExtensions. AppSetup.setupEnvironment({ return NoopCallMessageHandler() @@ -86,8 +90,6 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // upgrade process may depend on Environment. VersionMigrations.performUpdateCheck() - self.loadViewController = SAELoadViewController(delegate:self) - self.pushViewController(loadViewController, animated: false) self.isNavigationBarHidden = true // We don't need to use "screen protection" in the SAE. @@ -290,13 +292,6 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE } private func showErrorView(title: String, message: String) { - // ensure view is visible. - self.view.layer.removeAllAnimations() - UIView.animate(withDuration: 0.1, delay: 0, options: [.curveEaseInOut], animations: { - - self.view.alpha = 1 - }, completion: nil) - let viewController = SAEFailedViewController(delegate:self, title:title, message:message) self.setViewControllers([viewController], animated: false) } @@ -364,19 +359,21 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // MARK: Helpers private func presentConversationPicker() { - // pause any animation revealing the "loading" screen - self.view.layer.removeAllAnimations() - - // Once we've presented the conversation picker, we hide the loading VC - // so that it's not revealed when we eventually dismiss the share extension. - loadViewController.view.isHidden = true - self.buildAttachment().then { attachment -> Void in let conversationPicker = SharingThreadPickerViewController(shareViewDelegate: self) let navigationController = UINavigationController(rootViewController: conversationPicker) navigationController.isNavigationBarHidden = true conversationPicker.attachment = attachment - self.present(navigationController, animated: true, completion: nil) + if let presentedViewController = self.presentedViewController { + Logger.debug("\(self.logTag) dismissing \(presentedViewController) before presenting conversation picker") + self.dismiss(animated: true) { + self.present(navigationController, animated: true) + } + } else { + Logger.debug("\(self.logTag) no other modal, presenting conversation picker immediately") + self.present(navigationController, animated: true) + } + Logger.info("showing picker with attachment: \(attachment)") }.catch { error in let alertTitle = NSLocalizedString("SHARE_EXTENSION_UNABLE_TO_BUILD_ATTACHMENT_ALERT_TITLE", comment: "Shown when trying to share content to a Signal user for the share extension. Followed by failure details.") @@ -465,8 +462,13 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE } guard !SignalAttachment.isInvalidVideo(dataSource: dataSource, dataUTI: specificUTIType) else { - let (promise, exportSession) = SignalAttachment.compressVideoAsMp4(dataSource: dataSource, dataUTI: specificUTIType, imageQuality: .medium) // TODO show progress with exportSession + let (promise, exportSession) = SignalAttachment.compressVideoAsMp4(dataSource: dataSource, dataUTI: specificUTIType, imageQuality: .medium) + + // TODO use `exportSession.progress` to show a more precise progress indicator in the loadView, maybe sharing the "sending" progress UI. + // TODO expose "Cancel" + // Can we move this process to the end of the share flow rather than up front? + // Currently we aren't able to generate a proper thumbnail or play the video in the app extension without first converting it. return promise } From 4ce2eb3c6ccb4527c318c915a74aa067987e43b4 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Sun, 10 Dec 2017 18:39:11 -0500 Subject: [PATCH 04/15] Show ProgressView for video conversion // FREEBIE --- .../SAELoadViewController.swift | 50 +++++++++++--- .../ShareViewController.swift | 66 ++++++++++++++++++- 2 files changed, 103 insertions(+), 13 deletions(-) diff --git a/SignalShareExtension/SAELoadViewController.swift b/SignalShareExtension/SAELoadViewController.swift index 9857d5e93..651b85733 100644 --- a/SignalShareExtension/SAELoadViewController.swift +++ b/SignalShareExtension/SAELoadViewController.swift @@ -10,7 +10,36 @@ class SAELoadViewController: UIViewController { weak var delegate: ShareViewDelegate? - var activityIndicator: UIActivityIndicatorView? + var activityIndicator: UIActivityIndicatorView! + var progressView: UIProgressView! + + var progress: Progress? { + didSet { + guard progressView != nil else { + return + } + + updateProgressViewVisability() + progressView.observedProgress = progress + } + } + + func updateProgressViewVisability() { + guard progressView != nil, activityIndicator != nil else { + return + } + + // Prefer to show progress view when progress is present + if self.progress == nil { + activityIndicator.startAnimating() + self.progressView.isHidden = true + self.activityIndicator.isHidden = false + } else { + activityIndicator.stopAnimating() + self.progressView.isHidden = false + self.activityIndicator.isHidden = true + } + } // MARK: Initializers and Factory Methods @@ -39,6 +68,16 @@ class SAELoadViewController: UIViewController { self.view.addSubview(activityIndicator) activityIndicator.autoCenterInSuperview() + progressView = UIProgressView(progressViewStyle: .default) + progressView.observedProgress = progress + + self.view.addSubview(progressView) + progressView.autoVCenterInSuperview() + progressView.autoPinWidthToSuperview(withMargin: ScaleFromIPhone5(30)) + progressView.progressTintColor = UIColor.white + + updateProgressViewVisability() + let label = UILabel() label.textColor = UIColor.white label.font = UIFont.ows_mediumFont(withSize: 18) @@ -53,20 +92,11 @@ class SAELoadViewController: UIViewController { super.viewWillAppear(animated) self.navigationController?.isNavigationBarHidden = false - - guard let activityIndicator = activityIndicator else { - return - } - activityIndicator.startAnimating() } override func viewDidDisappear(_ animated: Bool) { super.viewDidDisappear(animated) - guard let activityIndicator = activityIndicator else { - return - } - activityIndicator.stopAnimating() } // MARK: - Event Handlers diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index f13aaf8b3..4584b7b06 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -15,6 +15,9 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE private var hasInitialRootViewController = false private var isReadyForAppExtensions = false + var progressPoller: ProgressPoller? + var loadViewController: SAELoadViewController? + override open func loadView() { super.loadView() @@ -65,6 +68,9 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE return } + let loadViewController = SAELoadViewController(delegate: self) + self.loadViewController = loadViewController + // Don't display load screen immediately, in hopes that we can avoid it altogether. after(seconds: 2).then { () -> Void in guard self.presentedViewController == nil else { @@ -74,7 +80,6 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE Logger.debug("\(self.logTag) setup is slow - showing loading screen") - let loadViewController = SAELoadViewController(delegate: self) let navigationController = UINavigationController(rootViewController: loadViewController) self.present(navigationController, animated: true) }.retainUntilComplete() @@ -465,10 +470,21 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // TODO show progress with exportSession let (promise, exportSession) = SignalAttachment.compressVideoAsMp4(dataSource: dataSource, dataUTI: specificUTIType, imageQuality: .medium) - // TODO use `exportSession.progress` to show a more precise progress indicator in the loadView, maybe sharing the "sending" progress UI. - // TODO expose "Cancel" // Can we move this process to the end of the share flow rather than up front? // Currently we aren't able to generate a proper thumbnail or play the video in the app extension without first converting it. + if let exportSession = exportSession { + let progressPoller = ProgressPoller(timeInterval: 0.1, ratioCompleteBlock: { return exportSession.progress }) + self.progressPoller = progressPoller + progressPoller.startPolling() + + guard let loadViewController = self.loadViewController else { + owsFail("load view controller was unexpectedly nil") + return promise + } + + loadViewController.progress = progressPoller.progress + } + return promise } @@ -477,3 +493,47 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE } } } + +class ProgressPoller { + + let TAG = "[ProgressPoller]" + + let progress: Progress + private(set) var timer: Timer? + + // Higher number offers higher ganularity + let progressTotalUnitCount: Int64 = 10000 + private let timeInterval: Double + private let ratioCompleteBlock: () -> Float + + init(timeInterval: TimeInterval, ratioCompleteBlock: @escaping () -> Float) { + self.timeInterval = timeInterval + self.ratioCompleteBlock = ratioCompleteBlock + + self.progress = Progress() + + progress.totalUnitCount = progressTotalUnitCount + progress.completedUnitCount = Int64(ratioCompleteBlock() * Float(progressTotalUnitCount)) + } + + func startPolling() { + guard self.timer == nil else { + owsFail("already started timer") + return + } + + self.timer = WeakTimer.scheduledTimer(timeInterval: timeInterval, target: self, userInfo: nil, repeats: true) { [weak self] (timer) in + guard let strongSelf = self else { + return + } + + let completedUnitCount = Int64(strongSelf.ratioCompleteBlock() * Float(strongSelf.progressTotalUnitCount)) + strongSelf.progress.completedUnitCount = completedUnitCount + + if completedUnitCount == strongSelf.progressTotalUnitCount { + Logger.debug("\(strongSelf.TAG) progress complete") + timer.invalidate() + } + } + } +} From 90e9b4a4f2bafc476e7eb1214fdce828a2a3bd92 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 11 Dec 2017 13:51:36 -0500 Subject: [PATCH 05/15] WIP - send all video types --- Signal.xcodeproj/project.pbxproj | 6 +++++ .../attachments/MediaMessageView.swift | 10 ++++++++ .../attachments/SignalAttachment.swift | 12 ++++++--- .../ShareViewController.swift | 2 ++ .../SignalShareExtension-Bridging-Header.h | 25 ++++++++++--------- .../utils/NSItemProvider+OWS.h | 11 ++++++++ .../utils/NSItemProvider+OWS.m | 17 +++++++++++++ 7 files changed, 68 insertions(+), 15 deletions(-) create mode 100644 SignalShareExtension/utils/NSItemProvider+OWS.h create mode 100644 SignalShareExtension/utils/NSItemProvider+OWS.m diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index 3c480ff54..f1398c69e 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -291,6 +291,7 @@ 45CB2FA81CB7146C00E1B343 /* Launch Screen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 45CB2FA71CB7146C00E1B343 /* Launch Screen.storyboard */; }; 45CD81EF1DC030E7004C9430 /* SyncPushTokensJob.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45CD81EE1DC030E7004C9430 /* SyncPushTokensJob.swift */; }; 45D231771DC7E8F10034FA89 /* SessionResetJob.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45D231761DC7E8F10034FA89 /* SessionResetJob.swift */; }; + 45D55A231FDF08C6003767F0 /* NSItemProvider+OWS.m in Sources */ = {isa = PBXBuildFile; fileRef = 45D55A221FDF08C6003767F0 /* NSItemProvider+OWS.m */; }; 45DF5DF21DDB843F00C936C7 /* CompareSafetyNumbersActivity.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45DF5DF11DDB843F00C936C7 /* CompareSafetyNumbersActivity.swift */; }; 45E547201FD755E700DFC09E /* AttachmentApprovalViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45E5471F1FD755E700DFC09E /* AttachmentApprovalViewController.swift */; }; 45E5A6991F61E6DE001E4A8A /* MarqueeLabel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45E5A6981F61E6DD001E4A8A /* MarqueeLabel.swift */; }; @@ -823,6 +824,8 @@ 45CB2FA71CB7146C00E1B343 /* Launch Screen.storyboard */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.storyboard; name = "Launch Screen.storyboard"; path = "Signal/src/util/Launch Screen.storyboard"; sourceTree = SOURCE_ROOT; }; 45CD81EE1DC030E7004C9430 /* SyncPushTokensJob.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SyncPushTokensJob.swift; sourceTree = ""; }; 45D231761DC7E8F10034FA89 /* SessionResetJob.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SessionResetJob.swift; sourceTree = ""; }; + 45D55A211FDF08C6003767F0 /* NSItemProvider+OWS.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "NSItemProvider+OWS.h"; sourceTree = ""; }; + 45D55A221FDF08C6003767F0 /* NSItemProvider+OWS.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "NSItemProvider+OWS.m"; sourceTree = ""; }; 45DF5DF11DDB843F00C936C7 /* CompareSafetyNumbersActivity.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CompareSafetyNumbersActivity.swift; sourceTree = ""; }; 45E282DE1D08E67800ADD4C8 /* gl */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = gl; path = translations/gl.lproj/Localizable.strings; sourceTree = ""; }; 45E282DF1D08E6CC00ADD4C8 /* id */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = id; path = translations/id.lproj/Localizable.strings; sourceTree = ""; }; @@ -1074,6 +1077,8 @@ children = ( 34480B341FD0929200BC14EF /* ShareAppExtensionContext.h */, 34480B351FD0929200BC14EF /* ShareAppExtensionContext.m */, + 45D55A211FDF08C6003767F0 /* NSItemProvider+OWS.h */, + 45D55A221FDF08C6003767F0 /* NSItemProvider+OWS.m */, ); path = utils; sourceTree = ""; @@ -2714,6 +2719,7 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( + 45D55A231FDF08C6003767F0 /* NSItemProvider+OWS.m in Sources */, 4535186B1FC635DD00210559 /* ShareViewController.swift in Sources */, 34480B361FD0929200BC14EF /* ShareAppExtensionContext.m in Sources */, 3461284B1FD0B94000532771 /* SAELoadViewController.swift in Sources */, diff --git a/SignalMessaging/attachments/MediaMessageView.swift b/SignalMessaging/attachments/MediaMessageView.swift index 7bee41fcc..5360f329c 100644 --- a/SignalMessaging/attachments/MediaMessageView.swift +++ b/SignalMessaging/attachments/MediaMessageView.swift @@ -464,11 +464,21 @@ public class MediaMessageView: UIView, OWSAudioAttachmentPlayerDelegate { @objc public func playVideo() { guard let dataUrl = attachment.dataUrl else { + owsFail("\(self.logTag) attachment is missing dataUrl") return } + + let filePath = dataUrl.path + guard FileManager.default.fileExists(atPath: filePath) else { + owsFail("\(self.logTag) file at \(filePath) doesn't exist") + return + } + guard let videoPlayer = MPMoviePlayerController(contentURL: dataUrl) else { + owsFail("\(self.logTag) unable to build moview player controller") return } + videoPlayer.prepareToPlay() NotificationCenter.default.addObserver(forName: .MPMoviePlayerWillExitFullscreen, object: nil, queue: nil) { [weak self] _ in diff --git a/SignalMessaging/attachments/SignalAttachment.swift b/SignalMessaging/attachments/SignalAttachment.swift index d9af6d74b..868eda051 100644 --- a/SignalMessaging/attachments/SignalAttachment.swift +++ b/SignalMessaging/attachments/SignalAttachment.swift @@ -240,7 +240,13 @@ public class SignalAttachment: NSObject { } do { - let asset = AVURLAsset(url:mediaUrl) + let filePath = mediaUrl.path + guard FileManager.default.fileExists(atPath: filePath) else { + owsFail("asset at \(filePath) doesn't exist") + return nil + } + + let asset = AVURLAsset(url: mediaUrl) let generator = AVAssetImageGenerator(asset: asset) generator.appliesPreferredTrackTransform = true let cgImage = try generator.copyCGImage(at: CMTimeMake(0, 1), actualTime: nil) @@ -784,9 +790,9 @@ public class SignalAttachment: NSObject { if !isInputVideoValidOutputVideo(dataSource: dataSource, dataUTI: dataUTI) { // Most people won't hit this because we convert video when picked from the media picker - // But the current API allos sending videos that some Signal clients will not + // But the current API allows sending videos that some Signal clients will not // be able to view. (e.g. when picked from document picker) - owsFail("building video with invalid output, migrate to async API using compressVideoAsMp4") +// owsFail("building video with invalid output, migrate to async API using compressVideoAsMp4") } return newAttachment(dataSource: dataSource, diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index 4584b7b06..4eea4f4c0 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -488,6 +488,8 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE return promise } + // DO NOT COMMIT +// specificUTIType = "com.apple.quicktime-movie" let attachment = SignalAttachment.attachment(dataSource: dataSource, dataUTI: specificUTIType) return Promise(value: attachment) } diff --git a/SignalShareExtension/SignalShareExtension-Bridging-Header.h b/SignalShareExtension/SignalShareExtension-Bridging-Header.h index 42008bc60..8746dfe0a 100644 --- a/SignalShareExtension/SignalShareExtension-Bridging-Header.h +++ b/SignalShareExtension/SignalShareExtension-Bridging-Header.h @@ -6,19 +6,20 @@ #import // Separate iOS Frameworks from other imports. -#import "DebugLogger.h" -#import "Environment.h" -#import "OWSContactsManager.h" -#import "OWSContactsSyncing.h" -#import "OWSLogger.h" -#import "OWSMath.h" -#import "OWSPreferences.h" -#import "Release.h" +#import "NSItemProvider+OWS.h" #import "ShareAppExtensionContext.h" -#import "UIColor+OWS.h" -#import "UIFont+OWS.h" -#import "UIView+OWS.h" -#import "VersionMigrations.h" +#import +#import +#import +#import +#import +#import +#import +#import +#import +#import +#import +#import #import #import #import diff --git a/SignalShareExtension/utils/NSItemProvider+OWS.h b/SignalShareExtension/utils/NSItemProvider+OWS.h new file mode 100644 index 000000000..aaa6f1d24 --- /dev/null +++ b/SignalShareExtension/utils/NSItemProvider+OWS.h @@ -0,0 +1,11 @@ +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// + +typedef void (^OWSItemProviderDataCompletionHandler)(NSData *_Nullable data, NSError *_Nullable error); + +@implementation NSItemProvider (OWS) + +- (void)loadItemForTypeIdentifier:(NSString *)typeIdentifier options:(nullable NSDictionary *)options dataCompletionHandler:(nullable OWSItemProviderDataCompletionHandler)completionHandler; + +@end diff --git a/SignalShareExtension/utils/NSItemProvider+OWS.m b/SignalShareExtension/utils/NSItemProvider+OWS.m new file mode 100644 index 000000000..412de795a --- /dev/null +++ b/SignalShareExtension/utils/NSItemProvider+OWS.m @@ -0,0 +1,17 @@ +// +// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// + +#import "NSItemProvider+OWS.h" + +@implementation NSItemProvider (OWS) + +/* +// Only override drawRect: if you perform custom drawing. +// An empty implementation adversely affects performance during animation. +- (void)drawRect:(CGRect)rect { + // Drawing code +} +*/ + +@end From 65f79770acc693443f6c85a48b9b5d0572509418 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 11 Dec 2017 14:11:58 -0500 Subject: [PATCH 06/15] rebase fixup --- Signal.xcodeproj/project.pbxproj | 2 +- SignalShareExtension/ShareViewController.swift | 4 ++-- SignalShareExtension/utils/NSItemProvider+OWS.h | 7 ++++++- 3 files changed, 9 insertions(+), 4 deletions(-) diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index f1398c69e..4f3fe39d1 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -91,7 +91,7 @@ 346129FF1FD5F31400532771 /* OWS103EnableVideoCalling.m in Sources */ = {isa = PBXBuildFile; fileRef = 346129F21FD5F31400532771 /* OWS103EnableVideoCalling.m */; }; 34612A001FD5F31400532771 /* OWS105AttachmentFilePaths.h in Headers */ = {isa = PBXBuildFile; fileRef = 346129F31FD5F31400532771 /* OWS105AttachmentFilePaths.h */; }; 34612A011FD5F31400532771 /* OWS104CreateRecipientIdentities.h in Headers */ = {isa = PBXBuildFile; fileRef = 346129F41FD5F31400532771 /* OWS104CreateRecipientIdentities.h */; }; - 34612A061FD7238600532771 /* OWSContactsSyncing.h in Headers */ = {isa = PBXBuildFile; fileRef = 34612A041FD7238500532771 /* OWSContactsSyncing.h */; }; + 34612A061FD7238600532771 /* OWSContactsSyncing.h in Headers */ = {isa = PBXBuildFile; fileRef = 34612A041FD7238500532771 /* OWSContactsSyncing.h */; settings = {ATTRIBUTES = (Public, ); }; }; 34612A071FD7238600532771 /* OWSContactsSyncing.m in Sources */ = {isa = PBXBuildFile; fileRef = 34612A051FD7238500532771 /* OWSContactsSyncing.m */; }; 346B66311F4E29B200E5122F /* CropScaleImageViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 346B66301F4E29B200E5122F /* CropScaleImageViewController.swift */; }; 3471B1DA1EB7C63600F6AEC8 /* NewNonContactConversationViewController.m in Sources */ = {isa = PBXBuildFile; fileRef = 3471B1D91EB7C63600F6AEC8 /* NewNonContactConversationViewController.m */; }; diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index 4eea4f4c0..ada9bb84d 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -468,7 +468,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE guard !SignalAttachment.isInvalidVideo(dataSource: dataSource, dataUTI: specificUTIType) else { // TODO show progress with exportSession - let (promise, exportSession) = SignalAttachment.compressVideoAsMp4(dataSource: dataSource, dataUTI: specificUTIType, imageQuality: .medium) + let (promise, exportSession) = SignalAttachment.compressVideoAsMp4(dataSource: dataSource, dataUTI: specificUTIType) // Can we move this process to the end of the share flow rather than up front? // Currently we aren't able to generate a proper thumbnail or play the video in the app extension without first converting it. @@ -490,7 +490,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // DO NOT COMMIT // specificUTIType = "com.apple.quicktime-movie" - let attachment = SignalAttachment.attachment(dataSource: dataSource, dataUTI: specificUTIType) + let attachment = SignalAttachment.attachment(dataSource: dataSource, dataUTI: specificUTIType, imageQuality: .medium) return Promise(value: attachment) } } diff --git a/SignalShareExtension/utils/NSItemProvider+OWS.h b/SignalShareExtension/utils/NSItemProvider+OWS.h index aaa6f1d24..9f43052f4 100644 --- a/SignalShareExtension/utils/NSItemProvider+OWS.h +++ b/SignalShareExtension/utils/NSItemProvider+OWS.h @@ -2,10 +2,15 @@ // Copyright (c) 2017 Open Whisper Systems. All rights reserved. // +NS_ASSUME_NONNULL_BEGIN + typedef void (^OWSItemProviderDataCompletionHandler)(NSData *_Nullable data, NSError *_Nullable error); -@implementation NSItemProvider (OWS) + +@interface NSItemProvider (OWS) - (void)loadItemForTypeIdentifier:(NSString *)typeIdentifier options:(nullable NSDictionary *)options dataCompletionHandler:(nullable OWSItemProviderDataCompletionHandler)completionHandler; @end + +NS_ASSUME_NONNULL_END From 56f1bf0305a3e419547921ea51433b91f25f6acc Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 11 Dec 2017 19:37:38 -0500 Subject: [PATCH 07/15] cleanup --- .../attachments/SignalAttachment.swift | 13 +++++++- SignalServiceKit/src/Util/MIMETypeUtil.m | 9 +++--- .../ShareViewController.swift | 30 ++++++++++++++++--- 3 files changed, 43 insertions(+), 9 deletions(-) diff --git a/SignalMessaging/attachments/SignalAttachment.swift b/SignalMessaging/attachments/SignalAttachment.swift index 868eda051..d1a590cba 100644 --- a/SignalMessaging/attachments/SignalAttachment.swift +++ b/SignalMessaging/attachments/SignalAttachment.swift @@ -792,7 +792,7 @@ public class SignalAttachment: NSObject { // Most people won't hit this because we convert video when picked from the media picker // But the current API allows sending videos that some Signal clients will not // be able to view. (e.g. when picked from document picker) -// owsFail("building video with invalid output, migrate to async API using compressVideoAsMp4") + owsFail("building video with invalid output, migrate to async API using compressVideoAsMp4") } return newAttachment(dataSource: dataSource, @@ -801,6 +801,17 @@ public class SignalAttachment: NSObject { maxFileSize: kMaxFileSizeVideo) } + public class func copyToVideoTempDir(url fromUrl: URL) throws -> URL { + let baseDir = SignalAttachment.videoTempPath.appendingPathComponent(UUID().uuidString, isDirectory: true) + OWSFileSystem.ensureDirectoryExists(baseDir.path) + let toUrl = baseDir.appendingPathComponent(fromUrl.lastPathComponent) + + Logger.debug("\(self.logTag) moving \(fromUrl) -> \(toUrl)") + try FileManager.default.copyItem(at: fromUrl, to: toUrl) + + return toUrl + } + private class var videoTempPath: URL { let videoDir = URL(fileURLWithPath: NSTemporaryDirectory()).appendingPathComponent("video") OWSFileSystem.ensureDirectoryExists(videoDir.path) diff --git a/SignalServiceKit/src/Util/MIMETypeUtil.m b/SignalServiceKit/src/Util/MIMETypeUtil.m index e779d2dbb..ff9f7e0de 100644 --- a/SignalServiceKit/src/Util/MIMETypeUtil.m +++ b/SignalServiceKit/src/Util/MIMETypeUtil.m @@ -211,19 +211,20 @@ NSString *const kSyncMessageFileExtension = @"bin"; } + (BOOL)isSupportedVideoFile:(NSString *)filePath { - return [[self supportedVideoExtensionTypesToMIMETypes] objectForKey:[filePath pathExtension]] != nil; + return [[self supportedVideoExtensionTypesToMIMETypes] objectForKey:filePath.pathExtension.lowercaseString] != nil; } + (BOOL)isSupportedAudioFile:(NSString *)filePath { - return [[self supportedAudioExtensionTypesToMIMETypes] objectForKey:[filePath pathExtension]] != nil; + return [[self supportedAudioExtensionTypesToMIMETypes] objectForKey:filePath.pathExtension.lowercaseString] != nil; } + (BOOL)isSupportedImageFile:(NSString *)filePath { - return [[self supportedImageExtensionTypesToMIMETypes] objectForKey:[filePath pathExtension]] != nil; + return [[self supportedImageExtensionTypesToMIMETypes] objectForKey:filePath.pathExtension.lowercaseString] != nil; } + (BOOL)isSupportedAnimatedFile:(NSString *)filePath { - return [[self supportedAnimatedExtensionTypesToMIMETypes] objectForKey:[filePath pathExtension]] != nil; + return + [[self supportedAnimatedExtensionTypesToMIMETypes] objectForKey:filePath.pathExtension.lowercaseString] != nil; } + (nullable NSString *)getSupportedExtensionFromVideoMIMEType:(NSString *)supportedMIMEType diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index ada9bb84d..47ea8c284 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -450,7 +450,30 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // TODO accept other data types // TODO whitelist attachment types // TODO coerce when necessary and possible - return promise.then { (url: URL) -> Promise in + return promise.then { (itemUrl: URL) -> Promise in + + let url: URL = try { + // iOS converts at least some video formats (e.g. com.apple.quicktime-movie) into mp4s as part of the + // NSItemProvider `loadItem` API. + // However, for some reason, AVFoundation operations such as generating a preview image and playing + // the url in the AVMoviePlayer fail on these converted formats until unless we first copy the media + // into our container. (These operations succeed when resending mp4s received and sent in Signal) + // + // I don't understand why this is, and I haven't found any relevant documentation in the NSItemProvider + // or AVFoundation docs. + // + // I *did* verify that the size and sah256 sum of the original url matches that of the copied url. + // Perhaps the AVFoundation API's require some extra file system permssion we don't have in the + // passed through URL. + if MIMETypeUtil.isSupportedVideoFile(itemUrl.path) { + return try SignalAttachment.copyToVideoTempDir(url: itemUrl) + } else { + return itemUrl + } + }() + + Logger.debug("\(self.logTag) building DataSource with url: \(url)") + guard let dataSource = DataSourcePath.dataSource(with: url) else { throw ShareViewControllerError.assertionError(description: "Unable to read attachment data") } @@ -467,7 +490,8 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE } guard !SignalAttachment.isInvalidVideo(dataSource: dataSource, dataUTI: specificUTIType) else { - // TODO show progress with exportSession + // This can happen, e.g. when sharing a quicktime-video from iCloud drive. + let (promise, exportSession) = SignalAttachment.compressVideoAsMp4(dataSource: dataSource, dataUTI: specificUTIType) // Can we move this process to the end of the share flow rather than up front? @@ -488,8 +512,6 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE return promise } - // DO NOT COMMIT -// specificUTIType = "com.apple.quicktime-movie" let attachment = SignalAttachment.attachment(dataSource: dataSource, dataUTI: specificUTIType, imageQuality: .medium) return Promise(value: attachment) } From 7d0acc94ff3ea6d88be9f2c24efcdc94e749ff1e Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 11 Dec 2017 19:40:35 -0500 Subject: [PATCH 08/15] cleanup // FREEBIE --- Signal.xcodeproj/project.pbxproj | 6 ------ .../SignalShareExtension-Bridging-Header.h | 1 - SignalShareExtension/utils/NSItemProvider+OWS.h | 16 ---------------- SignalShareExtension/utils/NSItemProvider+OWS.m | 17 ----------------- 4 files changed, 40 deletions(-) delete mode 100644 SignalShareExtension/utils/NSItemProvider+OWS.h delete mode 100644 SignalShareExtension/utils/NSItemProvider+OWS.m diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index 4f3fe39d1..9aa3e60a5 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -291,7 +291,6 @@ 45CB2FA81CB7146C00E1B343 /* Launch Screen.storyboard in Resources */ = {isa = PBXBuildFile; fileRef = 45CB2FA71CB7146C00E1B343 /* Launch Screen.storyboard */; }; 45CD81EF1DC030E7004C9430 /* SyncPushTokensJob.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45CD81EE1DC030E7004C9430 /* SyncPushTokensJob.swift */; }; 45D231771DC7E8F10034FA89 /* SessionResetJob.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45D231761DC7E8F10034FA89 /* SessionResetJob.swift */; }; - 45D55A231FDF08C6003767F0 /* NSItemProvider+OWS.m in Sources */ = {isa = PBXBuildFile; fileRef = 45D55A221FDF08C6003767F0 /* NSItemProvider+OWS.m */; }; 45DF5DF21DDB843F00C936C7 /* CompareSafetyNumbersActivity.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45DF5DF11DDB843F00C936C7 /* CompareSafetyNumbersActivity.swift */; }; 45E547201FD755E700DFC09E /* AttachmentApprovalViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45E5471F1FD755E700DFC09E /* AttachmentApprovalViewController.swift */; }; 45E5A6991F61E6DE001E4A8A /* MarqueeLabel.swift in Sources */ = {isa = PBXBuildFile; fileRef = 45E5A6981F61E6DD001E4A8A /* MarqueeLabel.swift */; }; @@ -824,8 +823,6 @@ 45CB2FA71CB7146C00E1B343 /* Launch Screen.storyboard */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = file.storyboard; name = "Launch Screen.storyboard"; path = "Signal/src/util/Launch Screen.storyboard"; sourceTree = SOURCE_ROOT; }; 45CD81EE1DC030E7004C9430 /* SyncPushTokensJob.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SyncPushTokensJob.swift; sourceTree = ""; }; 45D231761DC7E8F10034FA89 /* SessionResetJob.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SessionResetJob.swift; sourceTree = ""; }; - 45D55A211FDF08C6003767F0 /* NSItemProvider+OWS.h */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.h; path = "NSItemProvider+OWS.h"; sourceTree = ""; }; - 45D55A221FDF08C6003767F0 /* NSItemProvider+OWS.m */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.c.objc; path = "NSItemProvider+OWS.m"; sourceTree = ""; }; 45DF5DF11DDB843F00C936C7 /* CompareSafetyNumbersActivity.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CompareSafetyNumbersActivity.swift; sourceTree = ""; }; 45E282DE1D08E67800ADD4C8 /* gl */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = gl; path = translations/gl.lproj/Localizable.strings; sourceTree = ""; }; 45E282DF1D08E6CC00ADD4C8 /* id */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = id; path = translations/id.lproj/Localizable.strings; sourceTree = ""; }; @@ -1077,8 +1074,6 @@ children = ( 34480B341FD0929200BC14EF /* ShareAppExtensionContext.h */, 34480B351FD0929200BC14EF /* ShareAppExtensionContext.m */, - 45D55A211FDF08C6003767F0 /* NSItemProvider+OWS.h */, - 45D55A221FDF08C6003767F0 /* NSItemProvider+OWS.m */, ); path = utils; sourceTree = ""; @@ -2719,7 +2714,6 @@ isa = PBXSourcesBuildPhase; buildActionMask = 2147483647; files = ( - 45D55A231FDF08C6003767F0 /* NSItemProvider+OWS.m in Sources */, 4535186B1FC635DD00210559 /* ShareViewController.swift in Sources */, 34480B361FD0929200BC14EF /* ShareAppExtensionContext.m in Sources */, 3461284B1FD0B94000532771 /* SAELoadViewController.swift in Sources */, diff --git a/SignalShareExtension/SignalShareExtension-Bridging-Header.h b/SignalShareExtension/SignalShareExtension-Bridging-Header.h index 8746dfe0a..cdad2cb66 100644 --- a/SignalShareExtension/SignalShareExtension-Bridging-Header.h +++ b/SignalShareExtension/SignalShareExtension-Bridging-Header.h @@ -6,7 +6,6 @@ #import // Separate iOS Frameworks from other imports. -#import "NSItemProvider+OWS.h" #import "ShareAppExtensionContext.h" #import #import diff --git a/SignalShareExtension/utils/NSItemProvider+OWS.h b/SignalShareExtension/utils/NSItemProvider+OWS.h deleted file mode 100644 index 9f43052f4..000000000 --- a/SignalShareExtension/utils/NSItemProvider+OWS.h +++ /dev/null @@ -1,16 +0,0 @@ -// -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. -// - -NS_ASSUME_NONNULL_BEGIN - -typedef void (^OWSItemProviderDataCompletionHandler)(NSData *_Nullable data, NSError *_Nullable error); - - -@interface NSItemProvider (OWS) - -- (void)loadItemForTypeIdentifier:(NSString *)typeIdentifier options:(nullable NSDictionary *)options dataCompletionHandler:(nullable OWSItemProviderDataCompletionHandler)completionHandler; - -@end - -NS_ASSUME_NONNULL_END diff --git a/SignalShareExtension/utils/NSItemProvider+OWS.m b/SignalShareExtension/utils/NSItemProvider+OWS.m deleted file mode 100644 index 412de795a..000000000 --- a/SignalShareExtension/utils/NSItemProvider+OWS.m +++ /dev/null @@ -1,17 +0,0 @@ -// -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. -// - -#import "NSItemProvider+OWS.h" - -@implementation NSItemProvider (OWS) - -/* -// Only override drawRect: if you perform custom drawing. -// An empty implementation adversely affects performance during animation. -- (void)drawRect:(CGRect)rect { - // Drawing code -} -*/ - -@end From 031e40d090f8f50ec8f3fc0dfa131d427b742e1f Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 11 Dec 2017 20:16:08 -0500 Subject: [PATCH 09/15] Use SignalAttachment logic in conversation view too // FREEBIE --- .../ConversationViewController.m | 70 +++++++------------ .../attachments/SignalAttachment.swift | 21 ++++++ .../Promise+retainUntilComplete.swift | 15 ++++ 3 files changed, 63 insertions(+), 43 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 28ae423b7..48e6936f3 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -62,6 +62,7 @@ #import #import #import +#import #import #import #import @@ -2722,50 +2723,33 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { presentFromViewController:self canCancel:YES backgroundBlock:^(ModalActivityIndicatorViewController *modalActivityIndicator) { - AVAsset *video = [AVAsset assetWithURL:movieURL]; - AVAssetExportSession *exportSession = - [AVAssetExportSession exportSessionWithAsset:video - presetName:AVAssetExportPresetMediumQuality]; - exportSession.shouldOptimizeForNetworkUse = YES; - exportSession.outputFileType = AVFileTypeMPEG4; - NSURL *compressedVideoUrl = [[self videoTempFolder] - URLByAppendingPathComponent:[[[NSUUID UUID] UUIDString] - stringByAppendingPathExtension:@"mp4"]]; - exportSession.outputURL = compressedVideoUrl; - [exportSession exportAsynchronouslyWithCompletionHandler:^{ - dispatch_async(dispatch_get_main_queue(), ^{ - OWSAssert([NSThread isMainThread]); - - if (modalActivityIndicator.wasCancelled) { - return; + DataSource *dataSource = [DataSourcePath dataSourceWithURL:movieURL]; + dataSource.sourceFilename = filename; + VideoCompressionResult *compressionResult = + [SignalAttachment compressVideoAsMp4WithDataSource:dataSource + dataUTI:(NSString *)kUTTypeMPEG4]; + [compressionResult.attachmentPromise retainUntilComplete]; + + compressionResult.attachmentPromise.then(^(SignalAttachment *attachment) { + OWSAssert([NSThread isMainThread]); + OWSAssert([attachment isKindOfClass:[SignalAttachment class]]); + + if (modalActivityIndicator.wasCancelled) { + return; + } + + [modalActivityIndicator dismissWithCompletion:^{ + if (!attachment || [attachment hasError]) { + DDLogError(@"%@ %s Invalid attachment: %@.", + self.logTag, + __PRETTY_FUNCTION__, + attachment ? [attachment errorName] : @"Missing data"); + [self showErrorAlertForAttachment:attachment]; + } else { + [self tryToSendAttachmentIfApproved:attachment skipApprovalDialog:skipApprovalDialog]; } - - [modalActivityIndicator dismissWithCompletion:^{ - - NSString *baseFilename = filename.stringByDeletingPathExtension; - NSString *mp4Filename = [baseFilename stringByAppendingPathExtension:@"mp4"]; - DataSource *_Nullable dataSource = - [DataSourcePath dataSourceWithURL:compressedVideoUrl]; - [dataSource setSourceFilename:mp4Filename]; - - // Remove temporary file when complete. - [dataSource setShouldDeleteOnDeallocation]; - SignalAttachment *attachment = - [SignalAttachment attachmentWithDataSource:dataSource - dataUTI:(NSString *)kUTTypeMPEG4]; - if (!attachment || [attachment hasError]) { - DDLogError(@"%@ %s Invalid attachment: %@.", - self.logTag, - __PRETTY_FUNCTION__, - attachment ? [attachment errorName] : @"Missing data"); - [self showErrorAlertForAttachment:attachment]; - } else { - [self tryToSendAttachmentIfApproved:attachment - skipApprovalDialog:skipApprovalDialog]; - } - }]; - }); - }]; + }]; + }); }]; } diff --git a/SignalMessaging/attachments/SignalAttachment.swift b/SignalMessaging/attachments/SignalAttachment.swift index d1a590cba..c9758defc 100644 --- a/SignalMessaging/attachments/SignalAttachment.swift +++ b/SignalMessaging/attachments/SignalAttachment.swift @@ -866,6 +866,27 @@ public class SignalAttachment: NSObject { return (promise, exportSession) } + @objc + public class VideoCompressionResult: NSObject { + @objc + public let attachmentPromise: AnyPromise + + @objc + public let exportSession: AVAssetExportSession? + + fileprivate init(attachmentPromise: Promise, exportSession: AVAssetExportSession?) { + self.attachmentPromise = AnyPromise(attachmentPromise) + self.exportSession = exportSession + super.init() + } + } + + @objc + public class func compressVideoAsMp4(dataSource: DataSource, dataUTI: String) -> VideoCompressionResult { + let (attachmentPromise, exportSession) = compressVideoAsMp4(dataSource: dataSource, dataUTI: dataUTI) + return VideoCompressionResult(attachmentPromise: attachmentPromise, exportSession: exportSession) + } + public class func isInvalidVideo(dataSource: DataSource, dataUTI: String) -> Bool { guard videoUTISet.contains(dataUTI) else { // not a video diff --git a/SignalMessaging/categories/Promise+retainUntilComplete.swift b/SignalMessaging/categories/Promise+retainUntilComplete.swift index d3a74a61a..8f45c95c6 100644 --- a/SignalMessaging/categories/Promise+retainUntilComplete.swift +++ b/SignalMessaging/categories/Promise+retainUntilComplete.swift @@ -4,6 +4,21 @@ import PromiseKit +public extension AnyPromise { + /** + * Sometimes there isn't a straight forward candidate to retain a promise, in that case we tell the + * promise to self retain, until it completes to avoid the risk it's GC'd before completion. + */ + func retainUntilComplete() { + // Unfortunately, there is (currently) no way to surpress the + // compiler warning: "Variable 'retainCycle' was written to, but never read" + var retainCycle: AnyPromise? = self + self.always { + retainCycle = nil + } + } +} + public extension Promise { /** * Sometimes there isn't a straight forward candidate to retain a promise, in that case we tell the From 8996741277bbdff28bc7a6f4341142b36d801c17 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 11 Dec 2017 20:30:34 -0500 Subject: [PATCH 10/15] DocumentPicker converts to mp4 when necessary // FREEBIE --- .../ConversationView/ConversationViewController.m | 8 ++++++++ SignalMessaging/attachments/SignalAttachment.swift | 3 --- 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 48e6936f3..c1e879fea 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -2483,6 +2483,14 @@ typedef NS_ENUM(NSInteger, MessagesRangeSizeMode) { } [dataSource setSourceFilename:filename]; + + // Although we want to be able to send higher quality attachments throught the document picker + // it's more imporant that we ensure the sent format is one all clients can accept (e.g. *not* quicktime .mov) + if ([SignalAttachment isInvalidVideoWithDataSource:dataSource dataUTI:type]) { + [self sendQualityAdjustedAttachmentForVideo:url filename:filename skipApprovalDialog:NO]; + return; + } + // "Document picker" attachments _SHOULD NOT_ be resized, if possible. SignalAttachment *attachment = [SignalAttachment attachmentWithDataSource:dataSource dataUTI:type imageQuality:TSImageQualityOriginal]; diff --git a/SignalMessaging/attachments/SignalAttachment.swift b/SignalMessaging/attachments/SignalAttachment.swift index c9758defc..bab8fb4bb 100644 --- a/SignalMessaging/attachments/SignalAttachment.swift +++ b/SignalMessaging/attachments/SignalAttachment.swift @@ -789,9 +789,6 @@ public class SignalAttachment: NSObject { } if !isInputVideoValidOutputVideo(dataSource: dataSource, dataUTI: dataUTI) { - // Most people won't hit this because we convert video when picked from the media picker - // But the current API allows sending videos that some Signal clients will not - // be able to view. (e.g. when picked from document picker) owsFail("building video with invalid output, migrate to async API using compressVideoAsMp4") } From 47e92dbad5e5d1b4831dbd4ca1d9e4461f3e6bb5 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Mon, 11 Dec 2017 20:47:16 -0500 Subject: [PATCH 11/15] cleanup // FREEBIE --- SignalShareExtension/ShareViewController.swift | 13 ++++++++----- 1 file changed, 8 insertions(+), 5 deletions(-) diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index 47ea8c284..48e6dbb72 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -15,7 +15,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE private var hasInitialRootViewController = false private var isReadyForAppExtensions = false - var progressPoller: ProgressPoller? + private var progressPoller: ProgressPoller? var loadViewController: SAELoadViewController? override open func loadView() { @@ -457,7 +457,8 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // NSItemProvider `loadItem` API. // However, for some reason, AVFoundation operations such as generating a preview image and playing // the url in the AVMoviePlayer fail on these converted formats until unless we first copy the media - // into our container. (These operations succeed when resending mp4s received and sent in Signal) + // into our container. (These operations succeed when sending a non-converted mp4 (e.g. one received, + // saved, and resent in Signal) // // I don't understand why this is, and I haven't found any relevant documentation in the NSItemProvider // or AVFoundation docs. @@ -494,8 +495,9 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE let (promise, exportSession) = SignalAttachment.compressVideoAsMp4(dataSource: dataSource, dataUTI: specificUTIType) - // Can we move this process to the end of the share flow rather than up front? - // Currently we aren't able to generate a proper thumbnail or play the video in the app extension without first converting it. + // TODO: How can we move waiting for this export to the end of the share flow rather than having to do it up front? + // Ideally we'd be able to start it here, and not block the UI on conversion unless there's still work to be done + // when the user hit's "send". if let exportSession = exportSession { let progressPoller = ProgressPoller(timeInterval: 0.1, ratioCompleteBlock: { return exportSession.progress }) self.progressPoller = progressPoller @@ -518,7 +520,8 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE } } -class ProgressPoller { +// Exposes a Progress object, whose progress is updated by polling the return of a given block +private class ProgressPoller { let TAG = "[ProgressPoller]" From 813f4e474e5cedb7af66463fbb7ef62932d04538 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 12 Dec 2017 11:09:23 -0500 Subject: [PATCH 12/15] Respond to CR // FREEBIE --- .../attachments/SignalAttachment.swift | 11 ++-- .../ShareViewController.swift | 50 +++++++++++++------ 2 files changed, 40 insertions(+), 21 deletions(-) diff --git a/SignalMessaging/attachments/SignalAttachment.swift b/SignalMessaging/attachments/SignalAttachment.swift index bab8fb4bb..e410aaf87 100644 --- a/SignalMessaging/attachments/SignalAttachment.swift +++ b/SignalMessaging/attachments/SignalAttachment.swift @@ -582,7 +582,7 @@ public class SignalAttachment: NSObject { } attachment.cachedImage = image - if isInputImageValidOutputImage(image: image, dataSource: dataSource, dataUTI: dataUTI, imageQuality:imageQuality) { + if isValidOutputImage(image: image, dataSource: dataSource, dataUTI: dataUTI, imageQuality:imageQuality) { if let sourceFilename = dataSource.sourceFilename, let sourceFileExtension = sourceFilename.fileExtension, ["heic", "heif"].contains(sourceFileExtension.lowercased()) { @@ -613,7 +613,7 @@ public class SignalAttachment: NSObject { // If the proposed attachment already conforms to the // file size and content size limits, don't recompress it. - private class func isInputImageValidOutputImage(image: UIImage?, dataSource: DataSource?, dataUTI: String, imageQuality: TSImageQuality) -> Bool { + private class func isValidOutputImage(image: UIImage?, dataSource: DataSource?, dataUTI: String, imageQuality: TSImageQuality) -> Bool { guard let image = image else { return false } @@ -788,7 +788,7 @@ public class SignalAttachment: NSObject { return attachment } - if !isInputVideoValidOutputVideo(dataSource: dataSource, dataUTI: dataUTI) { + if !isValidOutputVideo(dataSource: dataSource, dataUTI: dataUTI) { owsFail("building video with invalid output, migrate to async API using compressVideoAsMp4") } @@ -854,6 +854,7 @@ public class SignalAttachment: NSObject { return } + dataSource.setShouldDeleteOnDeallocation() dataSource.sourceFilename = mp4Filename let attachment = SignalAttachment(dataSource: dataSource, dataUTI: kUTTypeMPEG4 as String) @@ -890,7 +891,7 @@ public class SignalAttachment: NSObject { return false } - guard isInputVideoValidOutputVideo(dataSource: dataSource, dataUTI: dataUTI) else { + guard isValidOutputVideo(dataSource: dataSource, dataUTI: dataUTI) else { // found a video which needs to be converted return true } @@ -899,7 +900,7 @@ public class SignalAttachment: NSObject { return false } - private class func isInputVideoValidOutputVideo(dataSource: DataSource?, dataUTI: String) -> Bool { + private class func isValidOutputVideo(dataSource: DataSource?, dataUTI: String) -> Bool { guard let dataSource = dataSource else { return false } diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index 48e6dbb72..3fd41a196 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -72,7 +72,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE self.loadViewController = loadViewController // Don't display load screen immediately, in hopes that we can avoid it altogether. - after(seconds: 2).then { () -> Void in + after(seconds: 0.5).then { () -> Void in guard self.presentedViewController == nil else { Logger.debug("\(self.logTag) setup completed quickly, no need to present load view controller.") return @@ -453,20 +453,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE return promise.then { (itemUrl: URL) -> Promise in let url: URL = try { - // iOS converts at least some video formats (e.g. com.apple.quicktime-movie) into mp4s as part of the - // NSItemProvider `loadItem` API. - // However, for some reason, AVFoundation operations such as generating a preview image and playing - // the url in the AVMoviePlayer fail on these converted formats until unless we first copy the media - // into our container. (These operations succeed when sending a non-converted mp4 (e.g. one received, - // saved, and resent in Signal) - // - // I don't understand why this is, and I haven't found any relevant documentation in the NSItemProvider - // or AVFoundation docs. - // - // I *did* verify that the size and sah256 sum of the original url matches that of the copied url. - // Perhaps the AVFoundation API's require some extra file system permssion we don't have in the - // passed through URL. - if MIMETypeUtil.isSupportedVideoFile(itemUrl.path) { + if self.isVideoNeedingRelocation(itemProvider: itemProvider, itemUrl: itemUrl) { return try SignalAttachment.copyToVideoTempDir(url: itemUrl) } else { return itemUrl @@ -497,7 +484,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // TODO: How can we move waiting for this export to the end of the share flow rather than having to do it up front? // Ideally we'd be able to start it here, and not block the UI on conversion unless there's still work to be done - // when the user hit's "send". + // when the user hits "send". if let exportSession = exportSession { let progressPoller = ProgressPoller(timeInterval: 0.1, ratioCompleteBlock: { return exportSession.progress }) self.progressPoller = progressPoller @@ -518,6 +505,37 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE return Promise(value: attachment) } } + + // Some host apps (e.g. iOS Photos.app) converts some video formats (e.g. com.apple.quicktime-movie) + // into mp4s as part of the NSItemProvider `loadItem` API. + // + // However, when using this url to the converted item, AVFoundation operations such as generating a + // preview image and playing the url in the AVMoviePlayer fails with an unhelpful error: "The operation could not be completed" + // + // We can work around this by first copying the media into our container. + // + // I don't understand why this is, and I haven't found any relevant documentation in the NSItemProvider + // or AVFoundation docs. + // + // Notes: + // + // These operations succeed when sending a video which initially existed on disk as an mp4. + // (e.g. Alice sends a video to Bob through the main app, which ensures it's an mp4. Bob saves it, then re-shares it) + // + // I *did* verify that the size and SHA256 sum of the original url matches that of the copied url. So there + // is no difference between the contents of the file, yet one works one doesn't. + // Perhaps the AVFoundation APIs require some extra file system permssion we don't have in the + // passed through URL. + private func isVideoNeedingRelocation(itemProvider: NSItemProvider, itemUrl: URL) -> Bool { + guard MIMETypeUtil.isSupportedVideoFile(itemUrl.path) else { + // not a video, isn't affected + return false + } + + // If video file already existed on disk as an mp4, then the host app didn't need to + // apply any conversion, so no need to relocate the app. + return !itemProvider.registeredTypeIdentifiers.contains(kUTTypeMPEG4 as String) + } } // Exposes a Progress object, whose progress is updated by polling the return of a given block From 03220ffa7904ffa1b61fa55ddfda9d71556905ea Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 12 Dec 2017 12:30:45 -0500 Subject: [PATCH 13/15] CR: Faster animation from loading -> picker // FREEBIE --- .../ShareViewController.swift | 44 +++++++++++-------- 1 file changed, 26 insertions(+), 18 deletions(-) diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index 3fd41a196..f7de42acf 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -18,9 +18,10 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE private var progressPoller: ProgressPoller? var loadViewController: SAELoadViewController? + let shareViewNavigationController: UINavigationController = UINavigationController() + override open func loadView() { super.loadView() - Logger.debug("\(self.logTag) \(#function)") // This should be the first thing we do. @@ -56,7 +57,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // most of the singletons, etc.). We just want to show an error view and // abort. isReadyForAppExtensions = OWSPreferences.isReadyForAppExtensions() - if !isReadyForAppExtensions { + guard isReadyForAppExtensions else { // If we don't have TSSStorageManager, we can't consult TSAccountManager // for isRegistered, so we use OWSPreferences which is usually-accurate // copy of that state. @@ -79,9 +80,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE } Logger.debug("\(self.logTag) setup is slow - showing loading screen") - - let navigationController = UINavigationController(rootViewController: loadViewController) - self.present(navigationController, animated: true) + self.showPrimaryViewController(loadViewController) }.retainUntilComplete() // We shouldn't set up our environment until after we've consulted isReadyForAppExtensions. @@ -117,6 +116,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE } deinit { + Logger.info("\(self.logTag) dealloc") NotificationCenter.default.removeObserver(self) } @@ -298,7 +298,7 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE private func showErrorView(title: String, message: String) { let viewController = SAEFailedViewController(delegate:self, title:title, message:message) - self.setViewControllers([viewController], animated: false) + self.showPrimaryViewController(viewController) } // MARK: View Lifecycle @@ -363,22 +363,30 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // MARK: Helpers + // This view controller is not visible to the user. It exists to intercept touches, set up the + // extensions dependencies, and eventually present a visible view to the user. + // For speed of presentation, we only present a single modal, and if it's already been presented + // we swap out the contents. + // e.g. if loading is taking a while, the user will see the load screen presented with a modal + // animation. Next, when loading completes, the load view will be switched out for the contact + // picker view. + private func showPrimaryViewController(_ viewController: UIViewController) { + shareViewNavigationController.setViewControllers([viewController], animated: false) + if self.presentedViewController == nil { + Logger.debug("\(self.logTag) presenting modally: \(viewController)") + self.present(shareViewNavigationController, animated: true) + } else { + Logger.debug("\(self.logTag) modal already presented. swapping modal content for: \(viewController)") + assert(self.presentedViewController == shareViewNavigationController) + } + } + private func presentConversationPicker() { self.buildAttachment().then { attachment -> Void in let conversationPicker = SharingThreadPickerViewController(shareViewDelegate: self) - let navigationController = UINavigationController(rootViewController: conversationPicker) - navigationController.isNavigationBarHidden = true conversationPicker.attachment = attachment - if let presentedViewController = self.presentedViewController { - Logger.debug("\(self.logTag) dismissing \(presentedViewController) before presenting conversation picker") - self.dismiss(animated: true) { - self.present(navigationController, animated: true) - } - } else { - Logger.debug("\(self.logTag) no other modal, presenting conversation picker immediately") - self.present(navigationController, animated: true) - } - + self.shareViewNavigationController.isNavigationBarHidden = true + self.showPrimaryViewController(conversationPicker) Logger.info("showing picker with attachment: \(attachment)") }.catch { error in let alertTitle = NSLocalizedString("SHARE_EXTENSION_UNABLE_TO_BUILD_ATTACHMENT_ALERT_TITLE", comment: "Shown when trying to share content to a Signal user for the share extension. Followed by failure details.") From 849388feb76979a504c4822c0b29bc7af2beab93 Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 12 Dec 2017 12:53:09 -0500 Subject: [PATCH 14/15] CR: clean up loading assets once no longer needed // FREEBIE --- SignalShareExtension/ShareViewController.swift | 2 ++ 1 file changed, 2 insertions(+) diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index f7de42acf..1347b95e7 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -386,6 +386,8 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE let conversationPicker = SharingThreadPickerViewController(shareViewDelegate: self) conversationPicker.attachment = attachment self.shareViewNavigationController.isNavigationBarHidden = true + self.progressPoller = nil + self.loadViewController = nil self.showPrimaryViewController(conversationPicker) Logger.info("showing picker with attachment: \(attachment)") }.catch { error in From f9d22545b11078cd332a9a84de0746813d4bed5d Mon Sep 17 00:00:00 2001 From: Michael Kirk Date: Tue, 12 Dec 2017 12:53:40 -0500 Subject: [PATCH 15/15] Only copy imported video when necessary. Non-mp4's will be moved as part of their conversion. We only need to move mp4's which were auto-converted // FREEBIE --- SignalShareExtension/ShareViewController.swift | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/SignalShareExtension/ShareViewController.swift b/SignalShareExtension/ShareViewController.swift index 1347b95e7..090e5b942 100644 --- a/SignalShareExtension/ShareViewController.swift +++ b/SignalShareExtension/ShareViewController.swift @@ -516,8 +516,8 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE } } - // Some host apps (e.g. iOS Photos.app) converts some video formats (e.g. com.apple.quicktime-movie) - // into mp4s as part of the NSItemProvider `loadItem` API. + // Some host apps (e.g. iOS Photos.app) sometimes auto-converts some video formats (e.g. com.apple.quicktime-movie) + // into mp4s as part of the NSItemProvider `loadItem` API. (Some files the Photo's app doesn't auto-convert) // // However, when using this url to the converted item, AVFoundation operations such as generating a // preview image and playing the url in the AVMoviePlayer fails with an unhelpful error: "The operation could not be completed" @@ -537,8 +537,9 @@ public class ShareViewController: UINavigationController, ShareViewDelegate, SAE // Perhaps the AVFoundation APIs require some extra file system permssion we don't have in the // passed through URL. private func isVideoNeedingRelocation(itemProvider: NSItemProvider, itemUrl: URL) -> Bool { - guard MIMETypeUtil.isSupportedVideoFile(itemUrl.path) else { - // not a video, isn't affected + guard MIMETypeUtil.utiType(forFileExtension: itemUrl.pathExtension) == kUTTypeMPEG4 as String else { + // Either it's not a video or it was a video which was not auto-converted to mp4. + // Not affected by the issue. return false }