Fixes for conformance testing

[osamu620]

Some code fixes have been made for the conformance testing defined in ISO/IEC 15444-4.

Motivation:

• Conformance testing results of jasper 2.0.19.xlsx shows that the current version of JasPer does not conform to the JPEG 2000 conformance testing defined as ISO/IEC 15444-4.

Description:

3204e67

• Why: If the LSB of the Sqcd or Sqcc in the QCD or QCC marker is equal to "1" , this incorrect calculation of derived stepsizes produces wrongly reconstructed sample values.
• How: The calculation of derived stepsizes in calcstepsizes() has been fixed according to the JPEG 2000 spec defined as the ISO/IEC 15444-1.

1c220a1

• Why: If the DWT transform differs among components, the information of DWT transform used for each component should be found as the COC marker segment. This information is wrongly used for dequantization and rounding in JasPer.
• How: Fixed dequantization and rounding in jpc_dec_tiledecode() to use ccp->qmfbid which has the information of DWT transform.

cc814c9

• Why: If a tile has multiple tile-parts and all the tile-parts have the TNsot (a parameter in the tile-part header) = 0, such a tile is never decoded.
• How: Modified the condition whether jpc_dec_tiledecode() shall be invoked or not in jpc_dec_process_eoc()

2004078

• Why: The condition for edge detection of precincts in jpc_pi_nextrpcl is wrong. (For example the p1_07 codestream used for the conformance contains first the packet for component 1, resolution 0, component 0, position (4,0) before the packet for component 0, resolution 0, component 0, position (8,0), but JasPer reads them in the wrong order.)
• How: Fixed the condition according to the JPEG 2000 spec defined as the ISO/IEC 15444-1.

55562ce

• Why: In the JPEG 2000 spec, the reconstruction parameter is introduced in the Annex E (Quantization). This reconstruction parameter can be arbitrarily chosen by the decoder, however, it is important to produce the best visual or objective quality for reconstruction. Generally, values for this parameter fall in the range [0, 1) and a common value is 0.5.
• How: Added new variable as the reconstruction parameter in jpc_dequantize() and set the value to 0.5.

Results:

• Conformance testing results of jasper 2.0.19_PR.xlsx
• It is confirmed that PAE (Peak Absolute Error) of decoded images are under allowable values in the spec (except for p1_03.)
• MSE values are still over allowable values for some test codestreams. Please consider changing the number of fractional bits(JPC_FIX_FRACBITS) from 13 to 15.
  Note - It has been confirmed that the PAE for p1_03 is under the allowable value with JPC_FIX_FRACBITS = 15.

[MaxKellermann]

There's now a lot of text in this PR, but the commit messages are still very short one-liners without any explanations.
Always remember: PR texts are only in GitHub's proprietary database and will be lost after merging. Only commit messages are available to anybody!

[osamu620]

There's now a lot of text in this PR, but the commit messages are still very short one-liners without any explanations.

Always remember: PR texts are only in GitHub's proprietary database and will be lost after merging. Only commit messages are available to anybody!

I got what you meant. Actually, I’m not familiar with git. Do you think the commit messages should be modified?

[MaxKellermann]

Do you think the commit messages should be modified?

Yes. True, this is advanced git usage - the tool of my choice is "stgit" (http://www.procode.org/stgit/doc/tutorial.html) - it lets me re-edit several patches-in-progress easily, and I can go back and forth in the patch series.

I'll try to review your changes (your explanations are there), but before merging your code, I think it's important to preserve your text in commit messages. Commit messages are important, even more so for people less familiar with the JPEG2000 standard than you. If I ever find problems in your code, it will be vital to be able to find your comments in the git history.

[jubalh]

Maybe this helps: https://gist.github.com/robertpainsi/b632364184e70900af4ab688decf6f53

Usually commit messages are written (like yours) with a subject. In case of your first commit this is Fix incorrect calculation of derived stepsizes. Which is fine.
After that you have a blank line and then in the "commit body" one can explain why the change is needed and other details.

So that would be:

    Why: If the LSB of the Sqcd or Sqcc in the QCD or QCC marker is equal to "1" , this incorrect calculation of derived stepsizes produces wrongly reconstructed sample values.
    How: The calculation of derived stepsizes in calcstepsizes() has been fixed according to the JPEG 2000 spec defined as the ISO/IEC 15444-1.

The advantage is that if GitHub goes down, or the repository is migrated or someone is offline (etc) one can still read all relevant details in the git commit messages using git log (for example).

So if you want to add your detailed messages you can use git rebase to do so.

git rebase -i HEAD~5

If you run this the first word will be pick you can change that to reword and save. Then your editor will pop open a couple of times so you rewrite the messages.

After that you need to force push to your branch osamu620:hotfix1-2.0.19

[MaxKellermann]

Question about "Fix incorrect calculation of derived stepsizes": which section of the standard describes the corrected formula? (The JasPer source code should really be full of code comments referring to the standard's section numbers!)

[osamu620]

Question about "Fix incorrect calculation of derived stepsizes": which section of the standard describes the corrected formula? (The JasPer source code should really be full of code comments referring to the standard's section numbers!)

The correct formula is written in Section E.1.1.1 of Annex E. (as Equation (E-5)) I'll put this information to the code.

[osamu620]

@MaxKellermann

Answers to your questions made on the commits before force pushing are here.

e45a7fe#r41003970

f0ecbe3#r41004133

[osamu620]

@MaxKellermann @jubalh I modified all the commit messages.
Inspired by @MaxKellermann 's question (e45a7fe#r41003970), I made edits on 50555b4. You can see those edits at 1c220a1.

[osamu620]

Maybe this helps: https://gist.github.com/robertpainsi/b632364184e70900af4ab688decf6f53

Usually commit messages are written (like yours) with a subject. In case of your first commit this is Fix incorrect calculation of derived stepsizes. Which is fine.
After that you have a blank line and then in the "commit body" one can explain why the change is needed and other details.

So that would be:

    Why: If the LSB of the Sqcd or Sqcc in the QCD or QCC marker is equal to "1" , this incorrect calculation of derived stepsizes produces wrongly reconstructed sample values.
    How: The calculation of derived stepsizes in calcstepsizes() has been fixed according to the JPEG 2000 spec defined as the ISO/IEC 15444-1.

The advantage is that if GitHub goes down, or the repository is migrated or someone is offline (etc) one can still read all relevant details in the git commit messages using git log (for example).

So if you want to add your detailed messages you can use git rebase to do so.

git rebase -i HEAD~5

If you run this the first word will be pick you can change that to reword and save. Then your editor will pop open a couple of times so you rewrite the messages.

After that you need to force push to your branch osamu620:hotfix1-2.0.19

Thank you for the guidance. I think the commits have been appropriately modified (less confidence)

[MaxKellermann]

Thanks @osamu620, I have spent a few hours today verifying your work (the JPC_TILE_ACTIVELAST change and the JasPer state machine), and everything looks quite right (I'm not an expert on the JPEG2000 spec, mostly I've been checking generic code stuff).
I wish the JasPer code were easier to read. State machines like the one in JasPer are hard to grasp and hard to verify. (While trying to understand it, I found yet another memory leak, which I'll submit a fix for later today.)

[osamu620]

Thanks @osamu620, I have spent a few hours today verifying your work (the JPC_TILE_ACTIVELAST change and the JasPer state machine), and everything looks quite right (I'm not an expert on the JPEG2000 spec, mostly I've been checking generic code stuff).

Thanks for your great effort.

I wish the JasPer code were easier to read. State machines like the one in JasPer are hard to grasp and hard to verify. (While trying to understand it, I found yet another memory leak, which I'll submit a fix for later today.)

I agree. I think every variable which directly corresponds to the standard should be noted with the appropriate part of the spec. Ideally, we should make our "consensus" to do so.

[mdadams]

Question about "Fix incorrect calculation of derived stepsizes": which section of the standard describes the corrected formula? (The JasPer source code should really be full of code comments referring to the standard's section numbers!)

The correct formula is written in Section E.1.1.1 of Annex E. (as Equation (E-5)) I'll put this information to the code.

@osamu620 Would it be possible to provide a test case (or two) that we could add to the JasPer test suite that would catch the quantization/dequantization bug that you found? Or, if you can suggest the tightening of any of the PSNR thresholds used in some of our existing test cases that would have caught this bug, this might be better (as this would allow existing test images to be used). This type of bug would be a good thing to be able to catch via one (or more) test cases. (If you are able to provide a test case and you use an image not already used for testing in JasPer, please ensure that it does not pose any copyright issues with respect to image ownership.)

[osamu620]

@osamu620 Would it be possible to provide a test case (or two) that we could add to the JasPer test suite that would catch the quantization/dequantization bug that you found? Or, if you can suggest the tightening of any of the PSNR thresholds used in some of our existing test cases that would have caught this bug, this might be better (as this would allow existing test images to be used). This type of bug would be a good thing to be able to catch via one (or more) test cases. (If you are able to provide a test case and you use an image not already used for testing in JasPer, please ensure that it does not pose any copyright issues with respect to image ownership.)

@mdadams I'll look the codes of JasPer test suite and will provide some solutions. (I’m afraid that I’m not familiar with the JasPer test suites.)

[mdadams]

@osamu620 The tests that would be of interest in your case are the ones driven off the file called test/bin/codec_tests. This file has a record for each test. The important thing is that these tests can specify a threshold on the PSNR (or PAE). If there is a bug in the quantization/dequantization, then the fact that all of the tests with PSNR thresholds are passing suggests that some of these thresholds are set too low (so they are too easily satisfied). For example, one record in codec_tests reads as follows:
/******************************************************************************\

• Basic Lossy Compression
  ******************************************************************************/

/* grayscale image, irreversible 9/7 WT */
BEGIN id=lossy_0 image=stawamuschief_gray.pnm mode=real rate=0.05 numrlvls=5 psnr=28.0

The above entry says that the image stawamuschief_gray.pnm is to be encoded with mode=real (9/7 wavelet transform) at a rate of 0.05 with 5 resolution levels and then the result is decoded and the PSNR is checked to be at least 28.0 dB. If this PSNR requirement is not met, the test fails.

If there is a bug in quantization/dequanitzation, the some of the "psnr=" thresholds must be set too low. Does that help?

Incidentally, the test images are in the directory data/images.