danvergara / morphos

Self-hosted file converter server
MIT License
749 stars 33 forks source link

Recommendation: webp support via libvips #44

Open mkalus opened 3 months ago

mkalus commented 3 months ago

I noticed your support for webp and was curious what you would use. You utilize the https://github.com/chai2010/webp library, but I would recommend to switch to https://github.com/h2non/bimg - it is better supported, more popular and faster. It also uses more recent libraries, as far as I have seen. The change would not be a big one.

danvergara commented 3 months ago

If you can make it work, feel free to drop an PR. I'll be more than happy to review it and provide feedback.

mkalus commented 3 months ago

All right, I ran a little benchmark to see, if the conversion is really worth it. bimg is a bit faster on my machine, although only by around 10% - not a huge difference.

goos: linux
goarch: amd64
pkg: github.com/danvergara/morphos/pkg/files/images
cpu: 12th Gen Intel(R) Core(TM) i7-12700H
BenchmarkConvertToWebpUsingBimg
BenchmarkConvertToWebpUsingBimg-20             8     138560112 ns/op
BenchmarkConvertToWebpUsingWebp
BenchmarkConvertToWebpUsingWebp-20             7     154431626 ns/op

The benchmarking code is like this:

func BenchmarkConvertToWebpUsingBimg(b *testing.B) {
    inputImg, err := os.ReadFile("testdata/gopher_pirate.png")
    require.NoError(b, err)

    img, err := png.Decode(bytes.NewReader(inputImg))
    require.NoError(b, err)

    for i := 0; i < b.N; i++ {
        buf := new(bytes.Buffer)

        // to plain bitmap in order to convert to webp.
        if err := bmp.Encode(buf, img); err != nil {
            require.NoError(b, err)
        }
        // encode the image as a WEPB file.
        _, err = bimg.NewImage(buf.Bytes()).Convert(bimg.WEBP)
        require.NoError(b, err)
    }
}

func BenchmarkConvertToWebpUsingWebp(b *testing.B) {
    inputImg, err := os.ReadFile("testdata/gopher_pirate.png")
    require.NoError(b, err)

    img, err := png.Decode(bytes.NewReader(inputImg))
    require.NoError(b, err)

    for i := 0; i < b.N; i++ {
        buf := new(bytes.Buffer)

        err := webp.Encode(buf, img, nil)
        require.NoError(b, err)
    }
}

One might want to test jpg, tiffs, gifs, etc, too. This will give a complete view, but I guess the numbers will not differ too greatly. The drawback in the test was that you pass a golang image to the function and I have to convert this back to some form of buffer in order to make bimg work (which uses buffers only and detects image formats itself). In the benchmark this is not a huge performance hit, percent wise at least, but you will feel it a bit. Since one could use bimg to convert everything using the libvips library, we could pass plain byte buffers to all the functions...

Should I provide a merge request and you can test - or do you think this is over the top? You need libvips as a dependency after all which makes installation a bit more convoluted. On the other hand, you need a cgo to use github.com/chai2010/webp, too - and my UI tells me that the module includes a high vulnerability described in CVE-2023-4863.

danvergara commented 3 months ago

Yeah, please, drop a Pull Request and I'm gonna review it. Don't forget to install libvips in the container image and make sure everything runs smoothly. The container image is the only distribution package we provide at the moment.

mkalus commented 3 weeks ago

Yes sure - I am drowning in work.