diff --git a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m index b449b77db..6e0219f23 100644 --- a/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m +++ b/Signal/src/ViewControllers/ConversationView/Cells/OWSMessageBubbleView.m @@ -654,7 +654,6 @@ NS_ASSUME_NONNULL_BEGIN - (nullable id)tryToLoadCellMedia:(nullable id (^)(void))loadCellMediaBlock mediaView:(UIView *)mediaView cacheKey:(NSString *)cacheKey - shouldSkipCache:(BOOL)shouldSkipCache canLoadAsync:(BOOL)canLoadAsync { OWSAssert(self.attachmentStream); @@ -674,9 +673,7 @@ NS_ASSUME_NONNULL_BEGIN cellMedia = loadCellMediaBlock(); if (cellMedia) { DDLogVerbose(@"%@ cell media cache miss", self.logTag); - if (!shouldSkipCache) { - [self.cellMediaCache setObject:cellMedia forKey:cacheKey]; - } + [self.cellMediaCache setObject:cellMedia forKey:cacheKey]; } else if (!canLoadAsync) { DDLogError(@"%@ Failed to load cell media: %@", [self logTag], [self.attachmentStream originalMediaURL]); self.viewItem.didCellMediaFailToLoad = YES; @@ -837,9 +834,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) { @@ -849,16 +845,11 @@ NS_ASSUME_NONNULL_BEGIN if (stillImageView.image) { return; } - // Don't cache large still images. - // - // TODO: Don't use full size images in the message cells. - const NSUInteger kMaxCachableSize = 1024 * 1024; - BOOL shouldSkipCache = - [OWSFileSystem fileSizeOfPath:strongSelf.attachmentStream.originalFilePath].unsignedIntegerValue - < kMaxCachableSize; stillImageView.image = [strongSelf tryToLoadCellMedia:^{ OWSCAssert([strongSelf.attachmentStream isImage]); + OWSCAssert([strongSelf.attachmentStream isValidImage]); + return [strongSelf.attachmentStream thumbnailImageMediumWithSuccess:^(UIImage *image) { weakImageView.image = image; @@ -869,7 +860,6 @@ NS_ASSUME_NONNULL_BEGIN } mediaView:stillImageView cacheKey:strongSelf.attachmentStream.uniqueId - shouldSkipCache:shouldSkipCache canLoadAsync:YES]; }; self.unloadCellContentBlock = ^{ @@ -909,6 +899,7 @@ NS_ASSUME_NONNULL_BEGIN animatedImageView.image = [strongSelf tryToLoadCellMedia:^{ OWSCAssert([strongSelf.attachmentStream isAnimated]); + OWSCAssert([strongSelf.attachmentStream isValidImage]); NSString *_Nullable filePath = [strongSelf.attachmentStream originalFilePath]; YYImage *_Nullable animatedImage = nil; @@ -919,7 +910,6 @@ NS_ASSUME_NONNULL_BEGIN } mediaView:animatedImageView cacheKey:strongSelf.attachmentStream.uniqueId - shouldSkipCache:NO canLoadAsync:NO]; }; self.unloadCellContentBlock = ^{ @@ -980,6 +970,7 @@ NS_ASSUME_NONNULL_BEGIN }]; __weak OWSMessageBubbleView *weakSelf = self; + __weak UIImageView *weakImageView = stillImageView; self.loadCellContentBlock = ^{ OWSMessageBubbleView *strongSelf = weakSelf; if (!strongSelf) { @@ -992,13 +983,19 @@ NS_ASSUME_NONNULL_BEGIN stillImageView.image = [strongSelf tryToLoadCellMedia:^{ OWSCAssert([strongSelf.attachmentStream isVideo]); + OWSCAssert([strongSelf.attachmentStream isValidVideo]); - return strongSelf.attachmentStream.originalImage; + return [strongSelf.attachmentStream + thumbnailImageMediumWithSuccess:^(UIImage *image) { + weakImageView.image = image; + } + failure:^{ + DDLogError(@"Could not load thumbnail."); + }]; } mediaView:stillImageView cacheKey:strongSelf.attachmentStream.uniqueId - shouldSkipCache:NO - canLoadAsync:NO]; + canLoadAsync:YES]; }; self.unloadCellContentBlock = ^{ OWSMessageBubbleView *strongSelf = weakSelf; diff --git a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m index b71da965b..3378f63ba 100644 --- a/Signal/src/ViewControllers/ConversationView/ConversationViewController.m +++ b/Signal/src/ViewControllers/ConversationView/ConversationViewController.m @@ -2220,7 +2220,7 @@ typedef enum : NSUInteger { NSFileManager *fileManager = [NSFileManager defaultManager]; if (![fileManager fileExistsAtPath:attachmentStream.originalFilePath]) { - OWSFail(@"%@ Missing video file: %@", self.logTag, attachmentStream.originalFilePath); + OWSFail(@"%@ Missing audio file: %@", self.logTag, attachmentStream.originalFilePath); } [self dismissKeyBoard]; diff --git a/Signal/src/ViewControllers/MediaDetailViewController.m b/Signal/src/ViewControllers/MediaDetailViewController.m index 6b232cab6..d2a7ca18e 100644 --- a/Signal/src/ViewControllers/MediaDetailViewController.m +++ b/Signal/src/ViewControllers/MediaDetailViewController.m @@ -35,11 +35,9 @@ NS_ASSUME_NONNULL_BEGIN @property (nonatomic) UIView *replacingView; @property (nonatomic) UIButton *shareButton; -@property (nonatomic) NSData *fileData; - @property (nonatomic) TSAttachmentStream *attachmentStream; @property (nonatomic, nullable) ConversationViewItem *viewItem; -@property (nonatomic, readonly) UIImage *image; +@property (nonatomic, nullable) UIImage *image; @property (nonatomic, nullable) OWSVideoPlayer *videoPlayer; @property (nonatomic, nullable) UIButton *playVideoButton; @@ -55,6 +53,8 @@ NS_ASSUME_NONNULL_BEGIN @end +#pragma mark - + @implementation MediaDetailViewController - (void)dealloc @@ -72,8 +72,18 @@ NS_ASSUME_NONNULL_BEGIN _galleryItemBox = galleryItemBox; _viewItem = viewItem; + // We cache the image data in case the attachment stream is deleted. - _image = galleryItemBox.attachmentStream.originalImage; + __weak MediaDetailViewController *weakSelf = self; + _image = [galleryItemBox.attachmentStream + thumbnailImageLargeWithSuccess:^(UIImage *image) { + weakSelf.image = image; + [weakSelf updateContents]; + [weakSelf updateMinZoomScale]; + } + failure:^{ + DDLogWarn(@"Could not load media."); + }]; return self; } @@ -83,22 +93,6 @@ NS_ASSUME_NONNULL_BEGIN return self.galleryItemBox.attachmentStream; } -- (NSURL *_Nullable)attachmentUrl -{ - return self.attachmentStream.originalMediaURL; -} - -- (NSData *)fileData -{ - if (!_fileData) { - NSURL *_Nullable url = self.attachmentUrl; - if (url) { - _fileData = [NSData dataWithContentsOfURL:url]; - } - } - return _fileData; -} - - (BOOL)isAnimated { return self.attachmentStream.isAnimated; @@ -115,7 +109,7 @@ NS_ASSUME_NONNULL_BEGIN self.view.backgroundColor = [UIColor clearColor]; - [self createContents]; + [self updateContents]; } - (void)viewWillAppear:(BOOL)animated @@ -134,6 +128,13 @@ NS_ASSUME_NONNULL_BEGIN - (void)updateMinZoomScale { + if (!self.image) { + self.scrollView.minimumZoomScale = 1.f; + self.scrollView.maximumZoomScale = 1.f; + self.scrollView.zoomScale = 1.f; + return; + } + CGSize viewSize = self.scrollView.bounds.size; UIImage *image = self.image; OWSAssert(image); @@ -163,8 +164,13 @@ NS_ASSUME_NONNULL_BEGIN #pragma mark - Initializers -- (void)createContents +- (void)updateContents { + [self.mediaView removeFromSuperview]; + [self.scrollView removeFromSuperview]; + [self.playVideoButton removeFromSuperview]; + [self.videoProgressBar removeFromSuperview]; + UIScrollView *scrollView = [UIScrollView new]; [self.view addSubview:scrollView]; self.scrollView = scrollView; @@ -184,19 +190,28 @@ NS_ASSUME_NONNULL_BEGIN if (self.isAnimated) { if (self.attachmentStream.isValidImage) { - YYImage *animatedGif = [YYImage imageWithData:self.fileData]; + YYImage *animatedGif = [YYImage imageWithContentsOfFile:self.attachmentStream.originalFilePath]; YYAnimatedImageView *animatedView = [YYAnimatedImageView new]; animatedView.image = animatedGif; self.mediaView = animatedView; } else { - self.mediaView = [UIImageView new]; + self.mediaView = [UIView new]; + self.mediaView.backgroundColor = Theme.offBackgroundColor; } + } else if (!self.image) { + // Still loading thumbnail. + self.mediaView = [UIView new]; + self.mediaView.backgroundColor = Theme.offBackgroundColor; } else if (self.isVideo) { - self.mediaView = [self buildVideoPlayerView]; + if (self.attachmentStream.isValidVideo) { + self.mediaView = [self buildVideoPlayerView]; + } else { + self.mediaView = [UIView new]; + self.mediaView.backgroundColor = Theme.offBackgroundColor; + } } else { // Present the static image using standard UIImageView UIImageView *imageView = [[UIImageView alloc] initWithImage:self.image]; - self.mediaView = imageView; } @@ -260,12 +275,14 @@ NS_ASSUME_NONNULL_BEGIN - (UIView *)buildVideoPlayerView { + NSURL *_Nullable attachmentUrl = self.attachmentStream.originalMediaURL; + NSFileManager *fileManager = [NSFileManager defaultManager]; - if (![fileManager fileExistsAtPath:[self.attachmentUrl path]]) { + if (![fileManager fileExistsAtPath:[attachmentUrl path]]) { OWSFail(@"%@ Missing video file", self.logTag); } - OWSVideoPlayer *player = [[OWSVideoPlayer alloc] initWithUrl:self.attachmentUrl]; + OWSVideoPlayer *player = [[OWSVideoPlayer alloc] initWithUrl:attachmentUrl]; [player seekToTime:kCMTimeZero]; player.delegate = self; self.videoPlayer = player; diff --git a/Signal/src/ViewControllers/MediaGalleryViewController.swift b/Signal/src/ViewControllers/MediaGalleryViewController.swift index cc79ccaa2..c5c278741 100644 --- a/Signal/src/ViewControllers/MediaGalleryViewController.swift +++ b/Signal/src/ViewControllers/MediaGalleryViewController.swift @@ -8,7 +8,7 @@ public enum GalleryDirection { case before, after, around } -public struct MediaGalleryItem: Equatable, Hashable { +public class MediaGalleryItem: Equatable, Hashable { let logTag = "[MediaGalleryItem]" let message: TSMessage @@ -38,15 +38,6 @@ public struct MediaGalleryItem: Equatable, Hashable { return attachmentStream.thumbnailImageSmall(success: async, failure: {}) } - var fullSizedImage: UIImage { - guard let image = attachmentStream.originalImage else { - owsFail("\(logTag) in \(#function) unexpectedly unable to build attachment image") - return UIImage() - } - - return image - } - // MARK: Equatable public static func == (lhs: MediaGalleryItem, rhs: MediaGalleryItem) -> Bool { @@ -304,7 +295,16 @@ class MediaGalleryViewController: OWSNavigationController, MediaGalleryDataSourc // loadView hasn't necessarily been called yet. self.loadViewIfNeeded() - self.presentationView.image = initialDetailItem.fullSizedImage + + self.presentationView.image = initialDetailItem.attachmentStream.thumbnailImageLarge(success: { [weak self] (image) in + guard let strongSelf = self else { + return + } + strongSelf.presentationView.image = image + }, failure: { + Logger.warn("Could not load presentation image.") + }) + self.applyInitialMediaViewConstraints() // Restore presentationView.alpha in case a previous dismiss left us in a bad state. @@ -481,7 +481,14 @@ class MediaGalleryViewController: OWSNavigationController, MediaGalleryDataSourc // it sits on the screen in the conversation view. let changedItems = currentItem != self.initialDetailItem if changedItems { - self.presentationView.image = currentItem.fullSizedImage + self.presentationView.image = currentItem.attachmentStream.thumbnailImageLarge(success: { [weak self] (image) in + guard let strongSelf = self else { + return + } + strongSelf.presentationView.image = image + }, failure: { + Logger.warn("Could not load presentation image.") + }) self.applyOffscreenMediaViewConstraints() } else { self.applyInitialMediaViewConstraints() diff --git a/SignalMessaging/ViewModels/OWSQuotedReplyModel.m b/SignalMessaging/ViewModels/OWSQuotedReplyModel.m index 5a3f1bb52..6cfbcc1e3 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.originalImage; + thumbnailImage = attachmentStream.thumbnailImageSmallSync; } } else if (attachmentInfo.thumbnailAttachmentPointerId) { // download failed, or hasn't completed yet. diff --git a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift index 9a2ad6910..77e7353a3 100644 --- a/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift +++ b/SignalServiceKit/src/Messages/Attachments/OWSThumbnailService.swift @@ -85,8 +85,7 @@ private struct OWSThumbnailRequest { return attachment.isImage || attachment.isAnimated || attachment.isVideo } - // completion will only be called on success. - // completion will be called async on the main thread. + // success and failure will be called async _off_ the main thread. @objc public func ensureThumbnail(forAttachment attachment: TSAttachmentStream, thumbnailDimensionPoints: UInt, @@ -114,13 +113,13 @@ private struct OWSThumbnailRequest { do { let loadedThumbnail = try process(thumbnailRequest: thumbnailRequest) - DispatchQueue.main.async { + DispatchQueue.global().async { thumbnailRequest.success(loadedThumbnail) } } catch { Logger.error("Could not create thumbnail: \(error)") - DispatchQueue.main.async { + DispatchQueue.global().async { thumbnailRequest.failure(error) } } @@ -146,6 +145,9 @@ private struct OWSThumbnailRequest { } return OWSLoadedThumbnail(image: image, filePath: thumbnailPath) } + + Logger.verbose("Creating thumbnail of size: \(thumbnailRequest.thumbnailDimensionPoints)") + let thumbnailDirPath = (thumbnailPath as NSString).deletingLastPathComponent guard OWSFileSystem.ensureDirectoryExists(thumbnailDirPath) else { throw OWSThumbnailError.failure(description: "Could not create attachment's thumbnail directory.") diff --git a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m index a8b7193e5..e6d7f301b 100644 --- a/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m +++ b/SignalServiceKit/src/Messages/Attachments/TSAttachmentStream.m @@ -7,6 +7,7 @@ #import "NSData+Image.h" #import "OWSFileSystem.h" #import "TSAttachmentPointer.h" +#import "Threading.h" #import #import #import @@ -20,7 +21,7 @@ const NSUInteger ThumbnailDimensionPointsLarge() { CGSize screenSizePoints = UIScreen.mainScreen.bounds.size; const CGFloat kMinZoomFactor = 2.f; - return MAX(screenSizePoints.width * kMinZoomFactor, screenSizePoints.height * kMinZoomFactor); + return MAX(screenSizePoints.width, screenSizePoints.height) * kMinZoomFactor; } typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); @@ -41,6 +42,9 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); // Optional property. Only set for attachments which need "lazy backup restore." @property (nonatomic, nullable) NSString *lazyRestoreFragmentId; +@property (atomic, nullable) NSNumber *isValidImageCached; +@property (atomic, nullable) NSNumber *isValidVideoCached; + @end #pragma mark - @@ -347,14 +351,33 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); { OWSAssert(self.isImage || self.isAnimated); - return [NSData ows_isValidImageAtPath:self.originalFilePath mimeType:self.contentType]; + if (self.lazyRestoreFragment) { + return NO; + } + + @synchronized(self) { + if (!self.isValidImageCached) { + self.isValidImageCached = + @([NSData ows_isValidImageAtPath:self.originalFilePath mimeType:self.contentType]); + } + return self.isValidImageCached.boolValue; + } } - (BOOL)isValidVideo { OWSAssert(self.isVideo); - return [OWSMediaUtils isValidVideoWithPath:self.originalFilePath]; + if (self.lazyRestoreFragment) { + return NO; + } + + @synchronized(self) { + if (!self.isValidVideoCached) { + self.isValidVideoCached = @([OWSMediaUtils isValidVideoWithPath:self.originalFilePath]); + } + return self.isValidVideoCached.boolValue; + } } #pragma mark - @@ -672,10 +695,16 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); { OWSLoadedThumbnail *_Nullable loadedThumbnail; loadedThumbnail = [self loadedThumbnailWithThumbnailDimensionPoints:thumbnailDimensionPoints - success:^(OWSLoadedThumbnail *loadedThumbnail) { - success(loadedThumbnail.image); - } - failure:failure]; + success:^(OWSLoadedThumbnail *loadedThumbnail) { + DispatchMainThreadSafe(^{ + success(loadedThumbnail.image); + }); + } + failure:^{ + DispatchMainThreadSafe(^{ + failure(); + }); + }]; return loadedThumbnail.image; } @@ -715,13 +744,14 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); - (nullable OWSLoadedThumbnail *)loadedThumbnailSmallSync { - __block dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); + dispatch_semaphore_t semaphore = dispatch_semaphore_create(0); - __block OWSLoadedThumbnail *_Nullable loadedThumbnail = nil; - loadedThumbnail = [self loadedThumbnailWithThumbnailDimensionPoints:kThumbnailDimensionPointsSmall + __block OWSLoadedThumbnail *_Nullable asyncLoadedThumbnail = nil; + OWSLoadedThumbnail *_Nullable syncLoadedThumbnail = nil; + syncLoadedThumbnail = [self loadedThumbnailWithThumbnailDimensionPoints:kThumbnailDimensionPointsSmall success:^(OWSLoadedThumbnail *asyncLoadedThumbnail) { @synchronized(self) { - loadedThumbnail = asyncLoadedThumbnail; + asyncLoadedThumbnail = asyncLoadedThumbnail; } dispatch_semaphore_signal(semaphore); } @@ -729,22 +759,36 @@ typedef void (^OWSLoadedThumbnailSuccess)(OWSLoadedThumbnail *loadedThumbnail); dispatch_semaphore_signal(semaphore); }]; + if (syncLoadedThumbnail) { + return syncLoadedThumbnail; + } + // Wait up to N seconds. dispatch_semaphore_wait(semaphore, dispatch_time(DISPATCH_TIME_NOW, (int64_t)(5 * NSEC_PER_SEC))); @synchronized(self) { - return loadedThumbnail; + return asyncLoadedThumbnail; } } - (nullable UIImage *)thumbnailImageSmallSync { - return [self loadedThumbnailSmallSync].image; + OWSLoadedThumbnail *_Nullable loadedThumbnail = [self loadedThumbnailSmallSync]; + if (!loadedThumbnail) { + DDLogInfo(@"Couldn't load small thumbnail sync."); + return nil; + } + return loadedThumbnail.image; } - (nullable NSData *)thumbnailDataSmallSync { + OWSLoadedThumbnail *_Nullable loadedThumbnail = [self loadedThumbnailSmallSync]; + if (!loadedThumbnail) { + DDLogInfo(@"Couldn't load small thumbnail sync."); + return nil; + } NSError *error; - NSData *_Nullable data = [[self loadedThumbnailSmallSync] dataAndReturnError:&error]; + NSData *_Nullable data = [loadedThumbnail dataAndReturnError:&error]; if (error || !data) { OWSFail(@"Couldn't load thumbnail data: %@", error); return nil; diff --git a/SignalServiceKit/src/Util/NSData+Image.m b/SignalServiceKit/src/Util/NSData+Image.m index ddcfe3f58..d81068645 100644 --- a/SignalServiceKit/src/Util/NSData+Image.m +++ b/SignalServiceKit/src/Util/NSData+Image.m @@ -136,7 +136,7 @@ typedef NS_ENUM(NSInteger, ImageFormat) { CGFloat bytesPerPixel = kWorseCastComponentsPerPixel * depthBytes; const CGFloat kExpectedBytePerPixel = 4; - const CGFloat kMaxValidImageDimension = 4 * 1024; + const CGFloat kMaxValidImageDimension = 8 * 1024; CGFloat kMaxBytes = kMaxValidImageDimension * kMaxValidImageDimension * kExpectedBytePerPixel; CGFloat actualBytes = width * height * bytesPerPixel; if (actualBytes > kMaxBytes) {