ainame / Swift-WebP

A thin libwebp wrapper in Swift that provides both encode/decode APIs
MIT License
163 stars 39 forks source link

Corrupt output WebP when using CGImage obtained from CIImage (rendered within CIContext) #40

Closed matylla closed 5 years ago

matylla commented 5 years ago

Hi again. I'm working on a simple command-line MacOS application. WebP encoding works just fine when simply re-encoding source CGImage to WebP. Let's discuss the very basic example:

import Foundation
import AppKit
import WebP

// input file path
let inputURL:URL = URL(fileURLWithPath: "/absolute/path/to/input.jpg")

// get CGImageSource
let cgSource = CGImageSourceCreateWithURL(inputURL as CFURL, nil)

// get CGImage from CGImageSource
let inputFile = CGImageSourceCreateImageAtIndex(cgSource!, 0, nil)

// convert CGImage to NSImage
let nsImage = NSImage.init(cgImage: inputFile!, size: NSZeroSize)

// encode NSImage to WebP format
let encoder = WebPEncoder()
let data = try encoder.encode(nsImage, config: .preset(.photo, quality: 95))

// write data to disk
try data.write(to: URL(fileURLWithPath: "/absolute/path/to/output.webp"))

However, when I first render input image in CIContext and then obtain CGimage from that context, the resulting WebP terribly corrupt. Here's a simple example:

import Foundation
import AppKit
import WebP

// input file path
let inputURL:URL = URL(fileURLWithPath: "/absolute/path/to/input.jpg")

// get CGImageSource
let cgSource = CGImageSourceCreateWithURL(inputURL as CFURL, nil)

// get CGImage from CGImageSource
let inputFile = CGImageSourceCreateImageAtIndex(cgSource!, 0, nil)

// convert inputFile (CGImage) to CIImage
let ciImage = CIImage(cgImage: inputFile!)

// render CIImage in CIContext and get CGImage back
let cgImage = context.createCGImage(
    ciImage,
    from: ciImage.extent,
    format: CIFormat.RGBA8,
    colorSpace: CGColorSpace(name: CGColorSpace.extendedSRGB)!
)

// convert CGImage to NSImage
let nsImage = NSImage.init(cgImage: inputFile!, size: NSZeroSize)

// encode NSImage to WebP format
let encoder = WebPEncoder()
let data = try encoder.encode(nsImage, config: .preset(.photo, quality: 95))

// write data to disk
try data.write(to: URL(fileURLWithPath: "/absolute/path/to/output.webp"))

Can you please verify on your end what could be causing output file corruption? Also, it would be extremely useful (at least for me) if the encoder would already accept CGImage without first converting it to NSimage. Thanks in advance!

ainame commented 5 years ago

@matylla Thank you for reporting this. Can you also create a GitHub repo including your code with Xcode project? That would be helpful as I can easily try it.

I haven't had time to look into this deeply yet but this just sounds like the matter of image format(colourspace). I should have mentioned it in docs or somewhere but WebPEncoder.encode(_ image: NSImage, config: WebPEncoderConfig) currently has an assumption that end-user passes an image in "RGB" colourspace (this is the exact RGB and not RGBA). Since you specify format: CIFormat.RGBA8 in the example, it doesn't work out well.

https://github.com/ainame/Swift-WebP/blob/bca6e51c719095b13bf93b8202a879d02cc9973a/Sources/WebP/WebPEncoder%2BPlatform.swift#L16-L24

You still may be able to use WebPEncoder.encode(RGBA: UnsafeMutablePointer<UInt8>, config: WebPEncoderConfig, originWidth: Int, originHeight: Int, stride: Int) directly (In this case, stride would be stride = Int(image.size.width) * MemoryLayout<UInt8>.size * 4 RGBA 4 bytes) but of course, usability isn't that good as the current terrible interface require originalHeight(Width) passed.

Perhaps, I can improve interfaces of this later. Nice catch!

matylla commented 5 years ago

Thanks for your input @ainame. I spent most of the time today trying to debug the issue and I came to the same conclusion. As you mentioned - I believe the extension for MacOS is limited. I ended up using the method from the iOS extension:

let stride = cgImage.bytesPerRow
let dataPtr = CFDataGetMutableBytePtr((cgImage.dataProvider!.data as! CFMutableData))!

let data = try encoder.encode(
    RGBA: dataPtr,
    config: config,
    originWidth: cgImage.width,
    originHeight: cgImage.height,
    stride: stride,
    resizeWidth: 0,
    resizeHeight: 0
)

I think the root cause of the problem with my previous approach is how you calculate the stride or simply speaking bytesPerRow within the MacOS extension. In my opinion a better approach would be to take bytesPerRow directly from the input image (as opposed to assuming just three channels).

Shall we keep this issue open for the time being?

ainame commented 5 years ago

In my opinion a better approach would be to take bytesPerRow directly from the input image (as opposed to assuming just three channels).

This sounds good👍

Shall we keep this issue open for the time being?

Yeah, I can work on this issue when I have time or can review a PR if you submit it🙈

ainame commented 5 years ago

What do you think about this? I'll merge this series of interfaces for improving the support of CGImage.

    public func encode(RGBA cgImage: CGImage, config: WebPEncoderConfig, resizeWidth: Int = 0, resizeHeight: Int = 0) throws -> Data {
        return try encode(RGBA: cgImage.getBaseAddress(), config: config,
                          originWidth: cgImage.width, originHeight: cgImage.height, stride: cgImage.bytesPerRow)
    }

If this makes sense, I'll release a new version later.

https://github.com/ainame/Swift-WebP/blob/0e9bfdc2a0b22262964c9d86b317c42417d946ae/Sources/WebP/WebPEncoder%2BCGImage.swift#L7-L35

matylla commented 5 years ago

That looks good, it makes perfect sense. Thanks for the update!

ainame commented 5 years ago

I feel like I should do a similar approach to the one for NS(UI)Image though 🤔