troosh / openjpeg

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

[1.5] 16bit compression failure #62

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago
What steps will reproduce the problem?
1. image_to_j2k -i tmp.raw -o tmp.j2k -F 512,512,1,16,u

if I compress using the signed flag, the compression goes well.

tmp.raw is attached here 7zipped.

What is the expected output? What do you see instead?
- codec crashes on freeing 

What version of the product are you using? On what operating system?
1.4, Win7 x64, library built 32bits

Original issue reported on code.google.com by ocla...@gmail.com on 31 Jan 2011 at 8:38

Attachments:

GoogleCodeExporter commented 9 years ago
see this thread : 
https://groups.google.com/d/topic/openjpeg/DLVrRKbTeI0/discussion

Hi,

The problem is the following:
- 'cblk->data' contains the compressed code-stream of a code-block
- In JPEG 2000, a code-block has a maximum of 4096 coefficients
- As a maximum of 16-bits precision is accepted, it seemed enough to developers 
to allocate (4096*2) bytes to store compressed data in a code-block (assuming 
that the compression ratio for a code-block is > 1, which is a good assumption).
- The problem not taken into account is : when converting pixels in the wavelet 
domain, 16-bits precision pixels might become 19-bits precision coefficients. 
- For very hard-to-compress code-blocks, it might therefore lead to buffer 
overflow.

In the present case, the original image is a *signed* image. If we try to 
compress it as an *unsigned* image, the compressor is very inefficient and 
leads to the scenario described above. This explains why it works when 
allocating bigger buffer (actually 19*4096/8 is enough). The blurred image 
described by Winfried is normal as it is originally a *signed* image.

Now the solutions : 
1) Replace '8192' by '9728' when allocating cblk->data in tcd_malloc_encode and 
tcd_init_encode.
2) Initialize the buffer to a size suited for most compression scenarios and 
reallocate it in case we face a potential overflow. Btw, it seems to be done 
that way for decoding images (see t2.c, l571) : 

cblk->data = (unsigned char*) opj_realloc(cblk->data, (cblk->len + seg->newlen) 
* sizeof(unsigned char*));

Solution 2 preferred of course. Solution 1 might be a temporary fix but uses a 
lot more memory.

NB : strangely, boths signed and unsigned scenarios work fine on macosx, 
without any fix.

Cheers,

Antonin

Original comment by anto...@gmail.com on 17 Feb 2011 at 8:08

GoogleCodeExporter commented 9 years ago

Original comment by mathieu.malaterre on 29 May 2012 at 12:44

GoogleCodeExporter commented 9 years ago

Original comment by mathieu.malaterre on 29 May 2012 at 5:12

GoogleCodeExporter commented 9 years ago
antonin the code should now read:

cblk->data = (unsigned char*) opj_realloc(cblk->data, (cblk->len + seg->newlen) 
* sizeof(unsigned char));

see r1322. Make sure your openjpeg copy is up to date.

Original comment by mathieu.malaterre on 29 May 2012 at 5:25

GoogleCodeExporter commented 9 years ago
This is still not fixed in v2.

The malloc is now :
p_code_block->data = (OPJ_BYTE*) opj_malloc(OPJ_J2K_DEFAULT_CBLK_DATA_SIZE);

with
#define OPJ_J2K_DEFAULT_CBLK_DATA_SIZE 8192

Original comment by jf.pamb...@gmail.com on 15 Dec 2013 at 6:21

GoogleCodeExporter commented 9 years ago
My issue was in part cause by the assumption that raw input is big-endian while 
my input file was little-endian. In the incorrect byte order, the wavelet 
transform is extremely inefficient and may require more than 16bits per 
coefficient. With the default codeblock size of 64x64 and buffer size of 8192 
,this may cause buffer overflows.

The documentation should clearly indicate that the raw input format assumes 
big-endian ordering.

Original comment by jf.pamb...@gmail.com on 16 Dec 2013 at 12:29

GoogleCodeExporter commented 9 years ago
This issue was closed by revision r2411.

Original comment by mathieu.malaterre on 25 Feb 2014 at 10:37

GoogleCodeExporter commented 9 years ago

Original comment by mathieu.malaterre on 25 Feb 2014 at 4:05