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鈥檒l occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add Gifski metadata to the generated GIF #93

Merged
merged 10 commits into from Jun 23, 2019

Conversation

@sunshinejr
Copy link
Collaborator

commented Jun 23, 2019

Fixes #61.

So this is not exactly using the IPTC properties mentioned in the issue but one of the common metadata properties from Apple's docs.

The reason is that GIF doesn't seem to support IPTC properties in the GIF itself, and when I tried to set these as extended attributes they weren't properly recognized as "real" IPTC properties (used exiftool/mdls for the check). But I found a nice extended attribute that macOS is also displaying on the sidebar in Finder (don't mind the polish language 馃槃):

Zrzut ekranu 2019-06-23 o 02 09 28

Let me know @sindresorhus @kornelski what do you think about it!


Edit: Also, there is an API for MDItem that could fetch attributes but for some reason there is no exposed function to set attributes. But, in fact, there is a runtime function for saving these properties and if we expose it in our objc header in Gifski with:

@import CoreServices;

Boolean MDItemSetAttribute(MDItemRef, CFStringRef name, CFTypeRef attr);

we have a "nice-looking" API to set these properties instead of using setxattr:

if let item = MDItemCreate(nil, url.path as CFString) {
	MDItemSetAttribute(item, kMDItemCreator, "Gifski 1.8.0 (17)" as CFTypeRef)
}

It also seems to be there forever but not sure how safe this is 馃槃


IssueHunt Summary

Referenced issues

This pull request has been submitted to:


IssueHunt has been backed by the following sponsors. Become a sponsor

@sunshinejr sunshinejr requested review from kornelski and sindresorhus Jun 23, 2019

@sindresorhus

This comment has been minimized.

Copy link
Owner

commented Jun 23, 2019

We cannot use private APIs as Gifski is on the Mac App Store.


Some more info about the method: https://github.com/adobe/chromium/blob/cfe5bf0b51b1f6b9fe239c2a3c2f2364da9967d7/content/browser/file_metadata_mac.mm#L40-L51


You should submit a feedback on Feedback Assistant to expose MDItemSetAttribute. It's weird that it's not exposed.

@sindresorhus

This comment has been minimized.

Copy link
Owner

commented Jun 23, 2019

I actually have a high-level xattr class in my private framework. We could use that:

extension POSIXError {
	/// Create an error from the global C `errno`.
	static var fromErrno: POSIXError {
		return self.init(errno: errno)
	}

	/**
	Create an error from the given C `errno`.

	```
	let length = getxattr(fileSystemPath, name, nil, 0, 0, 0)

	guard length >= 0 else {
		throw POSIXError(errno: errno)
	}
	```
	*/
	init(errno errorCode: Int32) {
		self.init(POSIXErrorCode(rawValue: errorCode) ?? .EPERM)
	}
}



final class ExtendedAttributes {
	let url: URL

	init(url: URL) {
		self.url = url
	}

	private func checkIfFileURL() throws {
		guard url.isFileURL else {
			throw CocoaError(.fileNoSuchFile)
		}
	}

	func has(_ name: String) -> Bool {
		guard url.isFileURL else {
			return false
		}

		return url.withUnsafeFileSystemRepresentation { fileSystemPath in
			getxattr(fileSystemPath, name, nil, 0, 0, 0) > 0
		}
	}

	func get(_ name: String) throws -> Data {
		try checkIfFileURL()

		return try url.withUnsafeFileSystemRepresentation { fileSystemPath in
			let length = getxattr(fileSystemPath, name, nil, 0, 0, 0)

			guard length >= 0 else {
				throw POSIXError.fromErrno
			}

			var data = Data(count: length)

			let result = data.withUnsafeMutableBytes {
				getxattr(fileSystemPath, name, $0.baseAddress, length, 0, 0)
			}

			guard result >= 0 else {
				throw POSIXError.fromErrno
			}

			return data
		}
	}

	/**
	- Note: Ensure you specify a type.

	```
	let isProtected: Bool = try? attributes.get("com.apple.rootless") ?? false
	```
	*/
	func get<T>(_ name: String) throws -> T {
		try checkIfFileURL()

		let data = try get(name)

		let value = try PropertyListSerialization.propertyList(from: data, options: [], format: nil)

		guard let result = value as? T else {
			throw CocoaError(.propertyListReadCorrupt)
		}

		return result
	}

	func set(_ name: String, data: Data) throws {
		try checkIfFileURL()

		try url.withUnsafeFileSystemRepresentation { fileSystemPath in
			let result = data.withUnsafeBytes {
				setxattr(fileSystemPath, name, $0.baseAddress, data.count, 0, 0)
			}

			guard result >= 0 else {
				throw POSIXError.fromErrno
			}
		}
	}

	func set<T>(_ name: String, value: T) throws {
		try checkIfFileURL()

		guard PropertyListSerialization.propertyList(value, isValidFor: .binary) else {
			throw CocoaError(.propertyListWriteInvalid)
		}

		let data = try PropertyListSerialization.data(fromPropertyList: value, format: .binary, options: 0)
		try set(name, data: data)
	}

	func remove(_ name: String) throws {
		try checkIfFileURL()

		try url.withUnsafeFileSystemRepresentation { fileSystemPath in
			guard removexattr(fileSystemPath, name, 0) >= 0 else {
				throw POSIXError.fromErrno
			}
		}
	}

	/// Get all the extended attribute names
	func all() throws -> [String] {
		try checkIfFileURL()

		let list: [String] = try url.withUnsafeFileSystemRepresentation { fileSystemPath in
			let length = listxattr(fileSystemPath, nil, 0, 0)

			guard length >= 0 else {
				throw POSIXError.fromErrno
			}

			var data = Data(count: length)

			let result = data.withUnsafeMutableBytes {
				listxattr(fileSystemPath, UnsafeMutablePointer<Int8>(OpaquePointer($0.baseAddress)), length, 0)
			}

			guard result >= 0 else {
				throw POSIXError.fromErrno
			}

			let list = data.split(separator: 0).compactMap {
				String(data: Data($0), encoding: .utf8)
			}

			return list
		}

		return list
	}

	func debug() {
		print("Extended attributes:\n\(try! all().joined(separator: .newline))")
	}
}


extension URL {
	var attributes: ExtendedAttributes {
		return ExtendedAttributes(url: self)
	}
}
@sunshinejr

This comment has been minimized.

Copy link
Collaborator Author

commented Jun 23, 2019

@sindresorhus yeah I submitted feedback, let's see if that helps. Also updated the PR with your ExtendedAttributes class (it's awesome btw) + an extension on URL. Currently implemented what we actually use but we can iterate on it further of course.

Gifski/util.swift Outdated Show resolved Hide resolved

sindresorhus and others added some commits Jun 23, 2019

sunshinejr and others added some commits Jun 23, 2019

sunshinejr and others added some commits Jun 23, 2019

Remove Gifski build number from Gif metadata
Co-Authored-By: Sindre Sorhus <sindresorhus@gmail.com>

@sindresorhus sindresorhus merged commit 90f8097 into master Jun 23, 2019

@sindresorhus sindresorhus deleted the feature/gifski_metadata branch Jun 23, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can鈥檛 perform that action at this time.