Am I expected to do something about this?  I seem to have misplaced my spare s390 system.

Adding Dan - sharkcz to cc. He can help you to get access to some s390.

Setting this bug to block the python-pillow one.

This is strange problem as libwebp passes the tests on ppc which is also a big endian platform, but fails on s390.

Rahul, you are a Red Hat employee, aren't you?

not anymore.

This should be fixed by this commit [1], possibly libwebp-0.3.1 is also necessary though.

[1] http://pkgs.fedoraproject.org/cgit/python-pillow.git/commit/?id=48894613f998e16b03a661fdbbdffc2d5ec5ad63

Uh no, the problem ist still here. Is there any chance of having access to a s390 machine to debug further?

This bug appears to have been reported against 'rawhide' during the Fedora 20 development cycle.
Changing version to '20'.

More information and reason for this action is here:
https://fedoraproject.org/wiki/BugZappers/HouseKeeping/Fedora20

It seems the repository already contains a newer version that apparently passed the tests.

Odd, last I tried the tests failed against libwep-0.3.1, which is still what is the current version in s390(x) (rawhide for primary arches now has libwebp-0.4.0). But if you confirm that things work now, I'll re-enable webp support on s390(x) in the master spec.

According to the following link both currently supported fedora releases already contain newer releases of the component ...

http://s390.koji.fedoraproject.org/koji/packageinfo?packageID=15514

I believe this bug can be closed.

python-pillow-2.2.1-1.fc20 and python-pillow-2.2.1-2.fc21 both contain

# Don't build with webp support on s390* archs, see bug #962091 (s390*)
%ifnarch s390 s390x
BuildRequires:  libwebp-devel
%endif

i.e. they were both built without webp support, hence the problem likely still exists. Would be nice to try building against libwebp-0.4.0 though.

Oh sorry ... if the tests are disabled, then it still needs to be investigated. But as I'm testing that locally, it works without problem with the libwebp-0.3.1 ...

Let's see -> http://s390.koji.fedoraproject.org/koji/taskinfo?taskID=1319964

Uhm failed but for completely unrelated reasons :s

I'm building the latest f20/python-pillow with the latest f20/libwebp and the build fails, but I'm unable to reproduce the scenario you described ....

# cwebp lena.png -o lena.webp
Saving file 'lena.webp'
File:      lena.png
Dimension: 128 x 128
Output:    3256 bytes Y-U-V-All-PSNR 36.85 41.23 42.04   37.94 dB
block count:  intra4: 63
              intra16: 1  (-> 1.56%)
              skipped block: 0 (0.00%)
bytes used:  header:             79  (2.4%)
             mode-partition:    338  (10.4%)
 Residuals bytes  |segment 1|segment 2|segment 3|segment 4|  total
    macroblocks:  |      15%|      25%|      34%|      25%|      64
      quantizer:  |      36 |      28 |      23 |      18 |
   filter level:  |      16 |       8 |       6 |       4 |

# dwebp lena.webp -o lena_webp_bits.png
Decoded lena.webp. Dimensions: 128 x 128. Now saving...
Saved file lena_webp_bits.png

Does that mean that test_file_webp.py passes? (not having access to s390(x) hardware I've never investigated the issue and don't know which is the specific part of the test which is failing).

Oh sorry ... it was Roman Rakus who reported the original bug. My mistake.

Currently the test fails with the following messages:

running test_file_webp ...
Tests/test_file_webp.py:21: image = Image.open(file_path) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp.py", line 21, in test_read_rgb
    image = Image.open(file_path)
  File "/root/rpmbuild/BUILD/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file
Tests/test_file_webp.py:45: image = Image.open(temp_file) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp.py", line 45, in test_write_rgb
    image = Image.open(temp_file)
  File "/root/rpmbuild/BUILD/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file
Tests/test_file_webp.py:84: image = Image.open(temp_file) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp.py", line 84, in test_write_rgba
    image = Image.open(temp_file)
  File "/root/rpmbuild/BUILD/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file
Tests/test_file_webp.py:101: image = Image.open(file_path) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp.py", line 101, in test_read_rgba
    image = Image.open(file_path)
  File "/root/rpmbuild/BUILD/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file
running test_file_webp_metadata ...
Tests/test_file_webp_metadata.py:44: webp_image = Image.open(buffer) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp_metadata.py", line 44, in test_write_exif_metadata
    webp_image = Image.open(buffer)
  File "/root/rpmbuild/BUILD/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file
Tests/test_file_webp_metadata.py:54: image = Image.open(file_path) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp_metadata.py", line 54, in test_read_icc_profile
    image = Image.open(file_path)
  File "/root/rpmbuild/BUILD/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file
Tests/test_file_webp_metadata.py:77: webp_image = Image.open(buffer) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp_metadata.py", line 77, in test_write_icc_metadata
    webp_image = Image.open(buffer)
  File "/root/rpmbuild/BUILD/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file

Uhm I think debugging something of that sorts on ppc I traced it back to the decoder failing (and pillow not giving a terribly helpful error message).
Running python through gdb, I *think* that breaking on WebPDecode_wrapper in _webp.c and stepping through the function should show where it is failing.

Btw, maybe you could test locally against libwebp-0.4.0?

Speaking as Fedora/s390x maintainer I think the first step should be to check whether the upstream test suite (libwebp-test-data from http://www.webmproject.org/code/) passes also on s390/s390x for libwebp, it didn't in the past when I tried it. ppc/ppc64 was OK which is also a big endian arch and one would think ppc and s390 should behave the same. And after that try python-pillow.

It seems the reproduction scenario described in the bug is a bit confusing ... finally I was able to reproduce the issue ...

The pre-generated lena.webp seems to have an incorrect format, but when I generate by hand, it's correct ...

The following comment might explain what happens ...

# If we're using the exact same version of webp, this test should pass.
# but it doesn't if the webp is generated on Ubuntu and tested on Fedora.
    
# generated with: dwebp -ppm temp.webp -o lena_webp_write.ppm
#target = Image.open('Tests/images/lena_webp_write.ppm')
#assert_image_equal(image, target)

# This test asserts that the images are similar. If the average pixel difference
# between the two images is less than the epsilon value, then we're going to
# accept that it's a reasonable lossy version of the image. The included lena images
# for webp are showing ~16 on Ubuntu, the jpegs are showing ~18.

I actually just recently pushed upstream a commit [1] which should make the webp test more robust against numerical noise. So if the webp image generated on s390(x) differs from one generated on x86(_64) only by numerical noise, then it should now work. Otherwise, it would be interesting to check whether the images are visually identical (if not, the issue should probably be reported in the upstream webp tracker).


[1] https://github.com/python-imaging/Pillow/commit/4de31b2693dcb76ce51744a986e8ac3598158c4c

Though on second thoughts the decoder really should not fail on an image generated on another architecture, so the issue is a deeper one.

More than that, I tried to replace the pre-generated lena.webp with another generated by me, but the _webp.so library still doesn't accept the file ... so, you're right, it smells.

But is seems the problem lies in the python-pillow webp plugin ... I'll continue tomorrow.

I found a possible bug in the vp8 decoder .... it might be unrelated to the failure we're facing, but needs to be fixed too. The number of partitions is incorrect when decoding on s390x.

I just tested the python-pillow build with libwebp 0.4.0 and the result seems to be just a bit different ...

Tests/test_file_webp.py:21: image = Image.open(file_path) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp.py", line 21, in test_read_rgb
    image = Image.open(file_path)
  File "/home/jcapik/projects/python-pillow/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file
Tests/test_file_webp.py:66: assert_image_similar(image, target, 20.0) failed:
- average pixel value difference 190.7027 > epsilon 20.0000
Tests/test_file_webp.py:84: image = Image.open(temp_file) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp.py", line 84, in test_write_rgba
    image = Image.open(temp_file)
  File "/home/jcapik/projects/python-pillow/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file
Tests/test_file_webp.py:101: image = Image.open(file_path) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp.py", line 101, in test_read_rgba
    image = Image.open(file_path)
  File "/home/jcapik/projects/python-pillow/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file
running test_file_webp_metadata ...
Tests/test_file_webp_metadata.py:44: webp_image = Image.open(buffer) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp_metadata.py", line 44, in test_write_exif_metadata
    webp_image = Image.open(buffer)
  File "/home/jcapik/projects/python-pillow/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file
Tests/test_file_webp_metadata.py:54: image = Image.open(file_path) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp_metadata.py", line 54, in test_read_icc_profile
    image = Image.open(file_path)
  File "/home/jcapik/projects/python-pillow/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file
Tests/test_file_webp_metadata.py:77: webp_image = Image.open(buffer) failed:
Traceback (most recent call last):
  File "Tests/test_file_webp_metadata.py", line 77, in test_write_icc_metadata
    webp_image = Image.open(buffer)
  File "/home/jcapik/projects/python-pillow/python-imaging-Pillow-3c2496e/build/lib.linux-s390x-2.7/PIL/Image.py", line 2007, in open
    raise IOError("cannot identify image file")
IOError: cannot identify image file

On s390x the following command returns different 'config.output' data than x86_64 and ppc64.

status = WebPDecode(data, data_size, &config);

frame.c: VP8EnterCritical()

const int extra_pixels = kFilterExtraRows[dec->filter_type_];


The value above is evaluated incorrectly on s390x.

VP8GetHeaders(dec, &io) returns incorrect 'dec.filter_type_' on s390x.

bit_reader.h: VP8LoadNewBytes()

gdb doesn't enter the __BIG_ENDIAN__ section of the function on s390x.

The __BIG_ENDIAN__ macro is NOT defined on s390x.

x86_64:
$ gcc -E -dM - < /dev/null |grep ENDIAN
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __ORDER_PDP_ENDIAN__ 3412
#define __ORDER_BIG_ENDIAN__ 4321
#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__


ppc64:
$ gcc -E -dM - < /dev/null |grep ENDIAN
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __BIG_ENDIAN__ 1
#define __FLOAT_WORD_ORDER__ __ORDER_BIG_ENDIAN__
#define __ORDER_PDP_ENDIAN__ 3412
#define _BIG_ENDIAN 1
#define __ORDER_BIG_ENDIAN__ 4321
#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__


s390x:
$ gcc -E -dM - < /dev/null |grep ENDIAN
#define __ORDER_LITTLE_ENDIAN__ 1234
#define __FLOAT_WORD_ORDER__ __ORDER_BIG_ENDIAN__
#define __ORDER_PDP_ENDIAN__ 3412
#define __ORDER_BIG_ENDIAN__ 4321
#define __BYTE_ORDER__ __ORDER_BIG_ENDIAN__

I think we should introduce the __BIG_ENDIAN__ macro for s390x.

The gcc changelog contains the following:

* quadmath-imp.h (__LITTLE_ENDIAN__): Don't define.
(ieee854_float128): Use __BYTE_ORDER == __ORDER_BIG_ENDIAN__
tests instead of __BIG_ENDIAN__.

... so, maybe the __BIG_ENDIAN__ and _BIG_ENDIAN macros are obsolete? Anyway, I checked the sources and the macros are used in more than just one software product and I think they should be both defined unless all the software that relies on them is fixed.

Changing the component to gcc to get a statement about that.

NOTE: After injecting the macros with -D__BIG_ENDIAN__ and -D_BIG_ENDIAN, the dwebp decoder started working properly and the python-pillow tests succeeded.

The funny thing is, that even some parts of the gcc internals depend on the __BIG_ENDIAN__ macro. How that comes it works correctly? Maybe it doesn't. Anyway, it's a mess that needs to be well analysed and then cleaned.

Thanks a lot Jaromír for your work on this.

(In reply to Sandro Mani from comment #39)
> Thanks a lot Jaromír for your work on this.

np ... it was my pleasure.

-----------------------------------

ppc64le:
$ gcc -E -dM - < /dev/null |grep ENDIAN
#define __ORDER_LITTLE_ENDIAN__ 1234
#define _LITTLE_ENDIAN 1
#define __FLOAT_WORD_ORDER__ __ORDER_LITTLE_ENDIAN__
#define __ORDER_PDP_ENDIAN__ 3412
#define __LITTLE_ENDIAN__ 1
#define __ORDER_BIG_ENDIAN__ 4321
#define __BYTE_ORDER__ __ORDER_LITTLE_ENDIAN__

(In reply to Jaromír Cápík from comment #38)
> The funny thing is, that even some parts of the gcc internals depend on the
> __BIG_ENDIAN__ macro. How that comes it works correctly? Maybe it doesn't.
> Anyway, it's a mess that needs to be well analysed and then cleaned.

The code in GCC depending on __BIG_ENDIAN__ is limited to targets known to define it.

I think the portable way of checking the endianess is checking __BYTE_ORDER. If you can come up with a number of packages really depending on __BIG_ENDIAN__ we probably have to revisit this. Otherwise I think it doesn't make much sense to introduce the macro now since new code could still not rely on it being defined as long as compilers are around which do not define it. So in any case I think it is preferable to fix the code actually doing the check.

Hi Andreas.

Well, I found 16 header files from 14 different software projects containing just the first __BIG_ENDIAN__ macro. And those are just the devel packages installed on my workstation. Now imagine the number of packages containing the second _BIG_ENDIAN macro and the total number of packages containing any combination of both. My estimation is that the number of affected packages might go over 100. We would have to search in all the sources to get the real stats. But even fixing 14 packages is time consuming. Therefore it would be safer and faster to just introduce the macros even when they're obsolete.

My proposal is to add the macros and additionally print compiler warnings, that the source file is using obsolete macros which should be replaced with the __BYTE_ORDER == __ORDER_BIG_ENDIAN__ condition. Does it sound acceptable?

Thanks,
Jaromir.

(In reply to Jaromír Cápík from comment #42)
> Hi Andreas.
> 
> Well, I found 16 header files from 14 different software projects containing
> just the first __BIG_ENDIAN__ macro. And those are just the devel packages
> installed on my workstation. Now imagine the number of packages containing
> the second _BIG_ENDIAN macro and the total number of packages containing any
> combination of both. My estimation is that the number of affected packages
> might go over 100.

So it appears to be a miracle that we run Linux distros for 15 years now without noticing right?! ;)

Perhaps your grep got confused a bit by the endianess checks emitted by the autotools? While the autotools of course have all the magic to detect the endianess properly they also seem to emit a check for __BIG_ENDIAN__ which is wrapped into #if defined AC_APPLE_UNIVERSAL_BUILD #endif. These are at least the only examples I can find on my system.

> My proposal is to add the macros and additionally print compiler warnings,
> that the source file is using obsolete macros which should be replaced with
> the __BYTE_ORDER == __ORDER_BIG_ENDIAN__ condition. Does it sound acceptable?

Sorry for being that stubborn but I'm still not convinced. I'm not opposed to defining the macro in the compiler but since we never had that problem in the past I would like to understand what is going wrong now. If we would introduce __BIG_ENDIAN__ for GCC 4.9 and perhaps also 4.8 there would still be many compilers around not defining the macro. And with these the packages then still would not work?! Are all these packages things we never had before in a distro?

Created attachment 882767 [details]
endian-grep.txt

Here's the grep output ...

Some of them are wrapped with the APPLE_UNIVERSAL stuff, but some of them aren't.

And now ... why nobody noticed that yet ... maybe because of missing tests and maybe the set of frequently used components and their features is unaffected by the issue. Maybe the users just do not report bugs ... I don't know.
The thing is, that it currently affects a subset of components and we need to decide what to do.
With other compilers not defining the macros the packages would not work, but they would work at least in our case and the warnings emitted by gcc would encourage maintainers to fix the issue and propagate fixes upstream.

Addition of the two macros seems to me the lesser evil at the moment. And fixing the packages one by one is a long term goal.

Eventually we could do something more aggressive. Maybe we could define the macro in a way that would cause build failures and all packages containing the macros would fail during the mass rebuild. That would force maintainers to fix their packages.

(In reply to Jaromír Cápík from comment #45)
> Some of them are wrapped with the APPLE_UNIVERSAL stuff, but some of them
> aren't.

But others are either only a fall back if the correct test does not work or are wrapped in platform specific macros. In the end there is probably not much left from the list.  But ...

> Addition of the two macros seems to me the lesser evil at the moment. And
> fixing the packages one by one is a long term goal.

... if it really helps some packages which already assume __BIG_ENDIAN__ to be defined it perhaps is the right way to fix it. 

Jakub, what do you think? Should we perhaps just define __BIG_ENDIAN__ on s390 to make these packages work? For 4.9 + backport to 4.8 perhaps?

I'd prefer not to introduce such macro, it is big endian what?  Does it talk about byte order, word order, floating point value order?  Furthermore, it is very confusing with endian.h's BIG_ENDIAN and __BIG_ENDIAN__ which are defined everywhere.

#include <endian.h>
#if BYTE_ORDER == BIG_ENDIAN
(for BSD feature set, or
#include <endian.h>
#if __BYTE_ORDER == __BIG_ENDIAN
otherwise), or with somewhat recent GCC versions
#if  __BYTE_ORDER__ == __ORDER_BIG_ENDIAN__
is what apps should use (or their own configure checks, autoconf has such tests).

BTW, looking at your grep results, some of the packages actually got it right, just try to be portable even to other OSes etc.

libwebp fixed in f20 and f21/rawhide ...

libwebp-0.3.1-3.fc20 has been submitted as an update for Fedora 20.
https://admin.fedoraproject.org/updates/libwebp-0.3.1-3.fc20

Package libwebp-0.3.1-3.fc20:
* should fix your issue,
* was pushed to the Fedora 20 testing repository,
* should be available at your local mirror within two days.
Update it with:
# su -c 'yum update --enablerepo=updates-testing libwebp-0.3.1-3.fc20'
as soon as you are able to.
Please go to the following url:
https://admin.fedoraproject.org/updates/FEDORA-2014-4991/libwebp-0.3.1-3.fc20
then log in and leave karma (feedback).

libwebp-0.3.1-3.fc20 has been pushed to the Fedora 20 stable repository.  If problems still persist, please make note of it in this bug report.