[GitHub] [commons-imaging] garydgregory commented on a diff in pull request #318: [IMAGING-316] Add support for BigTIFF format

2023-09-29 Thread via GitHub 


garydgregory commented on code in PR #318:
URL: https://github.com/apache/commons-imaging/pull/318#discussion_r1341381002


##
src/main/java/org/apache/commons/imaging/formats/tiff/TiffReader.java:
##
@@ -16,27 +16,29 @@
  */
 package org.apache.commons.imaging.formats.tiff;
 
-import static org.apache.commons.imaging.common.BinaryFunctions.read2Bytes;
-import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
-import static org.apache.commons.imaging.common.BinaryFunctions.readByte;
-import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
-import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
-import static 
org.apache.commons.imaging.formats.tiff.constants.TiffConstants.TIFF_ENTRY_MAX_VALUE_LENGTH;
-
 import java.io.IOException;
 import java.io.InputStream;
 import java.nio.ByteOrder;
 import java.util.ArrayList;
 import java.util.List;
-
 import org.apache.commons.imaging.FormatCompliance;
 import org.apache.commons.imaging.ImagingException;
 import org.apache.commons.imaging.bytesource.ByteSource;
 import org.apache.commons.imaging.common.BinaryFileParser;
+import static org.apache.commons.imaging.common.BinaryFunctions.read2Bytes;

Review Comment:
   All statics should be grouped at the top. Not sure if Checkstyle can check 
for us though.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] garydgregory commented on a diff in pull request #318: Imaging-316: Add support for BigTIFF format

2023-09-27 Thread via GitHub 


garydgregory commented on code in PR #318:
URL: https://github.com/apache/commons-imaging/pull/318#discussion_r1338737448


##
src/main/java/org/apache/commons/imaging/formats/tiff/TiffField.java:
##
@@ -234,6 +234,14 @@ public int[] getIntArrayValue() throws ImagingException {
 final int[] numbers = (int[]) o;
 return Arrays.copyOf(numbers, numbers.length);
 }
+if (o instanceof long[]){

Review Comment:
   The formatting seems off in the block, missing spaces.



##
src/main/java/org/apache/commons/imaging/formats/tiff/TiffField.java:
##
@@ -288,6 +296,55 @@ public int getIntValueOrArraySum() throws ImagingException 
{
 // return -1;
 }
 
+public long[] getLongArrayValue() throws ImagingException {

Review Comment:
   We should provide Javadocs on all new public and protected methods IMO.



##
src/main/java/org/apache/commons/imaging/formats/tiff/TiffReader.java:
##
@@ -455,27 +473,47 @@ private TiffHeader readTiffHeader(final ByteSource 
byteSource) throws ImagingExc
 }
 }
 
-private TiffHeader readTiffHeader(final InputStream is) throws 
ImagingException, IOException {
-final int byteOrder1 = readByte("BYTE_ORDER_1", is, "Not a Valid TIFF 
File");
-final int byteOrder2 = readByte("BYTE_ORDER_2", is, "Not a Valid TIFF 
File");
-if (byteOrder1 != byteOrder2) {
-throw new ImagingException("Byte Order bytes don't match (" + 
byteOrder1 + ", " + byteOrder2 + ").");
-}
-
-final ByteOrder byteOrder = getTiffByteOrder(byteOrder1);
-setByteOrder(byteOrder);
-
-final int tiffVersion = read2Bytes("tiffVersion", is, "Not a Valid 
TIFF File", getByteOrder());
-if (tiffVersion != 42) {
-throw new ImagingException("Unknown TIFF Version: " + tiffVersion);
-}
+   private TiffHeader readTiffHeader(final InputStream is) throws 
ImagingException, IOException {

Review Comment:
   Formatting seems odd.



##
src/main/java/org/apache/commons/imaging/common/ByteConversions.java:
##
@@ -399,6 +399,43 @@ private static int[] toUInt16s(final byte[] bytes, final 
int offset, final int l
 return result;
 }
 
+ public static long toLong(final byte[] bytes, final ByteOrder byteOrder) {

Review Comment:
   Also, it looks like there is an extra space in front of `public`?



##
src/main/java/org/apache/commons/imaging/common/ByteConversions.java:
##
@@ -399,6 +399,43 @@ private static int[] toUInt16s(final byte[] bytes, final 
int offset, final int l
 return result;
 }
 
+ public static long toLong(final byte[] bytes, final ByteOrder byteOrder) {

Review Comment:
   We should provide Javadocs on all new public and protected methods IMO.



##
src/main/java/org/apache/commons/imaging/formats/tiff/TiffField.java:
##
@@ -288,6 +296,55 @@ public int getIntValueOrArraySum() throws ImagingException 
{
 // return -1;
 }
 
+public long[] getLongArrayValue() throws ImagingException {
+final Object o = getValue();
+// if (o == null)
+// return null;
+
+if (o instanceof Number) {
+return new  long[] { ((Number) o).longValue() };
+}
+if (o instanceof Number[]) {
+final Number[] numbers = (Number[]) o;
+final long[] result = Allocator.longArray(numbers.length);
+Arrays.setAll(result, i -> numbers[i].longValue());
+return result;
+}
+if (o instanceof short[]) {
+final short[] numbers = (short[]) o;
+final long[] result = Allocator.longArray(numbers.length);
+Arrays.setAll(result, i ->  0x & numbers[i]);
+return result;
+}
+if (o instanceof int[]) {
+final int[] numbers = (int[]) o;
+final long[]result = Allocator.longArray(numbers.length);
+Arrays.setAll(result, i ->  0xL & numbers[i]);
+return result;
+}
+if (o instanceof long[]){
+  final long[] numbers = (long[]) o;
+  return Arrays.copyOf(numbers, numbers.length);
+}
+
+throw new ImagingException("Unknown value: " + o + " for: "
++ getTagInfo().getDescription());
+// return null;
+}
+
+public long getLongValue() throws ImagingException {

Review Comment:
   We should provide Javadocs on all new public and protected methods IMO.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] garydgregory commented on a diff in pull request #318: Imaging-316: Add support for BigTIFF format

2023-09-19 Thread via GitHub 


garydgregory commented on code in PR #318:
URL: https://github.com/apache/commons-imaging/pull/318#discussion_r1330152071


##
src/main/java/org/apache/commons/imaging/common/BinaryFunctions.java:
##
@@ -177,6 +177,44 @@ public static int read4Bytes(final String name, final 
InputStream is,
 return result;
 }
 
+   public static long read8Bytes(final String name,

Review Comment:
   Javadoc?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] garydgregory commented on a diff in pull request #318: Imaging-316: Add support for BigTIFF format

2023-09-19 Thread via GitHub 


garydgregory commented on code in PR #318:
URL: https://github.com/apache/commons-imaging/pull/318#discussion_r1330151666


##
src/main/java/org/apache/commons/imaging/formats/tiff/TiffReader.java:
##
@@ -455,27 +473,55 @@ private TiffHeader readTiffHeader(final ByteSource 
byteSource) throws ImagingExc
 }
 }
 
-private TiffHeader readTiffHeader(final InputStream is) throws 
ImagingException, IOException {
-final int byteOrder1 = readByte("BYTE_ORDER_1", is, "Not a Valid TIFF 
File");
-final int byteOrder2 = readByte("BYTE_ORDER_2", is, "Not a Valid TIFF 
File");
-if (byteOrder1 != byteOrder2) {
-throw new ImagingException("Byte Order bytes don't match (" + 
byteOrder1 + ", " + byteOrder2 + ").");
-}
-
-final ByteOrder byteOrder = getTiffByteOrder(byteOrder1);
-setByteOrder(byteOrder);
-
-final int tiffVersion = read2Bytes("tiffVersion", is, "Not a Valid 
TIFF File", getByteOrder());
-if (tiffVersion != 42) {
-throw new ImagingException("Unknown TIFF Version: " + tiffVersion);
-}
+   private TiffHeader readTiffHeader(final InputStream is) throws 
ImagingException, IOException {
+final int byteOrder1 = readByte("BYTE_ORDER_1", is, "Not a Valid TIFF 
File");
+final int byteOrder2 = readByte("BYTE_ORDER_2", is, "Not a Valid TIFF 
File");
+if (byteOrder1 != byteOrder2) {
+  throw new ImagingException("Byte Order bytes don't match (" + byteOrder1 
+ ", " + byteOrder2 + ").");
+}
 
-final long offsetToFirstIFD =
-0xL & read4Bytes("offsetToFirstIFD", is, "Not a Valid 
TIFF File", getByteOrder());
+final ByteOrder byteOrder = getTiffByteOrder(byteOrder1);
+setByteOrder(byteOrder);
+
+// verify that the file is a supported TIFF format using
+// the numeric indentifier
+// Classic TIFF (32 bit):42
+// Big TIFF (64 bit):43
+//
+final long offsetToFirstIFD;
+final int tiffVersion = read2Bytes("tiffVersion", is, "Not a Valid TIFF 
File", getByteOrder());
+if (tiffVersion == TIFF_VERSION_STANDARD) {
+  bigTiff = false;
+  standardTiff = true;
+  entryMaxValueLength = TIFF_ENTRY_MAX_VALUE_LENGTH;
+  offsetToFirstIFD
+= 0xL & read4Bytes(
+  "offsetToFirstIFD", is, "Not a Valid TIFF File", getByteOrder());
+} else if (tiffVersion == TIFF_VERSION_BIG) {
+  bigTiff = true;
+  standardTiff = false;
+   entryMaxValueLength = TIFF_ENTRY_MAX_VALUE_LENGTH_BIG;
+  int byteSize

Review Comment:
   I'm really not a fan of "newspaper" formatting, IOW very short lines. I've 
set the checkstyle max line length to 160 (you can rebase if you want to pick 
that up).



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org