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

Fix #612 #770

Closed
wants to merge 1 commit into from
Closed

Conversation

julienmalik
Copy link
Collaborator

Works for me under MemorySanitizer

@julienmalik
Copy link
Collaborator Author

Following @boxerab remark in #612 , I'm testing this fix.
It does avoid the uninitialized read.

I don't have much idea about the actual consequences, and about why this test was here in the first place.

So this needs to be validated by the experts :)
ping @detonin @mayeut

@julienmalik
Copy link
Collaborator Author

The test for bpno > 0 dates back to the early days, and appeared in

commit 2ad6a9770a867ddc3ecc31f61a248152b7c6eeac
Author: Sebastien Lugan <[email protected]>
Date:   Thu Nov 27 10:10:17 2003 +0000

    Initial revision

The available history does not go further in the past.

@boxerab
Copy link
Contributor

boxerab commented May 2, 2016

Looks like a 13 year old bug :) Surprising.

@mayeut
Copy link
Collaborator

mayeut commented May 2, 2016

My knowledge doesn't go that far... @detonin, any idea ?

detonin added a commit that referenced this pull request May 3, 2016
RESTART mode is now working, BYPASS still broken but much better
@detonin
Copy link
Contributor

detonin commented May 3, 2016

I've started investigated this bypass and restart problem and created a branch for this: https://github.com/uclouvain/openjpeg/tree/fix-bypass-restart
The whole function t1_encode_cblk would actually need a major refactoring.
For now, the RESTART mode seems to be fixed and the BYPASS mode is behaving better but not lossless yet.
The branch still needs refactoring before merging into master. I'll continue working on it.
ping #612 #674

@detonin
Copy link
Contributor

detonin commented May 3, 2016

More specifically, part of the problem comes from these lines:
https://github.com/uclouvain/openjpeg/blob/fix-bypass-restart/src/lib/openjp2/t1.c#L1689-L1697
that have been introduced in March 2007 (surprising indeed):
5a3c1ff

The FF-bytes management is something quite tricky in JPEG 2000 and the additional arithmetic coder termination brought by the BYPASS and RESTART modes can interfere with the way FF-bytes are usually managed (i.e. removed if they are the last bytes of a coding pass).

@rouault
Copy link
Collaborator

rouault commented Jun 13, 2017

Fixed per #949

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

Successfully merging this pull request may close these issues.

5 participants