openpreserve / jpylyzer

JP2 (JPEG 2000 Part 1) validator and properties extractor. Jpylyzer was specifically created to check that a JP2 file really conforms to the format's specifications. Additionally jpylyzer is able to extract technical characteristics.
http://jpylyzer.openpreservation.org/
Other
69 stars 28 forks source link

Calculation of numberOfTilesX and numberOfTilesY subtracts xOsiz/yOsiz instead of xTOsiz/yTOsiz #221

Closed bitsgalore closed 6 months ago

bitsgalore commented 7 months ago

As described here by @comstock and @aous72:

https://github.com/aous72/OpenJPH/issues/113#issuecomment-1806939351

Some of the files (both JPH and codestream) fail validation with:

    <tests>
        <foundExpectedNumberOfTiles>False</foundExpectedNumberOfTiles>
        <foundExpectedNumberOfTileParts>False</foundExpectedNumberOfTileParts>
    </tests>

TODO: figure out whether this is a Jpylyzer bug or a real fault of the images.

bitsgalore commented 7 months ago

Looks like a fault of the images, not Jpylyzer, see:

https://github.com/aous72/OpenJPH/issues/113#issuecomment-1808513743

aous72 commented 7 months ago

Hi Johan,

Thank you for looking into this -- it always useful to confirm against other peoples' code. I apologize for not having the time to have a more detailed report when I first looked at it.

With that said, -- please correct me if I am wrong -- the image simple_enc_irv97_tall_narrow1.j2c has TWO tiles.
Please refer to equation B-5 of the standard. This also explains the tpsot and tnsot values.

Thank you.

Kind regards, Aous.

bitsgalore commented 7 months ago

I think you're right - looking at the code again:

https://github.com/openpreserve/jpylyzer/blob/b5d4d979327bce09de17b4a5f013689b0a569aaa/jpylyzer/boxvalidator.py#L1924

Here I see:

        numberOfTilesX = math.ceil((xsiz - xOsiz) / xTsiz)
        numberOfTilesY = math.ceil((ysiz - yOsiz) / yTsiz)

But of course this should be:

          numberOfTilesX = math.ceil((xsiz - xTOsiz) / xTsiz)
          numberOfTilesY = math.ceil((ysiz - yTOsiz) / yTsiz)

Will correct this in the stable release.

Thanks again for reporting this, and apologies for any confusion caused by my initial response!

aous72 commented 7 months ago

Thank you Johan.

bitsgalore commented 6 months ago

Fixed: https://github.com/openpreserve/jpylyzer/commit/1d47452d68099c480a1de95104cb4c8d5eba999c Also added file that triggered this to test corpus + added test: https://github.com/openpreserve/jpylyzer/commit/fc1a2d09a384cdea9397f4d78f9804a7bd9f70be