From 498828f93c45640c3009e9bfeed2c29c1deff0fe Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 4 Sep 2018 10:25:42 -0400 Subject: [PATCH 01/21] Rename AttachmentStream methods. --- .../Cells/OWSAudioMessageView.m | 2 +- .../Cells/OWSGenericAttachmentView.m | 5 +- .../Cells/OWSMessageBubbleView.m | 7 +-- .../ConversationViewController.m | 7 +-- .../ConversationView/ConversationViewItem.m | 13 ++--- .../MediaDetailViewController.m | 2 +- .../MessageDetailViewController.swift | 2 +- Signal/src/util/OWSBackup.m | 4 +- Signal/src/util/OWSBackupExportJob.m | 2 +- Signal/src/util/OWSOrphanDataCleaner.m | 2 +- .../ConversationViewItemTest.m | 8 +-- .../ViewModels/OWSQuotedReplyModel.m | 2 +- .../attachments/AttachmentSharing.m | 2 +- .../Messages/Attachments/TSAttachmentStream.h | 5 +- .../Messages/Attachments/TSAttachmentStream.m | 51 ++++++++++--------- .../src/Messages/Interactions/TSMessage.m | 4 +- 16 files changed, 62 insertions(+), 56 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSAudioMessageView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSAudioMessageView.m index 13ecd63c9..0e1b2237f 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSAudioMessageView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSAudioMessageView.m @@ -192,7 +192,7 @@ NS_ASSUME_NONNULL_BEGIN NSString *filename = self.attachmentStream.sourceFilename; if (!filename) { - filename = [[self.attachmentStream filePath] lastPathComponent]; + filename = [self.attachmentStream.originalFilePath lastPathComponent]; } NSString *topText = [[filename stringByDeletingPathExtension] ows_stripped]; if (topText.length < 1) { diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSGenericAttachmentView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSGenericAttachmentView.m index 94da5eee6..ec2f2aedf 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSGenericAttachmentView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSGenericAttachmentView.m @@ -107,7 +107,7 @@ NS_ASSUME_NONNULL_BEGIN NSString *filename = self.attachmentStream.sourceFilename; if (!filename) { - filename = [[self.attachmentStream filePath] lastPathComponent]; + filename = [[self.attachmentStream originalFilePath] lastPathComponent]; } NSString *fileExtension = filename.pathExtension; if (fileExtension.length < 1) { @@ -149,7 +149,8 @@ NS_ASSUME_NONNULL_BEGIN NSError *error; unsigned long long fileSize = - [[NSFileManager defaultManager] attributesOfItemAtPath:[self.attachmentStream filePath] error:&error].fileSize; + [[NSFileManager defaultManager] attributesOfItemAtPath:[self.attachmentStream originalFilePath] error:&error] + .fileSize; OWSAssert(!error); NSString *bottomText = [OWSFormat formatFileSize:fileSize]; UILabel *bottomLabel = [UILabel new]; diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index 430bb92cf..bb70fb11f 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -677,7 +677,7 @@ NS_ASSUME_NONNULL_BEGIN [self.cellMediaCache setObject:cellMedia forKey:cacheKey]; } } else { - DDLogError(@"%@ Failed to load cell media: %@", [self logTag], [self.attachmentStream mediaURL]); + DDLogError(@"%@ Failed to load cell media: %@", [self logTag], [self.attachmentStream originalMediaURL]); self.viewItem.didCellMediaFailToLoad = YES; [self showAttachmentErrorViewWithMediaView:mediaView]; } @@ -851,7 +851,8 @@ NS_ASSUME_NONNULL_BEGIN // TODO: Don't use full size images in the message cells. const NSUInteger kMaxCachableSize = 1024 * 1024; BOOL shouldSkipCache = - [OWSFileSystem fileSizeOfPath:strongSelf.attachmentStream.filePath].unsignedIntegerValue < kMaxCachableSize; + [OWSFileSystem fileSizeOfPath:strongSelf.attachmentStream.originalFilePath].unsignedIntegerValue + < kMaxCachableSize; stillImageView.image = [strongSelf tryToLoadCellMedia:^{ OWSCAssert([strongSelf.attachmentStream isImage]); return strongSelf.attachmentStream.image; @@ -897,7 +898,7 @@ NS_ASSUME_NONNULL_BEGIN animatedImageView.image = [strongSelf tryToLoadCellMedia:^{ OWSCAssert([strongSelf.attachmentStream isAnimated]); - NSString *_Nullable filePath = [strongSelf.attachmentStream filePath]; + NSString *_Nullable filePath = [strongSelf.attachmentStream originalFilePath]; YYImage *_Nullable animatedImage = nil; if (strongSelf.attachmentStream.isValidImage && filePath) { animatedImage = [YYImage imageWithContentsOfFile:filePath]; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index 79c8d49f0..b71da965b 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -2219,8 +2219,8 @@ typedef enum : NSUInteger { OWSAssert(attachmentStream); NSFileManager *fileManager = [NSFileManager defaultManager]; - if (![fileManager fileExistsAtPath:[attachmentStream.mediaURL path]]) { - OWSFail(@"%@ Missing video file: %@", self.logTag, attachmentStream.mediaURL); + if (![fileManager fileExistsAtPath:attachmentStream.originalFilePath]) { + OWSFail(@"%@ Missing video file: %@", self.logTag, attachmentStream.originalFilePath); } [self dismissKeyBoard]; @@ -2235,7 +2235,8 @@ typedef enum : NSUInteger { [self.audioAttachmentPlayer stop]; self.audioAttachmentPlayer = nil; } - self.audioAttachmentPlayer = [[OWSAudioPlayer alloc] initWithMediaUrl:attachmentStream.mediaURL delegate:viewItem]; + self.audioAttachmentPlayer = + [[OWSAudioPlayer alloc] initWithMediaUrl:attachmentStream.originalMediaURL delegate:viewItem]; // Associate the player with this media adapter. self.audioAttachmentPlayer.owner = viewItem; [self.audioAttachmentPlayer playWithPlaybackAudioCategory]; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m index 2eb633997..f8a760b50 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewItem.m @@ -382,7 +382,8 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) return [self displayableTextForCacheKey:displayableTextCacheKey textBlock:^{ - NSData *textData = [NSData dataWithContentsOfURL:attachmentStream.mediaURL]; + NSData *textData = + [NSData dataWithContentsOfURL:attachmentStream.originalMediaURL]; NSString *text = [[NSString alloc] initWithData:textData encoding:NSUTF8StringEncoding]; return text; @@ -733,7 +734,7 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) OWSFail(@"%@ Unknown MIME type: %@", self.logTag, self.attachmentStream.contentType); utiType = (NSString *)kUTTypeGIF; } - NSData *data = [NSData dataWithContentsOfURL:[self.attachmentStream mediaURL]]; + NSData *data = [NSData dataWithContentsOfURL:[self.attachmentStream originalMediaURL]]; if (!data) { OWSFail(@"%@ Could not load attachment data", self.logTag); return; @@ -814,7 +815,7 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) case OWSMessageCellType_Audio: return NO; case OWSMessageCellType_Video: - return UIVideoAtPathIsCompatibleWithSavedPhotosAlbum(self.attachmentStream.mediaURL.path); + return UIVideoAtPathIsCompatibleWithSavedPhotosAlbum(self.attachmentStream.originalFilePath); case OWSMessageCellType_GenericAttachment: return NO; case OWSMessageCellType_DownloadingAttachment: { @@ -834,7 +835,7 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) break; case OWSMessageCellType_StillImage: case OWSMessageCellType_AnimatedImage: { - NSData *data = [NSData dataWithContentsOfURL:[self.attachmentStream mediaURL]]; + NSData *data = [NSData dataWithContentsOfURL:[self.attachmentStream originalMediaURL]]; if (!data) { OWSFail(@"%@ Could not load image data", self.logTag); return; @@ -853,8 +854,8 @@ NSString *NSStringForOWSMessageCellType(OWSMessageCellType cellType) OWSFail(@"%@ Cannot save media data.", self.logTag); break; case OWSMessageCellType_Video: - if (UIVideoAtPathIsCompatibleWithSavedPhotosAlbum(self.attachmentStream.mediaURL.path)) { - UISaveVideoAtPathToSavedPhotosAlbum(self.attachmentStream.mediaURL.path, self, nil, nil); + if (UIVideoAtPathIsCompatibleWithSavedPhotosAlbum(self.attachmentStream.originalFilePath)) { + UISaveVideoAtPathToSavedPhotosAlbum(self.attachmentStream.originalFilePath, self, nil, nil); } else { OWSFail(@"%@ Could not save incompatible video data.", self.logTag); } diff --git a/Signal/src/ViewControllers/MediaDetailViewController.m b/Signal/src/ViewControllers/MediaDetailViewController.m index fc28337a5..008a1179c 100644 --- a/Signal/src/ViewControllers/MediaDetailViewController.m +++ b/Signal/src/ViewControllers/MediaDetailViewController.m @@ -85,7 +85,7 @@ NS_ASSUME_NONNULL_BEGIN - (NSURL *_Nullable)attachmentUrl { - return self.attachmentStream.mediaURL; + return self.attachmentStream.originalMediaURL; } - (NSData *)fileData diff --git a/Signal/src/ViewControllers/MessageDetailViewController.swift b/Signal/src/ViewControllers/MessageDetailViewController.swift index 223ab41ae..82641bf38 100644 --- a/Signal/src/ViewControllers/MessageDetailViewController.swift +++ b/Signal/src/ViewControllers/MessageDetailViewController.swift @@ -651,7 +651,7 @@ class MessageDetailViewController: OWSViewController, MediaGalleryDataSourceDele func didTapAudioViewItem(_ viewItem: ConversationViewItem, attachmentStream: TSAttachmentStream) { SwiftAssertIsOnMainThread(#function) - guard let mediaURL = attachmentStream.mediaURL() else { + guard let mediaURL = attachmentStream.originalMediaURL() else { owsFail("\(logTag) in \(#function) mediaURL was unexpectedly nil for attachment: \(attachmentStream)") return } diff --git a/Signal/src/util/OWSBackup.m b/Signal/src/util/OWSBackup.m index c477b36d0..403e1c638 100644 --- a/Signal/src/util/OWSBackup.m +++ b/Signal/src/util/OWSBackup.m @@ -546,7 +546,7 @@ NS_ASSUME_NONNULL_BEGIN OWSAssert(backupIO); OWSAssert(completion); - NSString *_Nullable attachmentFilePath = [attachment filePath]; + NSString *_Nullable attachmentFilePath = [attachment originalFilePath]; if (attachmentFilePath.length < 1) { DDLogError(@"%@ Attachment has invalid file path.", self.logTag); return completion(NO); @@ -617,7 +617,7 @@ NS_ASSUME_NONNULL_BEGIN } } - NSString *_Nullable attachmentFilePath = [attachment filePath]; + NSString *_Nullable attachmentFilePath = [attachment originalFilePath]; if (attachmentFilePath.length < 1) { DDLogError(@"%@ Attachment has invalid file path.", self.logTag); return completion(NO); diff --git a/Signal/src/util/OWSBackupExportJob.m b/Signal/src/util/OWSBackupExportJob.m index 97ddab4c4..1703b02f1 100644 --- a/Signal/src/util/OWSBackupExportJob.m +++ b/Signal/src/util/OWSBackupExportJob.m @@ -539,7 +539,7 @@ NS_ASSUME_NONNULL_BEGIN return NO; } TSAttachmentStream *attachmentStream = object; - NSString *_Nullable filePath = attachmentStream.filePath; + NSString *_Nullable filePath = attachmentStream.originalFilePath; if (!filePath) { DDLogError(@"%@ attachment is missing file.", self.logTag); return NO; diff --git a/Signal/src/util/OWSOrphanDataCleaner.m b/Signal/src/util/OWSOrphanDataCleaner.m index 5f47d162e..a70b10d4c 100644 --- a/Signal/src/util/OWSOrphanDataCleaner.m +++ b/Signal/src/util/OWSOrphanDataCleaner.m @@ -316,7 +316,7 @@ typedef void (^OrphanDataBlock)(OWSOrphanData *); TSAttachmentStream *attachmentStream = (TSAttachmentStream *)attachment; attachmentStreamCount++; - NSString *_Nullable filePath = [attachmentStream filePath]; + NSString *_Nullable filePath = [attachmentStream originalFilePath]; if (filePath) { [allAttachmentFilePaths addObject:filePath]; } else { diff --git a/Signal/test/ViewControllers/ConversationViewItemTest.m b/Signal/test/ViewControllers/ConversationViewItemTest.m index 97cdd2dc7..eee47bc49 100644 --- a/Signal/test/ViewControllers/ConversationViewItemTest.m +++ b/Signal/test/ViewControllers/ConversationViewItemTest.m @@ -126,7 +126,7 @@ TSAttachment *_Nullable attachment = [TSAttachment fetchObjectWithUniqueID:attachmentId]; XCTAssertTrue([attachment isKindOfClass:[TSAttachmentStream class]]); TSAttachmentStream *_Nullable attachmentStream = (TSAttachmentStream *)attachment; - NSString *_Nullable filePath = attachmentStream.filePath; + NSString *_Nullable filePath = attachmentStream.originalFilePath; XCTAssertNotNil(filePath); XCTAssertNotNil([TSMessage fetchObjectWithUniqueID:viewItem.interaction.uniqueId]); @@ -148,7 +148,7 @@ TSAttachment *_Nullable attachment = [TSAttachment fetchObjectWithUniqueID:attachmentId]; XCTAssertTrue([attachment isKindOfClass:[TSAttachmentStream class]]); TSAttachmentStream *_Nullable attachmentStream = (TSAttachmentStream *)attachment; - NSString *_Nullable filePath = attachmentStream.filePath; + NSString *_Nullable filePath = attachmentStream.originalFilePath; XCTAssertNotNil(filePath); XCTAssertNotNil([TSMessage fetchObjectWithUniqueID:viewItem.interaction.uniqueId]); @@ -170,7 +170,7 @@ TSAttachment *_Nullable attachment = [TSAttachment fetchObjectWithUniqueID:attachmentId]; XCTAssertTrue([attachment isKindOfClass:[TSAttachmentStream class]]); TSAttachmentStream *_Nullable attachmentStream = (TSAttachmentStream *)attachment; - NSString *_Nullable filePath = attachmentStream.filePath; + NSString *_Nullable filePath = attachmentStream.originalFilePath; XCTAssertNotNil(filePath); XCTAssertNotNil([TSMessage fetchObjectWithUniqueID:viewItem.interaction.uniqueId]); @@ -192,7 +192,7 @@ TSAttachment *_Nullable attachment = [TSAttachment fetchObjectWithUniqueID:attachmentId]; XCTAssertTrue([attachment isKindOfClass:[TSAttachmentStream class]]); TSAttachmentStream *_Nullable attachmentStream = (TSAttachmentStream *)attachment; - NSString *_Nullable filePath = attachmentStream.filePath; + NSString *_Nullable filePath = attachmentStream.originalFilePath; XCTAssertNotNil(filePath); XCTAssertNotNil([TSMessage fetchObjectWithUniqueID:viewItem.interaction.uniqueId]); diff --git a/SignalMessaging/ViewModels/OWSQuotedReplyModel.m b/SignalMessaging/ViewModels/OWSQuotedReplyModel.m index ad8d578e3..6ebd3e143 100644 --- a/SignalMessaging/ViewModels/OWSQuotedReplyModel.m +++ b/SignalMessaging/ViewModels/OWSQuotedReplyModel.m @@ -179,7 +179,7 @@ NS_ASSUME_NONNULL_BEGIN hasText = YES; quotedText = @""; - NSData *_Nullable oversizeTextData = [NSData dataWithContentsOfFile:attachmentStream.filePath]; + NSData *_Nullable oversizeTextData = [NSData dataWithContentsOfFile:attachmentStream.originalFilePath]; if (oversizeTextData) { // We don't need to include the entire text body of the message, just // enough to render a snippet. kOversizeTextMessageSizeThreshold is our diff --git a/SignalMessaging/attachments/AttachmentSharing.m b/SignalMessaging/attachments/AttachmentSharing.m index eb8603c25..b32781c1d 100644 --- a/SignalMessaging/attachments/AttachmentSharing.m +++ b/SignalMessaging/attachments/AttachmentSharing.m @@ -16,7 +16,7 @@ NS_ASSUME_NONNULL_BEGIN { OWSAssert(stream); - [self showShareUIForURL:stream.mediaURL]; + [self showShareUIForURL:stream.originalMediaURL]; } + (void)showShareUIForURL:(NSURL *)url diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index 34f73f933..0dcfa51d5 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -47,11 +47,12 @@ NS_ASSUME_NONNULL_BEGIN - (BOOL)isImage; - (BOOL)isVideo; - (BOOL)isAudio; -- (nullable NSURL *)mediaURL; + (BOOL)hasThumbnailForMimeType:(NSString *)contentType; -- (nullable NSString *)filePath; +- (nullable NSString *)originalFilePath; +- (nullable NSURL *)originalMediaURL; + - (nullable NSString *)thumbnailPath; - (nullable NSData *)readDataFromFileWithError:(NSError **)error; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index 12aa2f582..cd619cbcc 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -156,7 +156,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; } self.localRelativeFilePath = localRelativeFilePath; - OWSAssert(self.filePath); + OWSAssert(self.originalFilePath); } #pragma mark - File Management @@ -164,7 +164,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; - (nullable NSData *)readDataFromFileWithError:(NSError **)error { *error = nil; - NSString *_Nullable filePath = self.filePath; + NSString *_Nullable filePath = self.originalFilePath; if (!filePath) { OWSFail(@"%@ Missing path for attachment.", self.logTag); return nil; @@ -177,7 +177,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; OWSAssert(data); *error = nil; - NSString *_Nullable filePath = self.filePath; + NSString *_Nullable filePath = self.originalFilePath; if (!filePath) { OWSFail(@"%@ Missing path for attachment.", self.logTag); return NO; @@ -190,7 +190,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; { OWSAssert(dataSource); - NSString *_Nullable filePath = self.filePath; + NSString *_Nullable filePath = self.originalFilePath; if (!filePath) { OWSFail(@"%@ Missing path for attachment.", self.logTag); return NO; @@ -229,7 +229,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; return attachmentsFolder; } -- (nullable NSString *)filePath +- (nullable NSString *)originalFilePath { if (!self.localRelativeFilePath) { OWSFail(@"%@ Attachment missing local file path.", self.logTag); @@ -241,7 +241,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; - (nullable NSString *)thumbnailPath { - NSString *filePath = self.filePath; + NSString *filePath = self.originalFilePath; if (!filePath) { OWSFail(@"%@ Attachment missing local file path.", self.logTag); return nil; @@ -258,9 +258,9 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; return [[containingDir stringByAppendingPathComponent:newFilename] stringByAppendingPathExtension:@"jpg"]; } -- (nullable NSURL *)mediaURL +- (nullable NSURL *)originalMediaURL { - NSString *_Nullable filePath = self.filePath; + NSString *_Nullable filePath = self.originalFilePath; if (!filePath) { OWSFail(@"%@ Missing path for attachment.", self.logTag); return nil; @@ -281,7 +281,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; } } - NSString *_Nullable filePath = self.filePath; + NSString *_Nullable filePath = self.originalFilePath; if (!filePath) { OWSFail(@"%@ Missing path for attachment.", self.logTag); return; @@ -321,14 +321,14 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; { OWSAssert(self.isImage || self.isAnimated); - return [NSData ows_isValidImageAtPath:self.filePath mimeType:self.contentType]; + return [NSData ows_isValidImageAtPath:self.originalFilePath mimeType:self.contentType]; } - (BOOL)isValidVideo { OWSAssert(self.isVideo); - return [NSData ows_isValidVideoAtURL:self.mediaURL]; + return [NSData ows_isValidVideoAtURL:self.originalMediaURL]; } #pragma mark - @@ -338,14 +338,14 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; if ([self isVideo]) { return [self videoStillImage]; } else if ([self isImage] || [self isAnimated]) { - NSURL *_Nullable mediaUrl = [self mediaURL]; + NSURL *_Nullable mediaUrl = self.originalMediaURL; if (!mediaUrl) { return nil; } if (![self isValidImage]) { return nil; } - return [[UIImage alloc] initWithContentsOfFile:self.filePath]; + return [[UIImage alloc] initWithContentsOfFile:self.originalFilePath]; } else { return nil; } @@ -362,12 +362,12 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; return nil; } - if (![NSData ows_isValidImageAtPath:self.filePath mimeType:self.contentType]) { + if (![NSData ows_isValidImageAtPath:self.originalFilePath mimeType:self.contentType]) { OWSFail(@"%@ skipping invalid image", self.logTag); return nil; } - return [NSData dataWithContentsOfFile:self.filePath]; + return [NSData dataWithContentsOfFile:self.originalFilePath]; } + (BOOL)hasThumbnailForMimeType:(NSString *)contentType @@ -425,8 +425,8 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; return; } - if (![[NSFileManager defaultManager] fileExistsAtPath:self.mediaURL.path]) { - DDLogError(@"%@ while generating thumbnail, source file doesn't exist: %@", self.logTag, self.mediaURL); + if (![[NSFileManager defaultManager] fileExistsAtPath:self.originalMediaURL.path]) { + DDLogError(@"%@ while generating thumbnail, source file doesn't exist: %@", self.logTag, self.originalMediaURL); // If we're not lazy-restoring this message, the attachment should exist on disk. OWSAssert(self.lazyRestoreFragmentId); return; @@ -438,11 +438,12 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; UIImage *_Nullable result; if (self.isImage || self.isAnimated) { if (![self isValidImage]) { - DDLogWarn(@"%@ skipping thumbnail generation for invalid image at path: %@", self.logTag, self.filePath); + DDLogWarn( + @"%@ skipping thumbnail generation for invalid image at path: %@", self.logTag, self.originalFilePath); return; } - CGImageSourceRef imageSource = CGImageSourceCreateWithURL((__bridge CFURLRef)self.mediaURL, NULL); + CGImageSourceRef imageSource = CGImageSourceCreateWithURL((__bridge CFURLRef)self.originalMediaURL, NULL); OWSAssert(imageSource != NULL); NSDictionary *imageOptions = @{ (NSString const *)kCGImageSourceCreateThumbnailFromImageIfAbsent : (NSNumber const *)kCFBooleanTrue, @@ -458,7 +459,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; } else if (self.isVideo) { if (![self isValidVideo]) { - DDLogWarn(@"%@ skipping thumbnail for invalid video at path: %@", self.logTag, self.filePath); + DDLogWarn(@"%@ skipping thumbnail for invalid video at path: %@", self.logTag, self.originalFilePath); return; } @@ -496,7 +497,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; maxSize.width = MIN(maxSize.width, kMaxVideoStillSize); maxSize.height = MIN(maxSize.height, kMaxVideoStillSize); - NSURL *_Nullable mediaUrl = [self mediaURL]; + NSURL *_Nullable mediaUrl = self.originalMediaURL; if (!mediaUrl) { return nil; } @@ -509,7 +510,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; CMTime time = CMTimeMake(1, 60); CGImageRef imgRef = [generator copyCGImageAtTime:time actualTime:NULL error:&err]; if (imgRef == NULL) { - DDLogError(@"Could not generate video still: %@", self.filePath.pathExtension); + DDLogError(@"Could not generate video still: %@", self.originalFilePath.pathExtension); return nil; } @@ -546,7 +547,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; } return [self videoStillImage].size; } else if ([self isImage] || [self isAnimated]) { - NSURL *_Nullable mediaUrl = [self mediaURL]; + NSURL *_Nullable mediaUrl = self.originalMediaURL; if (!mediaUrl) { return CGSizeZero; } @@ -656,7 +657,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; OWSAssert([self isAudio]); NSError *error; - AVAudioPlayer *audioPlayer = [[AVAudioPlayer alloc] initWithContentsOfURL:self.mediaURL error:&error]; + AVAudioPlayer *audioPlayer = [[AVAudioPlayer alloc] initWithContentsOfURL:self.originalMediaURL error:&error]; if (error && [error.domain isEqualToString:NSOSStatusErrorDomain] && (error.code == kAudioFileInvalidFileError || error.code == kAudioFileStreamError_InvalidFile)) { // Ignore "invalid audio file" errors. @@ -665,7 +666,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; if (!error) { return (CGFloat)[audioPlayer duration]; } else { - DDLogError(@"Could not find audio duration: %@", self.mediaURL); + DDLogError(@"Could not find audio duration: %@", self.originalMediaURL); return 0; } } diff --git a/SignalServiceKit/src/Messages/Interactions/TSMessage.m b/SignalServiceKit/src/Messages/Interactions/TSMessage.m index 314eea2c8..f0fb202d5 100644 --- a/SignalServiceKit/src/Messages/Interactions/TSMessage.m +++ b/SignalServiceKit/src/Messages/Interactions/TSMessage.m @@ -235,7 +235,7 @@ static const NSUInteger OWSMessageSchemaVersion = 4; [attachment isKindOfClass:TSAttachmentStream.class]) { TSAttachmentStream *attachmentStream = (TSAttachmentStream *)attachment; - NSData *_Nullable data = [NSData dataWithContentsOfFile:attachmentStream.filePath]; + NSData *_Nullable data = [NSData dataWithContentsOfFile:attachmentStream.originalFilePath]; if (data) { NSString *_Nullable text = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; if (text) { @@ -263,7 +263,7 @@ static const NSUInteger OWSMessageSchemaVersion = 4; // Handle oversize text attachments. if ([attachment isKindOfClass:[TSAttachmentStream class]]) { TSAttachmentStream *attachmentStream = (TSAttachmentStream *)attachment; - NSData *_Nullable data = [NSData dataWithContentsOfFile:attachmentStream.filePath]; + NSData *_Nullable data = [NSData dataWithContentsOfFile:attachmentStream.originalFilePath]; if (data) { NSString *_Nullable text = [[NSString alloc] initWithData:data encoding:NSUTF8StringEncoding]; if (text) { From 446ceb2b9cd52ed7b950e646ac3be9c963ae8ca0 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 4 Sep 2018 10:29:30 -0400 Subject: [PATCH 02/21] Rename AttachmentStream methods. --- .../ConversationView/Cells/OWSMessageBubbleView.m | 4 ++-- Signal/src/ViewControllers/MediaDetailViewController.m | 2 +- Signal/src/ViewControllers/MediaGalleryViewController.swift | 2 +- SignalMessaging/ViewModels/OWSQuotedReplyModel.m | 2 +- SignalServiceKit/src/Contacts/Threads/TSGroupThread.m | 2 +- .../src/Messages/Attachments/TSAttachmentStream.h | 2 +- .../src/Messages/Attachments/TSAttachmentStream.m | 2 +- 7 files changed, 8 insertions(+), 8 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index bb70fb11f..521630e6a 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -855,7 +855,7 @@ NS_ASSUME_NONNULL_BEGIN < kMaxCachableSize; stillImageView.image = [strongSelf tryToLoadCellMedia:^{ OWSCAssert([strongSelf.attachmentStream isImage]); - return strongSelf.attachmentStream.image; + return strongSelf.attachmentStream.originalImage; } mediaView:stillImageView cacheKey:strongSelf.attachmentStream.uniqueId @@ -979,7 +979,7 @@ NS_ASSUME_NONNULL_BEGIN stillImageView.image = [strongSelf tryToLoadCellMedia:^{ OWSCAssert([strongSelf.attachmentStream isVideo]); - return strongSelf.attachmentStream.image; + return strongSelf.attachmentStream.originalImage; } mediaView:stillImageView cacheKey:strongSelf.attachmentStream.uniqueId diff --git a/Signal/src/ViewControllers/MediaDetailViewController.m b/Signal/src/ViewControllers/MediaDetailViewController.m index 008a1179c..6b232cab6 100644 --- a/Signal/src/ViewControllers/MediaDetailViewController.m +++ b/Signal/src/ViewControllers/MediaDetailViewController.m @@ -73,7 +73,7 @@ NS_ASSUME_NONNULL_BEGIN _galleryItemBox = galleryItemBox; _viewItem = viewItem; // We cache the image data in case the attachment stream is deleted. - _image = galleryItemBox.attachmentStream.image; + _image = galleryItemBox.attachmentStream.originalImage; return self; } diff --git a/Signal/src/ViewControllers/MediaGalleryViewController.swift b/Signal/src/ViewControllers/MediaGalleryViewController.swift index 6a63a548e..903511ead 100644 --- a/Signal/src/ViewControllers/MediaGalleryViewController.swift +++ b/Signal/src/ViewControllers/MediaGalleryViewController.swift @@ -43,7 +43,7 @@ public struct MediaGalleryItem: Equatable, Hashable { } var fullSizedImage: UIImage { - guard let image = attachmentStream.image() else { + guard let image = attachmentStream.originalImage() else { owsFail("\(logTag) in \(#function) unexpectedly unable to build attachment image") return UIImage() } diff --git a/SignalMessaging/ViewModels/OWSQuotedReplyModel.m b/SignalMessaging/ViewModels/OWSQuotedReplyModel.m index 6ebd3e143..6c355c348 100644 --- a/SignalMessaging/ViewModels/OWSQuotedReplyModel.m +++ b/SignalMessaging/ViewModels/OWSQuotedReplyModel.m @@ -88,7 +88,7 @@ NS_ASSUME_NONNULL_BEGIN TSAttachmentStream *attachmentStream; if ([attachment isKindOfClass:[TSAttachmentStream class]]) { attachmentStream = (TSAttachmentStream *)attachment; - thumbnailImage = attachmentStream.image; + thumbnailImage = attachmentStream.originalImage; } } else if (attachmentInfo.thumbnailAttachmentPointerId) { // download failed, or hasn't completed yet. diff --git a/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m b/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m index 98a3ded81..c8bff39ad 100644 --- a/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m +++ b/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m @@ -191,7 +191,7 @@ NSString *const TSGroupThread_NotificationKey_UniqueId = @"TSGroupThread_Notific OWSAssert(attachmentStream); OWSAssert(transaction); - self.groupModel.groupImage = [attachmentStream image]; + self.groupModel.groupImage = [attachmentStream originalImage]; [self saveWithTransaction:transaction]; [transaction addCompletionQueue:nil diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index 0dcfa51d5..ddf2887c0 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -37,7 +37,7 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) NSDate *creationTimestamp; #if TARGET_OS_IPHONE -- (nullable UIImage *)image; +- (nullable UIImage *)originalImage; - (nullable UIImage *)thumbnailImage; - (nullable NSData *)thumbnailData; - (nullable NSData *)validStillImageData; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index cd619cbcc..f2f019d05 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -333,7 +333,7 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; #pragma mark - -- (nullable UIImage *)image +- (nullable UIImage *)originalImage { if ([self isVideo]) { return [self videoStillImage]; From 1831f0b1f8aa1fb2d1a59ca16fbac11ccf4bcfd0 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 4 Sep 2018 10:32:29 -0400 Subject: [PATCH 03/21] Reorder AttachmentStream methods. --- .../src/Messages/Attachments/TSAttachmentStream.h | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index ddf2887c0..3483184b5 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -37,9 +37,6 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) NSDate *creationTimestamp; #if TARGET_OS_IPHONE -- (nullable UIImage *)originalImage; -- (nullable UIImage *)thumbnailImage; -- (nullable NSData *)thumbnailData; - (nullable NSData *)validStillImageData; #endif @@ -48,13 +45,16 @@ NS_ASSUME_NONNULL_BEGIN - (BOOL)isVideo; - (BOOL)isAudio; -+ (BOOL)hasThumbnailForMimeType:(NSString *)contentType; - +- (nullable UIImage *)originalImage; - (nullable NSString *)originalFilePath; - (nullable NSURL *)originalMediaURL; +- (nullable UIImage *)thumbnailImage; +- (nullable NSData *)thumbnailData; - (nullable NSString *)thumbnailPath; ++ (BOOL)hasThumbnailForMimeType:(NSString *)contentType; + - (nullable NSData *)readDataFromFileWithError:(NSError **)error; - (BOOL)writeData:(NSData *)data error:(NSError **)error; - (BOOL)writeDataSource:(DataSource *)dataSource; From ac4365e1c982dc4b74ef6fffb8dabe061589dfd3 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 4 Sep 2018 15:21:48 -0400 Subject: [PATCH 04/21] Add OWSThumbnailService. --- Signal.xcodeproj/project.pbxproj | 4 - .../Attachments/OWSThumbnailService.swift | 194 ++++++++++++++++++ .../Messages/Attachments/TSAttachmentStream.h | 38 ++++ .../Messages/Attachments/TSAttachmentStream.m | 165 ++++++++++++++- .../src/Util}/SwiftSingletons.swift | 4 +- SignalServiceKit/src/Util/UIImage+OWS.h | 2 +- SignalServiceKit/src/Util/UIImage+OWS.m | 6 +- 7 files changed, 396 insertions(+), 17 deletions(-) create mode 100644 SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift rename {SignalMessaging/utils => SignalServiceKit/src/Util}/SwiftSingletons.swift (86%) diff --git a/Signal.xcodeproj/project.pbxproj b/Signal.xcodeproj/project.pbxproj index 65c0f8631..be57dca33 100644 --- a/Signal.xcodeproj/project.pbxproj +++ b/Signal.xcodeproj/project.pbxproj @@ -148,7 +148,6 @@ 3478504C1FD7496D007B8332 /* Images.xcassets in Resources */ = {isa = PBXBuildFile; fileRef = B66DBF4919D5BBC8006EA940 /* Images.xcassets */; }; 347850551FD749C0007B8332 /* Localizable.strings in Resources */ = {isa = PBXBuildFile; fileRef = B6F509951AA53F760068F56A /* Localizable.strings */; }; 347850571FD86544007B8332 /* SAEFailedViewController.swift in Sources */ = {isa = PBXBuildFile; fileRef = 347850561FD86544007B8332 /* SAEFailedViewController.swift */; }; - 347850591FD9972E007B8332 /* SwiftSingletons.swift in Sources */ = {isa = PBXBuildFile; fileRef = 347850581FD9972E007B8332 /* SwiftSingletons.swift */; }; 347850691FD9B78A007B8332 /* AppSetup.m in Sources */ = {isa = PBXBuildFile; fileRef = 347850651FD9B789007B8332 /* AppSetup.m */; }; 3478506A1FD9B78A007B8332 /* AppSetup.h in Headers */ = {isa = PBXBuildFile; fileRef = 347850661FD9B789007B8332 /* AppSetup.h */; settings = {ATTRIBUTES = (Public, ); }; }; 3478506B1FD9B78A007B8332 /* NoopCallMessageHandler.swift in Sources */ = {isa = PBXBuildFile; fileRef = 347850671FD9B78A007B8332 /* NoopCallMessageHandler.swift */; }; @@ -783,7 +782,6 @@ 34661FB720C1C0D60056EDD6 /* message_sent.aiff */ = {isa = PBXFileReference; lastKnownFileType = audio.aiff; name = message_sent.aiff; path = Signal/AudioFiles/message_sent.aiff; sourceTree = SOURCE_ROOT; }; 346B66301F4E29B200E5122F /* CropScaleImageViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = CropScaleImageViewController.swift; sourceTree = ""; }; 347850561FD86544007B8332 /* SAEFailedViewController.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SAEFailedViewController.swift; sourceTree = ""; }; - 347850581FD9972E007B8332 /* SwiftSingletons.swift */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.swift; path = SwiftSingletons.swift; sourceTree = ""; }; 3478505A1FD999D5007B8332 /* et */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = et; path = translations/et.lproj/Localizable.strings; sourceTree = ""; }; 3478505C1FD99A1F007B8332 /* zh_TW */ = {isa = PBXFileReference; lastKnownFileType = text.plist.strings; name = zh_TW; path = translations/zh_TW.lproj/Localizable.strings; sourceTree = ""; }; 347850651FD9B789007B8332 /* AppSetup.m */ = {isa = PBXFileReference; fileEncoding = 4; lastKnownFileType = sourcecode.c.objc; path = AppSetup.m; sourceTree = ""; }; @@ -1559,7 +1557,6 @@ 34641E1120878FB000E2EDE5 /* OWSWindowManager.h */, 34641E1020878FAF00E2EDE5 /* OWSWindowManager.m */, 45360B8C1F9521F800FA666C /* Searcher.swift */, - 347850581FD9972E007B8332 /* SwiftSingletons.swift */, 346129BD1FD2068600532771 /* ThreadUtil.h */, 346129BE1FD2068600532771 /* ThreadUtil.m */, B97940251832BD2400BD66CB /* UIUtil.h */, @@ -3208,7 +3205,6 @@ 346129AB1FD1F0EE00532771 /* OWSFormat.m in Sources */, 34AC0A12211B39EA00997B47 /* ContactTableViewCell.m in Sources */, 451F8A461FD715BA005CB9DA /* OWSGroupAvatarBuilder.m in Sources */, - 347850591FD9972E007B8332 /* SwiftSingletons.swift in Sources */, 346129961FD1E30000532771 /* OWSDatabaseMigration.m in Sources */, 346129FB1FD5F31400532771 /* OWS101ExistingUsersBlockOnIdentityChange.m in Sources */, 34AC09EA211B39B100997B47 /* ModalActivityIndicatorViewController.swift in Sources */, diff --git a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift new file mode 100644 index 000000000..f18f7af99 --- /dev/null +++ b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift @@ -0,0 +1,194 @@ +// +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// + +import Foundation + +private struct OWSThumbnailRequest { + public typealias CompletionBlock = (UIImage) -> Void + + let attachmentId: String + let thumbnailDimensionPoints: UInt + let completion: CompletionBlock + + init(attachmentId: String, thumbnailDimensionPoints: UInt, completion: @escaping CompletionBlock) { + self.attachmentId = attachmentId + self.thumbnailDimensionPoints = thumbnailDimensionPoints + self.completion = completion + } +} + +@objc public class OWSThumbnailService: NSObject { + + // MARK: - Singleton class + + @objc(shared) + public static let shared = OWSThumbnailService() + + public typealias CompletionBlock = (UIImage) -> Void + + private let serialQueue = DispatchQueue(label: "OWSThumbnailService") + + private let dbConnection: YapDatabaseConnection + + // This property should only be accessed on the serialQueue. + // + // We want to process requests in _reverse_ order in which they + // arrive so that we prioritize the most recent view state. + // This data structure is actually used like a stack. + private var thumbnailRequestQueue = [OWSThumbnailRequest]() + + private override init() { + + dbConnection = OWSPrimaryStorage.shared().newDatabaseConnection() + + super.init() + + SwiftSingletons.register(self) + } + + private func canThumbnailAttachment(attachment: TSAttachmentStream) -> Bool { + guard attachment.isImage() else { + return false + } + guard !attachment.isAnimated() else { + return false + } + guard attachment.isValidImage() else { + return false + } + return true + } + + // completion will only be called on success. + // completion will be called async on the main thread. + @objc public func ensureThumbnailForAttachmentId(attachmentId: String, + thumbnailDimensionPoints: UInt, + completion:@escaping CompletionBlock) { + guard attachmentId.count > 0 else { + owsFail("Empty attachment id.") + return + } + serialQueue.async { + let thumbnailRequest = OWSThumbnailRequest(attachmentId: attachmentId, thumbnailDimensionPoints: thumbnailDimensionPoints, completion: completion) + self.thumbnailRequestQueue.append(thumbnailRequest) + + self.processNextRequestSync() + } + } + + private func processNextRequestAsync() { + serialQueue.async { + self.processNextRequestSync() + } + } + + // This should only be called on the serialQueue. + private func processNextRequestSync() { + guard !thumbnailRequestQueue.isEmpty else { + return + } + let thumbnailRequest = thumbnailRequestQueue.removeLast() + + if let image = process(thumbnailRequest: thumbnailRequest) { + DispatchQueue.main.async { + thumbnailRequest.completion(image) + } + } + } + + // This should only be called on the serialQueue. + private func process(thumbnailRequest: OWSThumbnailRequest) -> UIImage? { + var possibleAttachment: TSAttachmentStream? + self.dbConnection.read({ (transaction) in + possibleAttachment = TSAttachmentStream.fetch(uniqueId: thumbnailRequest.attachmentId, transaction: transaction) + }) + guard let attachment = possibleAttachment else { + Logger.warn("Could not load attachment for thumbnailing.") + return nil + } + guard canThumbnailAttachment(attachment: attachment) else { + Logger.warn("Cannot thumbnail attachment.") + return nil + } + if let thumbnails = attachment.thumbnails { + for thumbnail in thumbnails { + if thumbnail.thumbnailDimensionPoints == thumbnailRequest.thumbnailDimensionPoints { + guard let filePath = attachment.path(for: thumbnail) else { + owsFail("Could not determine thumbnail path.") + return nil + } + guard let image = UIImage(contentsOfFile: filePath) else { + owsFail("Could not load thumbnail.") + return nil + } + return image + } + } + } + guard let originalFilePath = attachment.originalFilePath() else { + owsFail("Could not determine thumbnail path.") + return nil + } + guard let originalImage = UIImage(contentsOfFile: originalFilePath) else { + owsFail("Could not load original image.") + return nil + } + let originalSize = originalImage.size + guard originalSize.width > 0 && originalSize.height > 0 else { + owsFail("Original image has invalid size.") + return nil + } + var thumbnailSize = CGSize.zero + if originalSize.width > originalSize.height { + thumbnailSize.width = CGFloat(thumbnailRequest.thumbnailDimensionPoints) + thumbnailSize.height = round(CGFloat(thumbnailRequest.thumbnailDimensionPoints) * thumbnailSize.height / thumbnailSize.width) + } else { + thumbnailSize.width = round(CGFloat(thumbnailRequest.thumbnailDimensionPoints) * thumbnailSize.width / thumbnailSize.height) + thumbnailSize.height = CGFloat(thumbnailRequest.thumbnailDimensionPoints) + } + guard thumbnailSize.width > 0 && thumbnailSize.height > 0 else { + owsFail("Thumbnail has invalid size.") + return nil + } + guard originalSize.width < thumbnailSize.width && + originalSize.height < thumbnailSize.height else { + owsFail("Thumbnail isn't smaller than the original.") + return nil + } + // We use UIGraphicsBeginImageContextWithOptions() to scale. + // Core Image would provide better quality (e.g. Lanczos) but + // at perf cost we don't want to pay. We could also use + // CoreGraphics directly, but I'm not sure there's any benefit. + guard let thumbnailImage = originalImage.resizedImage(to: thumbnailSize) else { + owsFail("Could not thumbnail image.") + return nil + } + guard let thumbnailData = UIImageJPEGRepresentation(thumbnailImage, 0.85) else { + owsFail("Could not convert thumbnail to JPEG.") + return nil + } + let temporaryDirectory = NSTemporaryDirectory() + let thumbnailFilename = "\(NSUUID().uuidString).jpg" + let thumbnailFilePath = (temporaryDirectory as NSString).appendingPathComponent(thumbnailFilename) + do { + try thumbnailData.write(to: NSURL.fileURL(withPath: thumbnailFilePath), options: .atomicWrite) + } catch let error as NSError { + owsFail("File write failed: \(thumbnailFilePath), \(error)") + return nil + } + // It should be safe to assume that an attachment will never end up with two thumbnails of + // the same size since: + // + // * Thumbnails are only added by this method. + // * This method checks for an existing thumbnail using the same connection. + // * This method is performed on the serial queue. + self.dbConnection.readWrite({ (transaction) in + attachment.update(withNewThumbnail: thumbnailFilePath, + thumbnailDimensionPoints: thumbnailRequest.thumbnailDimensionPoints, + size: thumbnailSize, + transaction: transaction) + }) + return thumbnailImage + } +} diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index 3483184b5..649d9cfc4 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -17,6 +17,21 @@ NS_ASSUME_NONNULL_BEGIN @class TSAttachmentPointer; @class YapDatabaseReadWriteTransaction; +typedef void (^OWSThumbnailCompletion)(UIImage *image); + +@interface TSAttachmentThumbnail : MTLModel + +@property (nonatomic, readonly) NSString *filename; +@property (nonatomic, readonly) CGSize size; +// The length of the longer side. +@property (nonatomic, readonly) NSUInteger thumbnailDimensionPoints; + +- (instancetype)init NS_UNAVAILABLE; + +@end + +#pragma mark - + @interface TSAttachmentStream : TSAttachment - (instancetype)init NS_UNAVAILABLE; @@ -36,6 +51,8 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic, readonly) NSDate *creationTimestamp; +@property (nonatomic, nullable, readonly) NSArray *thumbnails; + #if TARGET_OS_IPHONE - (nullable NSData *)validStillImageData; #endif @@ -49,6 +66,7 @@ NS_ASSUME_NONNULL_BEGIN - (nullable NSString *)originalFilePath; - (nullable NSURL *)originalMediaURL; +// TODO: Rename to legacy... - (nullable UIImage *)thumbnailImage; - (nullable NSData *)thumbnailData; - (nullable NSString *)thumbnailPath; @@ -78,6 +96,20 @@ NS_ASSUME_NONNULL_BEGIN // Non-nil for attachments which need "lazy backup restore." - (nullable OWSBackupFragment *)lazyRestoreFragment; + +#pragma mark - Thumbnails + +// On cache hit, the thumbnail will be returned synchronously and completion will never be invoked. +// On cache miss, nil will be returned and the completion will be invoked async on main if +// thumbnail can be generated. +- (nullable UIImage *)thumbnailImageWithSizeHint:(CGSize)sizeHint completion:(OWSThumbnailCompletion)completion; +- (nullable UIImage *)thumbnailImageSmallWithCompletion:(OWSThumbnailCompletion)completion; +- (nullable UIImage *)thumbnailImageMediumWithCompletion:(OWSThumbnailCompletion)completion; +- (nullable UIImage *)thumbnailImageLargeWithCompletion:(OWSThumbnailCompletion)completion; + +// This method should only be invoked by OWSThumbnailService. +- (nullable NSString *)pathForThumbnail:(TSAttachmentThumbnail *)thumbnail; + #pragma mark - Validation - (BOOL)isValidImage; @@ -91,8 +123,14 @@ NS_ASSUME_NONNULL_BEGIN // Marks attachment as having completed "lazy backup restore." - (void)updateWithLazyRestoreComplete; +// TODO: Review. - (nullable TSAttachmentStream *)cloneAsThumbnail; +- (void)updateWithNewThumbnail:(NSString *)tempFilePath + thumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints + size:(CGSize)size + transaction:(YapDatabaseReadWriteTransaction *)transaction; + #pragma mark - Protobuf + (nullable SSKProtoAttachmentPointer *)buildProtoForAttachmentId:(nullable NSString *)attachmentId; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index f2f019d05..8f449d780 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -16,6 +16,36 @@ NS_ASSUME_NONNULL_BEGIN const CGFloat kMaxVideoStillSize = 1 * 1024; +const NSUInteger kThumbnailDimensionPointsSmall = 200; +const NSUInteger kThumbnailDimensionPointsMedium = 800; +// This size is large enough to render full screen. +const NSUInteger ThumbnailDimensionPointsLarge() { + CGSize screenSizePoints = UIScreen.mainScreen.bounds.size; + return MAX(screenSizePoints.width, screenSizePoints.height); +} + +@implementation TSAttachmentThumbnail + +- (instancetype)initWithFilename:(NSString *)filename + size:(CGSize)size + thumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints +{ + self = [super init]; + if (!self) { + return self; + } + + _filename = filename; + _size = size; + _thumbnailDimensionPoints = thumbnailDimensionPoints; + + return self; +} + +@end + +#pragma mark - + @interface TSAttachmentStream () // We only want to generate the file path for this attachment once, so that @@ -32,6 +62,8 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; // Optional property. Only set for attachments which need "lazy backup restore." @property (nonatomic, nullable) NSString *lazyRestoreFragmentId; +@property (nonatomic, nullable) NSArray *thumbnails; + @end #pragma mark - @@ -258,6 +290,18 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; return [[containingDir stringByAppendingPathComponent:newFilename] stringByAppendingPathExtension:@"jpg"]; } +- (nullable NSString *)pathForThumbnail:(TSAttachmentThumbnail *)thumbnail +{ + NSString *filePath = self.originalFilePath; + if (!filePath) { + OWSFail(@"%@ Attachment missing local file path.", self.logTag); + return nil; + } + + NSString *containingDir = filePath.stringByDeletingLastPathComponent; + return [containingDir stringByAppendingPathComponent:thumbnail.filename]; +} + - (nullable NSURL *)originalMediaURL { NSString *_Nullable filePath = self.originalFilePath; @@ -272,12 +316,22 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; { NSError *error; + for (TSAttachmentThumbnail *thumbnail in self.thumbnails) { + NSString *_Nullable thumbnailPath = [self pathForThumbnail:thumbnail]; + if (thumbnailPath) { + BOOL success = [[NSFileManager defaultManager] removeItemAtPath:thumbnailPath error:&error]; + if (error || !success) { + DDLogError(@"%@ remove thumbnail failed with: %@", self.logTag, error); + } + } + } + NSString *_Nullable thumbnailPath = self.thumbnailPath; if (thumbnailPath) { - [[NSFileManager defaultManager] removeItemAtPath:thumbnailPath error:&error]; + BOOL success = [[NSFileManager defaultManager] removeItemAtPath:thumbnailPath error:&error]; - if (error) { - DDLogError(@"%@ remove thumbnail errored with: %@", self.logTag, error); + if (error || !success) { + DDLogError(@"%@ remove legacy thumbnail failed with: %@", self.logTag, error); } } @@ -286,10 +340,9 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; OWSFail(@"%@ Missing path for attachment.", self.logTag); return; } - [[NSFileManager defaultManager] removeItemAtPath:filePath error:&error]; - - if (error) { - DDLogError(@"%@ remove file errored with: %@", self.logTag, error); + BOOL success = [[NSFileManager defaultManager] removeItemAtPath:filePath error:&error]; + if (error || !success) { + DDLogError(@"%@ remove file failed with: %@", self.logTag, error); } } @@ -728,6 +781,70 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; return string; } +#pragma mark - Thumbnails + +- (nullable UIImage *)thumbnailImageWithSizeHint:(CGSize)sizeHint completion:(OWSThumbnailCompletion)completion +{ + CGFloat maxDimensionHint = MAX(sizeHint.width, sizeHint.height); + NSUInteger thumbnailDimensionPoints; + if (maxDimensionHint <= kThumbnailDimensionPointsSmall) { + thumbnailDimensionPoints = kThumbnailDimensionPointsSmall; + } else if (maxDimensionHint <= kThumbnailDimensionPointsMedium) { + thumbnailDimensionPoints = kThumbnailDimensionPointsMedium; + } else { + thumbnailDimensionPoints = ThumbnailDimensionPointsLarge(); + } + + return [self thumbnailImageWithThumbnailDimensionPoints:thumbnailDimensionPoints completion:completion]; +} + +- (nullable UIImage *)thumbnailImageSmallWithCompletion:(OWSThumbnailCompletion)completion +{ + return [self thumbnailImageWithThumbnailDimensionPoints:kThumbnailDimensionPointsSmall completion:completion]; +} + +- (nullable UIImage *)thumbnailImageMediumWithCompletion:(OWSThumbnailCompletion)completion +{ + return [self thumbnailImageWithThumbnailDimensionPoints:kThumbnailDimensionPointsMedium completion:completion]; +} + +- (nullable UIImage *)thumbnailImageLargeWithCompletion:(OWSThumbnailCompletion)completion +{ + return [self thumbnailImageWithThumbnailDimensionPoints:ThumbnailDimensionPointsLarge() completion:completion]; +} + +- (nullable UIImage *)thumbnailImageWithThumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints + completion:(OWSThumbnailCompletion)completion +{ + CGSize originalSize = self.imageSize; + if (originalSize.width < 1 || originalSize.height < 1) { + return nil; + } + if (originalSize.width <= thumbnailDimensionPoints || originalSize.height <= thumbnailDimensionPoints) { + // There's no point in generating a thumbnail if the original is smaller than the + // thumbnail size. + return self.originalImage; + } + + for (TSAttachmentThumbnail *thumbnail in self.thumbnails) { + if (thumbnail.thumbnailDimensionPoints != thumbnailDimensionPoints) { + continue; + } + NSString *_Nullable thumbnailPath = [self pathForThumbnail:thumbnail]; + if (!thumbnailPath) { + OWSFail(@"Missing thumbnail path."); + continue; + } + UIImage *_Nullable image = [UIImage imageWithContentsOfFile:thumbnailPath]; + return image; + } + + [OWSThumbnailService.shared ensureThumbnailForAttachmentIdWithAttachmentId:self.uniqueId + thumbnailDimensionPoints:thumbnailDimensionPoints + completion:completion]; + return nil; +} + #pragma mark - Update With... Methods - (void)markForLazyRestoreWithFragment:(OWSBackupFragment *)lazyRestoreFragment @@ -783,6 +900,40 @@ const CGFloat kMaxVideoStillSize = 1 * 1024; return thumbnailAttachment; } +- (void)updateWithNewThumbnail:(NSString *)tempFilePath + thumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints + size:(CGSize)size + transaction:(YapDatabaseReadWriteTransaction *)transaction +{ + OWSAssert(tempFilePath.length > 0); + OWSAssert(thumbnailDimensionPoints > 0); + OWSAssert(size.width > 0 && size.height); + OWSAssert(transaction); + + NSString *filename = tempFilePath.lastPathComponent; + NSString *containingDir = self.originalFilePath.stringByDeletingLastPathComponent; + NSString *filePath = [containingDir stringByAppendingPathComponent:filename]; + + NSError *_Nullable error; + BOOL success = [[NSFileManager defaultManager] moveItemAtPath:tempFilePath toPath:filePath error:&error]; + if (error || !success) { + OWSFail(@"Could not move new thumbnail image: %@.", error); + return; + } + TSAttachmentThumbnail *newThumbnail = [[TSAttachmentThumbnail alloc] initWithFilename:filename + size:size + thumbnailDimensionPoints:thumbnailDimensionPoints]; + + [self applyChangeToSelfAndLatestCopy:transaction + changeBlock:^(TSAttachmentStream *attachment) { + NSMutableArray *thumbnails + = (attachment.thumbnails ? [attachment.thumbnails mutableCopy] + : [NSMutableArray new]); + [thumbnails addObject:newThumbnail]; + [attachment setThumbnails:thumbnails]; + }]; +} + // MARK: Protobuf serialization + (nullable SSKProtoAttachmentPointer *)buildProtoForAttachmentId:(nullable NSString *)attachmentId diff --git a/SignalMessaging/utils/SwiftSingletons.swift b/SignalServiceKit/src/Util/SwiftSingletons.swift similarity index 86% rename from SignalMessaging/utils/SwiftSingletons.swift rename to SignalServiceKit/src/Util/SwiftSingletons.swift index cccfa2e7e..d02ad65a2 100644 --- a/SignalMessaging/utils/SwiftSingletons.swift +++ b/SignalServiceKit/src/Util/SwiftSingletons.swift @@ -1,5 +1,5 @@ // -// Copyright (c) 2017 Open Whisper Systems. All rights reserved. +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. // import Foundation @@ -20,7 +20,7 @@ public class SwiftSingletons: NSObject { guard _isDebugAssertConfiguration() else { return } - let singletonClassName = String(describing:type(of:singleton)) + let singletonClassName = String(describing: type(of: singleton)) guard !classSet.contains(singletonClassName) else { owsFail("\(self.logTag) in \(#function) Duplicate singleton: \(singletonClassName).") return diff --git a/SignalServiceKit/src/Util/UIImage+OWS.h b/SignalServiceKit/src/Util/UIImage+OWS.h index 35eb5d673..f8e08a8b2 100644 --- a/SignalServiceKit/src/Util/UIImage+OWS.h +++ b/SignalServiceKit/src/Util/UIImage+OWS.h @@ -9,7 +9,7 @@ NS_ASSUME_NONNULL_BEGIN - (UIImage *)normalizedImage; - (UIImage *)resizedWithQuality:(CGInterpolationQuality)quality rate:(CGFloat)rate; -- (UIImage *)resizedImageToSize:(CGSize)dstSize; +- (nullable UIImage *)resizedImageToSize:(CGSize)dstSize; - (UIImage *)resizedImageToFillPixelSize:(CGSize)boundingSize; + (UIImage *)imageWithColor:(UIColor *)color; diff --git a/SignalServiceKit/src/Util/UIImage+OWS.m b/SignalServiceKit/src/Util/UIImage+OWS.m index 300fd07e3..2f67225fd 100644 --- a/SignalServiceKit/src/Util/UIImage+OWS.m +++ b/SignalServiceKit/src/Util/UIImage+OWS.m @@ -37,7 +37,7 @@ // Source: https://github.com/AliSoftware/UIImage-Resize -- (UIImage *)resizedImageToSize:(CGSize)dstSize +- (nullable UIImage *)resizedImageToSize:(CGSize)dstSize { CGImageRef imgRef = self.CGImage; // the below values are regardless of orientation : for UIImages from Camera, width>height (landscape) @@ -106,10 +106,10 @@ UIGraphicsBeginImageContextWithOptions(dstSize, NO, self.scale); CGContextRef context = UIGraphicsGetCurrentContext(); - if (!context) { return nil; } + CGContextSetInterpolationQuality(context, kCGInterpolationHigh); if (orient == UIImageOrientationRight || orient == UIImageOrientationLeft) { CGContextScaleCTM(context, -scaleRatio, scaleRatio); @@ -124,7 +124,7 @@ // we use srcSize (and not dstSize) as the size to specify is in user space (and we use the CTM to apply a // scaleRatio) CGContextDrawImage(UIGraphicsGetCurrentContext(), CGRectMake(0, 0, srcSize.width, srcSize.height), imgRef); - UIImage *resizedImage = UIGraphicsGetImageFromCurrentImageContext(); + UIImage *_Nullable resizedImage = UIGraphicsGetImageFromCurrentImageContext(); UIGraphicsEndImageContext(); return resizedImage; From 3437361d70a2f16540e18756e28c6a8eb26542fa Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 4 Sep 2018 15:50:54 -0400 Subject: [PATCH 05/21] Use new thumbnails in media gallery thumbnails. --- .../MediaGalleryViewController.swift | 10 ++--- .../MediaTileViewController.swift | 19 +++++++- Signal/src/util/OWSOrphanDataCleaner.m | 2 +- .../ViewModels/OWSQuotedReplyModel.m | 2 +- .../Attachments/OWSThumbnailService.swift | 8 ++-- .../Messages/Attachments/TSAttachmentStream.h | 6 +-- .../Messages/Attachments/TSAttachmentStream.m | 45 ++++++------------- 7 files changed, 43 insertions(+), 49 deletions(-) diff --git a/Signal/src/ViewControllers/MediaGalleryViewController.swift b/Signal/src/ViewControllers/MediaGalleryViewController.swift index 903511ead..10b94da74 100644 --- a/Signal/src/ViewControllers/MediaGalleryViewController.swift +++ b/Signal/src/ViewControllers/MediaGalleryViewController.swift @@ -33,13 +33,9 @@ public struct MediaGalleryItem: Equatable, Hashable { return attachmentStream.isImage() } - var thumbnailImage: UIImage { - guard let image = attachmentStream.thumbnailImage() else { - owsFail("\(logTag) in \(#function) unexpectedly unable to build attachment thumbnail") - return UIImage() - } - - return image + public typealias AsyncThumbnailBlock = (UIImage) -> Void + func thumbnailImage(async:@escaping AsyncThumbnailBlock) -> UIImage? { + return attachmentStream.thumbnailImageSmall(completion: async) } var fullSizedImage: UIImage { diff --git a/Signal/src/ViewControllers/MediaTileViewController.swift b/Signal/src/ViewControllers/MediaTileViewController.swift index 33cb6adbd..dd94ae094 100644 --- a/Signal/src/ViewControllers/MediaTileViewController.swift +++ b/Signal/src/ViewControllers/MediaTileViewController.swift @@ -922,7 +922,24 @@ private class MediaGalleryCell: UICollectionViewCell { public func configure(item: MediaGalleryItem) { self.item = item - self.imageView.image = item.thumbnailImage + if let image = item.thumbnailImage(async: { + [weak self] (image) in + guard let strongSelf = self else { + return + } + guard strongSelf.item == item else { + return + } + strongSelf.imageView.image = image + strongSelf.imageView.backgroundColor = UIColor.clear + }) { + self.imageView.image = image + self.imageView.backgroundColor = UIColor.clear + } else { + // TODO: Show a placeholder? + self.imageView.backgroundColor = Theme.offBackgroundColor + } + if item.isVideo { self.contentTypeBadgeView.isHidden = false self.contentTypeBadgeView.image = MediaGalleryCell.videoBadgeImage diff --git a/Signal/src/util/OWSOrphanDataCleaner.m b/Signal/src/util/OWSOrphanDataCleaner.m index a70b10d4c..1773450a5 100644 --- a/Signal/src/util/OWSOrphanDataCleaner.m +++ b/Signal/src/util/OWSOrphanDataCleaner.m @@ -323,7 +323,7 @@ typedef void (^OrphanDataBlock)(OWSOrphanData *); OWSFail(@"%@ attachment has no file path.", self.logTag); } - NSString *_Nullable thumbnailPath = [attachmentStream thumbnailPath]; + NSString *_Nullable thumbnailPath = [attachmentStream legacyThumbnailPath]; if (thumbnailPath.length > 0) { [allAttachmentFilePaths addObject:thumbnailPath]; } diff --git a/SignalMessaging/ViewModels/OWSQuotedReplyModel.m b/SignalMessaging/ViewModels/OWSQuotedReplyModel.m index 6c355c348..e48b83ccc 100644 --- a/SignalMessaging/ViewModels/OWSQuotedReplyModel.m +++ b/SignalMessaging/ViewModels/OWSQuotedReplyModel.m @@ -227,7 +227,7 @@ NS_ASSUME_NONNULL_BEGIN authorId:authorId body:quotedText bodySource:TSQuotedMessageContentSourceLocal - thumbnailImage:quotedAttachment.thumbnailImage + thumbnailImage:quotedAttachment.legacyThumbnailImage contentType:quotedAttachment.contentType sourceFilename:quotedAttachment.sourceFilename attachmentStream:quotedAttachment diff --git a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift index f18f7af99..97aaac571 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift +++ b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift @@ -142,17 +142,17 @@ private struct OWSThumbnailRequest { var thumbnailSize = CGSize.zero if originalSize.width > originalSize.height { thumbnailSize.width = CGFloat(thumbnailRequest.thumbnailDimensionPoints) - thumbnailSize.height = round(CGFloat(thumbnailRequest.thumbnailDimensionPoints) * thumbnailSize.height / thumbnailSize.width) + thumbnailSize.height = round(CGFloat(thumbnailRequest.thumbnailDimensionPoints) * originalSize.height / originalSize.width) } else { - thumbnailSize.width = round(CGFloat(thumbnailRequest.thumbnailDimensionPoints) * thumbnailSize.width / thumbnailSize.height) + thumbnailSize.width = round(CGFloat(thumbnailRequest.thumbnailDimensionPoints) * originalSize.width / originalSize.height) thumbnailSize.height = CGFloat(thumbnailRequest.thumbnailDimensionPoints) } guard thumbnailSize.width > 0 && thumbnailSize.height > 0 else { owsFail("Thumbnail has invalid size.") return nil } - guard originalSize.width < thumbnailSize.width && - originalSize.height < thumbnailSize.height else { + guard originalSize.width > thumbnailSize.width && + originalSize.height > thumbnailSize.height else { owsFail("Thumbnail isn't smaller than the original.") return nil } diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index 649d9cfc4..66e908663 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -67,9 +67,9 @@ typedef void (^OWSThumbnailCompletion)(UIImage *image); - (nullable NSURL *)originalMediaURL; // TODO: Rename to legacy... -- (nullable UIImage *)thumbnailImage; -- (nullable NSData *)thumbnailData; -- (nullable NSString *)thumbnailPath; +- (nullable UIImage *)legacyThumbnailImage; +//- (nullable NSData *)legacyThumbnailData; +- (nullable NSString *)legacyThumbnailPath; + (BOOL)hasThumbnailForMimeType:(NSString *)contentType; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index 8f449d780..99fe956b9 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -16,7 +16,7 @@ NS_ASSUME_NONNULL_BEGIN const CGFloat kMaxVideoStillSize = 1 * 1024; -const NSUInteger kThumbnailDimensionPointsSmall = 200; +const NSUInteger kThumbnailDimensionPointsSmall = 300; const NSUInteger kThumbnailDimensionPointsMedium = 800; // This size is large enough to render full screen. const NSUInteger ThumbnailDimensionPointsLarge() { @@ -129,7 +129,7 @@ const NSUInteger ThumbnailDimensionPointsLarge() { } // This is going to be slow the first time it runs. - [self ensureThumbnail]; + [self ensureLegacyThumbnail]; return self; } @@ -137,7 +137,7 @@ const NSUInteger ThumbnailDimensionPointsLarge() { - (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction { [super saveWithTransaction:transaction]; - [self ensureThumbnail]; + [self ensureLegacyThumbnail]; } - (void)upgradeFromAttachmentSchemaVersion:(NSUInteger)attachmentSchemaVersion @@ -271,7 +271,7 @@ const NSUInteger ThumbnailDimensionPointsLarge() { return [[[self class] attachmentsFolder] stringByAppendingPathComponent:self.localRelativeFilePath]; } -- (nullable NSString *)thumbnailPath +- (nullable NSString *)legacyThumbnailPath { NSString *filePath = self.originalFilePath; if (!filePath) { @@ -326,9 +326,9 @@ const NSUInteger ThumbnailDimensionPointsLarge() { } } - NSString *_Nullable thumbnailPath = self.thumbnailPath; - if (thumbnailPath) { - BOOL success = [[NSFileManager defaultManager] removeItemAtPath:thumbnailPath error:&error]; + NSString *_Nullable legacyThumbnailPath = self.legacyThumbnailPath; + if (legacyThumbnailPath) { + BOOL success = [[NSFileManager defaultManager] removeItemAtPath:legacyThumbnailPath error:&error]; if (error || !success) { DDLogError(@"%@ remove legacy thumbnail failed with: %@", self.logTag, error); @@ -429,28 +429,9 @@ const NSUInteger ThumbnailDimensionPointsLarge() { [MIMETypeUtil isAnimated:contentType]); } -- (nullable UIImage *)thumbnailImage +- (nullable NSData *)legacyThumbnailData { - NSString *thumbnailPath = self.thumbnailPath; - if (!thumbnailPath) { - OWSAssert(!self.isImage && !self.isVideo && !self.isAnimated); - - return nil; - } - - if (![[NSFileManager defaultManager] fileExistsAtPath:thumbnailPath]) { - // This isn't true for some useful edge cases tested by the Debug UI. - DDLogError(@"%@ missing thumbnail for attachmentId: %@", self.logTag, self.uniqueId); - - return nil; - } - - return [UIImage imageWithContentsOfFile:self.thumbnailPath]; -} - -- (nullable NSData *)thumbnailData -{ - NSString *thumbnailPath = self.thumbnailPath; + NSString *thumbnailPath = self.legacyThumbnailPath; if (!thumbnailPath) { OWSAssert(!self.isImage && !self.isVideo && !self.isAnimated); @@ -463,12 +444,12 @@ const NSUInteger ThumbnailDimensionPointsLarge() { return nil; } - return [NSData dataWithContentsOfFile:self.thumbnailPath]; + return [NSData dataWithContentsOfFile:thumbnailPath]; } -- (void)ensureThumbnail +- (void)ensureLegacyThumbnail { - NSString *thumbnailPath = self.thumbnailPath; + NSString *thumbnailPath = self.legacyThumbnailPath; if (!thumbnailPath) { return; } @@ -877,7 +858,7 @@ const NSUInteger ThumbnailDimensionPointsLarge() { - (nullable TSAttachmentStream *)cloneAsThumbnail { - NSData *thumbnailData = self.thumbnailData; + NSData *thumbnailData = self.legacyThumbnailData; // Only some media types have thumbnails if (!thumbnailData) { return nil; From cf469da9439c55559340e8681825efa6d7b18be7 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 4 Sep 2018 15:59:48 -0400 Subject: [PATCH 06/21] Use new thumbnails in conversation cells. --- .../Cells/OWSMessageBubbleView.m | 65 +++++++++++-------- 1 file changed, 38 insertions(+), 27 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index 521630e6a..1e0efb456 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -655,6 +655,7 @@ NS_ASSUME_NONNULL_BEGIN mediaView:(UIView *)mediaView cacheKey:(NSString *)cacheKey shouldSkipCache:(BOOL)shouldSkipCache + canLoadAsync:(BOOL)canLoadAsync { OWSAssert(self.attachmentStream); OWSAssert(mediaView); @@ -676,7 +677,7 @@ NS_ASSUME_NONNULL_BEGIN if (!shouldSkipCache) { [self.cellMediaCache setObject:cellMedia forKey:cacheKey]; } - } else { + } else if (!canLoadAsync) { DDLogError(@"%@ Failed to load cell media: %@", [self logTag], [self.attachmentStream originalMediaURL]); self.viewItem.didCellMediaFailToLoad = YES; [self showAttachmentErrorViewWithMediaView:mediaView]; @@ -836,6 +837,8 @@ NS_ASSUME_NONNULL_BEGIN stillImageView.backgroundColor = [UIColor whiteColor]; [self addAttachmentUploadViewIfNecessary]; + __weak UIImageView *weakImageView = stillImageView; + __weak OWSMessageBubbleView *weakSelf = self; self.loadCellContentBlock = ^{ OWSMessageBubbleView *strongSelf = weakSelf; @@ -853,13 +856,17 @@ NS_ASSUME_NONNULL_BEGIN BOOL shouldSkipCache = [OWSFileSystem fileSizeOfPath:strongSelf.attachmentStream.originalFilePath].unsignedIntegerValue < kMaxCachableSize; - stillImageView.image = [strongSelf tryToLoadCellMedia:^{ - OWSCAssert([strongSelf.attachmentStream isImage]); - return strongSelf.attachmentStream.originalImage; - } - mediaView:stillImageView - cacheKey:strongSelf.attachmentStream.uniqueId - shouldSkipCache:shouldSkipCache]; + stillImageView.image = [strongSelf + tryToLoadCellMedia:^{ + OWSCAssert([strongSelf.attachmentStream isImage]); + return [strongSelf.attachmentStream thumbnailImageMediumWithCompletion:^(UIImage *image) { + weakImageView.image = image; + }]; + } + mediaView:stillImageView + cacheKey:strongSelf.attachmentStream.uniqueId + shouldSkipCache:shouldSkipCache + canLoadAsync:YES]; }; self.unloadCellContentBlock = ^{ OWSMessageBubbleView *strongSelf = weakSelf; @@ -895,19 +902,21 @@ NS_ASSUME_NONNULL_BEGIN if (animatedImageView.image) { return; } - animatedImageView.image = [strongSelf tryToLoadCellMedia:^{ - OWSCAssert([strongSelf.attachmentStream isAnimated]); - - NSString *_Nullable filePath = [strongSelf.attachmentStream originalFilePath]; - YYImage *_Nullable animatedImage = nil; - if (strongSelf.attachmentStream.isValidImage && filePath) { - animatedImage = [YYImage imageWithContentsOfFile:filePath]; + animatedImageView.image = [strongSelf + tryToLoadCellMedia:^{ + OWSCAssert([strongSelf.attachmentStream isAnimated]); + + NSString *_Nullable filePath = [strongSelf.attachmentStream originalFilePath]; + YYImage *_Nullable animatedImage = nil; + if (strongSelf.attachmentStream.isValidImage && filePath) { + animatedImage = [YYImage imageWithContentsOfFile:filePath]; + } + return animatedImage; } - return animatedImage; - } - mediaView:animatedImageView - cacheKey:strongSelf.attachmentStream.uniqueId - shouldSkipCache:NO]; + mediaView:animatedImageView + cacheKey:strongSelf.attachmentStream.uniqueId + shouldSkipCache:NO + canLoadAsync:NO]; }; self.unloadCellContentBlock = ^{ OWSMessageBubbleView *strongSelf = weakSelf; @@ -976,14 +985,16 @@ NS_ASSUME_NONNULL_BEGIN if (stillImageView.image) { return; } - stillImageView.image = [strongSelf tryToLoadCellMedia:^{ - OWSCAssert([strongSelf.attachmentStream isVideo]); + stillImageView.image = [strongSelf + tryToLoadCellMedia:^{ + OWSCAssert([strongSelf.attachmentStream isVideo]); - return strongSelf.attachmentStream.originalImage; - } - mediaView:stillImageView - cacheKey:strongSelf.attachmentStream.uniqueId - shouldSkipCache:NO]; + return strongSelf.attachmentStream.originalImage; + } + mediaView:stillImageView + cacheKey:strongSelf.attachmentStream.uniqueId + shouldSkipCache:NO + canLoadAsync:NO]; }; self.unloadCellContentBlock = ^{ OWSMessageBubbleView *strongSelf = weakSelf; From f6e792c7075ce69e06b598427a88648255a9187d Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 4 Sep 2018 18:37:57 -0400 Subject: [PATCH 07/21] Add failure methods to thumbnail service. --- .../Attachments/OWSThumbnailService.swift | 37 ++++++++++++------- 1 file changed, 24 insertions(+), 13 deletions(-) diff --git a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift index 97aaac571..8ada87365 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift +++ b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift @@ -5,16 +5,19 @@ import Foundation private struct OWSThumbnailRequest { - public typealias CompletionBlock = (UIImage) -> Void + public typealias SuccessBlock = (UIImage) -> Void + public typealias FailureBlock = () -> Void let attachmentId: String let thumbnailDimensionPoints: UInt - let completion: CompletionBlock + let success: SuccessBlock + let failure: FailureBlock - init(attachmentId: String, thumbnailDimensionPoints: UInt, completion: @escaping CompletionBlock) { + init(attachmentId: String, thumbnailDimensionPoints: UInt, success: @escaping SuccessBlock, failure: @escaping FailureBlock) { self.attachmentId = attachmentId self.thumbnailDimensionPoints = thumbnailDimensionPoints - self.completion = completion + self.success = success + self.failure = failure } } @@ -25,7 +28,8 @@ private struct OWSThumbnailRequest { @objc(shared) public static let shared = OWSThumbnailService() - public typealias CompletionBlock = (UIImage) -> Void + public typealias SuccessBlock = (UIImage) -> Void + public typealias FailureBlock = () -> Void private let serialQueue = DispatchQueue(label: "OWSThumbnailService") @@ -35,8 +39,7 @@ private struct OWSThumbnailRequest { // // We want to process requests in _reverse_ order in which they // arrive so that we prioritize the most recent view state. - // This data structure is actually used like a stack. - private var thumbnailRequestQueue = [OWSThumbnailRequest]() + private var thumbnailRequestStack = [OWSThumbnailRequest]() private override init() { @@ -64,14 +67,18 @@ private struct OWSThumbnailRequest { // completion will be called async on the main thread. @objc public func ensureThumbnailForAttachmentId(attachmentId: String, thumbnailDimensionPoints: UInt, - completion:@escaping CompletionBlock) { + success: @escaping SuccessBlock, + failure: @escaping FailureBlock) { guard attachmentId.count > 0 else { owsFail("Empty attachment id.") + DispatchQueue.main.async { + failure() + } return } serialQueue.async { - let thumbnailRequest = OWSThumbnailRequest(attachmentId: attachmentId, thumbnailDimensionPoints: thumbnailDimensionPoints, completion: completion) - self.thumbnailRequestQueue.append(thumbnailRequest) + let thumbnailRequest = OWSThumbnailRequest(attachmentId: attachmentId, thumbnailDimensionPoints: thumbnailDimensionPoints, success: success, failure: failure) + self.thumbnailRequestStack.append(thumbnailRequest) self.processNextRequestSync() } @@ -85,14 +92,18 @@ private struct OWSThumbnailRequest { // This should only be called on the serialQueue. private func processNextRequestSync() { - guard !thumbnailRequestQueue.isEmpty else { + guard !thumbnailRequestStack.isEmpty else { return } - let thumbnailRequest = thumbnailRequestQueue.removeLast() + let thumbnailRequest = thumbnailRequestStack.removeLast() if let image = process(thumbnailRequest: thumbnailRequest) { DispatchQueue.main.async { - thumbnailRequest.completion(image) + thumbnailRequest.success(image) + } + } else { + DispatchQueue.main.async { + thumbnailRequest.failure() } } } From 206432fdf0cbb3dbec8b339ab906524f8a10a560 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 4 Sep 2018 20:38:07 -0400 Subject: [PATCH 08/21] Add failure methods to thumbnail service. --- .../Cells/OWSMessageBubbleView.m | 10 +- .../MediaGalleryViewController.swift | 2 +- Signal/src/util/OWSOrphanDataCleaner.m | 6 +- .../ViewModels/OWSQuotedReplyModel.m | 2 +- .../Attachments/OWSThumbnailService.swift | 39 ++++- .../Messages/Attachments/TSAttachmentStream.h | 20 +-- .../Messages/Attachments/TSAttachmentStream.m | 139 +++++++++++++----- 7 files changed, 152 insertions(+), 66 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index 1e0efb456..b449b77db 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -859,9 +859,13 @@ NS_ASSUME_NONNULL_BEGIN stillImageView.image = [strongSelf tryToLoadCellMedia:^{ OWSCAssert([strongSelf.attachmentStream isImage]); - return [strongSelf.attachmentStream thumbnailImageMediumWithCompletion:^(UIImage *image) { - weakImageView.image = image; - }]; + return [strongSelf.attachmentStream + thumbnailImageMediumWithSuccess:^(UIImage *image) { + weakImageView.image = image; + } + failure:^{ + DDLogError(@"Could not load thumbnail."); + }]; } mediaView:stillImageView cacheKey:strongSelf.attachmentStream.uniqueId diff --git a/Signal/src/ViewControllers/MediaGalleryViewController.swift b/Signal/src/ViewControllers/MediaGalleryViewController.swift index 10b94da74..4d0998051 100644 --- a/Signal/src/ViewControllers/MediaGalleryViewController.swift +++ b/Signal/src/ViewControllers/MediaGalleryViewController.swift @@ -35,7 +35,7 @@ public struct MediaGalleryItem: Equatable, Hashable { public typealias AsyncThumbnailBlock = (UIImage) -> Void func thumbnailImage(async:@escaping AsyncThumbnailBlock) -> UIImage? { - return attachmentStream.thumbnailImageSmall(completion: async) + return attachmentStream.thumbnailImageSmall(success: async, failure: {}) } var fullSizedImage: UIImage { diff --git a/Signal/src/util/OWSOrphanDataCleaner.m b/Signal/src/util/OWSOrphanDataCleaner.m index 1773450a5..0bd07e0aa 100644 --- a/Signal/src/util/OWSOrphanDataCleaner.m +++ b/Signal/src/util/OWSOrphanDataCleaner.m @@ -323,10 +323,8 @@ typedef void (^OrphanDataBlock)(OWSOrphanData *); OWSFail(@"%@ attachment has no file path.", self.logTag); } - NSString *_Nullable thumbnailPath = [attachmentStream legacyThumbnailPath]; - if (thumbnailPath.length > 0) { - [allAttachmentFilePaths addObject:thumbnailPath]; - } + [allAttachmentFilePaths + addObjectsFromArray:attachmentStream.allThumbnailPaths]; }]; if (shouldAbort) { diff --git a/SignalMessaging/ViewModels/OWSQuotedReplyModel.m b/SignalMessaging/ViewModels/OWSQuotedReplyModel.m index e48b83ccc..5a3f1bb52 100644 --- a/SignalMessaging/ViewModels/OWSQuotedReplyModel.m +++ b/SignalMessaging/ViewModels/OWSQuotedReplyModel.m @@ -227,7 +227,7 @@ NS_ASSUME_NONNULL_BEGIN authorId:authorId body:quotedText bodySource:TSQuotedMessageContentSourceLocal - thumbnailImage:quotedAttachment.legacyThumbnailImage + thumbnailImage:quotedAttachment.thumbnailImageSmallSync contentType:quotedAttachment.contentType sourceFilename:quotedAttachment.sourceFilename attachmentStream:quotedAttachment diff --git a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift index 8ada87365..861ef7fcf 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift +++ b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift @@ -4,8 +4,33 @@ import Foundation +@objc public class OWSLoadedThumbnail: NSObject { + public typealias DataSourceBlock = () throws -> Data + + @objc public let image: UIImage + let dataSourceBlock: DataSourceBlock + + @objc public init(image: UIImage, filePath: String) { + self.image = image + self.dataSourceBlock = { + return try Data(contentsOf: URL(fileURLWithPath: filePath)) + } + } + + @objc public init(image: UIImage, data: Data) { + self.image = image + self.dataSourceBlock = { + return data + } + } + + @objc public func data() throws -> Data { + return try dataSourceBlock() + } +} + private struct OWSThumbnailRequest { - public typealias SuccessBlock = (UIImage) -> Void + public typealias SuccessBlock = (OWSLoadedThumbnail) -> Void public typealias FailureBlock = () -> Void let attachmentId: String @@ -28,7 +53,7 @@ private struct OWSThumbnailRequest { @objc(shared) public static let shared = OWSThumbnailService() - public typealias SuccessBlock = (UIImage) -> Void + public typealias SuccessBlock = (OWSLoadedThumbnail) -> Void public typealias FailureBlock = () -> Void private let serialQueue = DispatchQueue(label: "OWSThumbnailService") @@ -97,9 +122,9 @@ private struct OWSThumbnailRequest { } let thumbnailRequest = thumbnailRequestStack.removeLast() - if let image = process(thumbnailRequest: thumbnailRequest) { + if let loadedThumbnail = process(thumbnailRequest: thumbnailRequest) { DispatchQueue.main.async { - thumbnailRequest.success(image) + thumbnailRequest.success(loadedThumbnail) } } else { DispatchQueue.main.async { @@ -109,7 +134,7 @@ private struct OWSThumbnailRequest { } // This should only be called on the serialQueue. - private func process(thumbnailRequest: OWSThumbnailRequest) -> UIImage? { + private func process(thumbnailRequest: OWSThumbnailRequest) -> OWSLoadedThumbnail? { var possibleAttachment: TSAttachmentStream? self.dbConnection.read({ (transaction) in possibleAttachment = TSAttachmentStream.fetch(uniqueId: thumbnailRequest.attachmentId, transaction: transaction) @@ -133,7 +158,7 @@ private struct OWSThumbnailRequest { owsFail("Could not load thumbnail.") return nil } - return image + return OWSLoadedThumbnail(image: image, filePath: filePath) } } } @@ -200,6 +225,6 @@ private struct OWSThumbnailRequest { size: thumbnailSize, transaction: transaction) }) - return thumbnailImage + return OWSLoadedThumbnail(image: thumbnailImage, data: thumbnailData) } } diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index 66e908663..e93a19a9a 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -17,7 +17,8 @@ NS_ASSUME_NONNULL_BEGIN @class TSAttachmentPointer; @class YapDatabaseReadWriteTransaction; -typedef void (^OWSThumbnailCompletion)(UIImage *image); +typedef void (^OWSThumbnailSuccess)(UIImage *image); +typedef void (^OWSThumbnailFailure)(void); @interface TSAttachmentThumbnail : MTLModel @@ -66,10 +67,7 @@ typedef void (^OWSThumbnailCompletion)(UIImage *image); - (nullable NSString *)originalFilePath; - (nullable NSURL *)originalMediaURL; -// TODO: Rename to legacy... -- (nullable UIImage *)legacyThumbnailImage; -//- (nullable NSData *)legacyThumbnailData; -- (nullable NSString *)legacyThumbnailPath; +- (NSArray *)allThumbnailPaths; + (BOOL)hasThumbnailForMimeType:(NSString *)contentType; @@ -96,16 +94,18 @@ typedef void (^OWSThumbnailCompletion)(UIImage *image); // Non-nil for attachments which need "lazy backup restore." - (nullable OWSBackupFragment *)lazyRestoreFragment; - #pragma mark - Thumbnails // On cache hit, the thumbnail will be returned synchronously and completion will never be invoked. // On cache miss, nil will be returned and the completion will be invoked async on main if // thumbnail can be generated. -- (nullable UIImage *)thumbnailImageWithSizeHint:(CGSize)sizeHint completion:(OWSThumbnailCompletion)completion; -- (nullable UIImage *)thumbnailImageSmallWithCompletion:(OWSThumbnailCompletion)completion; -- (nullable UIImage *)thumbnailImageMediumWithCompletion:(OWSThumbnailCompletion)completion; -- (nullable UIImage *)thumbnailImageLargeWithCompletion:(OWSThumbnailCompletion)completion; +- (nullable UIImage *)thumbnailImageWithSizeHint:(CGSize)sizeHint + completion:(OWSThumbnailSuccess)success + failure:(OWSThumbnailFailure)failure; +- (nullable UIImage *)thumbnailImageSmallWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; +- (nullable UIImage *)thumbnailImageMediumWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; +- (nullable UIImage *)thumbnailImageLargeWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; +- (nullable UIImage *)thumbnailImageSmallSync; // This method should only be invoked by OWSThumbnailService. - (nullable NSString *)pathForThumbnail:(TSAttachmentThumbnail *)thumbnail; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index 99fe956b9..f996c6af0 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -24,6 +24,8 @@ const NSUInteger ThumbnailDimensionPointsLarge() { return MAX(screenSizePoints.width, screenSizePoints.height); } +typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); + @implementation TSAttachmentThumbnail - (instancetype)initWithFilename:(NSString *)filename @@ -128,18 +130,9 @@ const NSUInteger ThumbnailDimensionPointsLarge() { _creationTimestamp = [NSDate new]; } - // This is going to be slow the first time it runs. - [self ensureLegacyThumbnail]; - return self; } -- (void)saveWithTransaction:(YapDatabaseReadWriteTransaction *)transaction -{ - [super saveWithTransaction:transaction]; - [self ensureLegacyThumbnail]; -} - - (void)upgradeFromAttachmentSchemaVersion:(NSUInteger)attachmentSchemaVersion { [super upgradeFromAttachmentSchemaVersion:attachmentSchemaVersion]; @@ -429,24 +422,6 @@ const NSUInteger ThumbnailDimensionPointsLarge() { [MIMETypeUtil isAnimated:contentType]); } -- (nullable NSData *)legacyThumbnailData -{ - NSString *thumbnailPath = self.legacyThumbnailPath; - if (!thumbnailPath) { - OWSAssert(!self.isImage && !self.isVideo && !self.isAnimated); - - return nil; - } - - if (![[NSFileManager defaultManager] fileExistsAtPath:thumbnailPath]) { - OWSFail(@"%@ missing thumbnail for attachmentId: %@", self.logTag, self.uniqueId); - - return nil; - } - - return [NSData dataWithContentsOfFile:thumbnailPath]; -} - - (void)ensureLegacyThumbnail { NSString *thumbnailPath = self.legacyThumbnailPath; @@ -764,7 +739,9 @@ const NSUInteger ThumbnailDimensionPointsLarge() { #pragma mark - Thumbnails -- (nullable UIImage *)thumbnailImageWithSizeHint:(CGSize)sizeHint completion:(OWSThumbnailCompletion)completion +- (nullable UIImage *)thumbnailImageWithSizeHint:(CGSize)sizeHint + success:(OWSThumbnailSuccess)success + failure:(OWSThumbnailFailure)failure { CGFloat maxDimensionHint = MAX(sizeHint.width, sizeHint.height); NSUInteger thumbnailDimensionPoints; @@ -776,26 +753,46 @@ const NSUInteger ThumbnailDimensionPointsLarge() { thumbnailDimensionPoints = ThumbnailDimensionPointsLarge(); } - return [self thumbnailImageWithThumbnailDimensionPoints:thumbnailDimensionPoints completion:completion]; + return [self thumbnailImageWithThumbnailDimensionPoints:thumbnailDimensionPoints success:success failure:failure]; } -- (nullable UIImage *)thumbnailImageSmallWithCompletion:(OWSThumbnailCompletion)completion +- (nullable UIImage *)thumbnailImageSmallWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure { - return [self thumbnailImageWithThumbnailDimensionPoints:kThumbnailDimensionPointsSmall completion:completion]; + return [self thumbnailImageWithThumbnailDimensionPoints:kThumbnailDimensionPointsSmall + success:success + failure:failure]; } -- (nullable UIImage *)thumbnailImageMediumWithCompletion:(OWSThumbnailCompletion)completion +- (nullable UIImage *)thumbnailImageMediumWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure { - return [self thumbnailImageWithThumbnailDimensionPoints:kThumbnailDimensionPointsMedium completion:completion]; + return [self thumbnailImageWithThumbnailDimensionPoints:kThumbnailDimensionPointsMedium + success:success + failure:failure]; } -- (nullable UIImage *)thumbnailImageLargeWithCompletion:(OWSThumbnailCompletion)completion +- (nullable UIImage *)thumbnailImageLargeWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure { - return [self thumbnailImageWithThumbnailDimensionPoints:ThumbnailDimensionPointsLarge() completion:completion]; + return [self thumbnailImageWithThumbnailDimensionPoints:ThumbnailDimensionPointsLarge() + success:success + failure:failure]; } - (nullable UIImage *)thumbnailImageWithThumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints - completion:(OWSThumbnailCompletion)completion + success:(OWSThumbnailSuccess)success + failure:(OWSThumbnailFailure)failure +{ + OWSLoadedThumbnail *_Nullable loadedThumbnail; + loadedThumbnail = [self loadedThumbnailWithThumbnailDimensionPoints:thumbnailDimensionPoints + success:^(OWSLoadedThumbnail *loadedThumbnail) { + success(loadedThumbnail.image); + } + failure:failure]; + return loadedThumbnail.image; +} + +- (nullable OWSLoadedThumbnail *)loadedThumbnailWithThumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints + success:(OWSLoadedThumbnailSuccess)success + failure:(OWSThumbnailFailure)failure { CGSize originalSize = self.imageSize; if (originalSize.width < 1 || originalSize.height < 1) { @@ -804,7 +801,7 @@ const NSUInteger ThumbnailDimensionPointsLarge() { if (originalSize.width <= thumbnailDimensionPoints || originalSize.height <= thumbnailDimensionPoints) { // There's no point in generating a thumbnail if the original is smaller than the // thumbnail size. - return self.originalImage; + return [[OWSLoadedThumbnail alloc] initWithImage:self.originalImage filePath:self.originalFilePath]; } for (TSAttachmentThumbnail *thumbnail in self.thumbnails) { @@ -817,15 +814,77 @@ const NSUInteger ThumbnailDimensionPointsLarge() { continue; } UIImage *_Nullable image = [UIImage imageWithContentsOfFile:thumbnailPath]; - return image; + if (!image) { + OWSFail(@"couldn't load image."); + continue; + } + return [[OWSLoadedThumbnail alloc] initWithImage:image filePath:thumbnailPath]; } [OWSThumbnailService.shared ensureThumbnailForAttachmentIdWithAttachmentId:self.uniqueId thumbnailDimensionPoints:thumbnailDimensionPoints - completion:completion]; + success:success + failure:failure]; return nil; } +- (nullable OWSLoadedThumbnail *)loadedThumbnailSmallSync +{ + __block dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); + + __block OWSLoadedThumbnail *_Nullable loadedThumbnail = nil; + loadedThumbnail = [self loadedThumbnailWithThumbnailDimensionPoints:kThumbnailDimensionPointsSmall + success:^(OWSLoadedThumbnail *asyncLoadedThumbnail) { + @synchronized(self) { + loadedThumbnail = asyncLoadedThumbnail; + } + dispatch_semaphore_signal(semaphore); + } + failure:^{ + dispatch_semaphore_signal(semaphore); + }]; + // Wait up to five seconds. + dispatch_semaphore_wait(semaphore, dispatch_time(DISPATCH_TIME_NOW, (int64_t)(5 * NSEC_PER_SEC))); + @synchronized(self) { + return loadedThumbnail; + } +} + +- (nullable UIImage *)thumbnailImageSmallSync +{ + return [self loadedThumbnailSmallSync].image; +} + +- (nullable NSData *)thumbnailDataSmallSync +{ + NSError *error; + NSData *_Nullable data = [[self loadedThumbnailSmallSync] dataAndReturnError:&error]; + if (error || !data) { + OWSFail(@"Couldn't load thumbnail data: %@", error); + return nil; + } + return data; +} + +- (NSArray *)allThumbnailPaths +{ + NSMutableArray *result = [NSMutableArray new]; + for (TSAttachmentThumbnail *thumbnail in self.thumbnails) { + NSString *_Nullable thumbnailPath = [self pathForThumbnail:thumbnail]; + if (!thumbnailPath) { + OWSFail(@"Missing thumbnail path."); + continue; + } + [result addObject:thumbnailPath]; + } + NSString *_Nullable legacyThumbnailPath = self.legacyThumbnailPath; + if (legacyThumbnailPath && [[NSFileManager defaultManager] fileExistsAtPath:legacyThumbnailPath]) { + [result addObject:legacyThumbnailPath]; + } + + return result; +} + #pragma mark - Update With... Methods - (void)markForLazyRestoreWithFragment:(OWSBackupFragment *)lazyRestoreFragment @@ -858,7 +917,7 @@ const NSUInteger ThumbnailDimensionPointsLarge() { - (nullable TSAttachmentStream *)cloneAsThumbnail { - NSData *thumbnailData = self.legacyThumbnailData; + NSData *_Nullable thumbnailData = self.thumbnailDataSmallSync; // Only some media types have thumbnails if (!thumbnailData) { return nil; From 8748dc9b2ec44cc57922cf8af22997096f7d00b9 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 4 Sep 2018 21:50:38 -0400 Subject: [PATCH 09/21] Modify new thumbnail system to include video and GIF thumbnails. --- .../MediaGalleryViewController.swift | 8 +- .../MessageDetailViewController.swift | 2 +- .../Messages/Attachments/OWSMediaUtils.swift | 137 ++++++++++++++++++ .../Attachments/OWSThumbnailService.swift | 89 ++++-------- .../Messages/Attachments/TSAttachmentStream.h | 16 +- .../Messages/Attachments/TSAttachmentStream.m | 107 +------------- SignalServiceKit/src/Util/DataSource.m | 3 +- SignalServiceKit/src/Util/NSData+Image.h | 2 - SignalServiceKit/src/Util/NSData+Image.m | 23 --- 9 files changed, 187 insertions(+), 200 deletions(-) create mode 100644 SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift diff --git a/Signal/src/ViewControllers/MediaGalleryViewController.swift b/Signal/src/ViewControllers/MediaGalleryViewController.swift index 4d0998051..cc79ccaa2 100644 --- a/Signal/src/ViewControllers/MediaGalleryViewController.swift +++ b/Signal/src/ViewControllers/MediaGalleryViewController.swift @@ -22,15 +22,15 @@ public struct MediaGalleryItem: Equatable, Hashable { } var isVideo: Bool { - return attachmentStream.isVideo() + return attachmentStream.isVideo } var isAnimated: Bool { - return attachmentStream.isAnimated() + return attachmentStream.isAnimated } var isImage: Bool { - return attachmentStream.isImage() + return attachmentStream.isImage } public typealias AsyncThumbnailBlock = (UIImage) -> Void @@ -39,7 +39,7 @@ public struct MediaGalleryItem: Equatable, Hashable { } var fullSizedImage: UIImage { - guard let image = attachmentStream.originalImage() else { + guard let image = attachmentStream.originalImage else { owsFail("\(logTag) in \(#function) unexpectedly unable to build attachment image") return UIImage() } diff --git a/Signal/src/ViewControllers/MessageDetailViewController.swift b/Signal/src/ViewControllers/MessageDetailViewController.swift index 82641bf38..d8bad7f42 100644 --- a/Signal/src/ViewControllers/MessageDetailViewController.swift +++ b/Signal/src/ViewControllers/MessageDetailViewController.swift @@ -651,7 +651,7 @@ class MessageDetailViewController: OWSViewController, MediaGalleryDataSourceDele func didTapAudioViewItem(_ viewItem: ConversationViewItem, attachmentStream: TSAttachmentStream) { SwiftAssertIsOnMainThread(#function) - guard let mediaURL = attachmentStream.originalMediaURL() else { + guard let mediaURL = attachmentStream.originalMediaURL else { owsFail("\(logTag) in \(#function) mediaURL was unexpectedly nil for attachment: \(attachmentStream)") return } diff --git a/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift b/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift new file mode 100644 index 000000000..cd7923b0c --- /dev/null +++ b/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift @@ -0,0 +1,137 @@ +// +// Copyright (c) 2018 Open Whisper Systems. All rights reserved. +// + +import Foundation +import AVFoundation + +public enum OWSMediaError: Error { + case failure(description: String) +} + +@objc public class OWSMediaUtils: NSObject { + + @available(*, unavailable, message:"do not instantiate this class.") + private override init() { + } + + @objc public class func thumbnail(forImageAtPath path: String, maxDimension: CGFloat) throws -> UIImage { + guard FileManager.default.fileExists(atPath: path) else { + throw OWSMediaError.failure(description: "Media file missing.") + } + guard NSData.ows_isValidImage(atPath: path) else { + throw OWSMediaError.failure(description: "Invalid image.") + } + + guard let originalImage = UIImage(contentsOfFile: path) else { + throw OWSMediaError.failure(description: "Could not load original image.") + } + let originalSize = originalImage.size + guard originalSize.width > 0 && originalSize.height > 0 else { + throw OWSMediaError.failure(description: "Original image has invalid size.") + } + var thumbnailSize = CGSize.zero + if originalSize.width > originalSize.height { + thumbnailSize.width = CGFloat(maxDimension) + thumbnailSize.height = round(CGFloat(maxDimension) * originalSize.height / originalSize.width) + } else { + thumbnailSize.width = round(CGFloat(maxDimension) * originalSize.width / originalSize.height) + thumbnailSize.height = CGFloat(maxDimension) + } + guard thumbnailSize.width > 0 && thumbnailSize.height > 0 else { + throw OWSMediaError.failure(description: "Thumbnail has invalid size.") + } + guard originalSize.width > thumbnailSize.width && + originalSize.height > thumbnailSize.height else { + throw OWSMediaError.failure(description: "Thumbnail isn't smaller than the original.") + } + // We use UIGraphicsBeginImageContextWithOptions() to scale. + // Core Image would provide better quality (e.g. Lanczos) but + // at perf cost we don't want to pay. We could also use + // CoreGraphics directly, but I'm not sure there's any benefit. + guard let thumbnailImage = originalImage.resizedImage(to: thumbnailSize) else { + throw OWSMediaError.failure(description: "Could not thumbnail image.") + } + return thumbnailImage + } + +// @objc public class func thumbnail(forImageAtPath path: String, maxDimension : CGFloat) throws -> UIImage { +// guard FileManager.default.fileExists(atPath: path) else { +// throw OWSMediaError.failure(description: "Media file missing.") +// } +// guard NSData.ows_isValidImage(atPath: path) else { +// throw OWSMediaError.failure(description: "Invalid image.") +// } +// let url = URL(fileURLWithPath: path) +// guard let imageSource = CGImageSourceCreateWithURL(url as CFURL, nil) else { +// throw OWSMediaError.failure(description: "Could not create image source.") +// } +// let imageOptions : [String :Any] = [ +// kCGImageSourceCreateThumbnailFromImageIfAbsent as String: kCFBooleanTrue as NSNumber, +// kCGImageSourceThumbnailMaxPixelSize as String: maxDimension, +// kCGImageSourceCreateThumbnailWithTransform as String: kCFBooleanTrue as NSNumber] +// guard let thumbnail = CGImageSourceCreateThumbnailAtIndex(imageSource, 0, imageOptions as CFDictionary) else { +// throw OWSMediaError.failure(description: "Could not create image thumbnail.") +// } +// let image = UIImage(cgImage: thumbnail) +// return image +// } + + private static let kMaxVideoStillSize: CGFloat = 1024 + + @objc public class func thumbnail(forVideoAtPath path: String) throws -> UIImage { + return try thumbnail(forVideoAtPath: path, maxSize: CGSize(width: kMaxVideoStillSize, height: kMaxVideoStillSize)) + } + + @objc public class func thumbnail(forVideoAtPath path: String, maxSize: CGSize) throws -> UIImage { + var maxSize = maxSize + maxSize.width = min(maxSize.width, kMaxVideoStillSize) + maxSize.height = min(maxSize.height, kMaxVideoStillSize) + + guard FileManager.default.fileExists(atPath: path) else { + throw OWSMediaError.failure(description: "Media file missing.") + } + let url = URL(fileURLWithPath: path) + let asset = AVURLAsset(url: url, options: nil) + guard isValidVideo(asset: asset) else { + throw OWSMediaError.failure(description: "Invalid video.") + } + + let generator = AVAssetImageGenerator(asset: asset) + generator.maximumSize = maxSize + generator.appliesPreferredTrackTransform = true + let time: CMTime = CMTimeMake(1, 60) + let cgImage = try generator.copyCGImage(at: time, actualTime: nil) + let image = UIImage(cgImage: cgImage) + return image + } + + @objc public class func isValidVideo(path: String) -> Bool { + guard FileManager.default.fileExists(atPath: path) else { + Logger.error("Media file missing.") + return false + } + let url = URL(fileURLWithPath: path) + let asset = AVURLAsset(url: url, options: nil) + return isValidVideo(asset: asset) + } + + private class func isValidVideo(asset: AVURLAsset) -> Bool { + var maxTrackSize = CGSize.zero + for track: AVAssetTrack in asset.tracks(withMediaType: .video) { + let trackSize: CGSize = track.naturalSize + maxTrackSize.width = max(maxTrackSize.width, trackSize.width) + maxTrackSize.height = max(maxTrackSize.height, trackSize.height) + } + if maxTrackSize.width < 1.0 || maxTrackSize.height < 1.0 { + Logger.error("Invalid video size: \(maxTrackSize)") + return false + } + let kMaxValidSize: CGFloat = 3 * 1024.0 + if maxTrackSize.width > kMaxValidSize || maxTrackSize.height > kMaxValidSize { + Logger.error("Invalid video dimensions: \(maxTrackSize)") + return false + } + return true + } +} diff --git a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift index 861ef7fcf..b4e69ee7a 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift +++ b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift @@ -3,6 +3,11 @@ // import Foundation +import AVFoundation + +public enum OWSThumbnailError: Error { + case failure(description: String) +} @objc public class OWSLoadedThumbnail: NSObject { public typealias DataSourceBlock = () throws -> Data @@ -76,16 +81,7 @@ private struct OWSThumbnailRequest { } private func canThumbnailAttachment(attachment: TSAttachmentStream) -> Bool { - guard attachment.isImage() else { - return false - } - guard !attachment.isAnimated() else { - return false - } - guard attachment.isValidImage() else { - return false - } - return true + return attachment.isImage || attachment.isAnimated || attachment.isVideo } // completion will only be called on success. @@ -122,11 +118,14 @@ private struct OWSThumbnailRequest { } let thumbnailRequest = thumbnailRequestStack.removeLast() - if let loadedThumbnail = process(thumbnailRequest: thumbnailRequest) { + do { + let loadedThumbnail = try process(thumbnailRequest: thumbnailRequest) DispatchQueue.main.async { thumbnailRequest.success(loadedThumbnail) } - } else { + } catch { + Logger.error("Could not create thumbnail: \(error)") + DispatchQueue.main.async { thumbnailRequest.failure() } @@ -134,75 +133,46 @@ private struct OWSThumbnailRequest { } // This should only be called on the serialQueue. - private func process(thumbnailRequest: OWSThumbnailRequest) -> OWSLoadedThumbnail? { + private func process(thumbnailRequest: OWSThumbnailRequest) throws -> OWSLoadedThumbnail { var possibleAttachment: TSAttachmentStream? self.dbConnection.read({ (transaction) in possibleAttachment = TSAttachmentStream.fetch(uniqueId: thumbnailRequest.attachmentId, transaction: transaction) }) guard let attachment = possibleAttachment else { - Logger.warn("Could not load attachment for thumbnailing.") - return nil + throw OWSThumbnailError.failure(description: "Could not load attachment for thumbnailing.") } guard canThumbnailAttachment(attachment: attachment) else { - Logger.warn("Cannot thumbnail attachment.") - return nil + throw OWSThumbnailError.failure(description: "Cannot thumbnail attachment.") } if let thumbnails = attachment.thumbnails { for thumbnail in thumbnails { if thumbnail.thumbnailDimensionPoints == thumbnailRequest.thumbnailDimensionPoints { guard let filePath = attachment.path(for: thumbnail) else { - owsFail("Could not determine thumbnail path.") - return nil + throw OWSThumbnailError.failure(description: "Could not determine thumbnail path.") } guard let image = UIImage(contentsOfFile: filePath) else { - owsFail("Could not load thumbnail.") - return nil + throw OWSThumbnailError.failure(description: "Could not load thumbnail.") } return OWSLoadedThumbnail(image: image, filePath: filePath) } } } - guard let originalFilePath = attachment.originalFilePath() else { - owsFail("Could not determine thumbnail path.") - return nil - } - guard let originalImage = UIImage(contentsOfFile: originalFilePath) else { - owsFail("Could not load original image.") - return nil - } - let originalSize = originalImage.size - guard originalSize.width > 0 && originalSize.height > 0 else { - owsFail("Original image has invalid size.") - return nil + guard let originalFilePath = attachment.originalFilePath else { + throw OWSThumbnailError.failure(description: "Missing original file path.") } - var thumbnailSize = CGSize.zero - if originalSize.width > originalSize.height { - thumbnailSize.width = CGFloat(thumbnailRequest.thumbnailDimensionPoints) - thumbnailSize.height = round(CGFloat(thumbnailRequest.thumbnailDimensionPoints) * originalSize.height / originalSize.width) + let maxDimension = CGFloat(thumbnailRequest.thumbnailDimensionPoints) + let thumbnailImage: UIImage + if attachment.isImage || attachment.isAnimated { + thumbnailImage = try OWSMediaUtils.thumbnail(forImageAtPath: originalFilePath, maxDimension: maxDimension) + } else if attachment.isVideo { + let maxSize = CGSize(width: maxDimension, height: maxDimension) + thumbnailImage = try OWSMediaUtils.thumbnail(forVideoAtPath: originalFilePath, maxSize: maxSize) } else { - thumbnailSize.width = round(CGFloat(thumbnailRequest.thumbnailDimensionPoints) * originalSize.width / originalSize.height) - thumbnailSize.height = CGFloat(thumbnailRequest.thumbnailDimensionPoints) - } - guard thumbnailSize.width > 0 && thumbnailSize.height > 0 else { - owsFail("Thumbnail has invalid size.") - return nil - } - guard originalSize.width > thumbnailSize.width && - originalSize.height > thumbnailSize.height else { - owsFail("Thumbnail isn't smaller than the original.") - return nil - } - // We use UIGraphicsBeginImageContextWithOptions() to scale. - // Core Image would provide better quality (e.g. Lanczos) but - // at perf cost we don't want to pay. We could also use - // CoreGraphics directly, but I'm not sure there's any benefit. - guard let thumbnailImage = originalImage.resizedImage(to: thumbnailSize) else { - owsFail("Could not thumbnail image.") - return nil + throw OWSThumbnailError.failure(description: "Invalid attachment type.") } + let thumbnailSize = thumbnailImage.size guard let thumbnailData = UIImageJPEGRepresentation(thumbnailImage, 0.85) else { - owsFail("Could not convert thumbnail to JPEG.") - return nil + throw OWSThumbnailError.failure(description: "Could not convert thumbnail to JPEG.") } let temporaryDirectory = NSTemporaryDirectory() let thumbnailFilename = "\(NSUUID().uuidString).jpg" @@ -210,8 +180,7 @@ private struct OWSThumbnailRequest { do { try thumbnailData.write(to: NSURL.fileURL(withPath: thumbnailFilePath), options: .atomicWrite) } catch let error as NSError { - owsFail("File write failed: \(thumbnailFilePath), \(error)") - return nil + throw OWSThumbnailError.failure(description: "File write failed: \(thumbnailFilePath), \(error)") } // It should be safe to assume that an attachment will never end up with two thumbnails of // the same size since: diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index e93a19a9a..e9ccccc3a 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -58,14 +58,14 @@ typedef void (^OWSThumbnailFailure)(void); - (nullable NSData *)validStillImageData; #endif -- (BOOL)isAnimated; -- (BOOL)isImage; -- (BOOL)isVideo; -- (BOOL)isAudio; +@property (nonatomic, readonly) BOOL isAnimated; +@property (nonatomic, readonly) BOOL isImage; +@property (nonatomic, readonly) BOOL isVideo; +@property (nonatomic, readonly) BOOL isAudio; -- (nullable UIImage *)originalImage; -- (nullable NSString *)originalFilePath; -- (nullable NSURL *)originalMediaURL; +@property (nonatomic, readonly, nullable) UIImage *originalImage; +@property (nonatomic, readonly, nullable) NSString *originalFilePath; +@property (nonatomic, readonly, nullable) NSURL *originalMediaURL; - (NSArray *)allThumbnailPaths; @@ -100,7 +100,7 @@ typedef void (^OWSThumbnailFailure)(void); // On cache miss, nil will be returned and the completion will be invoked async on main if // thumbnail can be generated. - (nullable UIImage *)thumbnailImageWithSizeHint:(CGSize)sizeHint - completion:(OWSThumbnailSuccess)success + success:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; - (nullable UIImage *)thumbnailImageSmallWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; - (nullable UIImage *)thumbnailImageMediumWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index f996c6af0..67f278003 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -14,8 +14,6 @@ NS_ASSUME_NONNULL_BEGIN -const CGFloat kMaxVideoStillSize = 1 * 1024; - const NSUInteger kThumbnailDimensionPointsSmall = 300; const NSUInteger kThumbnailDimensionPointsMedium = 800; // This size is large enough to render full screen. @@ -374,7 +372,7 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); { OWSAssert(self.isVideo); - return [NSData ows_isValidVideoAtURL:self.originalMediaURL]; + return [OWSMediaUtils isValidVideoWithPath:self.originalFilePath]; } #pragma mark - @@ -422,108 +420,15 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); [MIMETypeUtil isAnimated:contentType]); } -- (void)ensureLegacyThumbnail -{ - NSString *thumbnailPath = self.legacyThumbnailPath; - if (!thumbnailPath) { - return; - } - - if ([[NSFileManager defaultManager] fileExistsAtPath:thumbnailPath]) { - // already exists - return; - } - - if (![[NSFileManager defaultManager] fileExistsAtPath:self.originalMediaURL.path]) { - DDLogError(@"%@ while generating thumbnail, source file doesn't exist: %@", self.logTag, self.originalMediaURL); - // If we're not lazy-restoring this message, the attachment should exist on disk. - OWSAssert(self.lazyRestoreFragmentId); - return; - } - - // TODO proper resolution? - CGFloat thumbnailSize = 200; - - UIImage *_Nullable result; - if (self.isImage || self.isAnimated) { - if (![self isValidImage]) { - DDLogWarn( - @"%@ skipping thumbnail generation for invalid image at path: %@", self.logTag, self.originalFilePath); - return; - } - - CGImageSourceRef imageSource = CGImageSourceCreateWithURL((__bridge CFURLRef)self.originalMediaURL, NULL); - OWSAssert(imageSource != NULL); - NSDictionary *imageOptions = @{ - (NSString const *)kCGImageSourceCreateThumbnailFromImageIfAbsent : (NSNumber const *)kCFBooleanTrue, - (NSString const *)kCGImageSourceThumbnailMaxPixelSize : @(thumbnailSize), - (NSString const *)kCGImageSourceCreateThumbnailWithTransform : (NSNumber const *)kCFBooleanTrue - }; - CGImageRef thumbnail - = CGImageSourceCreateThumbnailAtIndex(imageSource, 0, (__bridge CFDictionaryRef)imageOptions); - CFRelease(imageSource); - - result = [[UIImage alloc] initWithCGImage:thumbnail]; - CGImageRelease(thumbnail); - - } else if (self.isVideo) { - if (![self isValidVideo]) { - DDLogWarn(@"%@ skipping thumbnail for invalid video at path: %@", self.logTag, self.originalFilePath); - return; - } - - result = [self videoStillImageWithMaxSize:CGSizeMake(thumbnailSize, thumbnailSize)]; - } else { - OWSFail(@"%@ trying to generate thumnail for unexpected attachment: %@ of type: %@", - self.logTag, - self.uniqueId, - self.contentType); - } - - if (result == nil) { - DDLogError(@"Unable to build thumbnail for attachmentId: %@", self.uniqueId); - return; - } - - NSData *thumbnailData = UIImageJPEGRepresentation(result, 0.9); - - OWSAssert(thumbnailData.length > 0); - DDLogDebug(@"%@ generated thumbnail with size: %lu", self.logTag, (unsigned long)thumbnailData.length); - [thumbnailData writeToFile:thumbnailPath atomically:YES]; -} - - (nullable UIImage *)videoStillImage { - if (![self isValidVideo]) { - return nil; - } - // Uses the assets intrinsic size by default - return [self videoStillImageWithMaxSize:CGSizeMake(kMaxVideoStillSize, kMaxVideoStillSize)]; -} - -- (nullable UIImage *)videoStillImageWithMaxSize:(CGSize)maxSize -{ - maxSize.width = MIN(maxSize.width, kMaxVideoStillSize); - maxSize.height = MIN(maxSize.height, kMaxVideoStillSize); - - NSURL *_Nullable mediaUrl = self.originalMediaURL; - if (!mediaUrl) { - return nil; - } - AVURLAsset *asset = [[AVURLAsset alloc] initWithURL:mediaUrl options:nil]; - - AVAssetImageGenerator *generator = [[AVAssetImageGenerator alloc] initWithAsset:asset]; - generator.maximumSize = maxSize; - generator.appliesPreferredTrackTransform = YES; - NSError *err = NULL; - CMTime time = CMTimeMake(1, 60); - CGImageRef imgRef = [generator copyCGImageAtTime:time actualTime:NULL error:&err]; - if (imgRef == NULL) { - DDLogError(@"Could not generate video still: %@", self.originalFilePath.pathExtension); + NSError *error; + UIImage *_Nullable image = [OWSMediaUtils thumbnailForVideoAtPath:self.originalFilePath error:&error]; + if (error || !image) { + DDLogError(@"Could not create video still: %@.", error); return nil; } - - return [[UIImage alloc] initWithCGImage:imgRef]; + return image; } + (void)deleteAttachments diff --git a/SignalServiceKit/src/Util/DataSource.m b/SignalServiceKit/src/Util/DataSource.m index 784639091..fd3f88180 100755 --- a/SignalServiceKit/src/Util/DataSource.m +++ b/SignalServiceKit/src/Util/DataSource.m @@ -7,6 +7,7 @@ #import "NSData+Image.h" #import "NSString+SSK.h" #import "OWSFileSystem.h" +#import NS_ASSUME_NONNULL_BEGIN @@ -75,7 +76,7 @@ NS_ASSUME_NONNULL_BEGIN - (BOOL)isValidVideo { - return [NSData ows_isValidVideoAtURL:self.dataUrl]; + return [OWSMediaUtils isValidVideoWithPath:self.dataUrl.path]; } - (void)setSourceFilename:(nullable NSString *)sourceFilename diff --git a/SignalServiceKit/src/Util/NSData+Image.h b/SignalServiceKit/src/Util/NSData+Image.h index 8ddea1190..5c86b2bcc 100644 --- a/SignalServiceKit/src/Util/NSData+Image.h +++ b/SignalServiceKit/src/Util/NSData+Image.h @@ -11,6 +11,4 @@ - (BOOL)ows_isValidImage; - (BOOL)ows_isValidImageWithMimeType:(nullable NSString *)mimeType; -+ (BOOL)ows_isValidVideoAtURL:(NSURL *)url; - @end diff --git a/SignalServiceKit/src/Util/NSData+Image.m b/SignalServiceKit/src/Util/NSData+Image.m index dea34ea64..4e00c43bc 100644 --- a/SignalServiceKit/src/Util/NSData+Image.m +++ b/SignalServiceKit/src/Util/NSData+Image.m @@ -261,27 +261,4 @@ typedef NS_ENUM(NSInteger, ImageFormat) { return (width > 0 && width < kMaxValidSize && height > 0 && height < kMaxValidSize); } -+ (BOOL)ows_isValidVideoAtURL:(NSURL *)url -{ - OWSAssert(url); - AVURLAsset *asset = [[AVURLAsset alloc] initWithURL:url options:nil]; - - CGSize maxSize = CGSizeZero; - for (AVAssetTrack *track in [asset tracksWithMediaType:AVMediaTypeVideo]) { - CGSize trackSize = track.naturalSize; - maxSize.width = MAX(maxSize.width, trackSize.width); - maxSize.height = MAX(maxSize.height, trackSize.height); - } - if (maxSize.width < 1.f || maxSize.height < 1.f) { - DDLogError(@"Invalid video size: %@", NSStringFromCGSize(maxSize)); - return NO; - } - const CGFloat kMaxSize = 3 * 1024.f; - if (maxSize.width > kMaxSize || maxSize.height > kMaxSize) { - DDLogError(@"Invalid video dimensions: %@", NSStringFromCGSize(maxSize)); - return NO; - } - return YES; -} - @end From a9096209e90daffd84531feb1f54cffad22f2624 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Tue, 4 Sep 2018 22:00:51 -0400 Subject: [PATCH 10/21] Add failure methods to thumbnail service. --- .../src/Messages/Attachments/OWSThumbnailService.swift | 2 +- .../src/Messages/Attachments/TSAttachmentStream.m | 8 ++++---- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift index b4e69ee7a..6b470e00f 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift +++ b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift @@ -86,7 +86,7 @@ private struct OWSThumbnailRequest { // completion will only be called on success. // completion will be called async on the main thread. - @objc public func ensureThumbnailForAttachmentId(attachmentId: String, + @objc public func ensureThumbnail(forAttachmentId attachmentId: String, thumbnailDimensionPoints: UInt, success: @escaping SuccessBlock, failure: @escaping FailureBlock) { diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index 67f278003..2a7b92e75 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -726,10 +726,10 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); return [[OWSLoadedThumbnail alloc] initWithImage:image filePath:thumbnailPath]; } - [OWSThumbnailService.shared ensureThumbnailForAttachmentIdWithAttachmentId:self.uniqueId - thumbnailDimensionPoints:thumbnailDimensionPoints - success:success - failure:failure]; + [OWSThumbnailService.shared ensureThumbnailForAttachmentId:self.uniqueId + thumbnailDimensionPoints:thumbnailDimensionPoints + success:success + failure:failure]; return nil; } From 2daa66fdf6e298d1810bfe981b0f244d993bddc4 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 5 Sep 2018 10:50:36 -0400 Subject: [PATCH 11/21] Use thumbnails dir. --- .../Messages/Attachments/TSAttachmentStream.m | 26 ++++++++++--------- 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index 2a7b92e75..a27ea70a0 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -281,16 +281,20 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); return [[containingDir stringByAppendingPathComponent:newFilename] stringByAppendingPathExtension:@"jpg"]; } -- (nullable NSString *)pathForThumbnail:(TSAttachmentThumbnail *)thumbnail +- (NSString *)thumbnailsDirPath { - NSString *filePath = self.originalFilePath; - if (!filePath) { + if (!self.localRelativeFilePath) { OWSFail(@"%@ Attachment missing local file path.", self.logTag); return nil; } - NSString *containingDir = filePath.stringByDeletingLastPathComponent; - return [containingDir stringByAppendingPathComponent:thumbnail.filename]; + NSString *dirName = [NSString stringWithFormat:@"%@-thumbnails", self.uniqueId]; + return [[[self class] attachmentsFolder] stringByAppendingPathComponent:dirName]; +} + +- (nullable NSString *)pathForThumbnail:(TSAttachmentThumbnail *)thumbnail +{ + return [self.thumbnailsDirPath stringByAppendingPathComponent:thumbnail.filename]; } - (nullable NSURL *)originalMediaURL @@ -307,13 +311,11 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); { NSError *error; - for (TSAttachmentThumbnail *thumbnail in self.thumbnails) { - NSString *_Nullable thumbnailPath = [self pathForThumbnail:thumbnail]; - if (thumbnailPath) { - BOOL success = [[NSFileManager defaultManager] removeItemAtPath:thumbnailPath error:&error]; - if (error || !success) { - DDLogError(@"%@ remove thumbnail failed with: %@", self.logTag, error); - } + NSString *thumbnailsDirPath = self.thumbnailsDirPath; + if ([[NSFileManager defaultManager] fileExistsAtPath:thumbnailsDirPath]) { + BOOL success = [[NSFileManager defaultManager] removeItemAtPath:thumbnailsDirPath error:&error]; + if (error || !success) { + DDLogError(@"%@ remove thumbnails dir failed with: %@", self.logTag, error); } } From 8026d34651029cb5c7b573b7aeceacd643965440 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 5 Sep 2018 10:51:24 -0400 Subject: [PATCH 12/21] Remove full-screen thumbnail. --- .../Messages/Attachments/TSAttachmentStream.h | 1 - .../Messages/Attachments/TSAttachmentStream.m | 16 +--------------- 2 files changed, 1 insertion(+), 16 deletions(-) diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index e9ccccc3a..d5e196252 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -104,7 +104,6 @@ typedef void (^OWSThumbnailFailure)(void); failure:(OWSThumbnailFailure)failure; - (nullable UIImage *)thumbnailImageSmallWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; - (nullable UIImage *)thumbnailImageMediumWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; -- (nullable UIImage *)thumbnailImageLargeWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; - (nullable UIImage *)thumbnailImageSmallSync; // This method should only be invoked by OWSThumbnailService. diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index a27ea70a0..1cfb2b1f2 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -16,11 +16,6 @@ NS_ASSUME_NONNULL_BEGIN const NSUInteger kThumbnailDimensionPointsSmall = 300; const NSUInteger kThumbnailDimensionPointsMedium = 800; -// This size is large enough to render full screen. -const NSUInteger ThumbnailDimensionPointsLarge() { - CGSize screenSizePoints = UIScreen.mainScreen.bounds.size; - return MAX(screenSizePoints.width, screenSizePoints.height); -} typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); @@ -654,10 +649,8 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); NSUInteger thumbnailDimensionPoints; if (maxDimensionHint <= kThumbnailDimensionPointsSmall) { thumbnailDimensionPoints = kThumbnailDimensionPointsSmall; - } else if (maxDimensionHint <= kThumbnailDimensionPointsMedium) { - thumbnailDimensionPoints = kThumbnailDimensionPointsMedium; } else { - thumbnailDimensionPoints = ThumbnailDimensionPointsLarge(); + thumbnailDimensionPoints = kThumbnailDimensionPointsMedium; } return [self thumbnailImageWithThumbnailDimensionPoints:thumbnailDimensionPoints success:success failure:failure]; @@ -677,13 +670,6 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); failure:failure]; } -- (nullable UIImage *)thumbnailImageLargeWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure -{ - return [self thumbnailImageWithThumbnailDimensionPoints:ThumbnailDimensionPointsLarge() - success:success - failure:failure]; -} - - (nullable UIImage *)thumbnailImageWithThumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints success:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure From 32bf47fc74a8aa3fd4019575325bd5d6019289a2 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 5 Sep 2018 11:33:45 -0400 Subject: [PATCH 13/21] Don't track thumbnail metadata in db; improve thumbnail quality. --- .../Messages/Attachments/OWSMediaUtils.swift | 45 +------- .../Attachments/OWSThumbnailService.swift | 72 +++++-------- .../Messages/Attachments/TSAttachmentStream.h | 22 +--- .../Messages/Attachments/TSAttachmentStream.m | 102 ++++-------------- SignalServiceKit/src/Util/UIImage+OWS.h | 1 + SignalServiceKit/src/Util/UIImage+OWS.m | 29 +++++ 6 files changed, 79 insertions(+), 192 deletions(-) diff --git a/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift b/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift index cd7923b0c..64d59148a 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift +++ b/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift @@ -26,57 +26,16 @@ public enum OWSMediaError: Error { guard let originalImage = UIImage(contentsOfFile: path) else { throw OWSMediaError.failure(description: "Could not load original image.") } - let originalSize = originalImage.size - guard originalSize.width > 0 && originalSize.height > 0 else { - throw OWSMediaError.failure(description: "Original image has invalid size.") - } - var thumbnailSize = CGSize.zero - if originalSize.width > originalSize.height { - thumbnailSize.width = CGFloat(maxDimension) - thumbnailSize.height = round(CGFloat(maxDimension) * originalSize.height / originalSize.width) - } else { - thumbnailSize.width = round(CGFloat(maxDimension) * originalSize.width / originalSize.height) - thumbnailSize.height = CGFloat(maxDimension) - } - guard thumbnailSize.width > 0 && thumbnailSize.height > 0 else { - throw OWSMediaError.failure(description: "Thumbnail has invalid size.") - } - guard originalSize.width > thumbnailSize.width && - originalSize.height > thumbnailSize.height else { - throw OWSMediaError.failure(description: "Thumbnail isn't smaller than the original.") - } // We use UIGraphicsBeginImageContextWithOptions() to scale. // Core Image would provide better quality (e.g. Lanczos) but - // at perf cost we don't want to pay. We could also use + // at a perf cost we don't want to pay. We could also use // CoreGraphics directly, but I'm not sure there's any benefit. - guard let thumbnailImage = originalImage.resizedImage(to: thumbnailSize) else { + guard let thumbnailImage = originalImage.resized(withMaxDimensionPoints: maxDimension) else { throw OWSMediaError.failure(description: "Could not thumbnail image.") } return thumbnailImage } -// @objc public class func thumbnail(forImageAtPath path: String, maxDimension : CGFloat) throws -> UIImage { -// guard FileManager.default.fileExists(atPath: path) else { -// throw OWSMediaError.failure(description: "Media file missing.") -// } -// guard NSData.ows_isValidImage(atPath: path) else { -// throw OWSMediaError.failure(description: "Invalid image.") -// } -// let url = URL(fileURLWithPath: path) -// guard let imageSource = CGImageSourceCreateWithURL(url as CFURL, nil) else { -// throw OWSMediaError.failure(description: "Could not create image source.") -// } -// let imageOptions : [String :Any] = [ -// kCGImageSourceCreateThumbnailFromImageIfAbsent as String: kCFBooleanTrue as NSNumber, -// kCGImageSourceThumbnailMaxPixelSize as String: maxDimension, -// kCGImageSourceCreateThumbnailWithTransform as String: kCFBooleanTrue as NSNumber] -// guard let thumbnail = CGImageSourceCreateThumbnailAtIndex(imageSource, 0, imageOptions as CFDictionary) else { -// throw OWSMediaError.failure(description: "Could not create image thumbnail.") -// } -// let image = UIImage(cgImage: thumbnail) -// return image -// } - private static let kMaxVideoStillSize: CGFloat = 1024 @objc public class func thumbnail(forVideoAtPath path: String) throws -> UIImage { diff --git a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift index 6b470e00f..b67633025 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift +++ b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift @@ -38,13 +38,13 @@ private struct OWSThumbnailRequest { public typealias SuccessBlock = (OWSLoadedThumbnail) -> Void public typealias FailureBlock = () -> Void - let attachmentId: String + let attachment: TSAttachmentStream let thumbnailDimensionPoints: UInt let success: SuccessBlock let failure: FailureBlock - init(attachmentId: String, thumbnailDimensionPoints: UInt, success: @escaping SuccessBlock, failure: @escaping FailureBlock) { - self.attachmentId = attachmentId + init(attachment: TSAttachmentStream, thumbnailDimensionPoints: UInt, success: @escaping SuccessBlock, failure: @escaping FailureBlock) { + self.attachment = attachment self.thumbnailDimensionPoints = thumbnailDimensionPoints self.success = success self.failure = failure @@ -86,19 +86,12 @@ private struct OWSThumbnailRequest { // completion will only be called on success. // completion will be called async on the main thread. - @objc public func ensureThumbnail(forAttachmentId attachmentId: String, + @objc public func ensureThumbnail(forAttachment attachment: TSAttachmentStream, thumbnailDimensionPoints: UInt, success: @escaping SuccessBlock, failure: @escaping FailureBlock) { - guard attachmentId.count > 0 else { - owsFail("Empty attachment id.") - DispatchQueue.main.async { - failure() - } - return - } serialQueue.async { - let thumbnailRequest = OWSThumbnailRequest(attachmentId: attachmentId, thumbnailDimensionPoints: thumbnailDimensionPoints, success: success, failure: failure) + let thumbnailRequest = OWSThumbnailRequest(attachment: attachment, thumbnailDimensionPoints: thumbnailDimensionPoints, success: success, failure: failure) self.thumbnailRequestStack.append(thumbnailRequest) self.processNextRequestSync() @@ -133,29 +126,28 @@ private struct OWSThumbnailRequest { } // This should only be called on the serialQueue. + // + // It should be safe to assume that an attachment will never end up with two thumbnails of + // the same size since: + // + // * Thumbnails are only added by this method. + // * This method checks for an existing thumbnail using the same connection. + // * This method is performed on the serial queue. private func process(thumbnailRequest: OWSThumbnailRequest) throws -> OWSLoadedThumbnail { - var possibleAttachment: TSAttachmentStream? - self.dbConnection.read({ (transaction) in - possibleAttachment = TSAttachmentStream.fetch(uniqueId: thumbnailRequest.attachmentId, transaction: transaction) - }) - guard let attachment = possibleAttachment else { - throw OWSThumbnailError.failure(description: "Could not load attachment for thumbnailing.") - } + let attachment = thumbnailRequest.attachment guard canThumbnailAttachment(attachment: attachment) else { throw OWSThumbnailError.failure(description: "Cannot thumbnail attachment.") } - if let thumbnails = attachment.thumbnails { - for thumbnail in thumbnails { - if thumbnail.thumbnailDimensionPoints == thumbnailRequest.thumbnailDimensionPoints { - guard let filePath = attachment.path(for: thumbnail) else { - throw OWSThumbnailError.failure(description: "Could not determine thumbnail path.") - } - guard let image = UIImage(contentsOfFile: filePath) else { - throw OWSThumbnailError.failure(description: "Could not load thumbnail.") - } - return OWSLoadedThumbnail(image: image, filePath: filePath) - } + let thumbnailPath = attachment.path(forThumbnailDimensionPoints: thumbnailRequest.thumbnailDimensionPoints) + if FileManager.default.fileExists(atPath: thumbnailPath) { + guard let image = UIImage(contentsOfFile: thumbnailPath) else { + throw OWSThumbnailError.failure(description: "Could not load thumbnail.") } + return OWSLoadedThumbnail(image: image, filePath: thumbnailPath) + } + let thumbnailDirPath = (thumbnailPath as NSString).deletingLastPathComponent + if !FileManager.default.fileExists(atPath: thumbnailDirPath) { + try FileManager.default.createDirectory(atPath: thumbnailDirPath, withIntermediateDirectories: true, attributes: nil) } guard let originalFilePath = attachment.originalFilePath else { throw OWSThumbnailError.failure(description: "Missing original file path.") @@ -170,30 +162,14 @@ private struct OWSThumbnailRequest { } else { throw OWSThumbnailError.failure(description: "Invalid attachment type.") } - let thumbnailSize = thumbnailImage.size guard let thumbnailData = UIImageJPEGRepresentation(thumbnailImage, 0.85) else { throw OWSThumbnailError.failure(description: "Could not convert thumbnail to JPEG.") } - let temporaryDirectory = NSTemporaryDirectory() - let thumbnailFilename = "\(NSUUID().uuidString).jpg" - let thumbnailFilePath = (temporaryDirectory as NSString).appendingPathComponent(thumbnailFilename) do { - try thumbnailData.write(to: NSURL.fileURL(withPath: thumbnailFilePath), options: .atomicWrite) + try thumbnailData.write(to: NSURL.fileURL(withPath: thumbnailPath), options: .atomicWrite) } catch let error as NSError { - throw OWSThumbnailError.failure(description: "File write failed: \(thumbnailFilePath), \(error)") + throw OWSThumbnailError.failure(description: "File write failed: \(thumbnailPath), \(error)") } - // It should be safe to assume that an attachment will never end up with two thumbnails of - // the same size since: - // - // * Thumbnails are only added by this method. - // * This method checks for an existing thumbnail using the same connection. - // * This method is performed on the serial queue. - self.dbConnection.readWrite({ (transaction) in - attachment.update(withNewThumbnail: thumbnailFilePath, - thumbnailDimensionPoints: thumbnailRequest.thumbnailDimensionPoints, - size: thumbnailSize, - transaction: transaction) - }) return OWSLoadedThumbnail(image: thumbnailImage, data: thumbnailData) } } diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index d5e196252..017fd0efd 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -20,19 +20,6 @@ NS_ASSUME_NONNULL_BEGIN typedef void (^OWSThumbnailSuccess)(UIImage *image); typedef void (^OWSThumbnailFailure)(void); -@interface TSAttachmentThumbnail : MTLModel - -@property (nonatomic, readonly) NSString *filename; -@property (nonatomic, readonly) CGSize size; -// The length of the longer side. -@property (nonatomic, readonly) NSUInteger thumbnailDimensionPoints; - -- (instancetype)init NS_UNAVAILABLE; - -@end - -#pragma mark - - @interface TSAttachmentStream : TSAttachment - (instancetype)init NS_UNAVAILABLE; @@ -52,8 +39,6 @@ typedef void (^OWSThumbnailFailure)(void); @property (nonatomic, readonly) NSDate *creationTimestamp; -@property (nonatomic, nullable, readonly) NSArray *thumbnails; - #if TARGET_OS_IPHONE - (nullable NSData *)validStillImageData; #endif @@ -107,7 +92,7 @@ typedef void (^OWSThumbnailFailure)(void); - (nullable UIImage *)thumbnailImageSmallSync; // This method should only be invoked by OWSThumbnailService. -- (nullable NSString *)pathForThumbnail:(TSAttachmentThumbnail *)thumbnail; +- (NSString *)pathForThumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints; #pragma mark - Validation @@ -125,11 +110,6 @@ typedef void (^OWSThumbnailFailure)(void); // TODO: Review. - (nullable TSAttachmentStream *)cloneAsThumbnail; -- (void)updateWithNewThumbnail:(NSString *)tempFilePath - thumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints - size:(CGSize)size - transaction:(YapDatabaseReadWriteTransaction *)transaction; - #pragma mark - Protobuf + (nullable SSKProtoAttachmentPointer *)buildProtoForAttachmentId:(nullable NSString *)attachmentId; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index 1cfb2b1f2..2cbf1b137 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -19,28 +19,6 @@ const NSUInteger kThumbnailDimensionPointsMedium = 800; typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); -@implementation TSAttachmentThumbnail - -- (instancetype)initWithFilename:(NSString *)filename - size:(CGSize)size - thumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints -{ - self = [super init]; - if (!self) { - return self; - } - - _filename = filename; - _size = size; - _thumbnailDimensionPoints = thumbnailDimensionPoints; - - return self; -} - -@end - -#pragma mark - - @interface TSAttachmentStream () // We only want to generate the file path for this attachment once, so that @@ -57,8 +35,6 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); // Optional property. Only set for attachments which need "lazy backup restore." @property (nonatomic, nullable) NSString *lazyRestoreFragmentId; -@property (nonatomic, nullable) NSArray *thumbnails; - @end #pragma mark - @@ -287,9 +263,10 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); return [[[self class] attachmentsFolder] stringByAppendingPathComponent:dirName]; } -- (nullable NSString *)pathForThumbnail:(TSAttachmentThumbnail *)thumbnail +- (NSString *)pathForThumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints { - return [self.thumbnailsDirPath stringByAppendingPathComponent:thumbnail.filename]; + NSString *filename = [NSString stringWithFormat:@"thumbnail-%lu.jpg", (unsigned long)thumbnailDimensionPoints]; + return [self.thumbnailsDirPath stringByAppendingPathComponent:filename]; } - (nullable NSURL *)originalMediaURL @@ -697,27 +674,20 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); return [[OWSLoadedThumbnail alloc] initWithImage:self.originalImage filePath:self.originalFilePath]; } - for (TSAttachmentThumbnail *thumbnail in self.thumbnails) { - if (thumbnail.thumbnailDimensionPoints != thumbnailDimensionPoints) { - continue; - } - NSString *_Nullable thumbnailPath = [self pathForThumbnail:thumbnail]; - if (!thumbnailPath) { - OWSFail(@"Missing thumbnail path."); - continue; - } + NSString *thumbnailPath = [self pathForThumbnailDimensionPoints:thumbnailDimensionPoints]; + if ([[NSFileManager defaultManager] fileExistsAtPath:thumbnailPath]) { UIImage *_Nullable image = [UIImage imageWithContentsOfFile:thumbnailPath]; if (!image) { OWSFail(@"couldn't load image."); - continue; + return nil; } return [[OWSLoadedThumbnail alloc] initWithImage:image filePath:thumbnailPath]; } - [OWSThumbnailService.shared ensureThumbnailForAttachmentId:self.uniqueId - thumbnailDimensionPoints:thumbnailDimensionPoints - success:success - failure:failure]; + [OWSThumbnailService.shared ensureThumbnailForAttachment:self + thumbnailDimensionPoints:thumbnailDimensionPoints + success:success + failure:failure]; return nil; } @@ -762,14 +732,20 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); - (NSArray *)allThumbnailPaths { NSMutableArray *result = [NSMutableArray new]; - for (TSAttachmentThumbnail *thumbnail in self.thumbnails) { - NSString *_Nullable thumbnailPath = [self pathForThumbnail:thumbnail]; - if (!thumbnailPath) { - OWSFail(@"Missing thumbnail path."); - continue; + + NSString *thumbnailsDirPath = self.thumbnailsDirPath; + NSError *error; + NSArray *_Nullable fileNames = + [[NSFileManager defaultManager] contentsOfDirectoryAtPath:thumbnailsDirPath error:&error]; + if (error || !fileNames) { + OWSFail(@"contentsOfDirectoryAtPath failed with error: %@", error); + } else { + for (NSString *fileName in fileNames) { + NSString *filePath = [thumbnailsDirPath stringByAppendingPathComponent:fileName]; + [result addObject:filePath]; } - [result addObject:thumbnailPath]; } + NSString *_Nullable legacyThumbnailPath = self.legacyThumbnailPath; if (legacyThumbnailPath && [[NSFileManager defaultManager] fileExistsAtPath:legacyThumbnailPath]) { [result addObject:legacyThumbnailPath]; @@ -833,40 +809,6 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); return thumbnailAttachment; } -- (void)updateWithNewThumbnail:(NSString *)tempFilePath - thumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints - size:(CGSize)size - transaction:(YapDatabaseReadWriteTransaction *)transaction -{ - OWSAssert(tempFilePath.length > 0); - OWSAssert(thumbnailDimensionPoints > 0); - OWSAssert(size.width > 0 && size.height); - OWSAssert(transaction); - - NSString *filename = tempFilePath.lastPathComponent; - NSString *containingDir = self.originalFilePath.stringByDeletingLastPathComponent; - NSString *filePath = [containingDir stringByAppendingPathComponent:filename]; - - NSError *_Nullable error; - BOOL success = [[NSFileManager defaultManager] moveItemAtPath:tempFilePath toPath:filePath error:&error]; - if (error || !success) { - OWSFail(@"Could not move new thumbnail image: %@.", error); - return; - } - TSAttachmentThumbnail *newThumbnail = [[TSAttachmentThumbnail alloc] initWithFilename:filename - size:size - thumbnailDimensionPoints:thumbnailDimensionPoints]; - - [self applyChangeToSelfAndLatestCopy:transaction - changeBlock:^(TSAttachmentStream *attachment) { - NSMutableArray *thumbnails - = (attachment.thumbnails ? [attachment.thumbnails mutableCopy] - : [NSMutableArray new]); - [thumbnails addObject:newThumbnail]; - [attachment setThumbnails:thumbnails]; - }]; -} - // MARK: Protobuf serialization + (nullable SSKProtoAttachmentPointer *)buildProtoForAttachmentId:(nullable NSString *)attachmentId diff --git a/SignalServiceKit/src/Util/UIImage+OWS.h b/SignalServiceKit/src/Util/UIImage+OWS.h index f8e08a8b2..770bb3eb7 100644 --- a/SignalServiceKit/src/Util/UIImage+OWS.h +++ b/SignalServiceKit/src/Util/UIImage+OWS.h @@ -9,6 +9,7 @@ NS_ASSUME_NONNULL_BEGIN - (UIImage *)normalizedImage; - (UIImage *)resizedWithQuality:(CGInterpolationQuality)quality rate:(CGFloat)rate; +- (nullable UIImage *)resizedWithMaxDimensionPoints:(CGFloat)maxDimensionPoints; - (nullable UIImage *)resizedImageToSize:(CGSize)dstSize; - (UIImage *)resizedImageToFillPixelSize:(CGSize)boundingSize; diff --git a/SignalServiceKit/src/Util/UIImage+OWS.m b/SignalServiceKit/src/Util/UIImage+OWS.m index 2f67225fd..c8282e46a 100644 --- a/SignalServiceKit/src/Util/UIImage+OWS.m +++ b/SignalServiceKit/src/Util/UIImage+OWS.m @@ -35,6 +35,35 @@ return resized; } +- (nullable UIImage *)resizedWithMaxDimensionPoints:(CGFloat)maxDimensionPoints +{ + CGSize originalSize = self.size; + if (originalSize.width < 1 || originalSize.height < 1) { + DDLogError(@"Invalid original size: %@", NSStringFromCGSize(originalSize)); + return nil; + } + CGSize thumbnailSize = CGSizeZero; + if (originalSize.width > originalSize.height) { + thumbnailSize.width = maxDimensionPoints; + thumbnailSize.height = round(maxDimensionPoints * originalSize.height / originalSize.width); + } else { + thumbnailSize.width = round(maxDimensionPoints * originalSize.width / originalSize.height); + thumbnailSize.height = maxDimensionPoints; + } + if (thumbnailSize.width < 1 || thumbnailSize.height < 1) { + DDLogError(@"Invalid thumbnail size: %@", NSStringFromCGSize(thumbnailSize)); + return nil; + } + + UIGraphicsBeginImageContext(CGSizeMake(thumbnailSize.width, thumbnailSize.height)); + CGContextRef context = UIGraphicsGetCurrentContext(); + CGContextSetInterpolationQuality(context, kCGInterpolationHigh); + [self drawInRect:CGRectMake(0, 0, thumbnailSize.width, thumbnailSize.height)]; + UIImage *_Nullable resized = UIGraphicsGetImageFromCurrentImageContext(); + UIGraphicsEndImageContext(); + return resized; +} + // Source: https://github.com/AliSoftware/UIImage-Resize - (nullable UIImage *)resizedImageToSize:(CGSize)dstSize From ec83ed182b1a49aa7647a70be40d32ff09e3bd81 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 5 Sep 2018 11:43:02 -0400 Subject: [PATCH 14/21] Clean up. --- .../src/Messages/Attachments/TSAttachmentStream.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index 2cbf1b137..aaa3e7189 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -8,7 +8,6 @@ #import "OWSFileSystem.h" #import "TSAttachmentPointer.h" #import -#import #import #import @@ -706,7 +705,8 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); failure:^{ dispatch_semaphore_signal(semaphore); }]; - // Wait up to five seconds. + + // Wait up to N seconds. dispatch_semaphore_wait(semaphore, dispatch_time(DISPATCH_TIME_NOW, (int64_t)(5 * NSEC_PER_SEC))); @synchronized(self) { return loadedThumbnail; From 3a5d1877da665f522fa5b4a7c82f60aaecabd518 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 5 Sep 2018 11:44:51 -0400 Subject: [PATCH 15/21] Reduce thumbnail sizes. --- .../src/Messages/Attachments/TSAttachmentStream.m | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index aaa3e7189..fbeb56a97 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -13,8 +13,8 @@ NS_ASSUME_NONNULL_BEGIN -const NSUInteger kThumbnailDimensionPointsSmall = 300; -const NSUInteger kThumbnailDimensionPointsMedium = 800; +const NSUInteger kThumbnailDimensionPointsSmall = 200; +const NSUInteger kThumbnailDimensionPointsMedium = 450; typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); From 72a71c185b9d54b0427fe2dc6beaf2caeb96fb12 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 5 Sep 2018 11:54:39 -0400 Subject: [PATCH 16/21] Improve handling of thumbnails dir. --- .../Messages/Attachments/TSAttachmentStream.h | 1 - .../Messages/Attachments/TSAttachmentStream.m | 20 ++++++++++--------- 2 files changed, 11 insertions(+), 10 deletions(-) diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index 017fd0efd..09e6f3f2a 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -107,7 +107,6 @@ typedef void (^OWSThumbnailFailure)(void); // Marks attachment as having completed "lazy backup restore." - (void)updateWithLazyRestoreComplete; -// TODO: Review. - (nullable TSAttachmentStream *)cloneAsThumbnail; #pragma mark - Protobuf diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index fbeb56a97..83009a87a 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -734,15 +734,17 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); NSMutableArray *result = [NSMutableArray new]; NSString *thumbnailsDirPath = self.thumbnailsDirPath; - NSError *error; - NSArray *_Nullable fileNames = - [[NSFileManager defaultManager] contentsOfDirectoryAtPath:thumbnailsDirPath error:&error]; - if (error || !fileNames) { - OWSFail(@"contentsOfDirectoryAtPath failed with error: %@", error); - } else { - for (NSString *fileName in fileNames) { - NSString *filePath = [thumbnailsDirPath stringByAppendingPathComponent:fileName]; - [result addObject:filePath]; + if ([[NSFileManager defaultManager] fileExistsAtPath:thumbnailsDirPath]) { + NSError *error; + NSArray *_Nullable fileNames = + [[NSFileManager defaultManager] contentsOfDirectoryAtPath:thumbnailsDirPath error:&error]; + if (error || !fileNames) { + OWSFail(@"contentsOfDirectoryAtPath failed with error: %@", error); + } else { + for (NSString *fileName in fileNames) { + NSString *filePath = [thumbnailsDirPath stringByAppendingPathComponent:fileName]; + [result addObject:filePath]; + } } } From 9eb2a4f5add73ff07c6697bcbe50248ad44fa394 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 5 Sep 2018 13:14:29 -0400 Subject: [PATCH 17/21] Raise max valid image size. --- SignalServiceKit/src/Util/NSData+Image.m | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/SignalServiceKit/src/Util/NSData+Image.m b/SignalServiceKit/src/Util/NSData+Image.m index 4e00c43bc..75c4b44b5 100644 --- a/SignalServiceKit/src/Util/NSData+Image.m +++ b/SignalServiceKit/src/Util/NSData+Image.m @@ -135,7 +135,7 @@ typedef NS_ENUM(NSInteger, ImageFormat) { CGFloat kWorseCastComponentsPerPixel = 4; CGFloat bytesPerPixel = kWorseCastComponentsPerPixel * depthBytes; - CGFloat kMaxDimension = 2 * 1024; + CGFloat kMaxDimension = 4 * 1024; CGFloat kExpectedBytePerPixel = 4; CGFloat kMaxBytes = kMaxDimension * kMaxDimension * kExpectedBytePerPixel; CGFloat actualBytes = width * height * bytesPerPixel; From 748b243156f5cbf7f0a6f6ee88c2c52f166537bd Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 5 Sep 2018 13:47:37 -0400 Subject: [PATCH 18/21] Restore full-screen thumbnails. --- .../Cells/OWSMessageBubbleView.m | 20 +++++++++++++++++-- .../Messages/Attachments/TSAttachmentStream.h | 1 + .../Messages/Attachments/TSAttachmentStream.m | 18 ++++++++++++++++- 3 files changed, 36 insertions(+), 3 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index b449b77db..62d9e6074 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -837,9 +837,8 @@ NS_ASSUME_NONNULL_BEGIN stillImageView.backgroundColor = [UIColor whiteColor]; [self addAttachmentUploadViewIfNecessary]; - __weak UIImageView *weakImageView = stillImageView; - __weak OWSMessageBubbleView *weakSelf = self; + __weak UIImageView *weakImageView = stillImageView; self.loadCellContentBlock = ^{ OWSMessageBubbleView *strongSelf = weakSelf; if (!strongSelf) { @@ -980,6 +979,7 @@ NS_ASSUME_NONNULL_BEGIN }]; __weak OWSMessageBubbleView *weakSelf = self; + __weak UIImageView *weakImageView = stillImageView; self.loadCellContentBlock = ^{ OWSMessageBubbleView *strongSelf = weakSelf; if (!strongSelf) { @@ -990,6 +990,22 @@ NS_ASSUME_NONNULL_BEGIN return; } stillImageView.image = [strongSelf + tryToLoadCellMedia:^{ + OWSCAssert([strongSelf.attachmentStream isImage]); + return [strongSelf.attachmentStream + thumbnailImageMediumWithSuccess:^(UIImage *image) { + weakImageView.image = image; + } + failure:^{ + DDLogError(@"Could not load thumbnail."); + }]; + } + mediaView:stillImageView + cacheKey:strongSelf.attachmentStream.uniqueId + shouldSkipCache:shouldSkipCache + canLoadAsync:YES]; + + tryToLoadCellMedia:^{ OWSCAssert([strongSelf.attachmentStream isVideo]); diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index 09e6f3f2a..158c42e04 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -89,6 +89,7 @@ typedef void (^OWSThumbnailFailure)(void); failure:(OWSThumbnailFailure)failure; - (nullable UIImage *)thumbnailImageSmallWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; - (nullable UIImage *)thumbnailImageMediumWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; +- (nullable UIImage *)thumbnailImageLargeWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; - (nullable UIImage *)thumbnailImageSmallSync; // This method should only be invoked by OWSThumbnailService. diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index 83009a87a..e9976ae85 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -15,6 +15,13 @@ NS_ASSUME_NONNULL_BEGIN const NSUInteger kThumbnailDimensionPointsSmall = 200; const NSUInteger kThumbnailDimensionPointsMedium = 450; +// This size is large enough to render full screen. +const NSUInteger ThumbnailDimensionPointsLarge() +{ + CGSize screenSizePoints = UIScreen.mainScreen.bounds.size; + const CGFloat kMinZoomFactor = 2.f; + return MAX(screenSizePoints.width * kMinZoomFactor, screenSizePoints.height * kMinZoomFactor); +} typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); @@ -625,8 +632,10 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); NSUInteger thumbnailDimensionPoints; if (maxDimensionHint <= kThumbnailDimensionPointsSmall) { thumbnailDimensionPoints = kThumbnailDimensionPointsSmall; - } else { + } else if (maxDimensionHint <= kThumbnailDimensionPointsMedium) { thumbnailDimensionPoints = kThumbnailDimensionPointsMedium; + } else { + thumbnailDimensionPoints = ThumbnailDimensionPointsLarge(); } return [self thumbnailImageWithThumbnailDimensionPoints:thumbnailDimensionPoints success:success failure:failure]; @@ -646,6 +655,13 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); failure:failure]; } +- (nullable UIImage *)thumbnailImageLargeWithSuccess:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure +{ + return [self thumbnailImageWithThumbnailDimensionPoints:ThumbnailDimensionPointsLarge() + success:success + failure:failure]; +} + - (nullable UIImage *)thumbnailImageWithThumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints success:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure From ad0d09483161c03a9755494e060dd97daebd6e09 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 5 Sep 2018 13:49:16 -0400 Subject: [PATCH 19/21] Fix build breakage. --- .../Cells/OWSMessageBubbleView.m | 20 ++----------------- 1 file changed, 2 insertions(+), 18 deletions(-) diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index 62d9e6074..b449b77db 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -837,8 +837,9 @@ NS_ASSUME_NONNULL_BEGIN stillImageView.backgroundColor = [UIColor whiteColor]; [self addAttachmentUploadViewIfNecessary]; - __weak OWSMessageBubbleView *weakSelf = self; __weak UIImageView *weakImageView = stillImageView; + + __weak OWSMessageBubbleView *weakSelf = self; self.loadCellContentBlock = ^{ OWSMessageBubbleView *strongSelf = weakSelf; if (!strongSelf) { @@ -979,7 +980,6 @@ NS_ASSUME_NONNULL_BEGIN }]; __weak OWSMessageBubbleView *weakSelf = self; - __weak UIImageView *weakImageView = stillImageView; self.loadCellContentBlock = ^{ OWSMessageBubbleView *strongSelf = weakSelf; if (!strongSelf) { @@ -990,22 +990,6 @@ NS_ASSUME_NONNULL_BEGIN return; } stillImageView.image = [strongSelf - tryToLoadCellMedia:^{ - OWSCAssert([strongSelf.attachmentStream isImage]); - return [strongSelf.attachmentStream - thumbnailImageMediumWithSuccess:^(UIImage *image) { - weakImageView.image = image; - } - failure:^{ - DDLogError(@"Could not load thumbnail."); - }]; - } - mediaView:stillImageView - cacheKey:strongSelf.attachmentStream.uniqueId - shouldSkipCache:shouldSkipCache - canLoadAsync:YES]; - - tryToLoadCellMedia:^{ OWSCAssert([strongSelf.attachmentStream isVideo]); From b91751a1143ad1dd49086d5104ec99feac89d898 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 6 Sep 2018 00:01:35 -0400 Subject: [PATCH 20/21] Respond to CR. --- .../src/Contacts/Threads/TSGroupThread.m | 2 +- .../Messages/Attachments/OWSMediaUtils.swift | 16 +------ .../Attachments/OWSThumbnailService.swift | 44 +++++++++---------- .../Messages/Attachments/TSAttachmentStream.h | 6 ++- .../Messages/Attachments/TSAttachmentStream.m | 13 ++++-- SignalServiceKit/src/Util/NSData+Image.m | 8 ++-- SignalServiceKit/src/Util/UIImage+OWS.m | 7 +++ 7 files changed, 50 insertions(+), 46 deletions(-) diff --git a/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m b/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m index c8bff39ad..a06bdb092 100644 --- a/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m +++ b/SignalServiceKit/src/Contacts/Threads/TSGroupThread.m @@ -191,7 +191,7 @@ NSString *const TSGroupThread_NotificationKey_UniqueId = @"TSGroupThread_Notific OWSAssert(attachmentStream); OWSAssert(transaction); - self.groupModel.groupImage = [attachmentStream originalImage]; + self.groupModel.groupImage = [attachmentStream thumbnailImageSmallSync]; [self saveWithTransaction:transaction]; [transaction addCompletionQueue:nil diff --git a/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift b/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift index 64d59148a..4f78788c0 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift +++ b/SignalServiceKit/src/Messages/Attachments/OWSMediaUtils.swift @@ -26,26 +26,14 @@ public enum OWSMediaError: Error { guard let originalImage = UIImage(contentsOfFile: path) else { throw OWSMediaError.failure(description: "Could not load original image.") } - // We use UIGraphicsBeginImageContextWithOptions() to scale. - // Core Image would provide better quality (e.g. Lanczos) but - // at a perf cost we don't want to pay. We could also use - // CoreGraphics directly, but I'm not sure there's any benefit. guard let thumbnailImage = originalImage.resized(withMaxDimensionPoints: maxDimension) else { throw OWSMediaError.failure(description: "Could not thumbnail image.") } return thumbnailImage } - private static let kMaxVideoStillSize: CGFloat = 1024 - - @objc public class func thumbnail(forVideoAtPath path: String) throws -> UIImage { - return try thumbnail(forVideoAtPath: path, maxSize: CGSize(width: kMaxVideoStillSize, height: kMaxVideoStillSize)) - } - - @objc public class func thumbnail(forVideoAtPath path: String, maxSize: CGSize) throws -> UIImage { - var maxSize = maxSize - maxSize.width = min(maxSize.width, kMaxVideoStillSize) - maxSize.height = min(maxSize.height, kMaxVideoStillSize) + @objc public class func thumbnail(forVideoAtPath path: String, maxDimension: CGFloat) throws -> UIImage { + let maxSize = CGSize(width: maxDimension, height: maxDimension) guard FileManager.default.fileExists(atPath: path) else { throw OWSMediaError.failure(description: "Media file missing.") diff --git a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift index b67633025..9a2ad6910 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift +++ b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift @@ -7,36 +7,42 @@ import AVFoundation public enum OWSThumbnailError: Error { case failure(description: String) + case assertionFailure(description: String) + case externalError(description: String, underlyingError:Error) } @objc public class OWSLoadedThumbnail: NSObject { public typealias DataSourceBlock = () throws -> Data - @objc public let image: UIImage + @objc + public let image: UIImage let dataSourceBlock: DataSourceBlock - @objc public init(image: UIImage, filePath: String) { + @objc + public init(image: UIImage, filePath: String) { self.image = image self.dataSourceBlock = { return try Data(contentsOf: URL(fileURLWithPath: filePath)) } } - @objc public init(image: UIImage, data: Data) { + @objc + public init(image: UIImage, data: Data) { self.image = image self.dataSourceBlock = { return data } } - @objc public func data() throws -> Data { + @objc + public func data() throws -> Data { return try dataSourceBlock() } } private struct OWSThumbnailRequest { public typealias SuccessBlock = (OWSLoadedThumbnail) -> Void - public typealias FailureBlock = () -> Void + public typealias FailureBlock = (Error) -> Void let attachment: TSAttachmentStream let thumbnailDimensionPoints: UInt @@ -59,12 +65,10 @@ private struct OWSThumbnailRequest { public static let shared = OWSThumbnailService() public typealias SuccessBlock = (OWSLoadedThumbnail) -> Void - public typealias FailureBlock = () -> Void + public typealias FailureBlock = (Error) -> Void private let serialQueue = DispatchQueue(label: "OWSThumbnailService") - private let dbConnection: YapDatabaseConnection - // This property should only be accessed on the serialQueue. // // We want to process requests in _reverse_ order in which they @@ -72,9 +76,6 @@ private struct OWSThumbnailRequest { private var thumbnailRequestStack = [OWSThumbnailRequest]() private override init() { - - dbConnection = OWSPrimaryStorage.shared().newDatabaseConnection() - super.init() SwiftSingletons.register(self) @@ -86,7 +87,8 @@ private struct OWSThumbnailRequest { // completion will only be called on success. // completion will be called async on the main thread. - @objc public func ensureThumbnail(forAttachment attachment: TSAttachmentStream, + @objc + public func ensureThumbnail(forAttachment attachment: TSAttachmentStream, thumbnailDimensionPoints: UInt, success: @escaping SuccessBlock, failure: @escaping FailureBlock) { @@ -106,10 +108,9 @@ private struct OWSThumbnailRequest { // This should only be called on the serialQueue. private func processNextRequestSync() { - guard !thumbnailRequestStack.isEmpty else { + guard let thumbnailRequest = thumbnailRequestStack.popLast() else { return } - let thumbnailRequest = thumbnailRequestStack.removeLast() do { let loadedThumbnail = try process(thumbnailRequest: thumbnailRequest) @@ -120,7 +121,7 @@ private struct OWSThumbnailRequest { Logger.error("Could not create thumbnail: \(error)") DispatchQueue.main.async { - thumbnailRequest.failure() + thumbnailRequest.failure(error) } } } @@ -146,8 +147,8 @@ private struct OWSThumbnailRequest { return OWSLoadedThumbnail(image: image, filePath: thumbnailPath) } let thumbnailDirPath = (thumbnailPath as NSString).deletingLastPathComponent - if !FileManager.default.fileExists(atPath: thumbnailDirPath) { - try FileManager.default.createDirectory(atPath: thumbnailDirPath, withIntermediateDirectories: true, attributes: nil) + guard OWSFileSystem.ensureDirectoryExists(thumbnailDirPath) else { + throw OWSThumbnailError.failure(description: "Could not create attachment's thumbnail directory.") } guard let originalFilePath = attachment.originalFilePath else { throw OWSThumbnailError.failure(description: "Missing original file path.") @@ -157,18 +158,17 @@ private struct OWSThumbnailRequest { if attachment.isImage || attachment.isAnimated { thumbnailImage = try OWSMediaUtils.thumbnail(forImageAtPath: originalFilePath, maxDimension: maxDimension) } else if attachment.isVideo { - let maxSize = CGSize(width: maxDimension, height: maxDimension) - thumbnailImage = try OWSMediaUtils.thumbnail(forVideoAtPath: originalFilePath, maxSize: maxSize) + thumbnailImage = try OWSMediaUtils.thumbnail(forVideoAtPath: originalFilePath, maxDimension: maxDimension) } else { - throw OWSThumbnailError.failure(description: "Invalid attachment type.") + throw OWSThumbnailError.assertionFailure(description: "Invalid attachment type.") } guard let thumbnailData = UIImageJPEGRepresentation(thumbnailImage, 0.85) else { throw OWSThumbnailError.failure(description: "Could not convert thumbnail to JPEG.") } do { - try thumbnailData.write(to: NSURL.fileURL(withPath: thumbnailPath), options: .atomicWrite) + try thumbnailData.write(to: URL(fileURLWithPath: thumbnailPath), options: .atomicWrite) } catch let error as NSError { - throw OWSThumbnailError.failure(description: "File write failed: \(thumbnailPath), \(error)") + throw OWSThumbnailError.externalError(description: "File write failed: \(thumbnailPath), \(error)", underlyingError: error) } return OWSLoadedThumbnail(image: thumbnailImage, data: thumbnailData) } diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h index 158c42e04..06ad4f80f 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.h @@ -82,8 +82,10 @@ typedef void (^OWSThumbnailFailure)(void); #pragma mark - Thumbnails // On cache hit, the thumbnail will be returned synchronously and completion will never be invoked. -// On cache miss, nil will be returned and the completion will be invoked async on main if -// thumbnail can be generated. +// On cache miss, nil will be returned and success will be invoked if thumbnail can be generated; +// otherwise failure will be invoked. +// +// success and failure are invoked async on main. - (nullable UIImage *)thumbnailImageWithSizeHint:(CGSize)sizeHint success:(OWSThumbnailSuccess)success failure:(OWSThumbnailFailure)failure; diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index e9976ae85..a8b7193e5 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -265,8 +265,10 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); return nil; } + // Thumbnails are written to the caches directory, so that iOS can + // remove them if necessary. NSString *dirName = [NSString stringWithFormat:@"%@-thumbnails", self.uniqueId]; - return [[[self class] attachmentsFolder] stringByAppendingPathComponent:dirName]; + return [OWSFileSystem.cachesDirectoryPath stringByAppendingPathComponent:dirName]; } - (NSString *)pathForThumbnailDimensionPoints:(NSUInteger)thumbnailDimensionPoints @@ -403,7 +405,9 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); - (nullable UIImage *)videoStillImage { NSError *error; - UIImage *_Nullable image = [OWSMediaUtils thumbnailForVideoAtPath:self.originalFilePath error:&error]; + UIImage *_Nullable image = [OWSMediaUtils thumbnailForVideoAtPath:self.originalFilePath + maxDimension:ThumbnailDimensionPointsLarge() + error:&error]; if (error || !image) { DDLogError(@"Could not create video still: %@.", error); return nil; @@ -702,7 +706,10 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); [OWSThumbnailService.shared ensureThumbnailForAttachment:self thumbnailDimensionPoints:thumbnailDimensionPoints success:success - failure:failure]; + failure:^(NSError *error) { + DDLogError(@"Failed to create thumbnail: %@", error); + failure(); + }]; return nil; } diff --git a/SignalServiceKit/src/Util/NSData+Image.m b/SignalServiceKit/src/Util/NSData+Image.m index 75c4b44b5..ddcfe3f58 100644 --- a/SignalServiceKit/src/Util/NSData+Image.m +++ b/SignalServiceKit/src/Util/NSData+Image.m @@ -132,12 +132,12 @@ typedef NS_ENUM(NSInteger, ImageFormat) { } // We only support (A)RGB and (A)Grayscale, so worst case is 4. - CGFloat kWorseCastComponentsPerPixel = 4; + const CGFloat kWorseCastComponentsPerPixel = 4; CGFloat bytesPerPixel = kWorseCastComponentsPerPixel * depthBytes; - CGFloat kMaxDimension = 4 * 1024; - CGFloat kExpectedBytePerPixel = 4; - CGFloat kMaxBytes = kMaxDimension * kMaxDimension * kExpectedBytePerPixel; + const CGFloat kExpectedBytePerPixel = 4; + const CGFloat kMaxValidImageDimension = 4 * 1024; + CGFloat kMaxBytes = kMaxValidImageDimension * kMaxValidImageDimension * kExpectedBytePerPixel; CGFloat actualBytes = width * height * bytesPerPixel; if (actualBytes > kMaxBytes) { DDLogWarn(@"invalid dimensions width: %f, height %f, bytesPerPixel: %f", width, height, bytesPerPixel); diff --git a/SignalServiceKit/src/Util/UIImage+OWS.m b/SignalServiceKit/src/Util/UIImage+OWS.m index c8282e46a..e0ff330ca 100644 --- a/SignalServiceKit/src/Util/UIImage+OWS.m +++ b/SignalServiceKit/src/Util/UIImage+OWS.m @@ -42,6 +42,13 @@ DDLogError(@"Invalid original size: %@", NSStringFromCGSize(originalSize)); return nil; } + + CGFloat maxOriginalDimensionPoints = MAX(originalSize.width, originalSize.height); + if (maxOriginalDimension < maxDimensionPoints) { + // Don't bother scaling an image that is already smaller than the max dimension. + return self; + } + CGSize thumbnailSize = CGSizeZero; if (originalSize.width > originalSize.height) { thumbnailSize.width = maxDimensionPoints; From a088b94c743f2b939979ed9a15ac14a551eb812c Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Thu, 6 Sep 2018 15:37:24 -0400 Subject: [PATCH 21/21] Update Cocoapods, fix build breakage. --- Pods | 2 +- SignalServiceKit/src/Util/UIImage+OWS.m | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/Pods b/Pods index 9a3f6876d..f594e0655 160000 --- a/Pods +++ b/Pods @@ -1 +1 @@ -Subproject commit 9a3f6876d4a6086d10501383b96fb2d9d47a75b6 +Subproject commit f594e0655b038d7a809b2bcc0222a00e64586d9f diff --git a/SignalServiceKit/src/Util/UIImage+OWS.m b/SignalServiceKit/src/Util/UIImage+OWS.m index e0ff330ca..02b7cb001 100644 --- a/SignalServiceKit/src/Util/UIImage+OWS.m +++ b/SignalServiceKit/src/Util/UIImage+OWS.m @@ -44,7 +44,7 @@ } CGFloat maxOriginalDimensionPoints = MAX(originalSize.width, originalSize.height); - if (maxOriginalDimension < maxDimensionPoints) { + if (maxOriginalDimensionPoints < maxDimensionPoints) { // Don't bother scaling an image that is already smaller than the max dimension. return self; }