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

Test failures on big-endian architecture (s390x) #545

Closed
musicinmybrain opened this issue Mar 20, 2022 · 5 comments
Closed

Test failures on big-endian architecture (s390x) #545

musicinmybrain opened this issue Mar 20, 2022 · 5 comments

Comments

@musicinmybrain
Copy link

While trying to update the python-glymur package in Fedora Linux from 0.9.6, I found there are some test failures related to the new tiff2jp2k tool on s390x—Fedora Linux’s sole big-endian architecture.

=========================== short test summary info ============================
FAILED tests/test_tiff2jp2.py::TestSuite::test_minisblack_3strip_to_2x2 - Zer...
FAILED tests/test_tiff2jp2.py::TestSuite::test_partial_last_strip - ZeroDivis...
FAILED tests/test_tiff2jp2.py::TestSuite::test_partial_strip_and_partial_tiles
FAILED tests/test_tiff2jp2.py::TestSuite::test_psnr - AssertionError: False i...
FAILED tests/test_tiff2jp2.py::TestSuiteNoScikitImage::test_rgb_stripped - Ze...
FAILED tests/test_tiff2jp2.py::TestSuiteNoScikitImage::test_rgb_stripped_bottom_of_tile_coincides_with_bottom_of_strip
FAILED tests/test_tiff2jp2.py::TestSuiteNoScikitImage::test_stripped_logging
============= 7 failed, 525 passed, 1 skipped in 168.62s (0:02:48) =============

It looks like, in each of these, rps (read from the RowsPerStrip field), is zero. I think this is because the field is actually 32-bit instead of 16-bit in this test (“LONG”) instead of (“SHORT”), and reading a 16-bit value is getting only the top half of the word on big-endian platforms.

It seems like either type is allowed for this field, and a library should use libtiff’s TIFFFieldDataType to see which is in use.

This should probably be handled automatically and generally in glymur.lib.tiff.getFieldDefaulted(). I’ll probably offer a PR if I have some time to work on it.

@musicinmybrain
Copy link
Author

As a quick hack, I confirmed that hard-coding

TAGS["RowsPerStrip"]["type"] = ctypes.c_uint32

at the end of glymur/lib/tiff.py makes all but one of these tests pass on all platforms. It’s not a correct fix, because RowsPerStrip could be 16-bit, but it helps confirm the diagnosis.

The exception is tests/test_tiff2jp2.py::TestSuite::test_psnr, which appears to be failing due to a separate issue that I haven’t looked into at all.

@quintusdias
Copy link
Owner

Thanks for the report. I'll get to work on it.

@musicinmybrain
Copy link
Author

Thanks! Please let me know if I can do anything to help.

quintusdias added a commit that referenced this issue Mar 23, 2022
This API call can be problematic if the tag has more that one possible
datatype on a platform that is hard to test, say a big endian platform.
Get around this by using the IFD as read via programming to the TIFF
spec.

Refactor, pass image width, height to readRGBAImage instead of using
TIFFGetFieldDefaulted.

Closes: #545
@quintusdias
Copy link
Owner

Using TIFFFieldDatatype is tougher than it looks. I elected on doing this another way, and just bypass LibTIFF when retrieving the tag values. I already had a platform-independent way of getting all the tags for the purposes of creating a UUID box to preserve the original value of all the tags, and that should work here as well.

@musicinmybrain
Copy link
Author

Thanks! This seems logical, and I can confirm that it fixes this bug. I still have one test failure on s390x, which I’ll report separately since it seems to be unrelated.

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

No branches or pull requests

2 participants