Issue 239 in webp: alpha artifacts

[we...@googlecode.com]

Status: New
Owner: ----
Labels: Type-Defect Priority-Medium

New issue 239 by shekh.an...@gmail.com: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

When I encode and then decode images using simple api there are sometimes
defects in the alpha plane.
The defects vary depending on compression level and the original image.
Only few of hundreds images are affected (however I spot them by eye).

This is present if I compile current webp sources (latest attempt from
da0912126b62f2f9390bf2a149784f7a0ed547de).
I tried 4.2.0 sources and could not spot any problem.

01.tga, quality=90
defective alpha at (66,24): value=255
source value=2

00.tga, quality=90
defective alpha blocks at (19,42), (570,42): value=57
source value=0


Attachments:
images.zip 244 KB

--
You received this message because this project is configured to send all
issue notifications to this address.
You may adjust your notification preferences at:
https://code.google.com/hosting/settings

[we...@googlecode.com]

Updates:
Status: Accepted
Owner: pascal.m...@gmail.com

Comment #1 on issue 239 by pascal.m...@gmail.com: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

thanks for the report!
i can confirm some values are badly compressed on the alpha plane.
See attached the Y/U/V/A planes for a 00.tga compressed with 0.4.2 vs HEAD.
Investigating...

Attachments:
00_HEAD_bad.png 139 KB
00_0.4.0_good.png 29.9 KB

[we...@googlecode.com]

Comment #2 on issue 239 by pascal.m...@gmail.com: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

looks like the regression was introduced by
https://gerrit.chromium.org/gerrit/#/c/72101/1/src/enc/vp8l.c
with a bad interaction between references tokens and increased color_cache
bits.

I will submit an "emergency fix" to shut this new feature off until the
problem is figured out.
-> https://gerrit.chromium.org/gerrit/73565


btw: attaching the good 00_HEAD_bad.png this time, for future reference.

Attachments:
00_HEAD_bad.png 29.6 KB

[we...@googlecode.com]

Comment #3 on issue 239 by jz...@chromium.org: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

v0.3.1 appears to be correct, so using that as a base this bisects to:

64c844863ab0d13e8dd51a59001ab55fbe64065e is the first bad commit
commit 64c844863ab0d13e8dd51a59001ab55fbe64065e
Author: Urvang Joshi <urv...@google.com>
Date: Mon May 13 16:24:49 2013 -0700

Further reduce memory to decode lossy+alpha images

Earlier such images were using roughly 9 * width * height bytes for
decoding. Now, they take 6 * width * height memory.

Change-Id: Ie4a681ca5074d96d64f30b2597fafdca648dd8f7

:040000 040000 da89ac00cb7fa3ca1bd34073037c38d58f5cf420
8a8f8f98f2e1854a47a5eadf606e9809a9fd89ad M src

[we...@googlecode.com]

Comment #4 on issue 239 by pascal.m...@gmail.com: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

strange, 0.4.2 is correct here. The error seemed to have been introduced
since...

[we...@googlecode.com]

Comment #5 on issue 239 by urv...@chromium.org: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

As discussed online, that regression was fixed in:
https://gerrit.chromium.org/gerrit/#/c/58842/, and the image decodes fine
at that point.

So, I reran bisect from that commit, and here's what I got.

a4d5f59d9e0869009433103818e87d131a56270d is the first bad commit
commit a4d5f59d9e0869009433103818e87d131a56270d
Author: skal <pascal.m...@gmail.com>
Date: Mon Jun 24 09:34:30 2013 +0200

Faster lossless decoding

Specialize and simplify the alpha-decoding case, which is used when:
- no color-cache is use
- all red/blue/alpha values are the same (and hence their Huffman tree
has
only 1 symbol. We don't need to consume any bits for reading these).

+ revamped the loop to use size_t and offsets instead of pointers.

~2-3% faster on Unix (gcc) but up to 25% faster lossy+alpha decoding
on Mac (llvm) and ARM.

Change-Id: I43c9688d1e4811cab0ecf0108a5b8f45781083e6

:040000 040000 3f13501c31f3b1e28d9023d7336f16a8a8d03279
7504e82c2f23ca486a377eb4a6d93b1e342a8aea M src

[we...@googlecode.com]

Comment #6 on issue 239 by urv...@google.com: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

As discussed offline, the regression James found was fixed in:

[we...@googlecode.com]

Comment #7 on issue 239 by jz...@google.com: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

I ended up at the same point, earlier I was bisecting across 2 working
branches with a merge in between which may have caused the difference. When
I check from that merge (30e77d0 Merge branch '0.3.0') I hit the same
commit.

[we...@googlecode.com]

Comment #8 on issue 239 by urv...@google.com: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

Pascal, try to directly decode the alpha plane of the image encoded with
HEAD.
This works fine before the commit a4d5f59d9e0869009433103818e87d131a56270d
I mentioned above, but not anytime after. (0.4.2 is bad as well).

[we...@googlecode.com]

Comment #9 on issue 239 by jz...@google.com: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

This seems to be related to a change in DecodeImageData() from the earlier
macro implementation:

- if (src < src_end)
{ \
- htree_group = GetHtreeGroupForPos(hdr, col,
row); \
+ if (src < src_last) {
+ if (col & mask) htree_group = GetHtreeGroupForPos(hdr, col, row);

specifically the if condition, reverting that to src_end works here where
(row, col, last_row)==(40, 40, 40); I haven't looked more closely at the
logic itself...

[we...@googlecode.com]

Updates:
Owner: urv...@google.com

Comment #10 on issue 239 by urv...@google.com: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

Indeed, that is the problem.
In fact, no if condition is needed, as src <= src_end is implied at that
point.
Sending a patch shortly.

[we...@googlecode.com]

Comment #11 on issue 239 by pascal.m...@gmail.com: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

i think "if (src < src_end) { ... }" is indeed correctly fixing the issue

[we...@googlecode.com]

Updates:
Status: Fixed

Comment #12 on issue 239 by urv...@google.com: alpha artifacts
https://code.google.com/p/webp/issues/detail?id=239

Fixed here: https://gerrit.chromium.org/gerrit/#/c/73580/
Thanks again for the report!