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
The following commit(s) were added to refs/heads/geoapi-4.0 by this push:
new 130695a20b Fix the way reading and writing of single bits are handled
in `ChannelData` input/output classes.
130695a20b is described below
commit 130695a20b77553514c68e1faa1f551234005dc4
Author: Martin Desruisseaux <[email protected]>
AuthorDate: Thu Oct 26 20:47:54 2023 +0200
Fix the way reading and writing of single bits are handled in `ChannelData`
input/output classes.
---
.../sis/storage/geotiff/ReversedBitsChannel.java | 6 +-
.../main/org/apache/sis/io/stream/ChannelData.java | 88 +++++++-----------
.../org/apache/sis/io/stream/ChannelDataInput.java | 103 +++++++++++++++++----
.../apache/sis/io/stream/ChannelDataOutput.java | 81 +++++++++-------
.../sis/io/stream/ChannelImageInputStream.java | 26 +++++-
.../sis/io/stream/ChannelImageOutputStream.java | 14 +--
.../sis/io/stream/ChannelDataOutputTest.java | 11 +--
.../io/stream/ChannelImageOutputStreamTest.java | 12 +--
.../io/stream/MemoryCacheImageOutputStream.java | 68 +++++++++++++-
9 files changed, 270 insertions(+), 139 deletions(-)
diff --git
a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/ReversedBitsChannel.java
b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/ReversedBitsChannel.java
index 8d181cb727..f8dec2fd9a 100644
---
a/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/ReversedBitsChannel.java
+++
b/endorsed/src/org.apache.sis.storage.geotiff/main/org/apache/sis/storage/geotiff/ReversedBitsChannel.java
@@ -63,10 +63,8 @@ final class ReversedBitsChannel implements
ReadableByteChannel, SeekableByteChan
* and because a new buffer is created for each strip or tile to read.
*/
static ChannelDataInput wrap(final ChannelDataInput input) throws
IOException {
- final var buffer =
ByteBuffer.allocate(2048).order(input.buffer.order());
- final var reverse = new ChannelDataInput(input.filename, new
ReversedBitsChannel(input), buffer, true);
- input.yield(reverse);
- return reverse;
+ final var buffer =
ByteBuffer.allocate(2048).order(input.buffer.order());
+ return new ChannelDataInput(input, new ReversedBitsChannel(input),
buffer);
}
/**
diff --git
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelData.java
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelData.java
index 60bece689e..d9da89481f 100644
---
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelData.java
+++
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelData.java
@@ -42,7 +42,7 @@ public abstract class ChannelData implements Markable {
* assert (1 << BIT_OFFSET_SIZE) == Byte.SIZE;
* }
*/
- private static final int BIT_OFFSET_SIZE = 3;
+ static final int BIT_OFFSET_SIZE = 3;
/**
* A short identifier (typically a filename without path) used for
formatting error message.
@@ -68,13 +68,18 @@ public abstract class ChannelData implements Markable {
private long channelOffset;
/**
- * The channel position where is located the {@link #buffer} value at
index 0.
+ * The channel position where is located the {@link #buffer}.
* This is initially zero and shall be incremented as below:
*
* <ul>
* <li>By {@link ByteBuffer#position()} every time {@link
ByteBuffer#compact()} is invoked.</li>
* <li>By {@link ByteBuffer#limit()} every time {@link
ByteBuffer#clear()} is invoked.</li>
* </ul>
+ *
+ * <h4>Subclass-dependent interpretation</h4>
+ * The value depends on whether the buffer is used before or after
transfer from/to the channel.
+ * For {@link ChannelDataOutput}, the value is the channel position of
buffer position 0.
+ * For {@link ChannelDataInput}, the value is the channel position of
buffer limit.
*/
long bufferOffset;
@@ -83,9 +88,16 @@ public abstract class ChannelData implements Markable {
* and the remaining of the {@code long} value is the stream position
where the bit
* offset is valid.
*
+ * <h4>Subclass-dependent interpretation</h4>
+ * Which byte contains the bits depends on whether this {@code
ChannelData} is an input or an output.
+ * For {@link ChannelDataOutput}, the bits are stored in the byte before
the current buffer position.
+ * For {@link ChannelDataInput}, the bits are stored in the byte at the
current buffer position.
+ *
* @see #getBitOffset()
+ * @see #setBitOffset(int)
+ * @see #skipRemainingBits()
*/
- private long bitPosition;
+ long bitPosition;
/**
* The most recent mark, or {@code null} if none.
@@ -133,14 +145,15 @@ public abstract class ChannelData implements Markable {
* not be used anymore after this constructor.
*
* @param other the existing instance from which to takes the
channel and buffer.
+ * @param buffer either {@code other.buffer} or a new buffer with an
equivalent position.
* @param replacing {@code true} if {@code other} will be discarded in
favor of the new instance.
*/
- ChannelData(final ChannelData other, final boolean replacing) {
+ ChannelData(final ChannelData other, final ByteBuffer buffer, final
boolean replacing) {
filename = other.filename;
- buffer = other.buffer;
channelOffset = other.channelOffset;
bufferOffset = other.bufferOffset;
bitPosition = other.bitPosition;
+ this.buffer = buffer;
if (replacing) {
mark = other.mark;
}
@@ -189,21 +202,6 @@ public abstract class ChannelData implements Markable {
return -1;
}
- /**
- * Implementation of {@link ChannelDataInput#readBit()} provided here for
performance reasons.
- * It is caller responsibility to ensure that the {@link #buffer} contains
at least one byte.
- */
- final int readBitFromBuffer() {
- final int bp = buffer.position();
- final long position = Math.addExact(bufferOffset, bp);
- if ((bitPosition >>> BIT_OFFSET_SIZE) != position) {
- bitPosition = position << BIT_OFFSET_SIZE;
- }
- final int bitOffset = (Byte.SIZE - 1) - (int) (bitPosition++ & ((1L <<
BIT_OFFSET_SIZE) - 1));
- final byte value = (bitOffset != 0) ? buffer.get(bp) : buffer.get();
- return (value & (1 << bitOffset)) == 0 ? 0 : 1;
- }
-
/**
* Returns the current bit offset, as an integer between 0 and 7 inclusive.
*
@@ -214,11 +212,12 @@ public abstract class ChannelData implements Markable {
* @return the bit offset of the stream.
*/
public final int getBitOffset() {
- final long position = position();
- if ((bitPosition >>> BIT_OFFSET_SIZE) != position) {
- bitPosition = position << BIT_OFFSET_SIZE;
+ // No need to use `Math.addExact(…)` because integer overflow results
in `false`.
+ if ((bitPosition >>> BIT_OFFSET_SIZE) == bufferOffset +
buffer.position()) {
+ return (int) (bitPosition & ((1L << BIT_OFFSET_SIZE) - 1));
}
- return (int) (bitPosition & ((1L << BIT_OFFSET_SIZE) - 1));
+ bitPosition = 0; // Bits are invalid even if the user moves back to
this position later.
+ return 0;
}
/**
@@ -238,21 +237,7 @@ public abstract class ChannelData implements Markable {
* If the bit offset is zero, this method does nothing.
* Otherwise it skips the remaining bits in current byte.
*/
- public final void skipRemainingBits() {
- if (bitPosition != 0) { // Quick check for common case.
- if (getBitOffset() != 0) {
- buffer.get(); // Should never fail, otherwise
bit offset should have been invalid.
- }
- clearBitOffset();
- }
- }
-
- /**
- * Sets the bit offset to zero.
- */
- final void clearBitOffset() {
- bitPosition = 0;
- }
+ public abstract void skipRemainingBits();
/**
* Returns the current byte position of the stream.
@@ -417,7 +402,7 @@ public abstract class ChannelData implements Markable {
((SeekableByteChannel) channel).position(channelOffset);
buffer.clear().limit(0);
bufferOffset = 0;
- clearBitOffset();
+ bitPosition = 0;
mark = null;
return true;
}
@@ -425,26 +410,15 @@ public abstract class ChannelData implements Markable {
}
/**
- * Notifies two {@code ChannelData} instances that operations will
continue with the specified take over.
+ * Copies all field values to the given object for {@code yield(…)}
implementation in subclasses.
+ * This is used for notifying that read or write operations will continue
with the specified take over.
* The two {@code ChannelData} instances should share the same {@link
Channel}, or use two channels that
* are at the same {@linkplain SeekableByteChannel#position() channel
position}.
*
- * <h4>Usage</h4>
- * This method is used when a {@link ChannelDataInput} and a {@link
ChannelDataOutput} are wrapping
- * the same {@link java.nio.channels.ByteChannel} and used alternatively
for reading and writing.
- * After a read operation, {@code in.yield(put)} should be invoked for
ensuring that the output
- * position is valid for the new channel position.
- *
- * <h4>Implementation note</h4>
- * Subclasses <strong>must</strong> override this method and set the
buffer position to zero.
- * Whether this need to be done before or after {@code
super.field(takeOver)} depends on whether
- * this {@code ChannelData} is for input or output.
- *
- * @param takeOver the {@code ChannelData} which will continue
operations after this one.
- *
- * @see ChannelDataOutput#ChannelDataOutput(ChannelDataInput)
+ * @param takeOver the {@code ChannelData} which will continue
operations after this one.
*/
- public void yield(final ChannelData takeOver) throws IOException {
+ final void copyTo(final ChannelData takeOver) {
+ assert takeOver.channel() == channel();
takeOver.bufferOffset = bufferOffset;
takeOver.channelOffset = channelOffset;
takeOver.bitPosition = bitPosition;
@@ -478,7 +452,7 @@ public abstract class ChannelData implements Markable {
* Clearing the bit offset is needed if we don't want to handle the
case of `ChannelDataOutput`,
* which use a different stream position calculation when the bit
offset is non-zero.
*/
- clearBitOffset();
+ bitPosition = 0;
mark = null;
}
diff --git
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelDataInput.java
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelDataInput.java
index 2431168752..72b8e65ae1 100644
---
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelDataInput.java
+++
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelDataInput.java
@@ -107,6 +107,22 @@ public class ChannelDataInput extends ChannelData
implements DataInput {
}
}
+ /**
+ * Creates a new data input with the same name and position than the given
object, but a different channel.
+ * This is used when the channel performs some filtering on the data, for
example inflating a ZIP file.
+ *
+ * @param other the other stream from which to copy the filename and
position.
+ * @param channel the new channel to use. Stream position shall be the
same as {@code other.channel} position.
+ * @param buffer the new buffer to use. Its content will be discarded
(limit set to 0).
+ */
+ public ChannelDataInput(final ChannelDataInput other, final
ReadableByteChannel channel, final ByteBuffer buffer) {
+ super(other, buffer, false);
+ this.channel = channel;
+ moveBufferForward(other.buffer.limit());
+ buffer.limit(0);
+ bitPosition = 0;
+ }
+
/**
* Creates a new instance for a buffer filled with the bytes to use.
* This constructor uses an independent, read-only view of the given
buffer.
@@ -128,7 +144,7 @@ public class ChannelDataInput extends ChannelData
implements DataInput {
* @param input the existing instance from which to takes the channel
and buffer.
*/
ChannelDataInput(final ChannelDataInput input) {
- super(input, true);
+ super(input, input.buffer, true);
channel = input.channel;
}
@@ -169,6 +185,21 @@ public class ChannelDataInput extends ChannelData
implements DataInput {
return c;
}
+ /**
+ * Moves the stream position to the next byte boundary.
+ * If the bit offset is zero, this method does nothing.
+ * Otherwise it skips the remaining bits in current byte.
+ */
+ @Override
+ public final void skipRemainingBits() {
+ if (bitPosition != 0) { // Quick check for common case.
+ if (getBitOffset() != 0) {
+ buffer.get(); // Should never fail, otherwise
bit offset should have been invalid.
+ }
+ bitPosition = 0;
+ }
+ }
+
/**
* Returns {@code true} if the buffer or the channel has at least one byte
remaining.
* If the {@linkplain #buffer buffer} has no remaining bytes, then this
method will attempt
@@ -270,7 +301,14 @@ public class ChannelDataInput extends ChannelData
implements DataInput {
*/
public final int readBit() throws IOException {
ensureBufferContains(Byte.BYTES);
- return readBitFromBuffer();
+ final int bp = buffer.position();
+ final long position = Math.addExact(bufferOffset, bp); // =
position() but inlined for reusing `bp`.
+ if ((bitPosition >>> BIT_OFFSET_SIZE) != position) {
+ bitPosition = position << BIT_OFFSET_SIZE; // Clear
the bits and mark as valid position.
+ }
+ final int bitOffset = (Byte.SIZE - 1) - (int) (bitPosition++ & ((1L <<
BIT_OFFSET_SIZE) - 1));
+ final byte value = (bitOffset != 0) ? buffer.get(bp) : buffer.get();
+ return (value & (1 << bitOffset)) == 0 ? 0 : 1;
}
/**
@@ -1052,7 +1090,7 @@ loop: while (hasRemaining()) {
*/
throw new
InvalidSeekException(Resources.format(Resources.Keys.StreamIsForwardOnly_1,
filename));
}
- clearBitOffset();
+ bitPosition = 0;
assert position() == position : position;
}
@@ -1092,29 +1130,62 @@ loop: while (hasRemaining()) {
* This method should be invoked when read operations with this {@code
ChannelDataInput} are completed for
* now, and write operations are about to begin with a {@link
ChannelDataOutput} sharing the same channel.
*
+ * <h4>Usage</h4>
+ * This method is used when a {@link ChannelDataInput} and a {@link
ChannelDataOutput} are wrapping
+ * the same {@link java.nio.channels.ByteChannel} and used alternatively
for reading and writing.
+ * After a read operation, {@code in.yield(out)} should be invoked for
ensuring that the output
+ * position is valid for the new channel position.
+ *
* @param takeOver the {@link ChannelDataOutput} which will continue
operations after this instance.
+ *
+ * @see ChannelDataOutput#ChannelDataOutput(ChannelDataInput)
*/
- @Override
- public void yield(final ChannelData takeOver) throws IOException {
- if (getBitOffset() != 0) {
- clearBitOffset();
- buffer.get();
- }
+ public final void yield(final ChannelDataOutput takeOver) throws
IOException {
+ int bitOffset = 0;
+ byte bits = 0;
/*
* If we filled the buffer with more bytes than the buffer position,
* the channel position is too far ahead. We need to seek backward.
+ * Note that if `bitOffset` is not zero, then there is at least one
+ * remaining byte, which is the byte where bits are read from.
*/
if (buffer.hasRemaining()) {
- if (channel instanceof SeekableByteChannel) {
- ((SeekableByteChannel)
channel).position(toSeekableByteChannelPosition(position()));
- } else {
+ if (!(channel instanceof SeekableByteChannel)) {
throw new
IOException(Resources.format(Resources.Keys.StreamIsForwardOnly_1,
takeOver.filename));
}
+ bitOffset = getBitOffset();
+ if (bitOffset != 0) {
+ bits = savedBitsForOutput(bitOffset);
+ }
+ final long p = position();
+ ((SeekableByteChannel)
channel).position(toSeekableByteChannelPosition(p));
+ bufferOffset = p; // Modify object state only on
success.
+ } else {
+ moveBufferForward(buffer.limit());
}
- bufferOffset = position();
- if (buffer.limit(0) != takeOver.buffer) { // Must be after
`position()`.
- takeOver.buffer.limit(0);
+ copyTo(takeOver);
+ takeOver.buffer.limit(0); // Also set the position to 0.
+ if (bitOffset != 0) {
+ takeOver.buffer.limit(1).put(bits);
+ takeOver.bitPosition += (1L << BIT_OFFSET_SIZE); // In
output mode, position is after the byte.
}
- super.yield(takeOver);
+ }
+
+ /**
+ * Returns the bits to save in {@link ChannelDataOutput} for avoiding
information lost.
+ * This method is invoked by {@link #yield(ChannelDataOutput)} if this
input channel was reading
+ * some bits in the middle of a byte. In order to keep the same bit offset
in the output channel,
+ * the output buffer must contain that byte for allowing to continue to
write bits in that byte.
+ * This method returns that byte to copy from the input channel to the
output channel.
+ *
+ * @param bitOffset current value of {@link #getBitOffset()}, which must
be non-zero.
+ * @return the byte to copy from the input channel to the output channel.
+ */
+ byte savedBitsForOutput(final int bitOffset) {
+ /*
+ * We do not check the position validity because it is guaranteed
valid when bitOffset > 0.
+ * An IndexOutOfBoundsException here with would be a bug in the way we
manage bit offsets.
+ */
+ return buffer.get(buffer.position());
}
}
diff --git
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelDataOutput.java
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelDataOutput.java
index f24b19ced7..08300cdf1b 100644
---
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelDataOutput.java
+++
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelDataOutput.java
@@ -116,9 +116,9 @@ public class ChannelDataOutput extends ChannelData
implements DataOutput, Flusha
* @see #yield(ChannelData)
*/
public ChannelDataOutput(final ChannelDataInput input) {
- super(input, false);
+ super(input, input.buffer, false);
channel = (WritableByteChannel) input.channel; //
`ClassCastException` is part of the contract.
- // Do not invoke `synchronized(input)` because caller may want to do
some more read operations first.
+ // Do not invoke `input.yield(this)` because caller may want to do
some more read operations first.
}
/**
@@ -195,6 +195,15 @@ public class ChannelDataOutput extends ChannelData
implements DataOutput, Flusha
return position;
}
+ /**
+ * Moves the stream position to the next byte boundary.
+ */
+ @Override
+ public final void skipRemainingBits() {
+ // See the comment in `getStreamPosition()` method body.
+ bitPosition = 0;
+ }
+
/**
* Writes a single bit. This method uses only the rightmost bit of the
given argument;
* the upper 31 bits are ignored.
@@ -224,7 +233,7 @@ public class ChannelDataOutput extends ChannelData
implements DataOutput, Flusha
bits &= Numerics.bitmask(numBits) - 1; //
Make sure that high-order bits are zero.
final int r = numBits - (Byte.SIZE - bitOffset);
/*
- * `r` is the number of bits than we cannot store in the
current byte. This value may be negative,
+ * `r` is the number of bits that we cannot store in the
current byte. This value may be negative,
* which means that the current byte has space for more bits
than what we have, in which case some
* room will still exist after this method call (i.e. the
`bitOffset` will still non-zero).
*/
@@ -507,6 +516,11 @@ public class ChannelDataOutput extends ChannelData
implements DataOutput, Flusha
*/
@Override
public final void write(final byte[] src, int offset, int length) throws
IOException {
+ /*
+ * Since the `bitOffset` validity is determined by the position, if
the position
+ * did not changed, then we need to clear the `bitOffset` flag
manually.
+ */
+ skipRemainingBits();
if (length != 0) {
do {
final int n = Math.min(buffer.capacity(), length);
@@ -515,12 +529,6 @@ public class ChannelDataOutput extends ChannelData
implements DataOutput, Flusha
offset += n;
length -= n;
} while (length != 0);
- } else {
- /*
- * Since the `bitOffset` validity is determined by the position,
if the position
- * did not changed, then we need to clear the `bitOffset` flag
manually.
- */
- clearBitOffset();
}
}
@@ -566,7 +574,7 @@ public class ChannelDataOutput extends ChannelData
implements DataOutput, Flusha
* @throws IOException if an error occurred while writing the stream.
*/
final void writeFully(final int dataSize, int offset, int length)
throws IOException {
- clearBitOffset(); // Actually needed
only if `length` == 0.
+ skipRemainingBits(); // Actually needed
only if `length` = 0.
ensureBufferAccepts(Math.min(length * dataSize,
buffer.capacity()));
final Buffer view = createView(); // Must be after
`ensureBufferAccept(…)`
int n = Math.min(view.remaining(), length);
@@ -738,7 +746,7 @@ public class ChannelDataOutput extends ChannelData
implements DataOutput, Flusha
* @param value the byte to repeat the given amount of times.
*/
public final void repeat(long count, final byte value) throws IOException {
- clearBitOffset();
+ skipRemainingBits(); // Actually needed only if `count` = 0.
if (count > 0) {
/*
* Fill the buffer with the specified value. The filling is done
only once,
@@ -796,7 +804,7 @@ public class ChannelDataOutput extends ChannelData
implements DataOutput, Flusha
* Requested position is inside the current limits of the buffer.
*/
buffer.position((int) p);
- clearBitOffset();
+ bitPosition = 0;
} else if (channel instanceof SeekableByteChannel) {
/*
* Requested position is outside the current limits of the buffer,
@@ -828,7 +836,7 @@ public class ChannelDataOutput extends ChannelData
implements DataOutput, Flusha
*/
@Override
public final void flush() throws IOException {
- clearBitOffset();
+ skipRemainingBits();
writeFully();
bufferOffset = position();
buffer.limit(0);
@@ -871,30 +879,41 @@ public class ChannelDataOutput extends ChannelData
implements DataOutput, Flusha
* This method should be invoked when write operations with this {@code
ChannelDataOutput} are completed
* for now, and read operations are about to begin with a {@link
ChannelDataInput} sharing the same channel.
*
+ * <h4>Usage</h4>
+ * This method is used when a {@link ChannelDataInput} and a {@link
ChannelDataOutput} are wrapping
+ * the same {@link java.nio.channels.ByteChannel} and used alternatively
for reading and writing.
+ * After a read operation, {@code in.yield(out)} should be invoked for
ensuring that the output
+ * position is valid for the new channel position.
+ *
* @param takeOver the {@link ChannelDataInput} which will continue
operations after this instance.
+ *
+ * @see ChannelDataOutput#ChannelDataOutput(ChannelDataInput)
*/
- @Override
- public void yield(final ChannelData takeOver) throws IOException {
- final int position = buffer.position();
- final int limit = buffer.limit();
+ public final void yield(final ChannelDataInput takeOver) throws
IOException {
/*
* Flush the full buffer content for avoiding data lost. Note that the
buffer position
- * is not necessarily at the end, so we may write more bytes than the
stream position.
- * It may force us to seek backward after flushing.
+ * is not necessarily at the buffer limit, so some bytes may be
written past the position.
+ * The buffer content is still valid, so we configure `takeOver` as if
the write operation
+ * was immediately following by a read of the same bytes. This is not
only an optimization,
+ * but also needed for preserving the byte containing the bits when
`bitOffset` is non-zero.
*/
- clearBitOffset();
+ int position = buffer.position();
writeFully();
- if (position >= limit) {
- // We were at the end of the buffer, so the channel is already at
the right position.
- bufferOffset = position();
- buffer.limit(0);
- } else if (channel instanceof SeekableByteChannel) {
- // Do not move `bufferOffset`. Instead make the channel position
consistent with it.
- buffer.limit(limit).position(position);
- ((SeekableByteChannel)
channel).position(toSeekableByteChannelPosition(Math.addExact(bufferOffset,
limit)));
- } else {
- throw new
IOException(Resources.format(Resources.Keys.StreamIsForwardOnly_1, filename));
+ if (getBitOffset() != 0) {
+ position--; // For input, the byte containing bits is at
instead of after the position.
+ bitPosition -= (1L << BIT_OFFSET_SIZE);
}
- super.yield(takeOver);
+ buffer.position(position);
+ copyTo(takeOver);
+ assert takeOver.buffer == buffer;
+ /*
+ * If above assertion is false, we could still work with:
+ *
+ * moveBufferForward(buffer.limit());
+ * takeOver.buffer.limit(0);
+ *
+ * However there is a slight complication because of the need to copy
bits if `bitOffset` is non-zero
+ * (see ChannelDataInput.yield(…) for code example). For now it is not
needed.
+ */
}
}
diff --git
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelImageInputStream.java
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelImageInputStream.java
index 9426cb2caa..c243e1a778 100644
---
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelImageInputStream.java
+++
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelImageInputStream.java
@@ -38,7 +38,13 @@ import javax.imageio.stream.ImageInputStream;
* Furthermore, this class allows us to reuse an existing buffer (especially
direct buffer, which are costly to create)
* and allow subclasses to store additional information, for example the file
path.
*
- * <p>This class is used when compatibility with {@link
javax.imageio.ImageReader} is needed.</p>
+ * <p>This class is used when compatibility with {@link
javax.imageio.ImageReader} is needed.
+ * The following methods behave in a slightly different way compared to {@link
ChannelDataInput}:</p>
+ *
+ * <ul>
+ * <li>{@link #skipBytes(int)} with a more restrictive interpretation of
method contract.</li>
+ * <li>{@link #yield(ChannelDataOutput)} when the bit offset is
non-zero.</li>
+ * </ul>
*
* @author Martin Desruisseaux (Geomatys)
*
@@ -176,7 +182,7 @@ public class ChannelImageInputStream extends
ChannelDataInput implements ImageIn
*/
@Override
public final void readBytes(final IIOByteBuffer dest, int length) throws
IOException {
- clearBitOffset();
+ bitPosition = 0;
final byte[] data;
final int offset;
if (buffer.hasArray()) {
@@ -214,7 +220,7 @@ public class ChannelImageInputStream extends
ChannelDataInput implements ImageIn
long p = buffer.position() + n;
if (p >= 0 && p <= buffer.limit()) {
buffer.position((int) p);
- clearBitOffset();
+ bitPosition = 0;
} else {
final long offset = getStreamPosition();
p = Math.max(Math.addExact(offset, n), 0);
@@ -310,4 +316,18 @@ public class ChannelImageInputStream extends
ChannelDataInput implements ImageIn
public final void close() throws IOException {
channel.close();
}
+
+ /**
+ * Returns the bits to save in {@link ChannelDataOutput} for avoiding
information lost.
+ * The Image I/O specification requires that we discard (force to zero)
all bits after
+ * the last bit that we have read. It may cause a lost of some bits, so we
do that only
+ * if an {@link ImageInputStream} was requested.
+ *
+ * @param bitOffset current value of {@link #getBitOffset()}, which must
be non-zero.
+ * @return the byte to copy from the input channel to the output channel.
+ */
+ @Override
+ final byte savedBitsForOutput(final int bitOffset) {
+ return (byte) (super.savedBitsForOutput(bitOffset) & ~((1 <<
(Byte.SIZE - bitOffset)) - 1));
+ }
}
diff --git
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelImageOutputStream.java
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelImageOutputStream.java
index 037b81ee80..46f8afe93c 100644
---
a/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelImageOutputStream.java
+++
b/endorsed/src/org.apache.sis.storage/main/org/apache/sis/io/stream/ChannelImageOutputStream.java
@@ -68,11 +68,6 @@ public class ChannelImageOutputStream extends OutputStream
implements ImageOutpu
output = new ChannelDataOutput(filename, channel, buffer);
}
- /** Declares that this stream does not cache data itself. */
- @Override public boolean isCached() {return false;}
- @Override public boolean isCachedMemory() {return false;}
- @Override public boolean isCachedFile() {return false;}
-
/**
* Returns the {@linkplain #input} or {@linkplain #output},
* depending on whether this stream is reading or writing.
@@ -112,6 +107,9 @@ public class ChannelImageOutputStream extends OutputStream
implements ImageOutpu
}
/** Delegates to the reader or writer. */
+ @Override public boolean isCached()
{return input.isCached();}
+ @Override public boolean isCachedMemory()
{return input.isCachedMemory();}
+ @Override public boolean isCachedFile()
{return input.isCachedFile();}
@Override public long length() throws
IOException {return current().length();}
@Override public void mark()
{ current().mark();}
@Override public void reset() throws
IOException { current().reset();}
@@ -178,15 +176,13 @@ public class ChannelImageOutputStream extends
OutputStream implements ImageOutpu
/**
* Discards the initial position of the stream prior to the current stream
position.
+ * Note that this behavior is as specified by Image I/O, but different
than the flush
+ * method of {@link OutputStream}.
*
* @throws IOException if an I/O error occurred.
*/
@Override
public void flush() throws IOException {
- if (writing) {
- // Reproduce the behavior of Imava I/O implementation.
- output.clearBitOffset();
- }
current().flushBefore(getStreamPosition());
}
diff --git
a/endorsed/src/org.apache.sis.storage/test/org/apache/sis/io/stream/ChannelDataOutputTest.java
b/endorsed/src/org.apache.sis.storage/test/org/apache/sis/io/stream/ChannelDataOutputTest.java
index 763c4ecfdc..e8d2f80fb0 100644
---
a/endorsed/src/org.apache.sis.storage/test/org/apache/sis/io/stream/ChannelDataOutputTest.java
+++
b/endorsed/src/org.apache.sis.storage/test/org/apache/sis/io/stream/ChannelDataOutputTest.java
@@ -224,7 +224,7 @@ public final class ChannelDataOutputTest extends
ChannelDataTestCase {
* @throws IOException should never happen since we read and write in
memory only.
*/
private void writeInStreams() throws IOException {
- transferRandomData(testedStream, testedStreamBackingArray.length -
ARRAY_MAX_LENGTH, 21);
+ transferRandomData(testedStream, testedStreamBackingArray.length -
ARRAY_MAX_LENGTH, 20);
}
/**
@@ -271,11 +271,10 @@ public final class ChannelDataOutputTest extends
ChannelDataTestCase {
t.flushBefore(flushedPosition);
break;
}
- case 20: {
- r.flush();
- t.flush();
- break;
- }
+ /*
+ * Do not test flush, because `ChannelDataOutput.flush()` semantic
+ * is very different than the `ImageOutputStream.flush()` semantic.
+ */
}
assertEquals(r.getBitOffset(), t.getBitOffset(),
"getBitOffset()");
assertEquals(r.getStreamPosition(), t.getStreamPosition(),
"getStreamPosition()");
diff --git
a/endorsed/src/org.apache.sis.storage/test/org/apache/sis/io/stream/ChannelImageOutputStreamTest.java
b/endorsed/src/org.apache.sis.storage/test/org/apache/sis/io/stream/ChannelImageOutputStreamTest.java
index 0d647db467..9f33e7490b 100644
---
a/endorsed/src/org.apache.sis.storage/test/org/apache/sis/io/stream/ChannelImageOutputStreamTest.java
+++
b/endorsed/src/org.apache.sis.storage/test/org/apache/sis/io/stream/ChannelImageOutputStreamTest.java
@@ -97,6 +97,7 @@ public final class ChannelImageOutputStreamTest extends
ChannelDataTestCase {
testWrapper = new ChannelData(testName, channel,
testedStream.input().buffer) {
@Override public Channel channel() {return channel;}
@Override public long getStreamPosition() {return
testedStream.getStreamPosition();}
+ @Override public void skipRemainingBits() {fail("Should not be
invoked.");}
@Override public void seek(long p) {fail("Should not be
invoked.");}
@Override void flushNBytes(int n) {fail("Should not be
invoked.");}
};
@@ -111,7 +112,7 @@ public final class ChannelImageOutputStreamTest extends
ChannelDataTestCase {
@Test
public void testAllMethods() throws IOException {
initialize("testAllMethods", STREAM_LENGTH, randomBufferCapacity());
- transferRandomData(testWrapper, testedStreamBackingArray.length -
ARRAY_MAX_LENGTH, 22);
+ transferRandomData(testWrapper, testedStreamBackingArray.length -
ARRAY_MAX_LENGTH, 21);
assertStreamContentEquals();
}
@@ -170,16 +171,11 @@ public final class ChannelImageOutputStreamTest extends
ChannelDataTestCase {
t.flushBefore(flushedPosition);
break;
}
- case 20: {
- r.flush();
- t.flush();
- break;
- }
/*
* Seek operation, potentially followed by a few read operations.
* The seek is necessary for moving to a position where there is
something to read.
*/
- case 21: {
+ case 20: {
long length = r.length();
assertTrue(length >= 0, "length");
assertEquals(length, t.length(), "length");
@@ -193,7 +189,7 @@ public final class ChannelImageOutputStreamTest extends
ChannelDataTestCase {
position = r.getStreamPosition();
assertEquals(position, t.getStreamPosition());
if (position >= length) break;
- switch (random.nextInt(11)) { //
TODO: should be 12. See class javadoc.
+ switch (random.nextInt(12)) {
default: throw new AssertionError();
case 0: assertEquals(r.read(),
t.read(), "read()"); break;
case 1: assertEquals(r.readBoolean(),
t.readBoolean(), "readBoolean()"); break;
diff --git
a/endorsed/src/org.apache.sis.storage/test/org/apache/sis/io/stream/MemoryCacheImageOutputStream.java
b/endorsed/src/org.apache.sis.storage/test/org/apache/sis/io/stream/MemoryCacheImageOutputStream.java
index 43124092ef..0b671e9187 100644
---
a/endorsed/src/org.apache.sis.storage/test/org/apache/sis/io/stream/MemoryCacheImageOutputStream.java
+++
b/endorsed/src/org.apache.sis.storage/test/org/apache/sis/io/stream/MemoryCacheImageOutputStream.java
@@ -30,6 +30,13 @@ import org.apache.sis.util.Workaround;
*/
@Workaround(library = "JDK", version = "1.8")
final class MemoryCacheImageOutputStream extends
javax.imageio.stream.MemoryCacheImageOutputStream {
+ /**
+ * Whether this stream potentially needs a flush of bits.
+ * This is needed because {@link #readBit()} implementation in super-class
invokes {@link #seek(long)}.
+ * If our subclass invokes {@link #flushBits()}, it causes the data to be
overwritten by garbage data.
+ */
+ private boolean needFlush;
+
/**
* Creates a new instance which will write the data in the given stream.
*/
@@ -37,6 +44,44 @@ final class MemoryCacheImageOutputStream extends
javax.imageio.stream.MemoryCach
super(stream);
}
+ /**
+ * Reads a bit.
+ */
+ @Override
+ public int readBit() throws IOException {
+ needFlush = false;
+ return super.readBit();
+ }
+
+ /**
+ * Reads many bits.
+ */
+ @Override
+ public long readBits(final int numBits) throws IOException {
+ needFlush = false;
+ return super.readBit();
+ }
+
+ /**
+ * Writes the given bit and remember that this bit may need to be flushed.
+ */
+ @Override
+ public void writeBit(final int bit) throws IOException {
+ needFlush = false;
+ super.writeBit(bit);
+ needFlush = true;
+ }
+
+ /**
+ * Writes the given bits and remember that those bits may need to be
flushed.
+ */
+ @Override
+ public void writeBits(final long bits, final int numBits) throws
IOException {
+ needFlush = false;
+ super.writeBits(bits, numBits);
+ needFlush = true;
+ }
+
/**
* Writes the bits (if any) before to seek.
*
@@ -46,7 +91,10 @@ final class MemoryCacheImageOutputStream extends
javax.imageio.stream.MemoryCach
*/
@Override
public void seek(final long pos) throws IOException {
- flushBits();
+ if (needFlush) {
+ needFlush = false;
+ flushBits();
+ }
super.seek(pos);
}
@@ -54,9 +102,16 @@ final class MemoryCacheImageOutputStream extends
javax.imageio.stream.MemoryCach
* Writes the bits (if any) before to flush the stream.
*/
@Override
- public void flush() throws IOException {
- flushBits();
- super.flush();
+ public void flushBefore(final long pos) throws IOException {
+ if (needFlush) {
+ needFlush = false;
+ long p = getStreamPosition();
+ int b = getBitOffset();
+ flushBits();
+ super.seek(p);
+ setBitOffset(b);
+ }
+ super.flushBefore(pos);
}
/**
@@ -64,7 +119,10 @@ final class MemoryCacheImageOutputStream extends
javax.imageio.stream.MemoryCach
*/
@Override
public void close() throws IOException {
- flushBits();
+ if (needFlush) {
+ needFlush = false;
+ flushBits();
+ }
super.close();
}
}
