Fix “cannot decode” error & improve performance

Fixes #202 Related #220
sindresorhus · Jan 22, 2021 · 25533d9 · 25533d9
1 parent 34ed7a0
commit 25533d9
Show file tree

Hide file tree

Showing 2 changed files with 49 additions and 9 deletions.
diff --git a/Gifski/Gifski.swift b/Gifski/Gifski.swift
@@ -178,14 +178,29 @@ final class Gifski {
 
 			let generator = AVAssetImageGenerator(asset: asset)
 			generator.appliesPreferredTrackTransform = true
-			generator.requestedTimeToleranceBefore = .zero
-			generator.requestedTimeToleranceAfter = .zero
+
+			// `.zero` tolerance is much slower and fails a lot on macOS 11. (macOS 11.1)
+			if #available(macOS 11, *) {
+				generator.requestedTimeToleranceBefore = CMTime(seconds: 0.1, preferredTimescale: .video)
+				generator.requestedTimeToleranceAfter = CMTime(seconds: 0.1, preferredTimescale: .video)
+			} else {
+				generator.requestedTimeToleranceBefore = .zero
+				generator.requestedTimeToleranceAfter = .zero
+			}
+
+			// This improves the performance a little bit.
+			if let dimensions = conversion.dimensions {
+				generator.maximumSize = CGSize(widthHeight: dimensions.longestSide)
+			}
 
 			self.progress.cancellationHandler = {
 				generator.cancelAllCGImageGeneration()
 			}
 
-			let fps = (conversion.frameRate.map { Double($0) } ?? assetFrameRate).clamped(to: Constants.allowedFrameRate)
+			// Even though we enforce a minimum of 5 FPS in the GUI, a source video could have lower FPS, and we should allow that.
+			var fps = (conversion.frameRate.map { Double($0) } ?? assetFrameRate).clamped(to: 0.1...Constants.allowedFrameRate.upperBound)
+			fps = min(fps, assetFrameRate)
+
 			let videoRange = conversion.timeRange?.clamped(to: videoTrackRange) ?? videoTrackRange
 			let startTime = videoRange.lowerBound
 			let duration = videoRange.length

diff --git a/Gifski/Utilities.swift b/Gifski/Utilities.swift
@@ -342,8 +342,12 @@ extension AVAssetImageGenerator {
 		var completedCount = 0
 		var decodeFailureFrameCount = 0
 
-		generateCGImagesAsynchronously(forTimes: times) { requestedTime, image, actualTime, result, error in
-			if (Double(decodeFailureFrameCount) / Double(totalCount)) > 0.5 {
+		generateCGImagesAsynchronously(forTimes: times) { [weak self] requestedTime, image, actualTime, result, error in
+			guard let self = self else {
+				return
+			}
+
+			if (Double(decodeFailureFrameCount) / Double(totalCount)) > 0.8 {
 				completionHandler(.failure(NSError.appError("\(decodeFailureFrameCount) of \(totalCount) frames failed to decode. This is a bug in macOS. We are looking into workarounds.")))
 				return
 			}
@@ -374,11 +378,30 @@ extension AVAssetImageGenerator {
 						break
 					}
 
-					// macOS 11 started throwing “decode failed” error for some frames in screen recordings. As a workaround, we ignore these. We throw an error if more than 50% of the frames could not be decoded.
+					// macOS 11 (still an issue in macOS 11.1) started throwing “decode failed” error for some frames in screen recordings. As a workaround, we ignore these. We throw an error if more than 50% of the frames could not be decoded.
 					if error.code == .decodeFailed {
-						decodeFailureFrameCount += 1
-						totalCount -= 1
-						Crashlytics.recordNonFatalError(error: error, userInfo: ["requestedTime": requestedTime.seconds])
+						var newActualTime = CMTime.zero
+						if let image = try? self.copyCGImage(at: requestedTime, actualTime: &newActualTime) {
+							completedCount += 1
+
+							completionHandler(
+								.success(
+									CompletionHandlerResult(
+										image: image,
+										requestedTime: requestedTime,
+										actualTime: newActualTime,
+										completedCount: completedCount,
+										totalCount: totalCount,
+										isFinished: completedCount == totalCount
+									)
+								)
+							)
+						} else {
+							decodeFailureFrameCount += 1
+							totalCount -= 1
+							Crashlytics.recordNonFatalError(error: error, userInfo: ["requestedTime": requestedTime.seconds])
+						}
+
 						break
 					}
 				}
@@ -1938,6 +1961,8 @@ extension CGSize {
 
 	var cgRect: CGRect { .init(origin: .zero, size: self) }
 
+	var longestSide: CGFloat { max(width, height) }
+
 	func aspectFit(to boundingSize: CGSize) -> Self {
 		let ratio = min(boundingSize.width / width, boundingSize.height / height)
 		return self * ratio