-
Notifications
You must be signed in to change notification settings - Fork 8
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
bioformats compatibility #80
Comments
Hi again @FabianHoerst, I tried here with bioformats 7.1.0:
To make:
Then built libdicom, openslide, libvips and vipsdisp from git master. With:
(you can use any of the I see: Aside from the hilariously broken colour, it all seems to work. If I view properties, it seems to have the metadata too: I'll investigate the colour error. |
Thanks for investigating! I will test this on my Ubuntu System, I used the same bf version as you on my Mac. Maybe I should carefully rebuild the OpenSlide library, as I used the release 4.0.0 from October 2023, with which I was able to load other dicom files (e.g., leica proprietary) without any problems. |
It looks like the colour error is an RGB / YCbCr mixup. If you extract the raw JPEG tile from 2_0_4 (the lowest res pyr layer) it displays fine (it uses 4:2:0 YCbCr), but if you get tiles from 2_0_3 (the next larger layer) they are tagged as RGB but actually encoded as 4:4:4 YCbCr. It looks like openslide will need to override the JPEG colourspace with the DICOM one. There's some kind of rounding issue with 2_0_4 as well, it's not seeing all the tiles. I'll keep poking it. |
I think I've found the problem -- Aperio save higher resolution pyramid levels as RGB 4:4:4 (I suppose they want to avoid visible chroma subsampling), but they incorrectly tag these tiles as YCbCr, so when you decode, you see crazy colours. The solution is to override the JPEG colourspace with the photometric interpretation from the enclosing file. openslide do this for SVS images here: https://github.com/openslide/openslide/blob/main/src/openslide-decode-tiff.c#L245-L246 So the DICOM loader will need to do this too to handle dcms which have been derived directly from SVS. I'll make a PR for openslide to fix this. As a workaround, if you ask There's a separate issue with (I think?) a rounding error in the lowest-res pyr level, I'll make another PR for that. |
... this is slightly off-topic, but there's a vipsdisp binary here: https://flathub.org/apps/org.libvips.vipsdisp Which includes openslide and libdicom support. It should be easy to install on ubuntu (just a click). |
JPEG tile colourspace is not always set correctly and may sometimes need to be overridden by the photometric interpretation of the enclosing slide. see ImagingDataCommons/libdicom#80
I made a patch and However, |
Yes, I think Here's the PI it's setting:
Here's a sample tile pulled untouched from the file:
If you examine that JPEG you'll see it's a 4:2:0 YCbCr JPEG. |
... though QuPath manages to display both versions correctly, there's some trick I'm missing. |
Thanks for the hint! Trying to follow up on your discussion on that. I really appreciate your effort! |
... the zoom out error was a problem in vipsdisp, it's working now in git master. I'll look into the colour issue again and see if I can find another way around this. |
I've made some more progress. Here's what's happening:
|
JPEG tile colourspace is not always set correctly and may sometimes need to be overridden by the photometric interpretation of the enclosing slide. see ImagingDataCommons/libdicom#80 Signed-off-by: John Cupitt <jcupitt@gmail.com>
I made a PR and all cases seem to work now. Thanks for reporting this, @FabianHoerst |
JPEG tile colourspace is not always set correctly and may sometimes need to be overridden by the photometric interpretation of the enclosing slide. see ImagingDataCommons/libdicom#80 Signed-off-by: John Cupitt <jcupitt@gmail.com>
JPEG tile colourspace is not always set correctly and may sometimes need to be overridden by the photometric interpretation of the enclosing slide. see ImagingDataCommons/libdicom#80 Signed-off-by: John Cupitt <jcupitt@gmail.com>
Hello @jcupitt, Thanks for your effort!!! Do you think that this could also be an issue with the Aperio files, which they could fix in their scanning software? |
It's a very long-standing issue in SVS, so they probably can't change it without breaking compatibility. For example, openslide patch around this problem in their SVS reader here: https://github.com/openslide/openslide/blob/main/src/openslide-decode-tiff.c#L245-L246 I think it's probably something |
It could be beneficial; I'm considering bringing up this issue with them. Additionally, I've identified some other issues with their conversion process that I plan to address soon (especially when using -precompressed). However, I need to organize public data to help them replicate the issue beforehand. |
That sounds like a useful thing to do. I'd be happy to assist if you need anything. |
JPEG tile colourspace is not always set correctly and may sometimes need to be overridden by the photometric interpretation of the enclosing slide. see ImagingDataCommons/libdicom#80 Signed-off-by: John Cupitt <jcupitt@gmail.com>
JPEG tile colourspace is not always set correctly and may sometimes need to be overridden by the photometric interpretation of the enclosing slide. see ImagingDataCommons/libdicom#80 Signed-off-by: John Cupitt <jcupitt@gmail.com>
The DICOM PhotometricInterpretation element is the authoritative source for the colorspace of a JPEG tile. Usually libjpeg correctly guesses the colorspace, but not for slides with RGB JPEGs converted from Aperio SVS. Use the colorspace from DICOM metadata. See: ImagingDataCommons/libdicom#80 See: openslide#558 Signed-off-by: John Cupitt <jcupitt@gmail.com> Signed-off-by: Benjamin Gilbert <bgilbert@cs.cmu.edu>
bioformats are also working on fixing their side, so I think this is resolved. |
@FabianHoerst reports:
Our recent efforts have been centered around BioFormats. We performed a conversion on one of the CMU files using the following command:
However, when attempting to load the resulting file with OpenSlide, we encountered the following error message:
If you are interested in test files to replicate the error and adapt OpenSlide for the resulting BioFormats DICOM files, I'd be happy to provide them. I believe this could be beneficial as BioFormats currently lacks a Python API.
The text was updated successfully, but these errors were encountered: