webp artifacts with transparency

[totaam]

Issue migrated from trac ticket # 487

component: client | priority: critical | resolution: fixed

2014-01-09 06:48:17: totaam created the issue

---

Split from #385#comment:20

This affects all platforms, with or without opengl rendering. See screenshot:
[[Image(/raw-attachment/ticket/487/webp-transparency-artifacts.png)]]

Probably a regression since I must have tested transparency when it was added to webp, but 0.10.x is also affected.

[totaam]

2014-01-09 06:48:47: totaam uploaded file webp-transparency-artifacts.png (22.6 KiB)

shows the artifacts on transparent windows when using webp

[totaam]

2014-01-09 06:49:35: totaam changed status from new to assigned

[totaam]

2014-01-09 06:49:35: totaam changed owner from antoine to totaam

[totaam]

2014-01-09 06:49:35: totaam edited the issue description

[totaam]

2014-01-09 12:33:15: totaam commented

---

Transparency support was added in r3522, and works ok as I expected.
r3884, r4245 and r5091 don't change any of the actual encoder code, so the problem must come from somewhere else..

Bisecting:

• r4300: bad? (different corruption..)
• r3910: good?
• r4100: good?
• 4240: bad?
• r4170: bad
• r4135: good
• r4152: bad
• r4144: good
• r4148: good
• r4150: good
• r4151: bad

So the problem comes from r4151, likely to be in the argb code, I tested byteswapping on RGB components, but maybe it's the (un)premultiply codepath..

[totaam]

2014-01-09 15:55:39: totaam commented

---

This is a regression, needs to be in 0.11

Taking out the line:

return byte_buffer_to_buffer(unpremultiply_argb(img_data))

And returning the data as it is instead (premultiplied), the picture looks OK (albeit, with too much in the alpha channel)

What I don't understand is why this affects webp and not the other encodings, which also use the same codepath!? (ie: png via PIL also gives us a string buffer)

[totaam]

2014-01-10 07:53:30: totaam commented

---

Since webp is buggy and not going to be used by default (and maybe we should even disable it?), re-scheduling this ticket until after #491

[totaam]

2014-02-09 08:25:01: antoine commented

---

re-scheduling all webp stuff

[totaam]

2014-05-17 15:19:06: antoine commented

---

The problem: webp's simple API returns unpremultiplied alpha, which is not what we want and when we unpremultiply it again, it goes out of range... not sure about Pillow (the version in Fedora 20 does not seem to be able to decode webp..)

So, we have to:

• use the advanced API preferably, adding a decoder to the new webp codec module, so we can get unpremultiplied alpha just like with the other encodings
• add a flag to the webm (and Pillow?) decoded picture options (which will still be used as fallback), so the window backings will know it is unpremultiplied already... some backigns will skip the unpremultiply step, and the opengl case will re-premultiply it!
• remove webp from the list of encodings when using opengl without the advanced api decoder

Note: the corruption is not directly visible with the opengl backing because we now use the premultiplied alpha directly during rendering (it may just end up oversaturated instead)

[totaam]

2014-05-18 11:10:31: antoine commented

---

And... to make matters worse, webp decides if the picture has an alpha channel not using any API flags, but by inspecting the alpha component of every pixel in the input and deciding that if any values are not 0xFF then it is on.. WTH? And whose bright idea was this?

[totaam]

2014-05-18 12:16:29: antoine uploaded file argb-clamp.patch (0.9 KiB)

clamp rgb values when unpremultiplying to prevent overflow

[totaam]

2014-05-18 13:37:26: antoine commented

---

Implemented a new "advanced API" webp decoder in r6509:

• added opengl support for painting from buffer type in r6508
• use this decoder ahead of the others (python-webm and Pillow)

I'm not going to bother fixing the codepath for the other decoders (I have not tested to see if Pillow uses premultiplied alpha or not), you will just get the wrong transparency levels with those. (and eventually, maybe we should just drop them)

[totaam]

2014-05-18 14:26:02: antoine commented

---

5510 clears out the alpha channel before calling the webp Encode function, not very efficient but it works.

[totaam]

2014-05-18 15:18:38: antoine commented

---

Ready for testing. Related to #419.

afarr: please check that webp works as primary encoding:

• that the transparency is as it should be: using the transparent_colors.py test script (transparency is only supported on Linux clients and must be compared with running native on Linux without xpra - beware, the bug is a fairly subtle difference)
• that it works with: opengl on and off without any errors in the logs, with different settings for quality settings
• that the win32 builds show in Encoding_Info.exe: dec_webp and enc_webp at {{0.4.0}}}
• that the webp data we receive on the client does not contain any alpha when we don't want any: run the client with -d webp and you should see webp decompress found features: ... has_alpha=0 (has_alpha=1 when alpha is present)

• when the window does not have any alpha channel (most windows do not)
• when running on platforms that do not support alpha (win32)

• that it works reasonably well, without too many slowdowns (it isn't going to be as fast as h264..), even at fairly high resolution (the code should switch to more suitable encodings rather than crawl to a halt - see better/faster encoding selection #419 for details)

Important note: because of #419, the actual encoding used for each region of pixels we send may end up being different from the one we specify. The only way to verify what encoding is used is to turn on compression debugging with -d compress on the server and to manually verify what comes out.

Note: I am not applying the [/attachment/ticket/487/argb-clamp.patch] because it would actually hide any overflow, and we want to see it and fix it instead.

[totaam]

2014-05-18 15:18:53: antoine changed status from assigned to new

[totaam]

2014-05-18 15:18:53: antoine changed owner from totaam to afarr

[totaam]

2014-05-18 15:18:53: antoine commented

---

(re-assign)

[totaam]

2014-05-20 21:27:29: maxmylyn commented

---

Tested with Windows 7 - 64 bit, Windows 8.1 - 32 bit, and Fedora 20 - 64 bit:

• Tested transparent_colors.py local and through xpra with and without opengl, both looked identical (xpra and locally running), as far as I could tell
• win32 and win64 both show dec_webp and enc_webp as 0.4.0
• With and without opengl works as expected on both Windows and Fedora - no errors are outputted, only webp compress/decompress messages
• Fedora 20 client shows alpha=1 when using the transparent_colors.py
• Win7 client shows aplha=0 when using the transparent_colors.py

Of note, using both Windows and Fedora clients, running a high definition youtube video full screen with webp causes the framerate to slow down to a crawl. Otherwise, everything worked as expected.

[totaam]

2014-05-21 14:33:08: totaam commented

---

Thanks for testing, this reminds me of just how slow webp can be (see #419): it falls off a cliff in lossless mode, especially at low speed and at high resolutions (1080p and up). So I'll run some more tests tomorrow to ensure that we never use it as a temporary non-video encoding with those problematic combinations of options (this should already be the case, but best to triple check).

What was the resolution you tested at when fullscreen? 1080p? If so, unless you have other options set, it should have switched to high speed which should still be faster than png. Having xpra info of when it is really slow may help me figure things out.

[totaam]

2014-05-22 15:24:22: totaam changed status from new to closed

[totaam]

2014-05-22 15:24:22: totaam changed resolution from ** to fixed

[totaam]

2014-05-22 15:24:22: totaam commented

---

Re-tested some more today, and made some more tweaks (r6528, r6534, r6535), fixes (r6529, r6531) and improvements to the test code (r6530, r6532, r6533).

Closing.

[totaam]

2014-05-26 06:29:39: totaam commented

---

Note: in 0.14, the old python-webm is completely removed in favor of the new webp advanced codec, see r6555.