Improve file size estimate (#205)

sindresorhus · Oct 24, 2020 · e6c97bc · e6c97bc
1 parent b61e883
commit e6c97bc
Show file tree

Hide file tree

Showing 4 changed files with 332 additions and 13 deletions.
diff --git a/Gifski/ConversionViewController.swift b/Gifski/ConversionViewController.swift
@@ -80,7 +80,7 @@ final class ConversionViewController: NSViewController {
 				return
 			}
 
-			gifski.run(conversion) { result in
+			gifski.run(conversion, isEstimation: false) { result in
 				do {
 					let gifUrl = try self.generateTemporaryGifUrl(for: conversion.video)
 					try result.get().write(to: gifUrl, options: .atomic)

diff --git a/Gifski/EditVideoViewController.swift b/Gifski/EditVideoViewController.swift
@@ -49,6 +49,17 @@ final class EditVideoViewController: NSViewController {
 		maxWidth: 300
 	)
 
+	private var conversionSettings: Gifski.Conversion {
+		.init(
+			video: inputUrl,
+			timeRange: timeRange,
+			quality: Defaults[.outputQuality],
+			dimensions: resizableDimensions.changed(dimensionsType: .pixels).currentDimensions.value,
+			frameRate: frameRateSlider.integerValue,
+			loopGif: Defaults[.loopGif]
+		)
+	}
+
 	convenience init(
 		inputUrl: URL,
 		asset: AVAsset,
@@ -65,16 +76,7 @@ final class EditVideoViewController: NSViewController {
 
 	@IBAction
 	private func convert(_ sender: Any) {
-		let conversion = Gifski.Conversion(
-			video: inputUrl,
-			timeRange: timeRange,
-			quality: Defaults[.outputQuality],
-			dimensions: resizableDimensions.changed(dimensionsType: .pixels).currentDimensions.value,
-			frameRate: frameRateSlider.integerValue,
-			loopGif: Defaults[.loopGif]
-		)
-
-		let convert = ConversionViewController(conversion: conversion)
+		let convert = ConversionViewController(conversion: conversionSettings)
 		push(viewController: convert)
 	}
 
@@ -356,7 +358,13 @@ final class EditVideoViewController: NSViewController {
 		selectPredefinedSizeBasedOnCurrentDimensions()
 	}
 
-	private func estimateFileSize() {
+	private func setEstimatedFileSize(_ string: NSAttributedString) {
+		estimatedSizeLabel.attributedStringValue = "Estimated File Size: ".attributedString + string
+	}
+
+	private var gifski: Gifski?
+
+	private func getNaiveEstimate() -> String {
 		let duration: Double = {
 			guard let timeRange = timeRange else {
 				return videoMetadata.duration
@@ -369,7 +377,43 @@ final class EditVideoViewController: NSViewController {
 		let dimensions = resizableDimensions.changed(dimensionsType: .pixels).currentDimensions.value
 		var fileSize = (Double(dimensions.width) * Double(dimensions.height) * frameCount) / 3
 		fileSize = fileSize * (qualitySlider.doubleValue + 1.5) / 2.5
-		estimatedSizeLabel.stringValue = "Estimated File Size: " + formatter.string(fromByteCount: Int64(fileSize))
+
+		return formatter.string(fromByteCount: Int64(fileSize))
+	}
+
+	private func _estimateFileSize() {
+		// TODO: Deinit doesn't seem to be called.
+		self.gifski?.cancel()
+
+		let gifski = Gifski()
+		self.gifski = gifski
+
+		setEstimatedFileSize(getNaiveEstimate().attributedString + "   Calculating Accurate Estimate…".attributedString.withColor(.secondaryLabelColor).withFontSize(NSFont.smallSystemFontSize.double))
+
+		gifski.run(conversionSettings, isEstimation: true) { [weak self] result in
+			guard let self = self else {
+				return
+			}
+
+			switch result {
+			case .success(let data):
+				// We add 10% extra because it's better to estimate slightly too much than too little.
+				let fileSize = (Double(data.count) * gifski.sizeMultiplierForEstimation) * 1.1
+
+				self.setEstimatedFileSize(self.formatter.string(fromByteCount: Int64(fileSize)).attributedString)
+			case .failure(let error):
+				switch error {
+				case .cancelled:
+					break
+				default:
+					error.presentAsModalSheet(for: self.view.window)
+				}
+			}
+		}
+	}
+
+	private func estimateFileSize() {
+		Debouncer.debounce(delay: 0.5, action: _estimateFileSize)
 	}
 
 	private func updateDimensionsDisplay() {

diff --git a/Gifski/Gifski.swift b/Gifski/Gifski.swift
@@ -51,6 +51,12 @@ final class Gifski {
 	private var progress: Progress!
 	private var gifski: GifskiWrapper?
 
+	var sizeMultiplierForEstimation = 1.0
+
+	deinit {
+		cancel()
+	}
+
 	// TODO: Split this method up into smaller methods. It's too large.
 	/**
 	Converts a movie to GIF.
@@ -59,6 +65,7 @@ final class Gifski {
 	*/
 	func run(
 		_ conversion: Conversion,
+		isEstimation: Bool,
 		completionHandler: ((Result<Data, Error>) -> Void)?
 	) {
 		// For debugging.
@@ -204,6 +211,20 @@ final class Gifski {
 				)
 			}
 
+			// TODO: The whole estimation thing should be split out into a separate method and the things that are shared should also be split out.
+			if isEstimation {
+				let originalCount = frameForTimes.count
+
+				if originalCount > 25 {
+					frameForTimes = frameForTimes
+						.chunked(by: 5)
+						.sample(length: 5)
+						.flatten()
+				}
+
+				self.sizeMultiplierForEstimation = Double(originalCount) / Double(frameForTimes.count)
+			}
+
 			Crashlytics.record(
 				key: "\(debugKey): fps",
 				value: fps
@@ -276,4 +297,8 @@ final class Gifski {
 			}
 		}
 	}
+
+	func cancel() {
+		progress?.cancel()
+	}
 }