From c6d9e0b3c937db5e0510157ccb528d95ee93b29e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Daniel=20D=E2=80=99Aquino?= Date: Mon, 29 Apr 2024 12:45:40 -0700 Subject: [PATCH] Fix GIF uploads MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit fixes GIF uploads and improves GIF support: - MediaPicker will now skip location data removal processing, as it is not needed on GIF images and causes them to be converted to JPEG images - The uploader now sets more accurate MIME types on the upload request Issue Repro ----------- Device: iPhone 13 Mini iOS: 17.4.1 Damus: `ada99418f6fcdb1354bc5c1c3f3cc3b4db994ce6` Steps: 1. Download a GIF from GIPHY to the iOS photo gallery 2. Upload that and attach into a post in Damus 3. Check if GIF is animated. Results: GIF is not animated. Issue is reproduced. Testing ------- PASS Device: iPhone 13 Mini iOS: 17.4.1 Damus: this commit Steps: 1. Create a new post 2. Upload the same GIF as the repro and post 3. Make sure GIF is animated. PASS 4. Create a new post 5. Upload a new GIF image (that has never been uploaded by the user on the app) and post 6. Make sure the GIF is animated on the post. PASS 7. Make sure that JPEGs can still be successfully uploaded. PASS 8. Make sure that MP4s can be uploaded. 9. Make a new post that contains 1 JPEG, 1 MP4 file, and 2 GIF files. Make sure they are all uploaded correctly and all GIF files are animated. PASS Closes: https://github.com/damus-io/damus/issues/2157 Changelog-Fixed: Fix broken GIF uploads Signed-off-by: Daniel D’Aquino Reviewed-by: William Casarin --- damus/Models/ImageUploadModel.swift | 26 +++++++++++++++++++++++++ damus/Util/Images/ImageProcessing.swift | 6 +++--- damus/Util/Log.swift | 1 + damus/Views/AttachMediaUtility.swift | 2 +- damus/Views/MediaPicker.swift | 24 ++++++++++++++++++++++- 5 files changed, 54 insertions(+), 5 deletions(-) diff --git a/damus/Models/ImageUploadModel.swift b/damus/Models/ImageUploadModel.swift index e6a44d22..a8c9ce72 100644 --- a/damus/Models/ImageUploadModel.swift +++ b/damus/Models/ImageUploadModel.swift @@ -49,6 +49,32 @@ enum MediaUpload { return false } + + var mime_type: String { + switch self.file_extension { + case "jpg", "jpeg": + return "image/jpg" + case "png": + return "image/png" + case "gif": + return "image/gif" + case "tiff", "tif": + return "image/tiff" + case "mp4": + return "video/mp4" + case "ogg": + return "video/ogg" + case "webm": + return "video/webm" + default: + switch self { + case .image: + return "image/jpg" + case .video: + return "video/mp4" + } + } + } } class ImageUploadModel: NSObject, URLSessionTaskDelegate, ObservableObject { diff --git a/damus/Util/Images/ImageProcessing.swift b/damus/Util/Images/ImageProcessing.swift index 40abb699..a7e26228 100644 --- a/damus/Util/Images/ImageProcessing.swift +++ b/damus/Util/Images/ImageProcessing.swift @@ -30,7 +30,7 @@ func processImage(image: UIImage) -> URL? { } fileprivate func processImage(source: CGImageSource, fileExtension: String) -> URL? { - let destinationURL = createMediaURL(fileExtension: fileExtension) + let destinationURL = generateUniqueTemporaryMediaURL(fileExtension: fileExtension) guard let destination = removeGPSDataFromImage(source: source, url: destinationURL) else { return nil } @@ -45,7 +45,7 @@ func processVideo(videoURL: URL) -> URL? { } fileprivate func saveVideoToTemporaryFolder(videoURL: URL) -> URL? { - let destinationURL = createMediaURL(fileExtension: videoURL.pathExtension) + let destinationURL = generateUniqueTemporaryMediaURL(fileExtension: videoURL.pathExtension) do { try FileManager.default.copyItem(at: videoURL, to: destinationURL) @@ -57,7 +57,7 @@ fileprivate func saveVideoToTemporaryFolder(videoURL: URL) -> URL? { } /// Generate a temporary URL with a unique filename -fileprivate func createMediaURL(fileExtension: String) -> URL { +func generateUniqueTemporaryMediaURL(fileExtension: String) -> URL { let temporaryDirectoryURL = URL(fileURLWithPath: NSTemporaryDirectory(), isDirectory: true) let uniqueMediaName = "\(UUID().uuidString).\(fileExtension)" let temporaryMediaURL = temporaryDirectoryURL.appendingPathComponent(uniqueMediaName) diff --git a/damus/Util/Log.swift b/damus/Util/Log.swift index a879037f..d8a3208e 100644 --- a/damus/Util/Log.swift +++ b/damus/Util/Log.swift @@ -15,6 +15,7 @@ enum LogCategory: String { case storage case push_notifications case damus_purple + case image_uploading } /// Damus structured logger diff --git a/damus/Views/AttachMediaUtility.swift b/damus/Views/AttachMediaUtility.swift index ecf28cdd..e4f9c60b 100644 --- a/damus/Views/AttachMediaUtility.swift +++ b/damus/Views/AttachMediaUtility.swift @@ -17,7 +17,7 @@ enum ImageUploadResult { fileprivate func create_upload_body(mediaData: Data, boundary: String, mediaUploader: MediaUploader, mediaToUpload: MediaUpload) -> Data { let body = NSMutableData(); - let contentType = mediaToUpload.is_image ? "image/jpg" : "video/mp4" + let contentType = mediaToUpload.mime_type body.appendString(string: "Content-Type: multipart/form-data; boundary=\(boundary)\r\n\r\n") body.appendString(string: "--\(boundary)\r\n") body.appendString(string: "Content-Disposition: form-data; name=\(mediaUploader.nameParam); filename=\(mediaToUpload.genericFileName)\r\n") diff --git a/damus/Views/MediaPicker.swift b/damus/Views/MediaPicker.swift index 43f6fceb..d03598e4 100644 --- a/damus/Views/MediaPicker.swift +++ b/damus/Views/MediaPicker.swift @@ -36,7 +36,29 @@ struct MediaPicker: UIViewControllerRepresentable { result.itemProvider.loadItem(forTypeIdentifier: UTType.image.identifier, options: nil) { (item, error) in guard let url = item as? URL else { return } - if canGetSourceTypeFromUrl(url: url) { + if(url.pathExtension == "gif") { + // GIFs do not natively support location metadata (See https://superuser.com/a/556320 and https://www.w3.org/Graphics/GIF/spec-gif89a.txt) + // It is better to avoid any GPS data processing at all, as it can cause the image to be converted to JPEG. + // Therefore, we should load the file directtly and deliver it as "already processed". + + // Load the data for the GIF image + // - Don't load it as an UIImage since that can only get exported into JPEG/PNG + // - Don't load it as a file representation because it gets deleted before the upload can occur + _ = result.itemProvider.loadDataRepresentation(for: .gif, completionHandler: { imageData, error in + guard let imageData else { return } + let destinationURL = generateUniqueTemporaryMediaURL(fileExtension: "gif") + do { + try imageData.write(to: destinationURL) + Task { + await self.chooseMedia(.processed_image(destinationURL)) + } + } + catch { + Log.error("Failed to write GIF image data from Photo picker into a local copy", for: .image_uploading) + } + }) + } + else if canGetSourceTypeFromUrl(url: url) { // Media was not taken from camera self.attemptAcquireResourceAndChooseMedia( url: url,