Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

opj_write_bytes_BE is wrong in trunk #345

Closed
gcode-importer opened this issue May 11, 2014 · 6 comments
Closed

opj_write_bytes_BE is wrong in trunk #345

gcode-importer opened this issue May 11, 2014 · 6 comments

Comments

@gcode-importer
Copy link

Originally reported on Google Code with ID 345

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)


Reported by szukw000 on 2014-05-11 15:12:11

@gcode-importer
Copy link
Author

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 !

Reported by detonin on 2014-07-09 09:39:46

@gcode-importer
Copy link
Author

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

Reported by szukw000 on 2014-07-09 19:37:02


- _Attachment: [BE-INFO.txt.gz](https://storage.googleapis.com/google-code-attachments/openjpeg/issue-345/comment-2/BE-INFO.txt.gz)_

@gcode-importer
Copy link
Author


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 !

Reported by detonin on 2014-07-10 15:33:16

@gcode-importer
Copy link
Author

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 

Reported by szukw000 on 2014-11-26 05:41:43


- _Attachment: [kodak-orig-cio.j2k](https://storage.googleapis.com/google-code-attachments/openjpeg/issue-345/comment-4/kodak-orig-cio.j2k)_

@gcode-importer
Copy link
Author

The first image has disappeared.

winfried

Reported by szukw000 on 2014-11-26 05:43:00


- _Attachment: [kodak.j2k](https://storage.googleapis.com/google-code-attachments/openjpeg/issue-345/comment-5/kodak.j2k)_

@mayeut
Copy link
Collaborator

mayeut commented Jul 17, 2015

Related to PR #521

@mayeut mayeut closed this as completed in 8c4afef Jul 25, 2015
@mayeut mayeut added this to the OPJ v2.1.1 milestone Aug 27, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

2 participants