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

NDPI: support JPEG-XR and files larger than 4 GB #3505

Merged
merged 14 commits into from
Mar 11, 2020

Conversation

melissalinkert
Copy link
Member

Backported from private PRs.

Based on new (private) documentation, this updates NDPI support to allow JPEG-XR compressed tiles, full support for files larger than 4 GB, and some additional metadata tags.

JPEG-XR support required changing the component of the JPEG-XR codec and service from formats-gpl to formats-bsd. The jxrlib version also needed to be updated to include glencoesoftware/jxrlib#20, since overview images in files that use JPEG-XR compression store channels in BGR order.

Definitely requires a minor release; I would expect memo files to be affected. Requires some configuration changes to NDPI files larger than 4 GB, but otherwise should not fail existing tests.

I suspect, but have not yet thoroughly tested, that this will fix at least a few of the outstanding NDPI issues. To be tested:

CZI stores BGR JPEG-XR tiles, which the codec/service now internally reverses to RGB.
@dgault dgault added this to the 6.4.0 milestone Feb 6, 2020
@dgault dgault added exclude and removed exclude labels Feb 17, 2020
jxrlib appears not to be thread safe when reading images via Decode objects.
This caused intermittent repository test failures.  Until a version of jxrlib
with improved thread safety is available, this commit should restore tests to
a passing state.
@melissalinkert
Copy link
Member Author

melissalinkert commented Feb 27, 2020

311e1c8 is an attempt at fixing the ongoing repo test failures. With it, I can no longer reproduce failures locally, but builds should probably be green for several days in a row before thinking about merging.

I don't like this as a long-term solution, since it will slow down JPEG-XR tile reading especially in use cases like viewing in OMERO. Making jxrlib more thread safe (glencoesoftware/jxrlib#22) is better long-term, but not a quick project.

Another option is to re-add the original JPEGXRServiceImpl decoding logic as a new service method, revert the changes to ZeissCZIReader, and have ZeissCZIReader call the new service method. That's not great either and still not a long-term fix, but would hopefully reduce the scope of the performance impact.

Happy to discuss.

Reverted 311e1c8 in 199705a since it didn't actually fix the issue.

Revert to using Decode.decodeFirstFrame(...) instead of constructing a new Decode object
to get the raw pixels and add metadata parsing to get the pixel type directly.
@melissalinkert
Copy link
Member Author

One more try at fixing the ongoing test failures in 2381e5a.

@dgault
Copy link
Member

dgault commented Mar 11, 2020

Builds and repo tests have been green for a number of days now with the latest commit.

Manually tested using the new and updated files from the config PR. Each opened and displayed without issue and the pixel and metadata looked sensible. The additional tags all look like positive additions and I cannot see them causing any issues with existing filesets.

I also sanity checked some of the CZI files which had been failing in the builds previously. These too opened and displayed without any issues.

Merging for release with 6.4.0

@dgault dgault merged commit 7f5a068 into ome:develop Mar 11, 2020
@melissalinkert melissalinkert deleted the new-ndpi branch May 20, 2021 22:28
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.

2 participants