opj_write_bytes_BE is wrong in trunk

[GoogleCodeExporter]

The attached patch shows what is wrong with the BE code.

The original code goes from the address of p_value p_nb_bytes to
the right. From there it copies p_nb_bytes into the buffer.

The patched code goes from the address of p_value sizeof(OPJ_UINT32)
to the right and from there p_nb_bytes back. From there it copies
p_nb_bytes into the buffer.

The patched code has been proved to be correct on a G5 machine
working with Debian ppc64 code.

winfried

--- cio.c.orig  2014-04-26 01:12:48.653018821 +0000
+++ cio.c   2014-04-26 02:48:24.823047287 +0000
@@ -46,11 +46,11 @@

 void opj_write_bytes_BE (OPJ_BYTE * p_buffer, OPJ_UINT32 p_value, OPJ_UINT32 p_nb_bytes)
 {
-   const OPJ_BYTE * l_data_ptr = ((const OPJ_BYTE *) &p_value) + p_nb_bytes;
+   const OPJ_BYTE * l_data_ptr = ((const OPJ_BYTE *) &p_value);

    assert(p_nb_bytes > 0 && p_nb_bytes <=  sizeof(OPJ_UINT32));

-   memcpy(p_buffer,l_data_ptr,p_nb_bytes);
+   memcpy(p_buffer,l_data_ptr+sizeof(OPJ_UINT32)-p_nb_bytes,p_nb_bytes);
 }

 void opj_write_bytes_LE (OPJ_BYTE * p_buffer, OPJ_UINT32 p_value, OPJ_UINT32 p_nb_bytes)

Original issue reported on code.google.com by szukw...@arcor.de on 11 May 2014 at 3:12

[GoogleCodeExporter]

On our dashboard (http://my.cdash.org/index.php?project=OPENJPEG), the machine 
"macminig4" is big endian and succeed on a vast majority of tests, which would 
not be the case I guess if a central function as opj_writes_bytes would be 
wrong.

Could you provide a test (image + commandline) to be run on a BE machine that 
would demonstrate the issue you pointed out ?

Many thanks !

Original comment by antonin on 9 Jul 2014 at 9:39

[GoogleCodeExporter]

The files you have used are (supposed to be) 8-Bit images.

The difference can be found in 16-Bit images.

You can make a test patched/unpatched with

  data/input/conformance/file7.jp2

-------------------------------------------------------
file7.jp2 is a 16-Bit image with ICC and 3 components:

read_ihdr
    w(480) h(640) nc(3) bpc(15)
    signed(0) depth(16)
    compress(7) unknown_c(0) ipr(0)

-------------------------------------------------------
I have used an RGBA image for the test.

rgba-16bit.png is a 16-Bit image with 4 components.

p-rgba-16bit.png.jp2 has been created with the cio-patch:

read_ihdr
    w(300) h(300) nc(4) bpc(15)
    signed(0) depth(16)
    compress(7) unknown_c(0) ipr(0)

The attached info shows the difference between the unpatched
cio.c and the patched cio.c:

No 16-Bit image without the patch can be read both on a
BIG_ENDIAN and a LITTLE_ENDIAN machine.

There is another 16-Bit patch for TIFF in issue338.

winfried

Original comment by szukw...@arcor.de on 9 Jul 2014 at 7:37

Attachments:

• BE-INFO.txt.gz

[GoogleCodeExporter]

There are several tests with images > 8 bits, and they are successful on LE 
machines so I guess saying that no 16-bit image can be read on a LE machine is 
not correct.

As far as BE (and macminig4) is concerned, there are indeed strange behaviours 
in our tests and Mathieu will have a look at it (as owner of the macminig4 
machine). Thanks for having pointed this out !

Original comment by antonin on 10 Jul 2014 at 3:33

[GoogleCodeExporter]

Here is the difference. The KODAK image 'issue135-test-12-KODAK.j2c' is read
and then written. First with the changed cio.c code (kodak.j2k), then with
the original cio.c code (kodak-orig-cio.j2k).

I have chosen the KODAK image because on an LE machine, the image with 2 layers
is completely broken, with 1 layer it is OK.

On the BE machine the KODAK image has two broken speckles with layers 2, it is
OK with reduction 1 .. 5.

winfried 

Original comment by szukw...@arcor.de on 26 Nov 2014 at 5:41

Attachments:

• kodak-orig-cio.j2k

[GoogleCodeExporter]

The first image has disappeared.

winfried

Original comment by szukw...@arcor.de on 26 Nov 2014 at 5:43

Attachments:

• kodak.j2k