Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix “cannot decode” error & improve performance #221

Merged
merged 2 commits into from
Jan 22, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
22 changes: 19 additions & 3 deletions Gifski/Gifski.swift
Original file line number Diff line number Diff line change
Expand Up @@ -178,14 +178,30 @@ final class Gifski {

let generator = AVAssetImageGenerator(asset: asset)
generator.appliesPreferredTrackTransform = true
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)

// `.zero` tolerance is much slower and fails a lot on macOS 11. (macOS 11.1)
if #available(macOS 11, *) {
let tolerance = CMTime(seconds: 0.5 / fps, preferredTimescale: .video)
generator.requestedTimeToleranceBefore = tolerance
generator.requestedTimeToleranceAfter = tolerance
} else {
generator.requestedTimeToleranceBefore = .zero
generator.requestedTimeToleranceAfter = .zero
}

let videoRange = conversion.timeRange?.clamped(to: videoTrackRange) ?? videoTrackRange
let startTime = videoRange.lowerBound
let duration = videoRange.length
Expand Down
37 changes: 31 additions & 6 deletions Gifski/Utilities.swift
Original file line number Diff line number Diff line change
Expand Up @@ -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
}
Expand Down Expand Up @@ -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) {
kornelski marked this conversation as resolved.
Show resolved Hide resolved
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
}
}
Expand Down Expand Up @@ -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
Expand Down