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

Refactor opj_j2k_read_ppm & opj_j2k_read_ppt #533

Merged
merged 2 commits into from
Jul 20, 2015

Conversation

mayeut
Copy link
Collaborator

@mayeut mayeut commented Jul 12, 2015

Update #470
Update #288
Update #532

@mayeut
Copy link
Collaborator Author

mayeut commented Jul 12, 2015

Test data already updated in openjpeg-data
All tests are OK under CDash.

@mayeut
Copy link
Collaborator Author

mayeut commented Jul 19, 2015

@detonin, could you have a look at this one and tell me if it's consistent with what's asked in #470

@detonin
Copy link
Contributor

detonin commented Jul 20, 2015

Seems consistent indeed with #470 indeed. Great job.

Before merging, there is some kind of regression issue. I ran test suite and get 5 additional tests succeeding but 9 more failing:
http://my.cdash.org/viewTest.php?onlydelta&buildid=796159
What do you have on your side ?

@mayeut
Copy link
Collaborator Author

mayeut commented Jul 20, 2015

@detonin,

Can you check the version of master branch used to test the pull request ?
I suspect the PNG & TIFF updates are not applied (those tests where updated, see for TIFF in PR #535, for PNG I removed the screen gamma to get consistent conversions - see comment in code).

@detonin
Copy link
Contributor

detonin commented Jul 20, 2015

@mayeut My mistake indeed I forgot to merge master in pr/533 before running the test suite.
http://my.cdash.org/viewTest.php?buildid=796217
Test are ok now (18 failing only) but total time went from 3m29 to 5m11. I did not yet investigate what takes longer but this would be interesting to know.

@mayeut
Copy link
Collaborator Author

mayeut commented Jul 20, 2015

@detonin,
There are much more tests so that seems fair to get an increase.

mayeut added a commit that referenced this pull request Jul 20, 2015
Refactor opj_j2k_read_ppm & opj_j2k_read_ppt
Fixes #470
Fixes #288
Fixes #532
@mayeut mayeut merged commit 28c6f54 into uclouvain:master Jul 20, 2015
@mayeut mayeut deleted the refactor-read-ppX branch July 21, 2015 00:09
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.

None yet

2 participants