Skip to content

Commit

Permalink
Add more user-friendly error messages
Browse files Browse the repository at this point in the history 
- When the user selects a non-video.
- When the video dimensions are less than the minimum.
  • Loading branch information
@sindresorhus
sindresorhus committed May 5, 2019
1 parent df25de3 commit 96c60bb
Show file tree
Hide file tree
Showing 4 changed files with 213 additions and 16 deletions.
8 changes: 4 additions & 4 deletions Gifski/Gifski.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
)

Expand Down
47 changes: 44 additions & 3 deletions Gifski/MainWindowController.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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.",
Expand Down Expand Up @@ -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(
Expand All @@ -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(
Expand All @@ -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]
Expand Down
172 changes: 163 additions & 9 deletions Gifski/util.swift
View file
Original file line number Diff line number Diff line change
Expand Up @@ -22,7 +22,7 @@ func with<T>(_ item: T, update: (inout T) throws -> Void) rethrows -> T {

struct Meta {
static func openSubmitFeedbackPage(message: String? = nil) {
let defaultMessage = "<!-- Provide your feedback here. Include as many details as possible. -->"
let defaultMessage = "<!--\nProvide your feedback here. Include as many details as possible.\nYou can also email me at sindresorhus@gmail.com\n-->"

let body =
"""
Expand Down Expand Up @@ -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]
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -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.
Expand All @@ -676,7 +807,6 @@ extension AVAsset {
}
}


extension AVAsset {
/// Returns debug info for the asset to use in logging and error messages.
var debugInfo: String {
Expand All @@ -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)
Expand All @@ -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)
Expand Down Expand Up @@ -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))
Expand Down
2 changes: 2 additions & 0 deletions gifski-api/Cargo.lock
View file

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 96c60bb

Please sign in to comment.