Closed GoogleCodeExporter closed 8 years ago
Here is the correct image file.
Original comment by era...@phobicgames.com
on 25 Mar 2014 at 7:55
Attachments:
We are currently using this workaround:
// Building webp with -Dmalloc=hooked_webp_malloc
void* webp_hooked_malloc(size_t n)
{
unsigned char* ret = malloc(n + 4);
memset(ret + n, 0, 4);
return ret;
}
Original comment by yo...@phobicgames.com
on 26 Mar 2014 at 12:55
having a look (I'm surprised bypass_filtering has an impact on _lossless_
decoding though, which is not using lossy's in-loop filtering).
Original comment by pascal.m...@gmail.com
on 26 Mar 2014 at 2:32
update: i can reproduce with valgrind using dwebp: 'valgrind dwebp test.WEBP
-nofilter' exhibits the out-of-bound read.
Still debugging....
Original comment by pascal.m...@gmail.com
on 26 Mar 2014 at 2:35
When stepping in the code, we noticed that the last uint32_t is indexed with
row = width of image (256 not 255), x = 0. It seems that the code is reading
one extra uint32_t in the huffman image.
Original comment by yo...@phobicgames.com
on 26 Mar 2014 at 2:41
FYI we have to use bypass_filtering because otherwise decompressing is
extremely slow on ARMV7 class hardware. Decompressing a lossy 1024 x 1024 x
RGBA texture takes around 500ms with bypass_filtering = 0, otherwise with
bypass_filtering = 1 it takes 150-200 ms.
This is with NEON activated.
Original comment by yo...@phobicgames.com
on 26 Mar 2014 at 2:44
You can try the patch here: https://gerrit.chromium.org/gerrit/#/c/69363/
(still testing it, but it seems reasonably ok)
A note about filtering speed:
actually, the ARMv7 code still lacks NEON optimization for _strong_ filtering
(which is the default encoding option).
But support for light filtering in assembly is here. So you can maybe encode
the picture using -nostrong option in cwebp if that's a possibility. (asm
support for strong filtering is in the work. ETA unknown).
Also, you might be interested in using no_fancy_upsampling=1 to speed-up the
YUV->RGB conversion.
Original comment by s...@google.com
on 26 Mar 2014 at 4:10
We've tested no_fancy_upsampling and it does not affect performance
significantly.
Another issue we have on ARMv7 is memory usage. When not decoding
incrementally, libwebp mallocs at least the number of pixels required by the
output image. This is excluding the memory required for the output buffer. For
a complete decompression, this means around 8.5 megabytes of mallocs when
decompressing a lossy 1024 x 1024 RGBA image.
Moreover, webp does around 500 tiny mallocs on a lossy 1024 x 1024 RGBA. Most
of those allocs may be replaced by alloca style stack allocations. Especially
the tiny ones done by the huffman tree code.
Will apply your patch to webp 0.4 and run it on a few thousand images on 8
cores. These sort of issues are extremely rare to detect. We detected it
because we are hooking malloc/free in webp and we do not pad with memsetted
memory as is done by most libc mallocs.
Original comment by yo...@phobicgames.com
on 26 Mar 2014 at 4:20
I've verified that the patch works on our dataset after running the code using
the same memory a couple of times.
This is a quick-fix and seems to work but the core of the issue seems to be
that the caller of the patched function asks for a row/pixel out-of-bounds.
Thanks a lot Pascal, you saved us of a lot of headache!
Johan
Original comment by yo...@phobicgames.com
on 26 Mar 2014 at 4:37
Yohan,
> This is a quick-fix and seems to work but the core of the issue seems to be
that the caller of the patched function asks for a row/pixel out-of-bounds.
Actually, it really seems like a reasonable fix: we were calling
GetHtreeGroupForPos()
one extra time (only) outside of the loop at vp8l.cc:822
The loop itself was not executed since the decoding was already finished, but
this call was done anyway. So best is just to not even get into this function
if it's not needed.
Thanks for the tests. I will also have a look at the separate memory problems.
Original comment by s...@google.com
on 26 Mar 2014 at 5:03
Great. Will use this patch until the next version of WebP comes out.
Original comment by yo...@phobicgames.com
on 26 Mar 2014 at 6:19
patch submitted.
Original comment by pascal.m...@gmail.com
on 26 Mar 2014 at 10:23
Issue 194 has been merged into this issue.
Original comment by pascal.m...@gmail.com
on 27 Mar 2014 at 3:58
Original issue reported on code.google.com by
era...@phobicgames.com
on 25 Mar 2014 at 7:53Attachments: