From 32bf47fc74a8aa3fd4019575325bd5d6019289a2 Mon Sep 17 00:00:00 2001 From: Matthew Chen Date: Wed, 5 Sep 2018 11:33:45 -0400 Subject: [PATCH] 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