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’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Auxority opened this issue Feb 16, 2024 · 7 comments

Comments

@Auxority
Copy link
Contributor

Auxority commented Feb 16, 2024

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  | 
@Auxority Auxority changed the title Webpsave mux error when attempting to convert large image (40MB). image.ExportWebp - Webpsave mux error when attempting to convert large image (40MB) at high quality. Feb 16, 2024
@tonimelisma
Copy link
Collaborator

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
Copy link
Contributor Author

I'll give it a go on Monday :)

@Auxority
Copy link
Contributor Author

Auxority commented Feb 19, 2024

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
Copy link
Contributor Author

Auxority commented Feb 19, 2024

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
Copy link
Collaborator

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

@Auxority
Copy link
Contributor Author

Auxority commented Feb 22, 2024

Sure, will try to do that today.

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

@tonimelisma
Copy link
Collaborator

Thank you <3

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants