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

x/image/webp: WebP decode contrast issue #40173

Open
AliFlux opened this issue Jul 12, 2020 · 15 comments
Open

x/image/webp: WebP decode contrast issue #40173

AliFlux opened this issue Jul 12, 2020 · 15 comments
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Milestone

Comments

@AliFlux
Copy link

AliFlux commented Jul 12, 2020

What version of Go are you using (go version)?

go version go1.13.7 windows/amd64

Does this issue reproduce with the latest release?

Yes.

What operating system and processor architecture are you using (go env)?

go env Output
$ go env
set GO111MODULE=off
set GOARCH=amd64
set GOBIN=
set GOCACHE=C:\Users\Alpha\AppData\Local\go-build
set GOENV=C:\Users\Alpha\AppData\Roaming\go\env
set GOEXE=.exe
set GOFLAGS=
set GOHOSTARCH=amd64
set GOHOSTOS=windows
set GONOPROXY=
set GONOSUMDB=
set GOOS=windows
set GOPATH=C:\Users\Alpha\go
set GOPRIVATE=
set GOPROXY=https://proxy.golang.org,direct
set GOROOT=c:\go
set GOSUMDB=sum.golang.org
set GOTMPDIR=
set GOTOOLDIR=c:\go\pkg\tool\windows_amd64
set GCCGO=gccgo
set AR=ar
set CC=gcc
set CXX=g++
set CGO_ENABLED=1
set GOMOD=
set CGO_CFLAGS=-g -O2
set CGO_CPPFLAGS=
set CGO_CXXFLAGS=-g -O2
set CGO_FFLAGS=-g -O2
set CGO_LDFLAGS=-g -O2
set PKG_CONFIG=pkg-config
set GOGCCFLAGS=-m64 -mthreads -fmessage-length=0 -fdebug-prefix-map=C:\Users\Alpha\AppData\Local\Temp\go-build034800510=/tmp/go-build -gno-record-gcc-switches

What did you do?

I opened up a webp using go, and saved it back to png, The output was different from the source. Other golang webp decoding packages parsed it correctly.

package main

import (
	"image"
	"image/png"
	"os"

	// you can swap between these two libs, chai2010 is okay
	"golang.org/x/image/webp"
	//"github.com/chai2010/webp"
)

func main() {

	tileStream, _ := os.Open("full.webp")
	tile, _ := webp.Decode(tileStream)
	defer tileStream.Close()

	outputStream, _ := os.Create("full.png")
	png.Encode(outputStream, tile)

	defer outputStream.Close()
}

What did you expect to see?

b(1)

This is the source image, notice the details and the clarity.

What did you see instead?

full

This is the converted, the details are dimmed, the contrast is lower. Other webp decoding libs maintain the contrast.

@gopherbot gopherbot added this to the Unreleased milestone Jul 12, 2020
@agnivade
Copy link
Contributor

/cc @nigeltao

@agnivade agnivade added the NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one. label Jul 12, 2020
@dwbuiten
Copy link
Contributor

dwbuiten commented Jul 22, 2020

I'm almost certain this is due to x/image's VP8 decoder (which WebP lossy is internally) not handling the full/limited range flag:

https://github.com/golang/image/blob/69cc3646b96e61de0b417f4815b86c36e65783ee/vp8/decode.go#L332-L336

I'm also not sure if image.YCbCr can even handle full range vs limited range YCbCr content. It certainly can't handle any colorspace but BT.601 (which is fine for WebP, which is always BT.601, but won't hold true for newer formats.)

@AliFlux
Copy link
Author

AliFlux commented Aug 3, 2020

Is there any temporary hack that can fix this? I'm using python webp library to write the images.

@savtrip
Copy link

savtrip commented May 15, 2022

Out of curiosity, did anyone find a work around for this? Should we just use another webp library?

@AliFlux
Copy link
Author

AliFlux commented May 15, 2022

Yeah, I ended up using chai2010 webp library.

@savtrip
Copy link

savtrip commented May 15, 2022

@AliFlux thanks ali, appreciate the reply, I’ll try out the library.

By the way, the link you gave is broken, I still found the library though.

@savtrip
Copy link

savtrip commented May 23, 2022

Hey guys,

I really wanted to highlight that the problem is much worse when there is an image with a white background, for example, the image below:

Original:

google-search-results-how-to-market-my-business

After:

google-search-results-how-to-market-my-business_hubd5e904f9390325c44b8d7a5d156ca03_33118_1200x0_resize_q75_h2_box_2

I added this because I think it shows the problem more pronounced than the original image submitted by @AliFlux.

And also, a big thank you for all the open source work, I really appreciate it and use most of it in my day to day work. Your work obviously has a massive impact on lives around the world.

@savtrip
Copy link

savtrip commented Jun 5, 2022

@nigeltao @agnivade if I investigate myself and create a pull request will it be looked at?

@nigeltao
Copy link
Contributor

nigeltao commented Jun 6, 2022

It will be looked at (if you expliciltly add me to the PR) but:

  1. If it involves modifying the standard library's image or image/color packages (and their YCbCr representations), then there's a higher bar for accepting the pull request.
  2. If it involves general concepts of color correction at the API level (e.g. if it requires the caller passing in a color space somehow) then such API should ideally also work with any future color correction API in the standard library and x/image. Once again, there'd therefore be a higher bar. See x/image/draw: color space-correct interpolation #11420 for a discussion.

@savtrip
Copy link

savtrip commented Jun 6, 2022

Awesome, thanks @nigeltao, I appreciate it, I wanted to make sure I didn't spend a bunch of timing creating a solution that has no chance.

@TrentMoyar
Copy link

@savtrip are you still working on this issue, because if you aren't I'd be interested in taking over.

@savtrip
Copy link

savtrip commented Oct 11, 2022 via email

@seankhliao seankhliao changed the title x/image: WebP decode contrast issue x/image/webp: WebP decode contrast issue Jan 30, 2023
@slnc
Copy link

slnc commented Oct 20, 2024

Is there any update on this? This issue discourages using a technically superior image format (WebP) for users of tools like Hugo that depend on this library because the alternative with the least friction is to go back to jpeg or png.

@TrentMoyar
Copy link

The issue with golang's webp implementation is that webp (or more broadly vp8) only encodes and decodes to limited YCbCr using jpeg primaries and matrices, wheras jpeg uses full ycbcr. This means that in order for webp to work, either the go standard library would have to add a new, limited YCbCr type with the proper conversion to RGB, or such a type would have to be added to x/image.

This appears to be a substantial amount of work, as the YCbCr type appears in many places, such as this gen.go file. This means that fixing ycbcr in webp also entails rewriting chroma upsampling algorithms. If an instant fix is desired, one option would be to change the webp library to decode to some ycbcrl type, and then users would handle the rgb transform and chroma upsampling.

Here is a link to the documented conversion functions in the libwebp source.

@nigeltao what are your thoughts? You seem to have written the bulk of the current ycbcr code, so I'm curious if you have any suggestions.

I apologize for never getting around to an update. I realized the scale of potentially adding the new limited ycbcr and i set this aside, then difficulties in my life took priority.

@nigeltao
Copy link
Contributor

nigeltao commented Nov 6, 2024

Yeah, the bug is a fair complaint but fixing it properly (in the standard library) is a lot of work (see also my earlier comment) and I don't have the time to do it.

You could probably fix it (without touching the standard library) if golang.org/x/image/vp8 decoded directly to an image.RGBA (instead of to an image.YCbCr) or if it had its own FullYCbCr image.Image type. But that's still a substantial change and, again, I don't have the time to do it. Sorry.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
NeedsInvestigation Someone must examine and confirm this is a valid issue and not a duplicate of an existing one.
Projects
None yet
Development

No branches or pull requests

8 participants