search

jai-imageio / jai-imageio-jpeg2000

JPEG2000 support for Java Advanced Imaging Image I/O Tools API
Other
74 stars 56 forks source link

Bug fix in StdEntropyDecoder.java #24

Closed Jugen closed 5 years ago

Jugen commented 6 years ago

setmask = (3<<bp)>>1; works great for unsigned integers however setmask is a signed integer so when bp = 30 the result is negative (-536870912) and thus incorrect. The correct mask result for bit plane 30 should be 1610612736, which is correctly returned by 3<<(bp-1). This bug results in black 'holes' / 'stripes' in the image as shown here on StackOverflow (https://stackoverflow.com/questions/41977536/black-stain-when-extracting-page-to-image-on-pdfbox-2-0-4)

stain commented 6 years ago

Thanks! Could you investigate why the Travis tests fails? Might need an update to .travis.yml as in jai-imageio-core?

Jugen commented 6 years ago

@stain Hi Stian, if the PR "Java 9 support" by rleigh-codelibre is merged then if I'm not mistaken this PR's (and some others as well) Travis' tests will also pass.

THausherr commented 6 years ago

The same commit has been made in a fork https://github.com/faceless2/jpeg2000/commit/b20342956ac5d05bb97522ddeb41e9bed5abfa95 where it was made at two additional places. It would be nice to merge that part and release it. This bug has been a PITA for years.

cdokolas commented 5 years ago

Any plans on a release?

mu-majid commented 4 years ago

Is this fix released in a version? if so what is the version number ?

Lonzak commented 4 years ago

There are a lot of people waiting for this to be included. @stain It would be highly appreciated if you could update the status on this one. Is any more help necessary? Can it be included in the next version? Thanks

Update: Ah I see it was merged. Can you release a new version 1.3.1?

@stain I vote for a 1.3.1 release :-D

stain commented 3 years ago

I later got a merge conflict from this and #26 from @faceless2 - but they seem to give same result, is this still OK?

Their patch do:

int one = 1 << bp
int half = one >> 1
setmask = one | half

.. but changes not just this line but every occurrence of setmask = (3<<bp)>>1

faceless2 commented 3 years ago

(3<<bp)>>1 will sometimes result in an overflow - I changed it everywhere, but I believe it's only been demonstrated in one place, which is presumably where the other patch applied it.

Lonzak commented 3 years ago

@stain Does this answer your question? If not, can you post the merge-conflict file in here? Then we can take a look...

Lonzak commented 3 years ago

Thank you for finally releasing the new version 1.4.0!