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

Extend precompressed support to read DICOM and write TIFF/OME-TIFF #4190

Merged
merged 14 commits into from
Aug 26, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import loci.formats.FileStitcher;
import loci.formats.FormatException;
import loci.formats.FormatTools;
import loci.formats.ICompressedTileReader;
import loci.formats.IFormatReader;
import loci.formats.IFormatWriter;
import loci.formats.ImageReader;
Expand All @@ -71,7 +72,9 @@
import loci.formats.MetadataTools;
import loci.formats.MinMaxCalculator;
import loci.formats.MissingLibraryException;
import loci.formats.codec.Codec;
import loci.formats.codec.CodecOptions;
import loci.formats.codec.CompressionType;
import loci.formats.codec.JPEG2000CodecOptions;
import loci.formats.gui.Index16ColorModel;
import loci.formats.in.DynamicMetadataOptions;
Expand Down Expand Up @@ -558,6 +561,11 @@ public boolean testConvert(IFormatWriter writer, String[] args)

reader.setId(in);

if (compression == null && precompressed) {
compression = getReaderCodecName();
sbesson marked this conversation as resolved.
Show resolved Hide resolved
LOGGER.info("Implicitly using compression = {}", compression);
}

if (swapOrder != null) {
dimSwapper.swapDimensions(swapOrder);
}
Expand Down Expand Up @@ -759,7 +767,9 @@ else if (writer instanceof ImageWriter) {
int writerSeries = series == -1 ? q : 0;
writer.setSeries(writerSeries);
writer.setResolution(res);

writer.setInterleaved(reader.isInterleaved() && !autoscale);

writer.setValidBitsPerPixel(reader.getBitsPerPixel());
int numImages = writer.canDoStacks() ? reader.getImageCount() : 1;

Expand Down Expand Up @@ -832,6 +842,12 @@ else if (saveTileWidth > 0 && saveTileHeight > 0) {
}
}

if (precompressed && FormatTools.canUsePrecompressedTiles(reader, writer, writer.getSeries(), writer.getResolution())) {
if (getReaderCodecName().startsWith("JPEG")) {
writer.setInterleaved(true);
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is there a reason to special case here or should we ensure that reader.getInterleaved returns true appropriately so that

writer.setInterleaved(reader.isInterleaved() && !autoscale);

works correctly above?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SVSReader was the particular issue here; my concern was that swapping the return value of reader.isInterleaved is a potentially a lot more invasive. When that value changes, what openBytes returns has to change, which means things like pixels hashes change, etc.

}
}

int outputIndex = 0;
if (nextOutputIndex.containsKey(outputName)) {
outputIndex = nextOutputIndex.get(outputName);
Expand Down Expand Up @@ -1260,19 +1276,21 @@ private boolean isTiledWriter(IFormatWriter writer, String outputFile)
private boolean doTileConversion(IFormatWriter writer, String outputFile)
throws FormatException
{
if (writer instanceof DicomWriter ||
(writer instanceof ImageWriter && ((ImageWriter) writer).getWriter(outputFile) instanceof DicomWriter))
// if we asked to try a precompressed conversion,
// then the writer's tile sizes will have been set automatically
// according to the input data
// the conversion must then be performed tile-wise to match the tile sizes,
// even if precompression doesn't end up being possible
if (precompressed) {
return true;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Definitely good to see instanceof checks removed. Are there specific tests that should be carried out here if the target format is neither DICOM not TIFF/OME-TIFF?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See 29e8c05.

It's probably worth a couple of tests, maybe with fake data, for things like ICS. Keep in mind most writers do not allow tile-wise conversion at all (e.g. https://github.com/ome/bioformats/blob/develop/components/formats-bsd/src/loci/formats/out/APNGWriter.java#L84), and DICOM/TIFF/OME-TIFF are the only ones that would support pyramids.

}
// tile size has already been set in the writer,
// so tile-wise conversion should be performed
// independent of image size
if ((writer.getTileSizeX() > 0 && writer.getTileSizeX() < width) ||
(writer.getTileSizeY() > 0 && writer.getTileSizeY() < height))
{
// if we asked to try a precompressed conversion,
// then the writer's tile sizes will have been set automatically
// according to the input data
// the conversion must then be performed tile-wise to match the tile sizes,
// even if precompression doesn't end up being possible
if (precompressed) {
return true;
}
MetadataStore r = reader.getMetadataStore();
return !(r instanceof IPyramidStore) || ((IPyramidStore) r).getResolutionCount(reader.getSeries()) > 1;
return true;
}
return DataTools.safeMultiply64(width, height) >= DataTools.safeMultiply64(4096, 4096) ||
saveTileWidth > 0 || saveTileHeight > 0;
Expand Down Expand Up @@ -1318,6 +1336,18 @@ private void setCodecOptions(IFormatWriter writer) {
}
}

private String getReaderCodecName() throws FormatException, IOException {
if (reader instanceof ICompressedTileReader) {
ICompressedTileReader r = (ICompressedTileReader) reader;
Codec c = r.getTileCodec(0);
CompressionType type = CompressionType.get(c);
if (type != null) {
return type.getCompression();
}
}
return null;
}

// -- Main method --

public static void main(String[] args) throws FormatException, IOException {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,7 @@ default int getTileColumns(int no) {
*
* @param no plane index
* @param x tile X index (indexed from 0, @see getTileColumns(int))
* @param y tile Y index (indexed frmo 0, @see getTileRows(int))
* @param y tile Y index (indexed from 0, @see getTileRows(int))
* @return compressed tile bytes
*/
default byte[] openCompressedBytes(int no, int x, int y) throws FormatException, IOException {
Expand All @@ -79,7 +79,7 @@ default byte[] openCompressedBytes(int no, int x, int y) throws FormatException,
* @param no plane index
* @param buf pre-allocated buffer in which to store compressed bytes
* @param x tile X index (indexed from 0, @see getTileColumns(int))
* @param y tile Y index (indexed frmo 0, @see getTileRows(int))
* @param y tile Y index (indexed from 0, @see getTileRows(int))
* @return compressed tile bytes
*/
default byte[] openCompressedBytes(int no, byte[] buf, int x, int y) throws FormatException, IOException {
Expand All @@ -102,7 +102,7 @@ default Codec getTileCodec(int no) throws FormatException, IOException {
*
* @param no plane index
* @param x tile X index (indexed from 0, @see getTileColumns(int))
* @param y tile Y index (indexed frmo 0, @see getTileRows(int))
* @param y tile Y index (indexed from 0, @see getTileRows(int))
* @return codec options
* @see getTileCodec(int)
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -115,5 +115,27 @@ public int getCode() {
public String getCompression() {
return compression;
}


/**
* Look up the compression type by Codec instance.
*/
public static CompressionType get(Codec c) {
if (c instanceof ZlibCodec) {
return ZLIB;
}
if (c instanceof LZWCodec) {
return LZW;
}
if (c instanceof JPEGCodec) {
return JPEG;
}
if (c instanceof JPEG2000Codec) {
return J2K;
}
if (c instanceof PassthroughCodec) {
return UNCOMPRESSED;
}
return null;
}

}
Loading