qiqian / webp

Automatically exported from code.google.com/p/webp
0 stars 0 forks source link

investigate PPC (big-endian) targets #150

Closed GoogleCodeExporter closed 8 years ago

GoogleCodeExporter commented 8 years ago
There's an open question about a potential regression from 0.1.3 to 0.3.0 with 
big endian decodes.

https://groups.google.com/a/webmproject.org/d/msg/webp-discuss/ykAwWkjJPGA/rBjK-
5PX-nYJ

Original issue reported on code.google.com by jz...@google.com on 11 May 2013 at 1:05

GoogleCodeExporter commented 8 years ago
It seems BITS=24 (with and without WEBP_RIGHT_JUSTIFY) is broken on big endian 
machines. I didn't search out the root cause yet, but setting BITS=32 (the old 
default) for big endian passes all tests:

diff --git a/src/utils/bit_reader.h b/src/utils/bit_reader.h
index f1ded6f..ac4361a 100644
--- a/src/utils/bit_reader.h
+++ b/src/utils/bit_reader.h
@@ -70,6 +70,8 @@ extern "C" {
 #define BITS 16
 #elif defined(__arm__) || defined(_M_ARM)     // ARM
 #define BITS 24
+#elif defined(__BIG_ENDIAN__)
+#define BITS 32
 #else                      // reasonable default
 #define BITS 24
 #endif

b7490f8553f35298532457266790eeaad0493d53 is the first bad commit
commit b7490f8553f35298532457266790eeaad0493d53
Author: skal <pascal.massimino@gmail.com>
Date:   Thu Feb 14 15:46:12 2013 +0100

    introduce WEBP_REFERENCE_IMPLEMENTATION compile option

    This flag will make the code use no uint64, no asm, and no fancy
    trick, but instead aim at being as simple and straightforward as
    possible.
    Main use is to help emscripten generate proper JS code.
    More code needs to be simplified later.

    Also: tune the BITS values to be 24 and make use of WEBP_RIGHT_JUSTIFY
    Here are the typical timing for decoding a large image:

            ARM7-a:
            dwebp_justify_32_neon Time to decode picture: 3.280s
            dwebp_justify_24_neon Time to decode picture: 2.640s
            dwebp_justify_16_neon Time to decode picture: 2.723s
            dwebp_justify_8_neon Time to decode picture: 2.802s
            dwebp_justify_32 Time to decode picture: 4.264s
            dwebp_justify_24 Time to decode picture: 3.696s
            dwebp_justify_16 Time to decode picture: 3.779s
            dwebp_justify_8 Time to decode picture: 3.834s
            dwebp_32_neon Time to decode picture: 4.010s
            dwebp_24_neon Time to decode picture: 2.725s
            dwebp_16_neon Time to decode picture: 2.852s
            dwebp_8_neon Time to decode picture: 2.778s
            dwebp_32 Time to decode picture: 4.587s
            dwebp_24 Time to decode picture: 3.800s
            dwebp_16 Time to decode picture: 3.902s
            dwebp_8 Time to decode picture: 3.815s
            REFERENCE (HEAD) Time to decode picture: 3.818s

            x86_64:
            dwebp_justify_32 Time to decode picture: 0.473s
            dwebp_justify_24 Time to decode picture: 0.434s
            dwebp_justify_16 Time to decode picture: 0.450s
            dwebp_justify_8 Time to decode picture: 0.467s
            dwebp_32 Time to decode picture: 0.474s
            dwebp_24 Time to decode picture: 0.468s
            dwebp_16 Time to decode picture: 0.468s
            dwebp_8 Time to decode picture: 0.481s
            REFERENCE (HEAD) Time to decode picture: 0.436s

            i386:
            dwebp_justify_32 Time to decode picture: 0.723s
            dwebp_justify_24 Time to decode picture: 0.618s
            dwebp_justify_16 Time to decode picture: 0.626s
            dwebp_justify_8 Time to decode picture: 0.651s
            dwebp_32 Time to decode picture: 0.744s
            dwebp_24 Time to decode picture: 0.627s
            dwebp_16 Time to decode picture: 0.642s
            dwebp_8 Time to decode picture: 0.642s

    Change-Id: Ie56c7235733a24f94fbfc2e4351aae36ec39c225

:040000 040000 34a702a9819405a771edef3f387407fba339ed34 
f0d62a70616a4cc1b2de886131afcb4d4cde2633 M      src

Original comment by jz...@google.com on 11 May 2013 at 3:23

GoogleCodeExporter commented 8 years ago
https://gerrit.chromium.org/gerrit/50952 might be a likely fix

Original comment by pascal.m...@gmail.com on 11 May 2013 at 10:56

GoogleCodeExporter commented 8 years ago
submitted patch #50952 to fix the lossy decoder. Will cross-check encoder (and 
lossless mode) too.

Original comment by pascal.m...@gmail.com on 14 May 2013 at 8:59

GoogleCodeExporter commented 8 years ago
This did at least fix the problem I originally reported in my environment. 
After this patch, both lossy and lossless worked fine on big endian.

Thank you!

Original comment by gun...@gmail.com on 14 May 2013 at 8:55

GoogleCodeExporter commented 8 years ago
There was one issue with decoding lossless to BGRA that was also fixed:

https://gerrit.chromium.org/gerrit/#/c/51559/

2d6ac42 Merge "webp/lossless: fix big endian BGRA output"
2ca8396 webp/lossless: fix big endian BGRA output

The core library should behave the same as little-endian after this commit.

Original comment by jz...@google.com on 21 May 2013 at 12:44

GoogleCodeExporter commented 8 years ago
closing, since the original issue is fixed. Please feel free to re-open in case 
of new problem.

Original comment by pascal.m...@gmail.com on 8 Oct 2013 at 11:16