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

Allow reading and writing compressed tiles #3992

Merged
merged 30 commits into from
Dec 1, 2023
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
30 commits
Select commit Hold shift + click to select a range
ec17e0d
First draft of pre-compressed tile API
melissalinkert Apr 24, 2023
082b95a
SVS: initial pre-compressed tile API implementation
melissalinkert Apr 25, 2023
6c06e96
Add methods to get the number of tiles in X and Y
melissalinkert Apr 25, 2023
5c06784
Add documentation to ICompressedTileReader
melissalinkert Apr 25, 2023
f2cc370
Add example of working with pre-compressed tile API
melissalinkert Apr 25, 2023
6f676d9
Override ICompressedTileReader methods in ImageReader
melissalinkert Apr 25, 2023
a67df0f
First draft of pre-compressed tile writing API
melissalinkert Apr 25, 2023
fde8ea3
Initial implementation of compressed tile saving in DICOM
melissalinkert Apr 25, 2023
b433c11
Rough example of converting pre-compressed tiles
melissalinkert Apr 25, 2023
5d79b3e
Fix pre-compressed conversion example to correctly represent pyramids
melissalinkert Apr 26, 2023
0628584
bfconvert: add -precompressed option
melissalinkert Apr 26, 2023
ffca24b
SVS: fix compressed JPEG tile reading
melissalinkert Apr 30, 2023
f5496ca
Add helper method to check if precompressed tiles can be used
melissalinkert Apr 30, 2023
d75c969
Fix handling of variable tile sizes between pyramid resolutions
melissalinkert Apr 30, 2023
4032d57
Update DICOM/TIFF IFD when writing precompressed bytes
melissalinkert Apr 30, 2023
6ddf262
Warn if precompressed tiles are not used when doing a full-plane conv…
melissalinkert Apr 30, 2023
d9d8d79
Update precompressed tile conversion example
melissalinkert May 1, 2023
deb1c46
Merge branch 'develop' of github.com:openmicroscopy/bioformats into p…
melissalinkert May 1, 2023
5539188
Fix artifact uploading in GitHub Actions Ant workflow
melissalinkert May 1, 2023
e0b97a1
Merge branch 'develop' of github.com:openmicroscopy/bioformats into p…
melissalinkert May 10, 2023
5b67096
Update DicomWriter to accept JPEG-2000 tiles
melissalinkert May 10, 2023
6dcfbb6
Merge branch 'develop' of github.com:openmicroscopy/bioformats into p…
melissalinkert Jun 16, 2023
a50d09c
SVS: don't add JPEG table to zero-length compressed tiles
melissalinkert Jun 30, 2023
a7a2fea
DICOM writer: create blank tile if a zero-length compressed tile is p…
melissalinkert Jun 30, 2023
af837bf
Merge branch 'develop' of github.com:openmicroscopy/bioformats into p…
melissalinkert Aug 2, 2023
2b1d661
DICOM writer: use 256x256 as the default tile size if none was specified
melissalinkert Nov 28, 2023
3b63973
bfconvert: don't set a 0x0 tile size
melissalinkert Nov 28, 2023
cd6cf1e
If a whole resolution cannot use precompressed tiles, warn just once
melissalinkert Nov 28, 2023
30f1c3d
Compare reader and writer codecs when determining if precompression i…
melissalinkert Nov 28, 2023
0049264
Throw an exception if -autoscale is used with -precompressed
melissalinkert Nov 29, 2023
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 @@ -129,6 +129,8 @@ public final class ImageConverter {
private boolean noSequential = false;
private String swapOrder = null;
private Byte fillColor = null;
private boolean precompressed = false;
private boolean tryPrecompressed = false;

private IFormatReader reader;
private MinMaxCalculator minMax;
Expand Down Expand Up @@ -179,6 +181,7 @@ private boolean parseArgs(String[] args) {
else if (args[i].equals("-padded")) zeroPadding = true;
else if (args[i].equals("-noflat")) flat = false;
else if (args[i].equals("-no-sas")) originalMetadata = false;
else if (args[i].equals("-precompressed")) precompressed = true;
else if (args[i].equals("-cache")) useMemoizer = true;
else if (args[i].equals("-cache-dir")) {
cacheDir = args[++i];
Expand Down Expand Up @@ -340,6 +343,7 @@ private void printUsage() {
" [-option key value] [-novalid] [-validate] [-tilex tileSizeX]",
" [-tiley tileSizeY] [-pyramid-scale scale]",
" [-swap dimensionsOrderString] [-fill color]",
" [-precompressed]",
" [-pyramid-resolutions numResolutionLevels] in_file out_file",
"",
" -version: print the library version and exit",
Expand Down Expand Up @@ -384,6 +388,10 @@ private void printUsage() {
" -no-sequential: do not assume that planes are written in sequential order",
" -swap: override the default input dimension order; argument is f.i. XYCTZ",
" -fill: byte value to use for undefined pixels (0-255)",
" -precompressed: transfer compressed bytes from input dataset directly to output.",
" Most input and output formats do not support this option.",
Copy link
Member

Choose a reason for hiding this comment

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

Two quick brainstormy thoughts:

  • I could see an option to formatlist that would print out a table of which of these interfaces is supported by each format.
  • It would be interesting to see (OME-)TIFF as another supported writer format for this to have an N>2.

Copy link
Member Author

Choose a reason for hiding this comment

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

Agree these would be nice targets for future work, but are well outside the scope of this PR and not realistic for 7.1.0. OK to open new issues for each, so that we can schedule for future releases?

" Do not use -crop, -fill, or -autoscale, or pyramid generation options",
sbesson marked this conversation as resolved.
Show resolved Hide resolved
" with this option.",
"",
"The extension of the output file specifies the file format to use",
"for the conversion. The list of available formats and extensions is:",
Expand Down Expand Up @@ -455,6 +463,18 @@ public boolean testConvert(IFormatWriter writer, String[] args)
return false;
}

// TODO: there may be other options not compatible with -precompressed
if (precompressed &&
(width_crop > 0 || height_crop > 0 ||
pyramidResolutions > 1 ||
fillColor != null ||
autoscale
))
{
throw new UnsupportedOperationException("-precompressed not supported with " +
"-autoscale, -crop, -fill, -pyramid-scale, -pyramid-resolutions");
}

CommandLineTools.runUpgradeCheck(args);

if (new Location(out).exists()) {
Expand Down Expand Up @@ -712,8 +732,14 @@ else if (w instanceof DicomWriter) {

total += numImages;

writer.setTileSizeX(saveTileWidth);
writer.setTileSizeY(saveTileHeight);
if (precompressed) {
writer.setTileSizeX(reader.getOptimalTileWidth());
writer.setTileSizeY(reader.getOptimalTileHeight());
}
else if (saveTileWidth > 0 && saveTileHeight > 0) {
writer.setTileSizeX(saveTileWidth);
writer.setTileSizeY(saveTileHeight);
}

int count = 0;
for (int i=startPlane; i<endPlane; i++) {
Expand Down Expand Up @@ -834,13 +860,28 @@ private long convertPlane(IFormatWriter writer, int index, int outputIndex,
}
}

tryPrecompressed = precompressed && FormatTools.canUsePrecompressedTiles(reader, writer, writer.getSeries(), writer.getResolution());

byte[] buf = getTile(reader, writer.getResolution(), index,
xCoordinate, yCoordinate, width, height);

// if we asked for precompressed tiles, but that wasn't possible,
// then log that decompression/recompression happened
// TODO: decide if an exception is better here?
Copy link
Member

Choose a reason for hiding this comment

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

At least in my testing of the CMU-1.svs sample file from OpenSlide, there was only plane where this was an issue. I don't think throwing an exception is a better alternative since it might significantly restrict the usage of this feature and might happen quite late in the conversion as well which is frustrating for the end-user.

Additionally, there is still some significant gain associated to using precompressed tiles for the highest resolutions even if the lowest resolutions are decompressed.

Copy link
Member

Choose a reason for hiding this comment

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

At the risk of adding too much complexity, I could see having an option (flag or -option) to also provide whichever of warning or error is not the default.

Copy link
Member

Choose a reason for hiding this comment

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

I think for now having it logged is a better option than throwing the exception

if (precompressed && !tryPrecompressed) {
LOGGER.warn("Decompressed tile: series={}, resolution={}, x={}, y={}",
writer.getSeries(), writer.getResolution(), xCoordinate, yCoordinate);
}

autoscalePlane(buf, index);
applyLUT(writer);
long m = System.currentTimeMillis();
writer.saveBytes(outputIndex, buf);
if (tryPrecompressed) {
writer.saveCompressedBytes(outputIndex, buf, 0, 0, reader.getSizeX(), reader.getSizeY());
}
else {
writer.saveBytes(outputIndex, buf);
}
return m;
}

Expand Down Expand Up @@ -886,16 +927,37 @@ private long convertTilePlane(IFormatWriter writer, int index, int outputIndex,
nYTiles++;
}

// only warn once if the whole resolution will need to
// be decompressed and recompressed
boolean canPrecompressResolution = precompressed && FormatTools.canUsePrecompressedTiles(reader, writer, writer.getSeries(), writer.getResolution());
if (precompressed && !canPrecompressResolution) {
LOGGER.warn("Decompressing resolution: series={}, resolution={}",
writer.getSeries(), writer.getResolution());
tryPrecompressed = false;
}

Long m = null;
for (int y=0; y<nYTiles; y++) {
for (int x=0; x<nXTiles; x++) {
int tileX = xCoordinate + x * w;
int tileY = yCoordinate + y * h;
int tileWidth = x < nXTiles - 1 ? w : width - (w * x);
int tileHeight = y < nYTiles - 1 ? h : height - (h * y);

tryPrecompressed = precompressed && canPrecompressResolution &&
FormatTools.canUsePrecompressedTiles(reader, writer, writer.getSeries(), writer.getResolution());
byte[] buf = getTile(reader, writer.getResolution(),
index, tileX, tileY, tileWidth, tileHeight);

// if we asked for precompressed tiles, but that wasn't possible,
// then log that decompression/recompression happened
// this is mainly expected for edge tiles, which might be smaller than expected
// TODO: decide if an exception is better here?
if (precompressed && canPrecompressResolution && !tryPrecompressed) {
LOGGER.warn("Decompressed tile: series={}, resolution={}, x={}, y={}",
writer.getSeries(), writer.getResolution(), x, y);
Copy link
Member

Choose a reason for hiding this comment

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

This warning message is currently written for every single tile of the resolution but it applies to the entire resolution. Would it make sense to compute and display it outside the loop?
Otherwise we might want to align the display with the rest of the utility as this reads a bit confusing if you don't know what is happening

Tile size = 256 x 256
Decompressed tile: series=0, resolution=3, x=0, y=0
Decompressed tile: series=0, resolution=3, x=1, y=0
Decompressed tile: series=0, resolution=3, x=2, y=0
Decompressed tile: series=0, resolution=3, x=3, y=0
Decompressed tile: series=0, resolution=3, x=0, y=1
Decompressed tile: series=0, resolution=3, x=1, y=1
Decompressed tile: series=0, resolution=3, x=2, y=1
Decompressed tile: series=0, resolution=3, x=3, y=1
Decompressed tile: series=0, resolution=3, x=0, y=2
Decompressed tile: series=0, resolution=3, x=1, y=2
Decompressed tile: series=0, resolution=3, x=2, y=2
Decompressed tile: series=0, resolution=3, x=3, y=2
	Series 0: converted 1/1 planes (100%)

Copy link
Member Author

Choose a reason for hiding this comment

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

If the whole resolution needs to be decompressed/recompressed, this should now only warn once. If some of the tiles in the resolution work with -precompressed and some do not, it will still warn once for each tile that doesn't work. Mostly I'd expect that to happen with right and bottom edge tiles, which could be smaller and need to be padded to the correct size.

}

String tileName =
FormatTools.getTileFilename(x, y, y * nXTiles + x, currentFile);
if (!currentFile.equals(tileName)) {
Expand Down Expand Up @@ -957,18 +1019,8 @@ private long convertTilePlane(IFormatWriter writer, int index, int outputIndex,
outputY = 0;
}

if (writer instanceof TiffWriter) {
((TiffWriter) writer).saveBytes(outputIndex, buf,
outputX, outputY, tileWidth, tileHeight);
}
else if (writer instanceof ImageWriter) {
if (baseWriter instanceof TiffWriter) {
((TiffWriter) baseWriter).saveBytes(outputIndex, buf,
outputX, outputY, tileWidth, tileHeight);
}
else {
writer.saveBytes(outputIndex, buf, outputX, outputY, tileWidth, tileHeight);
}
if (tryPrecompressed) {
writer.saveCompressedBytes(outputIndex, buf, outputX, outputY, tileWidth, tileHeight);
}
else {
writer.saveBytes(outputIndex, buf, outputX, outputY, tileWidth, tileHeight);
Expand Down Expand Up @@ -1033,7 +1085,7 @@ public int getTileColumns(String outputName) {
private void autoscalePlane(byte[] buf, int index)
throws FormatException, IOException
{
if (autoscale) {
if (autoscale && !tryPrecompressed) {
Double min = null;
Double max = null;

Expand Down Expand Up @@ -1123,8 +1175,17 @@ private byte[] getTile(IFormatReader reader, int resolution,
{
if (resolution < reader.getResolutionCount()) {
reader.setResolution(resolution);
int optimalWidth = reader.getOptimalTileWidth();
int optimalHeight = reader.getOptimalTileHeight();
if (tryPrecompressed) {
return reader.openCompressedBytes(no, x / optimalWidth, y / optimalHeight);
}
tryPrecompressed = false;
return reader.openBytes(no, x, y, w, h);
}
if (tryPrecompressed) {
throw new UnsupportedOperationException("Cannot generate resolutions with precompressed tiles");
}
reader.setResolution(0);
IImageScaler scaler = new SimpleImageScaler();
int scale = (int) Math.pow(pyramidScale, resolution);
Expand Down
14 changes: 14 additions & 0 deletions components/formats-api/src/loci/formats/FormatReader.java
Original file line number Diff line number Diff line change
Expand Up @@ -1310,6 +1310,20 @@ public int getOptimalTileHeight() {
return (int) Math.min(maxHeight, getSizeY());
}

// -- ICompressedTileReader API methods --

@Override
public int getTileRows(int no) {
double rows = (double) getSizeY() / getOptimalTileHeight();
return (int) Math.ceil(rows);
}

@Override
public int getTileColumns(int no) {
double cols = (double) getSizeX() / getOptimalTileWidth();
return (int) Math.ceil(cols);
}

// -- Sub-resolution API methods --

@Override
Expand Down
54 changes: 54 additions & 0 deletions components/formats-api/src/loci/formats/FormatTools.java
Original file line number Diff line number Diff line change
Expand Up @@ -54,6 +54,7 @@
import loci.common.services.DependencyException;
import loci.common.services.ServiceException;
import loci.common.services.ServiceFactory;
import loci.formats.codec.Codec;
import loci.formats.meta.DummyMetadata;
import loci.formats.meta.MetadataRetrieve;
import loci.formats.meta.MetadataStore;
Expand Down Expand Up @@ -2127,4 +2128,57 @@ public static int getRequiredDirectories(String[] files)

return (int) Math.max(maxDirCount - dirCount, 0);
}

/**
* Compare the given reader and writer at the specified series and resolution to
* see if pre-compressed tiles can be transferred directly from the reader to the writer.
*/
public static boolean canUsePrecompressedTiles(IFormatReader reader, IFormatWriter writer,
int series, int resolution)
throws FormatException, IOException
{
if (!(reader instanceof ICompressedTileReader)) {
return false;
}
if (!(writer instanceof ICompressedTileWriter)) {
return false;
}

int readerSeries = reader.getSeries();
int readerRes = reader.getResolution();
reader.setSeries(series);
reader.setResolution(resolution);

int writerSeries = writer.getSeries();
int writerRes = writer.getResolution();
writer.setSeries(series);
writer.setResolution(resolution);

boolean sameTileWidth = reader.getOptimalTileWidth() == writer.getTileSizeX();
boolean sameTileHeight = reader.getOptimalTileHeight() == writer.getTileSizeY();

// reader and writer must use equivalent codecs
// the Codec objects are not expected to be strictly equal,
// but both should either be null, or non-null and instances of the same class
boolean sameCodec = true;
Codec writerCodec = ((ICompressedTileWriter) writer).getCodec();
for (int no=0; no<reader.getImageCount(); no++) {
Codec readerCodec = ((ICompressedTileReader) reader).getTileCodec(no);
if ((writerCodec == null && readerCodec != null) ||
(writerCodec != null && readerCodec == null) ||
!writerCodec.getClass().equals(readerCodec.getClass()))
{
sameCodec = false;
break;
}
}

reader.setSeries(readerSeries);
reader.setResolution(resolution);
writer.setSeries(series);
writer.setResolution(resolution);

return sameTileWidth && sameTileHeight && sameCodec;
}

}
5 changes: 5 additions & 0 deletions components/formats-api/src/loci/formats/FormatWriter.java
Original file line number Diff line number Diff line change
Expand Up @@ -266,6 +266,11 @@ public void setCodecOptions(CodecOptions options) {
this.options = options;
}

@Override
public CodecOptions getCodecOptions() {
return options;
}

/* @see IFormatWriter#getCompression() */
@Override
public String getCompression() {
Expand Down
Loading