This is an automated email from the ASF dual-hosted git repository. desruisseaux pushed a commit to branch geoapi-4.0 in repository https://gitbox.apache.org/repos/asf/sis.git
commit 07618780bcbff7aecd6ba7ad1683309e6ec2fe9c Author: Martin Desruisseaux <[email protected]> AuthorDate: Mon Jun 20 17:13:02 2022 +0200 Fix erroneous pixel values sometime computed for a compressed tile using horizontal predictor. --- .../storage/inflater/HorizontalPredictor.java | 49 +++++++++++++--------- .../internal/storage/inflater/PixelChannel.java | 2 +- .../storage/inflater/PredictorChannel.java | 34 ++++++++++----- .../internal/storage/inflater/package-info.java | 2 +- 4 files changed, 54 insertions(+), 33 deletions(-) diff --git a/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/HorizontalPredictor.java b/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/HorizontalPredictor.java index 54da47096f..9f9ac83576 100644 --- a/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/HorizontalPredictor.java +++ b/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/HorizontalPredictor.java @@ -28,7 +28,7 @@ import org.apache.sis.internal.jdk9.JDK9; * Values packed on 4, 2 or 1 bits are not yet supported. * * @author Martin Desruisseaux (Geomatys) - * @version 1.2 + * @version 1.3 * @since 1.1 * @module */ @@ -76,7 +76,7 @@ abstract class HorizontalPredictor extends PredictorChannel { private final int truncationMask; /** - * Creates a new predictor which will read compressed data from the given channel. + * Creates a new predictor which will read uncompressed data from the given channel. * The {@link #setInputRegion(long, long)} method must be invoked after construction * before a reading process can start. * @@ -141,7 +141,7 @@ abstract class HorizontalPredictor extends PredictorChannel { * unless the predictor needs more data for processing the last bytes. */ @Override - protected final int uncompress(final ByteBuffer buffer, final int start) { + protected final int apply(final ByteBuffer buffer, final int start) { final int limit = buffer.position(); final int limitMS = limit - sampleSizeM1; // Limit with enough space for last sample value. /* @@ -159,7 +159,8 @@ abstract class HorizontalPredictor extends PredictorChannel { * at the end of the last invocation of this method. Note that this will perform no operation * if the block above skipped fully the pixel in the first column. */ - position = applyOnFirst(buffer, position, Math.min(start + pixelStride, limitMS), position - start); + final int endOfFirstPixel = Math.min(Math.min(start + pixelStride, scanlineStride), limitMS); + position = applyOnFirst(buffer, position, endOfFirstPixel, position - start); if ((column += position - start) >= scanlineStride) { column = 0; } @@ -202,7 +203,7 @@ abstract class HorizontalPredictor extends PredictorChannel { /** * Applies the predictor on the specified region of the given buffer, but using {@code savedValues} * array as the source of previous values. This is used only for the first pixel in a new invocation - * of {@link #uncompress(ByteBuffer, int)}. + * of {@link #apply(ByteBuffer, int)}. * * @param buffer the buffer on which to apply the predictor. * @param position position of the first value to modify in the given buffer. @@ -242,8 +243,8 @@ abstract class HorizontalPredictor extends PredictorChannel { */ private static final class Bytes extends HorizontalPredictor { /** - * The trailing values of previous invocation of {@link #uncompress(ByteBuffer, int)}. - * After each call to {@code uncompress(…)}, the last values in the buffer are saved + * The trailing values of previous invocation of {@link #apply(ByteBuffer, int)}. + * After each call to {@code apply(…)}, the last values in the buffer are saved * for use by the next invocation. The buffer capacity is exactly one pixel. */ private final byte[] savedValues; @@ -268,7 +269,7 @@ abstract class HorizontalPredictor extends PredictorChannel { /** * Applies the predictor, using {@link #savedValues} as the source of previous values. - * Used only for the first pixel in a new invocation of {@link #uncompress(ByteBuffer, int)}. + * Used only for the first pixel in a new invocation of {@link #apply(ByteBuffer, int)}. */ @Override int applyOnFirst(final ByteBuffer buffer, int position, final int end, int offset) { @@ -299,8 +300,8 @@ abstract class HorizontalPredictor extends PredictorChannel { */ private static final class Shorts extends HorizontalPredictor { /** - * The trailing values of previous invocation of {@link #uncompress(ByteBuffer, int)}. - * After each call to {@code uncompress(…)}, the last values in the buffer are saved + * The trailing values of previous invocation of {@link #apply(ByteBuffer, int)}. + * After each call to {@code apply(…)}, the last values in the buffer are saved * for use by the next invocation. The buffer capacity is exactly one pixel. */ private final short[] savedValues; @@ -319,6 +320,7 @@ abstract class HorizontalPredictor extends PredictorChannel { */ @Override void saveLastPixel(final ByteBuffer buffer, int offset, int position) { + assert (offset % Short.BYTES) == 0 : offset; offset /= Short.BYTES; System.arraycopy(savedValues, savedValues.length - offset, savedValues, 0, offset); while (offset < savedValues.length) { @@ -329,10 +331,11 @@ abstract class HorizontalPredictor extends PredictorChannel { /** * Applies the predictor, using {@link #savedValues} as the source of previous values. - * Used only for the first pixel in a new invocation of {@link #uncompress(ByteBuffer, int)}. + * Used only for the first pixel in a new invocation of {@link #apply(ByteBuffer, int)}. */ @Override int applyOnFirst(final ByteBuffer buffer, int position, final int end, int offset) { + assert (offset % Short.BYTES) == 0 : offset; offset /= Short.BYTES; while (position < end) { buffer.putShort(position, (short) (buffer.getShort(position) + savedValues[offset++])); @@ -361,8 +364,8 @@ abstract class HorizontalPredictor extends PredictorChannel { */ private static final class Integers extends HorizontalPredictor { /** - * The trailing values of previous invocation of {@link #uncompress(ByteBuffer, int)}. - * After each call to {@code uncompress(…)}, the last values in the buffer are saved + * The trailing values of previous invocation of {@link #apply(ByteBuffer, int)}. + * After each call to {@code apply(…)}, the last values in the buffer are saved * for use by the next invocation. The buffer capacity is exactly one pixel. */ private final int[] savedValues; @@ -381,6 +384,7 @@ abstract class HorizontalPredictor extends PredictorChannel { */ @Override void saveLastPixel(final ByteBuffer buffer, int offset, int position) { + assert (offset % Integer.BYTES) == 0 : offset; offset /= Integer.BYTES; System.arraycopy(savedValues, savedValues.length - offset, savedValues, 0, offset); while (offset < savedValues.length) { @@ -391,10 +395,11 @@ abstract class HorizontalPredictor extends PredictorChannel { /** * Applies the predictor, using {@link #savedValues} as the source of previous values. - * Used only for the first pixel in a new invocation of {@link #uncompress(ByteBuffer, int)}. + * Used only for the first pixel in a new invocation of {@link #apply(ByteBuffer, int)}. */ @Override int applyOnFirst(final ByteBuffer buffer, int position, final int end, int offset) { + assert (offset % Integer.BYTES) == 0 : offset; offset /= Integer.BYTES; while (position < end) { buffer.putInt(position, buffer.getInt(position) + savedValues[offset++]); @@ -423,8 +428,8 @@ abstract class HorizontalPredictor extends PredictorChannel { */ private static final class Floats extends HorizontalPredictor { /** - * The trailing values of previous invocation of {@link #uncompress(ByteBuffer, int)}. - * After each call to {@code uncompress(…)}, the last values in the buffer are saved + * The trailing values of previous invocation of {@link #apply(ByteBuffer, int)}. + * After each call to {@code apply(…)}, the last values in the buffer are saved * for use by the next invocation. The buffer capacity is exactly one pixel. */ private final float[] savedValues; @@ -443,6 +448,7 @@ abstract class HorizontalPredictor extends PredictorChannel { */ @Override void saveLastPixel(final ByteBuffer buffer, int offset, int position) { + assert (offset % Float.BYTES) == 0 : offset; offset /= Float.BYTES; System.arraycopy(savedValues, savedValues.length - offset, savedValues, 0, offset); while (offset < savedValues.length) { @@ -453,10 +459,11 @@ abstract class HorizontalPredictor extends PredictorChannel { /** * Applies the predictor, using {@link #savedValues} as the source of previous values. - * Used only for the first pixel in a new invocation of {@link #uncompress(ByteBuffer, int)}. + * Used only for the first pixel in a new invocation of {@link #apply(ByteBuffer, int)}. */ @Override int applyOnFirst(final ByteBuffer buffer, int position, final int end, int offset) { + assert (offset % Float.BYTES) == 0 : offset; offset /= Float.BYTES; while (position < end) { buffer.putFloat(position, buffer.getFloat(position) + savedValues[offset++]); @@ -485,8 +492,8 @@ abstract class HorizontalPredictor extends PredictorChannel { */ private static final class Doubles extends HorizontalPredictor { /** - * The trailing values of previous invocation of {@link #uncompress(ByteBuffer, int)}. - * After each call to {@code uncompress(…)}, the last values in the buffer are saved + * The trailing values of previous invocation of {@link #apply(ByteBuffer, int)}. + * After each call to {@code apply(…)}, the last values in the buffer are saved * for use by the next invocation. The buffer capacity is exactly one pixel. */ private final double[] savedValues; @@ -505,6 +512,7 @@ abstract class HorizontalPredictor extends PredictorChannel { */ @Override void saveLastPixel(final ByteBuffer buffer, int offset, int position) { + assert (offset % Double.BYTES) == 0 : offset; offset /= Double.BYTES; System.arraycopy(savedValues, savedValues.length - offset, savedValues, 0, offset); while (offset < savedValues.length) { @@ -515,10 +523,11 @@ abstract class HorizontalPredictor extends PredictorChannel { /** * Applies the predictor, using {@link #savedValues} as the source of previous values. - * Used only for the first pixel in a new invocation of {@link #uncompress(ByteBuffer, int)}. + * Used only for the first pixel in a new invocation of {@link #apply(ByteBuffer, int)}. */ @Override int applyOnFirst(final ByteBuffer buffer, int position, final int end, int offset) { + assert (offset % Double.BYTES) == 0 : offset; offset /= Double.BYTES; while (position < end) { buffer.putDouble(position, buffer.getDouble(position) + savedValues[offset++]); diff --git a/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/PixelChannel.java b/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/PixelChannel.java index 5247497466..6f805f0d43 100644 --- a/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/PixelChannel.java +++ b/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/PixelChannel.java @@ -45,7 +45,7 @@ abstract class PixelChannel implements ReadableByteChannel { } /** - * Prepares this channel for reading a new tile or a new band of a tile. + * Prepares this channel for reading a new tile or a new band of a planar image. * * @param start stream position where to start reading. * @param byteCount number of bytes to read from the input. diff --git a/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/PredictorChannel.java b/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/PredictorChannel.java index e10fc7698b..9202be74cf 100644 --- a/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/PredictorChannel.java +++ b/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/PredictorChannel.java @@ -27,7 +27,7 @@ import org.apache.sis.internal.jdk9.JDK9; * Implementation of a {@link Predictor} to be executed after decompression. * * @author Martin Desruisseaux (Geomatys) - * @version 1.2 + * @version 1.3 * @since 1.1 * @module */ @@ -61,7 +61,7 @@ abstract class PredictorChannel extends PixelChannel { } /** - * Prepares this predictor for reading a new tile or a new band of a tile. + * Prepares this predictor for reading a new tile or a new band of a planar image. * * @param start stream position where to start reading. * @param byteCount number of bytes to read from the input. @@ -82,10 +82,10 @@ abstract class PredictorChannel extends PixelChannel { * @return position after the last sample value processed. Should be {@link ByteBuffer#position()}, * unless the predictor needs more data for processing the last bytes. */ - protected abstract int uncompress(ByteBuffer buffer, int start); + protected abstract int apply(ByteBuffer buffer, int start); /** - * Decompresses some bytes from the {@linkplain #input input} into the given destination buffer. + * Decompresses some bytes from the {@linkplain #input} into the given destination buffer. * * @param target the buffer into which bytes are to be transferred. * @return the number of bytes read, or -1 if end-of-stream. @@ -95,19 +95,31 @@ abstract class PredictorChannel extends PixelChannel { public int read(final ByteBuffer target) throws IOException { final int start = target.position(); if (deferredCount != 0) { + /* + * If we had some bytes from the previous invocation that `apply(…)` could not use, + * append those bytes in the target buffer before to read new bytes from the channel. + * We may not be able to append all bytes. + */ final int n = Math.min(deferredCount, target.remaining()); target.put(deferred, 0, n); - deferredCount -= n; + System.arraycopy(deferred, n, deferred, 0, deferredCount -= n); } input.read(target); - final int end = uncompress(target, start); - deferredCount = target.position() - end; - if (deferredCount != 0) { - if (deferredCount > deferred.length) { - deferred = new byte[deferredCount]; + final int end = apply(target, start); + final int remaining = target.position() - end; + if (remaining != 0) { + /* + * If there is some bytes that `apply(…)` could not use, save those bytes for next + * invocation of this `read(…)` method. Those bytes may need to be appended after + * bytes that previous code block has not been able to put in the target buffer. + */ + final int length = deferredCount + remaining; + if (length > deferred.length) { + deferred = new byte[length]; } - JDK9.get(target, end, deferred, 0, deferredCount); + JDK9.get(target, end, deferred, deferredCount, remaining); target.position(end); + deferredCount = length; } return end - start; } diff --git a/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/package-info.java b/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/package-info.java index c975f32d84..7ff1f2a7af 100644 --- a/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/package-info.java +++ b/storage/sis-geotiff/src/main/java/org/apache/sis/internal/storage/inflater/package-info.java @@ -42,7 +42,7 @@ * </dl> * * @author Martin Desruisseaux (Geomatys) - * @version 1.2 + * @version 1.3 * @since 1.1 * @module */
