From 96c60bb5e76d0e8d80ce124aae76800a7beabcc2 Mon Sep 17 00:00:00 2001 From: Sindre Sorhus Date: Sun, 5 May 2019 14:14:02 +0700 Subject: [PATCH] Add more user-friendly error messages - When the user selects a non-video. - When the video dimensions are less than the minimum. --- Gifski/Gifski.swift | 8 +- Gifski/MainWindowController.swift | 47 +++++++- Gifski/util.swift | 172 ++++++++++++++++++++++++++++-- gifski-api/Cargo.lock | 2 + 4 files changed, 213 insertions(+), 16 deletions(-) diff --git a/Gifski/Gifski.swift b/Gifski/Gifski.swift index 56fd6b60..087fe5eb 100644 --- a/Gifski/Gifski.swift +++ b/Gifski/Gifski.swift @@ -90,19 +90,19 @@ final class Gifski { let asset = AVURLAsset(url: conversion.input, options: nil) Crashlytics.record( - key: "Does input file exist", + key: "Conversion: Does input file exist", value: conversion.input.exists ) Crashlytics.record( - key: "Is input file reachable", + key: "Conversion: Is input file reachable", value: try? conversion.input.checkResourceIsReachable() ) Crashlytics.record( - key: "Is input file readable", + key: "Conversion: Is input file readable", value: conversion.input.isReadable ) Crashlytics.record( - key: "AVAsset debug info 2", + key: "Conversion: AVAsset debug info", value: asset.debugInfo ) diff --git a/Gifski/MainWindowController.swift b/Gifski/MainWindowController.swift index ea0cb1f8..4342afa1 100644 --- a/Gifski/MainWindowController.swift +++ b/Gifski/MainWindowController.swift @@ -103,11 +103,34 @@ final class MainWindowController: NSWindowController { } func convert(_ inputUrl: URL) { + Crashlytics.record( + key: "Does input file exist", + value: inputUrl.exists + ) + Crashlytics.record( + key: "Is input file reachable", + value: try? inputUrl.checkResourceIsReachable() + ) + Crashlytics.record( + key: "Is input file readable", + value: inputUrl.isReadable + ) + + // This is very unlikely to happen. We have a lot of file type filters in place, so the only way this can happen is if the user right-clicks a non-video in Finder, chooses "Open With", then "Other…", chooses "All Applications", and then selects Gifski. Yet, some people are doing this… + guard inputUrl.isVideo else { + NSAlert.showModal( + for: window, + message: "The selected file is not a video.", + informativeText: "Gifski can only convert a video file." + ) + return + } + let asset = AVURLAsset(url: inputUrl) Crashlytics.record(key: "AVAsset debug info", value: asset.debugInfo) - guard asset.videoCodec != "rle" else { + guard asset.videoCodec != .appleAnimation else { NSAlert.showModal( for: window, message: "The QuickTime Animation format is not supported.", @@ -135,7 +158,7 @@ final class MainWindowController: NSWindowController { NSAlert.showModal( for: window, message: "The video file is not supported.", - informativeText: "Please open an issue on https://github.com/sindresorhus/gifski-app. ZIP the video and attach it to the issue.\n\nInclude this info:\n\(asset.debugInfo)" + informativeText: "Please open an issue on https://github.com/sindresorhus/gifski-app or email sindresorhus@gmail.com. ZIP the video and attach it.\n\nInclude this info:\n\(asset.debugInfo)" ) Crashlytics.recordNonFatalError( @@ -149,7 +172,7 @@ final class MainWindowController: NSWindowController { NSAlert.showModal( for: window, message: "The video metadata is not readable.", - informativeText: "Please open an issue on https://github.com/sindresorhus/gifski-app. ZIP the video and attach it to the issue.\n\nInclude this info:\n\(asset.debugInfo)" + informativeText: "Please open an issue on https://github.com/sindresorhus/gifski-app or email sindresorhus@gmail.com. ZIP the video and attach it.\n\nInclude this info:\n\(asset.debugInfo)" ) Crashlytics.recordNonFatalError( @@ -159,6 +182,24 @@ final class MainWindowController: NSWindowController { return } + guard + let dimensions = asset.dimensions, + dimensions.width > 10, + dimensions.height > 10 + else { + NSAlert.showModal( + for: window, + message: "The video dimensions must be at least 10×10.", + informativeText: "The dimensions of your video are \(asset.dimensions?.formatted ?? "0×0").\n\nIf you think this error is a mistake, please open an issue on https://github.com/sindresorhus/gifski-app or email sindresorhus@gmail.com. ZIP the video and attach it.\n\nInclude this info:\n\(asset.debugInfo)" + ) + + Crashlytics.recordNonFatalError( + title: "The video dimensions must be at least 10×10.", + message: asset.debugInfo + ) + return + } + let panel = NSSavePanel() panel.canCreateDirectories = true panel.allowedFileTypes = [FileType.gif.identifier] diff --git a/Gifski/util.swift b/Gifski/util.swift index 9ea1e885..664ad5d2 100644 --- a/Gifski/util.swift +++ b/Gifski/util.swift @@ -22,7 +22,7 @@ func with(_ item: T, update: (inout T) throws -> Void) rethrows -> T { struct Meta { static func openSubmitFeedbackPage(message: String? = nil) { - let defaultMessage = "" + let defaultMessage = "" let body = """ @@ -520,11 +520,19 @@ extension AVAssetTrack { /// Example: /// `avc1` (video) /// `aac` (audio) - var codec: String? { + var codecString: String? { let descriptions = formatDescriptions as! [CMFormatDescription] return descriptions.map { CMFormatDescriptionGetMediaSubType($0).toString() }.first } + var codec: AVFormat? { + guard let codecString = codecString else { + return nil + } + + return AVFormat(fourCC: codecString) + } + /// Returns a debug string with the media format. Example: `vide/avc1` var mediaFormat: String { let descriptions = formatDescriptions as! [CMFormatDescription] @@ -570,6 +578,130 @@ extension FourCharCode { } +// TODO: Support audio formats too. +enum AVFormat: String { + case hevc + case h264 + case appleProResRAWHQ + case appleProResRAW + case appleProRes4444XQ + case appleProRes4444 + case appleProRes422HQ + case appleProRes422 + case appleProRes422LT + case appleProRes422Proxy + case appleAnimation + + init?(fourCC: String) { + switch fourCC.trimmingCharacters(in: .whitespaces) { + case "hvc1": + self = .hevc + case "avc1": + self = .h264 + case "aprh": // From https://avpres.net/Glossar/ProResRAW.html + self = .appleProResRAWHQ + case "aprn": + self = .appleProResRAW + case "ap4x": + self = .appleProRes4444XQ + case "ap4h": + self = .appleProRes4444 + case "apch": + self = .appleProRes422HQ + case "apcn": + self = .appleProRes422 + case "apcs": + self = .appleProRes422LT + case "apco": + self = .appleProRes422Proxy + case "rle": + self = .appleAnimation + default: + return nil + } + } + + init?(fourCC: FourCharCode) { + self.init(fourCC: fourCC.toString()) + } + + var fourCC: String { + switch self { + case .hevc: + return "hvc1" + case .h264: + return "avc1" + case .appleProResRAWHQ: + return "aprh" + case .appleProResRAW: + return "aprn" + case .appleProRes4444XQ: + return "ap4x" + case .appleProRes4444: + return "ap4h" + case .appleProRes422HQ: + return "apcn" + case .appleProRes422: + return "apch" + case .appleProRes422LT: + return "apcs" + case .appleProRes422Proxy: + return "apco" + case .appleAnimation: + return "rle " + } + } + + var isAppleProRes: Bool { + return [ + .appleProResRAWHQ, + .appleProResRAW, + .appleProRes4444XQ, + .appleProRes4444, + .appleProRes422HQ, + .appleProRes422, + .appleProRes422LT, + .appleProRes422Proxy + ].contains(self) + } +} + +extension AVFormat: CustomStringConvertible { + var description: String { + switch self { + case .hevc: + return "HEVC" + case .h264: + return "H264" + case .appleProResRAWHQ: + return "Apple ProRes RAW HQ" + case .appleProResRAW: + return "Apple ProRes RAW" + case .appleProRes4444XQ: + return "Apple ProRes 4444 XQ" + case .appleProRes4444: + return "Apple ProRes 4444" + case .appleProRes422HQ: + return "Apple ProRes 422 HQ" + case .appleProRes422: + return "Apple ProRes 422" + case .appleProRes422LT: + return "Apple ProRes 422 LT" + case .appleProRes422Proxy: + return "Apple ProRes 422 Proxy" + case .appleAnimation: + return "Apple Animation" + } + } +} + +extension AVFormat: CustomDebugStringConvertible { + var debugDescription: String { + return "\(description) (\(fourCC))" + } +} + + extension AVMediaType: CustomDebugStringConvertible { public var debugDescription: String { switch self { @@ -650,15 +782,14 @@ extension AVAsset { } /// Returns the video codec of the first video track if any. - /// Example: `avc1` - var videoCodec: String? { + var videoCodec: AVFormat? { return firstVideoTrack?.codec } /// Returns the audio codec of the first audio track if any. /// Example: `aac` var audioCodec: String? { - return firstAudioTrack?.codec + return firstAudioTrack?.codecString } /// The file size of the asset in bytes. @@ -676,7 +807,6 @@ extension AVAsset { } } - extension AVAsset { /// Returns debug info for the asset to use in logging and error messages. var debugInfo: String { @@ -690,7 +820,7 @@ extension AVAsset { ## AVAsset debug info ## Extension: \(describing: (self as? AVURLAsset)?.url.fileExtension) - Video codec: \(describing: videoCodec) + Video codec: \(describing: videoCodec?.debugDescription) Audio codec: \(describing: audioCodec) Duration: \(describing: durationFormatter.string(from: duration.seconds)) Dimension: \(describing: dimensions?.formatted) @@ -708,10 +838,11 @@ extension AVAsset { """ Track #\(track.trackID) ---- - Type: \(String(reflecting: track.mediaType)) - Codec: \(describing: track.codec) + Type: \(track.mediaType.debugDescription) + Codec: \(describing: track.mediaType == .video ? track.codec?.debugDescription : track.codecString) Duration: \(describing: durationFormatter.string(from: track.timeRange.duration.seconds)) Dimensions: \(describing: track.dimensions?.formatted) + Natural size: \(describing: track.naturalSize) Frame rate: \(describing: track.frameRate?.rounded(toDecimalPlaces: 2).formatted) Is playable: \(track.isPlayable) Is decodable: \(track.isDecodable) @@ -1316,6 +1447,29 @@ extension URL { } } +extension URL { + /** + Check if the file conforms to the given type identifier + + ``` + URL(fileURLWithPath: "video.mp4").conformsTo(typeIdentifier: "public.movie") + //=> true + ``` + */ + func conformsTo(typeIdentifier parentTypeIdentifier: String) -> Bool { + guard let typeIdentifier = typeIdentifier else { + return false + } + + return UTTypeConformsTo(typeIdentifier as CFString, parentTypeIdentifier as CFString) + } + + /// - Important: This doesn't guarantee it's a video. A video container could contain only an audio track. Use the `AVAsset` properties to ensure it's something you can use. + var isVideo: Bool { + return conformsTo(typeIdentifier: kUTTypeMovie as String) + } +} + extension CGSize { static func * (lhs: CGSize, rhs: Double) -> CGSize { return CGSize(width: lhs.width * CGFloat(rhs), height: lhs.height * CGFloat(rhs)) diff --git a/gifski-api/Cargo.lock b/gifski-api/Cargo.lock index 96017312..6eb6a656 100644 --- a/gifski-api/Cargo.lock +++ b/gifski-api/Cargo.lock @@ -1,3 +1,5 @@ +# This file is automatically @generated by Cargo. +# It is not intended for manual editing. [[package]] name = "aho-corasick" version = "0.6.4"