Skip to content

Commit

Permalink
Fix crash on cancel (#166)
Browse files Browse the repository at this point in the history
  • Loading branch information
sindresorhus committed Dec 3, 2019
1 parent fc95a73 commit 21e3f49
Show file tree
Hide file tree
Showing 3 changed files with 17 additions and 16 deletions.
1 change: 1 addition & 0 deletions Gifski.xcodeproj/xcshareddata/xcschemes/Gifski.xcscheme
Expand Up @@ -54,6 +54,7 @@
buildConfiguration = "Debug"
selectedDebuggerIdentifier = "Xcode.DebuggerFoundation.Debugger.LLDB"
selectedLauncherIdentifier = "Xcode.DebuggerFoundation.Launcher.LLDB"
enableAddressSanitizer = "YES"
enableASanStackUseAfterReturn = "YES"
launchStyle = "0"
useCustomWorkingDirectory = "NO"
Expand Down
3 changes: 2 additions & 1 deletion Gifski/ConversionViewController.swift
Expand Up @@ -21,6 +21,7 @@ final class ConversionViewController: NSViewController {
private var conversion: Gifski.Conversion!
private var progress: Progress?
private var isRunning = false
private let gifski = Gifski()

convenience init(conversion: Gifski.Conversion) {
self.init()
Expand Down Expand Up @@ -80,7 +81,7 @@ final class ConversionViewController: NSViewController {
timeRemainingEstimator.start()

progress?.performAsCurrent(withPendingUnitCount: 1) { [weak self] in
Gifski.run(conversion) { result in
gifski.run(conversion) { result in
guard let self = self else {
return
}
Expand Down
29 changes: 14 additions & 15 deletions Gifski/Gifski.swift
Expand Up @@ -61,26 +61,24 @@ final class Gifski {
}
}

// This is carefully nil'd and should remain private.
private static var gifData: NSMutableData?
private var gifData = NSMutableData()
private var progress: Progress!

// TODO: Split this method up into smaller methods. It's too large.
/**
Converts a movie to GIF
- Parameter completionHandler: Guaranteed to be called on the main thread
*/
static func run(
func run(
_ conversion: Conversion,
completionHandler: ((Result<Data, Error>) -> Void)?
) {
var progress = Progress(parent: .current())
progress = Progress(parent: .current())

let completionHandlerOnce = Once().wrap { (_ result: Result<Data, Error>) -> Void in
gifData = nil // Resetting state

DispatchQueue.main.async {
guard !progress.isCancelled else {
guard !self.progress.isCancelled else {
completionHandler?(.failure(.cancelled))
return
}
Expand Down Expand Up @@ -108,8 +106,6 @@ final class Gifski {
return progress.isCancelled ? 0 : 1
}

gifData = NSMutableData()

gifski.setWriteCallback(context: &gifData) { bufferLength, bufferPointer, context in
guard
bufferLength > 0,
Expand Down Expand Up @@ -157,7 +153,7 @@ final class Gifski {
generator.requestedTimeToleranceBefore = .zero
generator.appliesPreferredTrackTransform = true

progress.cancellationHandler = {
self.progress.cancellationHandler = {
generator.cancelAllCGImageGeneration()
}

Expand All @@ -167,15 +163,19 @@ final class Gifski {
let duration = videoRange.length

let frameCount = Int(duration * fps)
progress.totalUnitCount = Int64(frameCount)
self.progress.totalUnitCount = Int64(frameCount)

var frameForTimes = [CMTime]()
for index in 0..<frameCount {
frameForTimes.append(CMTime(seconds: startTime + ((1 / fps) * Double(index)), preferredTimescale: .video))
}

generator.generateCGImagesAsynchronously(forTimePoints: frameForTimes) { result in
guard !progress.isCancelled else {
generator.generateCGImagesAsynchronously(forTimePoints: frameForTimes) { [weak self] result in
guard let self = self else {
return
}

guard !self.progress.isCancelled else {
completionHandlerOnce(.failure(.cancelled))
return
}
Expand Down Expand Up @@ -211,8 +211,7 @@ final class Gifski {
if result.isFinished {
do {
try gifski.finish()
// Force unwrapping here is safe because we nil gifData in one single place after this.
completionHandlerOnce(.success(gifData! as Data))
completionHandlerOnce(.success(self.gifData as Data))
} catch {
completionHandlerOnce(.failure(.writeFailed(error)))
}
Expand Down

0 comments on commit 21e3f49

Please sign in to comment.