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

Correctly decode files with incorrect tile-part header fields (TPsot==TNsot) #514

Merged
merged 6 commits into from
Jul 3, 2015

Conversation

mayeut
Copy link
Collaborator

@mayeut mayeut commented Jun 19, 2015

fixes #254

@mayeut
Copy link
Collaborator Author

mayeut commented Jun 19, 2015

@detonin ,

All tests OK (see #254 for details). Given all comments in #254, I think it's OK to merge.
I can add a build option (Not to change API/ABI at all) do disable/enable the check.
I can add a new function to enable/disable the check.
If any of the above are needed, we shall choose what's the default value

@szukw000
Copy link
Contributor

This patch for issue254 has a drawback. I downloaded openjpeg-master on 2015-06-23 and found
that my viewer showed a correct JP2 image for the respective image.

But the decompressed images (PNG, BMP, etc) are all unsharp and washed out as the 'original' JP2
image.

The last patch from mdarbois was OK: JP2 and decompressed images are all sharp.

winfried

@detonin
Copy link
Contributor

detonin commented Jul 3, 2015

@mayeut Thanks for the PR. For now, I propose you add a build option which would be enabled by default. Once we change API/ABI, I suggest to add a commandline option, such as "--robust" or sth like that to enable robustness against corrupted codestreams (at a price of small performance decrease). By the way, this PR triggers 4 additional MD5 failing tests. Corresponding MD5 files shall be updated accordingly ... Could you also take care of that ? Many thanks;

@szukw000 Could you give a link to the corresponding JP2 image ?

@szukw000
Copy link
Contributor

szukw000 commented Jul 3, 2015

The file:

data/input/nonregression/issue254.jp2

Broken opj_decompress in:

openjpeg-git-2015-06-29

Perfect opj_decompress in:

openjpeg-2.x-trunk-r3007 patched with:

For those interested,
Updated patch for r3007

Reported by mayeut on 2015-06-05 19:03:36

issue254-r3007.patch

winfried

@szukw000
Copy link
Contributor

szukw000 commented Jul 3, 2015

In openjpeg-git-2015-06-29 I have now included 'flviewer'. And found that the
file 'issue254.jp2' is broken as before: unsharp and washed out.

That my viewer shows a sharp JP2 image does have a simple reason: the viewer
uses the 'issue254-r3007.patch'.

Does the above mentioned fix really fix the cyrillic image of issue254?

winfried

@mayeut
Copy link
Collaborator Author

mayeut commented Jul 3, 2015

@szukw000 ,

The issue254-r3007.patch is this PR. It was created by a git diff between master (synced to r3007 thanks to @detonin ) & my issue-254 branch.
I'll make a final check with the up to date master before committing but I don't see why it wouldn't work as expected.
Here's the image I get so far (which is sharp & the correct one I think)
issue254-fix

@mayeut
Copy link
Collaborator Author

mayeut commented Jul 3, 2015

@detonin
1 test less failing after this PR (MD5 for issue254 was wrong previously).
That's 26 tests failing on my setup. When disabling the fix, goes up to 31 (4 already mentioned in #254 + issue254 MD5).

mayeut added a commit that referenced this pull request Jul 3, 2015
Correctly decode files with incorrect tile-part header fields (TPsot==TNsot)
Fixes #254
@mayeut mayeut merged commit 237ddd7 into uclouvain:master Jul 3, 2015
@mayeut mayeut deleted the issue-254 branch July 3, 2015 22:53
@szukw000
Copy link
Contributor

szukw000 commented Jul 4, 2015

@mayeut ,
yes: the patch for r3007 is correct and perfect. I misunderstood the first comment
on this page and thought that there was another, later patch.

winfried

@rouault
Copy link
Collaborator

rouault commented Nov 13, 2024

Behavior of this PR will be partly changed by #1560

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Loss decoding quality in 2.0.0
4 participants