proposal: image/png: Encoder should allow specifying encoding filter to be used

[shlompy]

Hi.

I'm working on an application which converts text subtitles to PNG images in real time.
I've been trying to squeeze as much performance as I can, so far by upgrading to go 1.20 which has an optimization for RGBA PNG encoding, setting compression to BestSpeed instead of default compression, and also re-using encoder buffer.

So far the above 3 changes improved the PNG encoding greatly, but since my application is running on the cloud with potential to serve +100K clients, I need to reduce the billing costs even more as currently most of the CPU which is consumed by this application is due to PNG encoding.

Doing some CPU profiling, it seems the PNG filter function has a high penalty on the encoding.
My understanding is that the PNG encoder calculates 5 different encoding methods for each line of the image, to choose the best algorithm for each line to provide better compression.

In my case, my images are compressed rather well, even with BestSpeed compression the file size remain the same as with default compression. The reason is simple, text images mostly contains very few pixel differences, as background is usually transparent or black, text is simply white, and there may be a black text border. Few pixels around the text may differ from one to another due to sub pixels obviously.

I've done a little research based on ~150 different subtitles images I'm converting.
I've added a map counter which counts how many time each filter algorithm was chosen (For all the lines in these 150 images).
As I was expected, the majority of the images / lines got the best selected filter (filter 2 - The up filter).

It seems to be a waste that all filters are being calculated to choose the best one, if one can already know that a specific filter is best suitable for the type of images he is working with (From performance perspective, not necessarily compression) - In my case, images with text which are compressed very we.

I've changed the PNG encoding code a bit so it will only use UP filter and will skip the rest of the filters calculations.
While forcing UP filter caused a total of 14% increase in resulting files size to all the 150 images, it yielded 34% improvement in processing time.

I could potentially also try the None filter which should squeeze more performance which will probably end up with even bigger file size, but I haven't attempted it yet.

Obviously I am not suggesting to change the existing way of calculating all the filters algorithms to choose the best one, this is perfectly sensible as a default behavior, but I think it would be very sensible to allow someone to explicitly set which filter to be used by the Encoder, in a similar way we can specific the compression and encoder buffer.

I would be more than happy to contribute (And probably frustrated since that would be my first contribution:) ), but I would like to hear your opinion on it and what would probably be the best code change to achieve this.

From API perspective, I would expect the Encoder struct to include it, such as:

type Encoder struct {
	CompressionLevel CompressionLevel

	// BufferPool optionally specifies a buffer pool to get temporary
	// EncoderBuffers when encoding an image.
	BufferPool EncoderBufferPool

	// Force a usage of specific encoding filter which will yield better encoding speed but with a potential of increasing the compressed image size.
	FilterAlgorithm int
}

I'm having difficults with running benchstat as it outputs nothing, so here are the 2 benchmarks, first one with existing code where all filters are performed to choose the best one, and the second one is using UP filter only:

// Auto filter selection (Existing code)
goos: windows
goarch: amd64
pkg: subtitle-converter/subtitleRenderer
cpu: 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
Benchmark_FullRender_FastPng_DefaultCompression-8            382           3006191 ns/op
Benchmark_FullRender_FastPng_DefaultCompression-8            404           2884266 ns/op
Benchmark_FullRender_FastPng_DefaultCompression-8            394           2938494 ns/op
Benchmark_FullRender_FastPng_DefaultCompression-8            405           2919398 ns/op
Benchmark_FullRender_FastPng_DefaultCompression-8            393           2910806 ns/op
PASS
ok      subtitle-converter/subtitleRenderer     7.059s

// UP filter only
goos: windows
goarch: amd64
pkg: subtitle-converter/subtitleRenderer
cpu: 11th Gen Intel(R) Core(TM) i7-1185G7 @ 3.00GHz
Benchmark_FullRender_FastPng_DefaultCompression-8            598           2036076 ns/op
Benchmark_FullRender_FastPng_DefaultCompression-8            594           1890689 ns/op
Benchmark_FullRender_FastPng_DefaultCompression-8            628           1873155 ns/op
Benchmark_FullRender_FastPng_DefaultCompression-8            592           1933793 ns/op
Benchmark_FullRender_FastPng_DefaultCompression-8            530           2009748 ns/op
PASS
ok      subtitle-converter/subtitleRenderer     7.967s

[ianlancetaylor]

CC @nigeltao

[shlompy]

Should have looked at opened proposals... I've found same request in #51982
But #51982 seems to have been deviated from the original request to export the filter type

[nigeltao]

Most of #51982 (comment) applies here too.

Bear in mind, though, that the standard library is especially constrained by the Go 1.x backwards compatibility promise, both in terms of (1) we can't change any existing API (e.g. adding new function arguments) or behavior if it will break previous users and (2) any new API we invent will be frozen and have to be supported for a long time.

The best solution may not necessarily be "change the stdlib PNG encoder" but "create a new PNG encoder package, under a different import path" instead - one that isn't subject to these constraints. The stdlib PNG package is open source and easily forked.

Along those lines, if you're looking for a PNG encoder more focused on compression speed than compression size, you might find inspiration in https://github.com/richgel999/fpng and https://github.com/veluca93/fpnge

Because of that first quoted paragraph, I'm also still hesitant to add ad hoc png.Encoder fields without having some sense of what the larger API design is for configuring all of the image/* encoders (and maybe the golang.org/x/image/* encoders too).

[shlompy]

Is this image/go package which is part of Go available as a standalone repository I can fork to be used by my application?
I wouldn't want to fork the entire Go repo and compile it...
I can also probably create a local copy of the png package with my modifications, but I wonder what I need to do in terms of license (Should it be suffice to include a copy of go/LICENSE file in my repository?

[nigeltao]

The src/image (or src/image/png) subdirectory of the Go repository isn't officially available as a standalone repository. But you should be able to just copy the entire src/image/png directory to a new repository.

Re licensing, it should suffice to copy Go's LICENSE file.

[shlompy]

Thanks!

…

On Sat, Oct 7, 2023, 04:30 Nigel Tao ***@***.***> wrote: The src/image (or src/image/png) subdirectory of the Go repository isn't officially available as a standalone repository. But you should be able to just copy the entire src/image/png directory to a new repository. Re licensing, it should suffice to copy Go's LICENSE file. — Reply to this email directly, view it on GitHub <#62009 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ABYTFJMZOX4QECGXHXKSOGDX6CWE3AVCNFSM6AAAAAA3PM5KKCVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTONJRGU2DOMRZGA> . You are receiving this because you authored the thread.Message ID: ***@***.***>