davidbyttow / govips

A lightning fast image processing and resizing library for Go
MIT License
1.31k stars 203 forks source link

image.ExportWebp - Webpsave mux error when attempting to convert large image (40MB) at high quality. #404

Closed Auxority closed 1 month ago

Auxority commented 9 months ago

Hey there, I ran into an issue while attempting to convert an image from JPG to WebP. It works fine when converting it from JPG to AVIF, but for some reason the JPG to WebP conversion doesn't work. At least, not at 100% quality. At lower quality values (below 89), the conversion works fine.

Code:

package main

import (
    "fmt"
    "os"

    "github.com/davidbyttow/govips/v2/vips"
)

func main() {
    config := vips.Config{
        ConcurrencyLevel: 1,
        MaxCacheFiles:    0,
        MaxCacheMem:      52428800,
        MaxCacheSize:     100,
    }

    vips.LoggingSettings(nil, vips.LogLevelWarning)
    vips.Startup(&config)
    defer vips.Shutdown()

    image, err := vips.NewImageFromFile("01.png")
    if err != nil {
        fmt.Println(err)
        return
    }
    defer image.Close()

    new_quality := 100

    params2 := vips.WebpExportParams{
        Quality: new_quality,
    }

    buffer, _, err := image.ExportWebp(&params2)
    if err != nil {
        fmt.Println(err)
        return
    }

    err = os.WriteFile("01.webp", buffer, 0644)
    if err != nil {
        fmt.Println(err)
        return
    }
}

Output:

app-1  | webpsave: unable to encode
app-1  | webpsave: mux error
app-1  | 
app-1  | Stack:
app-1  | goroutine 1 [running]:
app-1  | runtime/debug.Stack()
app-1  |        /usr/local/go/src/runtime/debug/stack.go:24 +0x5e
app-1  | github.com/davidbyttow/govips/v2/vips.handleVipsError()
app-1  |        /go/pkg/mod/github.com/davidbyttow/govips/v2@v2.13.0/vips/error.go:38 +0x4f
app-1  | github.com/davidbyttow/govips/v2/vips.handleSaveBufferError(0xc00018c000?)
app-1  |        /go/pkg/mod/github.com/davidbyttow/govips/v2@v2.13.0/vips/error.go:31 +0x1e
app-1  | github.com/davidbyttow/govips/v2/vips.vipsSaveToBuffer({0x7fffb46a4c90, 0x0, 0x2, 0x0, 0x0, 0x64, 0x0, 0x0, 0x1, 0x0, ...})
app-1  |        /go/pkg/mod/github.com/davidbyttow/govips/v2@v2.13.0/vips/foreign.go:465 +0xaa
app-1  | github.com/davidbyttow/govips/v2/vips.vipsSaveWebPToBuffer(0x7fffb46a4c90, {0x0, 0x64, 0x0, 0x0, 0x0, {0x0, 0x0}})
app-1  |        /go/pkg/mod/github.com/davidbyttow/govips/v2@v2.13.0/vips/foreign.go:386 +0x218
app-1  | github.com/davidbyttow/govips/v2/vips.(*ImageRef).ExportWebp(0xc000180050, 0x6?)
app-1  |        /go/pkg/mod/github.com/davidbyttow/govips/v2@v2.13.0/vips/image.go:924 +0xb6
app-1  | main.main()
app-1  |        /app/main.go:35 +0x1cb
app-1  | 
tonimelisma commented 9 months ago

Hey @Auxority looks like a bug. Would you be able to try debugging it and finding what the issue is? If you'll submit a PR with test cases I'd be more than happy to review and approve.

Auxority commented 9 months ago

I'll give it a go on Monday :)

Auxority commented 9 months ago

Sadly a simple make or make test on the latest tag or master branch already fails. And there are no clear instructions on how to build the project or how to use it.

I did look through the code and found that if you replace the Quality parameter with the Lossless: true parameter, that it works fine. Although this still means that the Quality parameter won't work for values between 86 and 100. It probably has something to do with this function:

func vipsSaveToBuffer(params C.struct_SaveParams) ([]byte, error) {
    if err := C.save_to_buffer(&params); err != 0 {
        return nil, handleSaveBufferError(params.outputBuffer)
    }

    buf := C.GoBytes(params.outputBuffer, C.int(params.outputLen))
    defer gFreePointer(params.outputBuffer)

    return buf, nil
}

Which is kinda strange, since that same method is used for AVIF, and not only for WebP images. Just to be sure I also ran the vips command vips copy 01.png 01.webp[Q=100] in my Docker container, and that worked fine.

I'd gladly contribute if the instructions were a bit clearer, but for now I'll leave it as is. I hope you guys are able to find a fix.

Edit: This is the stack trace with the relevant methods:

/my-code.go
params := vips.WebpExportParams{
    Quality: 100,
}
buffer, _, err := image.ExportWebp(&params)

/go/pkg/mod/github.com/davidbyttow/govips/v2@v2.13.0/vips/image.go:924
buf, err := vipsSaveWebPToBuffer(r.image, paramsWithIccProfile)

/go/pkg/mod/github.com/davidbyttow/govips/v2@v2.13.0/vips/foreign.go:386
return vipsSaveToBuffer(p)

/go/pkg/mod/github.com/davidbyttow/govips/v2@v2.13.0/vips/foreign.go:465
if err := C.save_to_buffer(&params); err != 0 {
        return nil, handleSaveBufferError(params.outputBuffer)
}

Means that it goes wrong here: C.save_to_buffer(&params); So it seems that there is an issue with the C bindings.

Auxority commented 9 months ago

Oh I think I found the issue! @tonimelisma

if (!ret && params->quality) {
    vips_object_set(VIPS_OBJECT(operation), "Q", params->quality, NULL);
}

The ret variable is NOT set here in foreign.c:299.

int set_webpsave_options(VipsOperation *operation, SaveParams *params) {
  int ret =
      vips_object_set(VIPS_OBJECT(operation),
                      "strip", params->stripMetadata,
                      "lossless", params->webpLossless,
                      "near_lossless", params->webpNearLossless,
                      "reduction_effort", params->webpReductionEffort,
                      "profile", params->webpIccProfile ? params->webpIccProfile : "none",
                      "min_size", params->webpMinSize,
                      "kmin", params->webpKMin,
                      "kmax", params->webpKMax,
                      NULL);

  if (!ret && params->quality) {
    vips_object_set(VIPS_OBJECT(operation), "Q", params->quality, NULL);
  }

  return ret;
}

While it is set in many other methods:

if (!ret && params->quality) {
    ret = vips_object_set(VIPS_OBJECT(operation), "Q", params->quality, NULL);
}

Although this still seems strange though, because it does work at lower quality values.

tonimelisma commented 9 months ago

Thank you so much @Auxority - great find. Do you think you could send a PR?

Auxority commented 9 months ago

Sure, will try to do that today.

edit: day was filled at work, trying again tomorrow.

tonimelisma commented 9 months ago

Thank you <3