kinow commented on code in PR #283:
URL: https://github.com/apache/commons-imaging/pull/283#discussion_r1139419012


##########
src/main/java/org/apache/commons/imaging/formats/jpeg/exif/ExifOrientationRewriter.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg.exif;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.ImageWriteException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.common.bytesource.ByteSourceArray;
+import org.apache.commons.imaging.common.bytesource.ByteSourceFile;
+import org.apache.commons.imaging.formats.jpeg.JpegImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffContents;
+import org.apache.commons.imaging.formats.tiff.TiffHeader;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputDirectory;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputField;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputSet;
+
+public class ExifOrientationRewriter {
+
+    public enum Orientation {
+        HORIZONTAL((short)1),
+        MIRROR_HORIZONTAL((short)2),
+        ROTATE_180((short)3),
+        MIRROR_VERTICAL((short)4),
+        MIRROR_HORIZONTAL_AND_ROTATE_270((short)5),
+        ROTATE_90((short)6),
+        MIRROR_HORIZONTAL_AND_ROTATE_90((short)7),
+        ROTATE_270((short)8);
+
+        private short val;
+
+        Orientation(short orVal) {
+            this.val = orVal;
+        }
+
+        public short getVal() {
+            return val;
+        }
+    }
+
+    private ByteSource fileSrc;
+
+    public ExifOrientationRewriter(File imageFile) {
+        fileSrc = new ByteSourceFile(imageFile);
+    }
+    public ExifOrientationRewriter(byte[] byteArray) {
+        fileSrc = new ByteSourceArray(byteArray);
+    }
+    public ExifOrientationRewriter(ByteSource byteSource) {
+        fileSrc = byteSource;
+    }
+
+    /***
+     * Get the orientation (enum) of the current image
+     * Returns horizontal by default
+     * @return Orientation enum
+     * @throws IOException
+     * @throws ImageReadException
+     * @throws ImageWriteException

Review Comment:
   These also need a comment explaining the exceptions.



##########
src/main/java/org/apache/commons/imaging/formats/jpeg/exif/ExifOrientationRewriter.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg.exif;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.ImageWriteException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.common.bytesource.ByteSourceArray;
+import org.apache.commons.imaging.common.bytesource.ByteSourceFile;
+import org.apache.commons.imaging.formats.jpeg.JpegImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffContents;
+import org.apache.commons.imaging.formats.tiff.TiffHeader;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputDirectory;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputField;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputSet;
+
+public class ExifOrientationRewriter {
+
+    public enum Orientation {
+        HORIZONTAL((short)1),
+        MIRROR_HORIZONTAL((short)2),
+        ROTATE_180((short)3),
+        MIRROR_VERTICAL((short)4),
+        MIRROR_HORIZONTAL_AND_ROTATE_270((short)5),
+        ROTATE_90((short)6),
+        MIRROR_HORIZONTAL_AND_ROTATE_90((short)7),
+        ROTATE_270((short)8);
+
+        private short val;
+
+        Orientation(short orVal) {
+            this.val = orVal;
+        }
+
+        public short getVal() {
+            return val;
+        }
+    }
+
+    private ByteSource fileSrc;
+
+    public ExifOrientationRewriter(File imageFile) {
+        fileSrc = new ByteSourceFile(imageFile);
+    }
+    public ExifOrientationRewriter(byte[] byteArray) {
+        fileSrc = new ByteSourceArray(byteArray);
+    }
+    public ExifOrientationRewriter(ByteSource byteSource) {
+        fileSrc = byteSource;
+    }
+
+    /***
+     * Get the orientation (enum) of the current image
+     * Returns horizontal by default
+     * @return Orientation enum
+     * @throws IOException
+     * @throws ImageReadException
+     * @throws ImageWriteException
+     */
+    public Orientation getExifOrientation() throws IOException, 
ImageReadException, ImageWriteException {
+
+        final JpegImageMetadata metadata = (JpegImageMetadata) 
Imaging.getMetadata(this.fileSrc.getAll());
+
+        if (metadata == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        final TiffImageMetadata exifMetadata = metadata.getExif();
+
+        if (exifMetadata == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        final TiffOutputSet outputSet = exifMetadata.getOutputSet();
+
+        TiffOutputDirectory tod = outputSet.getRootDirectory();
+        if (tod == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        TiffOutputField tof = 
tod.findField(TiffTagConstants.TIFF_TAG_ORIENTATION);
+        if (tof == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        short imageOrientationVal = (short) 
exifMetadata.getFieldValue(TiffTagConstants.TIFF_TAG_ORIENTATION);
+
+        for (Orientation orientation : Orientation.values()) {
+            if(orientation.getVal() == imageOrientationVal) {
+                return orientation;
+            }
+        }
+
+        return Orientation.HORIZONTAL;
+    }
+
+    /**
+     * A method that sets a new value to the orientation field in the EXIF 
metadata of a JPEG file.
+     * @param orientation the value as a enum of the direction to set as the 
new EXIF orientation
+     *
+     */
+    public void setExifOrientation(Orientation orientation) throws 
ImageWriteException, IOException, ImageReadException {
+
+        JpegImageMetadata metadata = (JpegImageMetadata) 
Imaging.getMetadata(this.fileSrc.getAll());
+
+        if (metadata == null) {
+            metadata = new JpegImageMetadata(null, new TiffImageMetadata(new 
TiffContents(new TiffHeader(ByteOrder.BIG_ENDIAN, 0, 0), new ArrayList<>(), new 
ArrayList<>())));
+        }
+
+        TiffImageMetadata exifMetadata = metadata.getExif();
+
+        if (exifMetadata == null) {
+            exifMetadata = new TiffImageMetadata(new TiffContents(new 
TiffHeader(ByteOrder.BIG_ENDIAN, 0, 0), new ArrayList<>(), new ArrayList<>()));
+        }
+
+        final TiffOutputSet outputSet = exifMetadata.getOutputSet();
+
+        TiffOutputDirectory tod =  outputSet.getOrCreateRootDirectory();
+        tod.removeField(TiffTagConstants.TIFF_TAG_ORIENTATION);
+        tod.add(TiffTagConstants.TIFF_TAG_ORIENTATION, orientation.getVal());
+
+        final ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        new ExifRewriter().updateExifMetadataLossy(this.fileSrc, baos, 
outputSet);
+
+        this.fileSrc = new ByteSourceArray(baos.toByteArray());
+    }
+
+    /**
+     * @return the ByteSource of the current file
+     */
+    public ByteSource getOutput() {
+        return fileSrc;
+    }
+
+    /**
+     *  Writes Bytesource to file with given path

Review Comment:
   Extra space before Writes, and s/Bytesource/ByteSource



##########
src/main/java/org/apache/commons/imaging/formats/jpeg/exif/ExifOrientationRewriter.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg.exif;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.ImageWriteException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.common.bytesource.ByteSourceArray;
+import org.apache.commons.imaging.common.bytesource.ByteSourceFile;
+import org.apache.commons.imaging.formats.jpeg.JpegImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffContents;
+import org.apache.commons.imaging.formats.tiff.TiffHeader;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputDirectory;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputField;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputSet;
+
+public class ExifOrientationRewriter {
+
+    public enum Orientation {
+        HORIZONTAL((short)1),
+        MIRROR_HORIZONTAL((short)2),
+        ROTATE_180((short)3),
+        MIRROR_VERTICAL((short)4),
+        MIRROR_HORIZONTAL_AND_ROTATE_270((short)5),
+        ROTATE_90((short)6),
+        MIRROR_HORIZONTAL_AND_ROTATE_90((short)7),
+        ROTATE_270((short)8);
+
+        private short val;
+
+        Orientation(short orVal) {
+            this.val = orVal;
+        }
+
+        public short getVal() {
+            return val;
+        }
+    }
+
+    private ByteSource fileSrc;
+
+    public ExifOrientationRewriter(File imageFile) {
+        fileSrc = new ByteSourceFile(imageFile);
+    }
+    public ExifOrientationRewriter(byte[] byteArray) {
+        fileSrc = new ByteSourceArray(byteArray);
+    }
+    public ExifOrientationRewriter(ByteSource byteSource) {
+        fileSrc = byteSource;
+    }
+
+    /***
+     * Get the orientation (enum) of the current image

Review Comment:
   Needs a dot. The other sentence can come directly after this one, or leave 
it in the next line (or if you intended to have two lines, use HTML to add a 
break line, new paragraph, etc.).



##########
src/main/java/org/apache/commons/imaging/formats/jpeg/exif/ExifOrientationRewriter.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg.exif;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.ImageWriteException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.common.bytesource.ByteSourceArray;
+import org.apache.commons.imaging.common.bytesource.ByteSourceFile;
+import org.apache.commons.imaging.formats.jpeg.JpegImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffContents;
+import org.apache.commons.imaging.formats.tiff.TiffHeader;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputDirectory;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputField;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputSet;
+
+public class ExifOrientationRewriter {

Review Comment:
   Missing javadoc with `@since` tag (see other classes).



##########
src/main/java/org/apache/commons/imaging/formats/jpeg/exif/ExifOrientationRewriter.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg.exif;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.ImageWriteException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.common.bytesource.ByteSourceArray;
+import org.apache.commons.imaging.common.bytesource.ByteSourceFile;
+import org.apache.commons.imaging.formats.jpeg.JpegImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffContents;
+import org.apache.commons.imaging.formats.tiff.TiffHeader;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputDirectory;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputField;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputSet;
+
+public class ExifOrientationRewriter {
+
+    public enum Orientation {
+        HORIZONTAL((short)1),
+        MIRROR_HORIZONTAL((short)2),
+        ROTATE_180((short)3),
+        MIRROR_VERTICAL((short)4),
+        MIRROR_HORIZONTAL_AND_ROTATE_270((short)5),
+        ROTATE_90((short)6),
+        MIRROR_HORIZONTAL_AND_ROTATE_90((short)7),
+        ROTATE_270((short)8);
+
+        private short val;
+
+        Orientation(short orVal) {
+            this.val = orVal;
+        }
+
+        public short getVal() {
+            return val;
+        }
+    }
+
+    private ByteSource fileSrc;
+
+    public ExifOrientationRewriter(File imageFile) {
+        fileSrc = new ByteSourceFile(imageFile);
+    }
+    public ExifOrientationRewriter(byte[] byteArray) {
+        fileSrc = new ByteSourceArray(byteArray);
+    }
+    public ExifOrientationRewriter(ByteSource byteSource) {
+        fileSrc = byteSource;
+    }
+
+    /***
+     * Get the orientation (enum) of the current image
+     * Returns horizontal by default
+     * @return Orientation enum
+     * @throws IOException
+     * @throws ImageReadException
+     * @throws ImageWriteException
+     */
+    public Orientation getExifOrientation() throws IOException, 
ImageReadException, ImageWriteException {
+
+        final JpegImageMetadata metadata = (JpegImageMetadata) 
Imaging.getMetadata(this.fileSrc.getAll());
+
+        if (metadata == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        final TiffImageMetadata exifMetadata = metadata.getExif();
+
+        if (exifMetadata == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        final TiffOutputSet outputSet = exifMetadata.getOutputSet();
+
+        TiffOutputDirectory tod = outputSet.getRootDirectory();
+        if (tod == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        TiffOutputField tof = 
tod.findField(TiffTagConstants.TIFF_TAG_ORIENTATION);
+        if (tof == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        short imageOrientationVal = (short) 
exifMetadata.getFieldValue(TiffTagConstants.TIFF_TAG_ORIENTATION);
+
+        for (Orientation orientation : Orientation.values()) {
+            if(orientation.getVal() == imageOrientationVal) {
+                return orientation;
+            }
+        }
+
+        return Orientation.HORIZONTAL;
+    }
+
+    /**
+     * A method that sets a new value to the orientation field in the EXIF 
metadata of a JPEG file.
+     * @param orientation the value as a enum of the direction to set as the 
new EXIF orientation
+     *
+     */
+    public void setExifOrientation(Orientation orientation) throws 
ImageWriteException, IOException, ImageReadException {
+
+        JpegImageMetadata metadata = (JpegImageMetadata) 
Imaging.getMetadata(this.fileSrc.getAll());
+
+        if (metadata == null) {
+            metadata = new JpegImageMetadata(null, new TiffImageMetadata(new 
TiffContents(new TiffHeader(ByteOrder.BIG_ENDIAN, 0, 0), new ArrayList<>(), new 
ArrayList<>())));
+        }
+
+        TiffImageMetadata exifMetadata = metadata.getExif();
+
+        if (exifMetadata == null) {
+            exifMetadata = new TiffImageMetadata(new TiffContents(new 
TiffHeader(ByteOrder.BIG_ENDIAN, 0, 0), new ArrayList<>(), new ArrayList<>()));
+        }
+
+        final TiffOutputSet outputSet = exifMetadata.getOutputSet();
+
+        TiffOutputDirectory tod =  outputSet.getOrCreateRootDirectory();
+        tod.removeField(TiffTagConstants.TIFF_TAG_ORIENTATION);
+        tod.add(TiffTagConstants.TIFF_TAG_ORIENTATION, orientation.getVal());
+
+        final ByteArrayOutputStream baos = new ByteArrayOutputStream();
+        new ExifRewriter().updateExifMetadataLossy(this.fileSrc, baos, 
outputSet);
+
+        this.fileSrc = new ByteSourceArray(baos.toByteArray());
+    }
+
+    /**
+     * @return the ByteSource of the current file
+     */
+    public ByteSource getOutput() {
+        return fileSrc;
+    }
+
+    /**
+     *  Writes Bytesource to file with given path
+     * @param path String of the path in which the file is written
+     * @throws IOException
+     */
+    public void getOutput(String path)

Review Comment:
   I think this is never tested?



##########
src/main/java/org/apache/commons/imaging/formats/jpeg/exif/ExifOrientationRewriter.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg.exif;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.ImageWriteException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.common.bytesource.ByteSourceArray;
+import org.apache.commons.imaging.common.bytesource.ByteSourceFile;
+import org.apache.commons.imaging.formats.jpeg.JpegImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffContents;
+import org.apache.commons.imaging.formats.tiff.TiffHeader;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputDirectory;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputField;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputSet;
+
+public class ExifOrientationRewriter {
+
+    public enum Orientation {

Review Comment:
   At least everything `public` must have a Javadoc :point_down:  (tests should 
be fine as we don't have that in other tests, unless you have time to add it 
too).



##########
src/main/java/org/apache/commons/imaging/formats/jpeg/exif/ExifOrientationRewriter.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg.exif;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.ImageWriteException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.common.bytesource.ByteSourceArray;
+import org.apache.commons.imaging.common.bytesource.ByteSourceFile;
+import org.apache.commons.imaging.formats.jpeg.JpegImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffContents;
+import org.apache.commons.imaging.formats.tiff.TiffHeader;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputDirectory;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputField;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputSet;
+
+public class ExifOrientationRewriter {
+
+    public enum Orientation {
+        HORIZONTAL((short)1),
+        MIRROR_HORIZONTAL((short)2),
+        ROTATE_180((short)3),
+        MIRROR_VERTICAL((short)4),
+        MIRROR_HORIZONTAL_AND_ROTATE_270((short)5),
+        ROTATE_90((short)6),
+        MIRROR_HORIZONTAL_AND_ROTATE_90((short)7),
+        ROTATE_270((short)8);
+
+        private short val;
+
+        Orientation(short orVal) {
+            this.val = orVal;
+        }
+
+        public short getVal() {
+            return val;
+        }
+    }
+
+    private ByteSource fileSrc;
+
+    public ExifOrientationRewriter(File imageFile) {
+        fileSrc = new ByteSourceFile(imageFile);
+    }
+    public ExifOrientationRewriter(byte[] byteArray) {
+        fileSrc = new ByteSourceArray(byteArray);
+    }
+    public ExifOrientationRewriter(ByteSource byteSource) {
+        fileSrc = byteSource;
+    }
+
+    /***
+     * Get the orientation (enum) of the current image
+     * Returns horizontal by default
+     * @return Orientation enum
+     * @throws IOException
+     * @throws ImageReadException
+     * @throws ImageWriteException
+     */
+    public Orientation getExifOrientation() throws IOException, 
ImageReadException, ImageWriteException {
+
+        final JpegImageMetadata metadata = (JpegImageMetadata) 
Imaging.getMetadata(this.fileSrc.getAll());
+
+        if (metadata == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        final TiffImageMetadata exifMetadata = metadata.getExif();
+
+        if (exifMetadata == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        final TiffOutputSet outputSet = exifMetadata.getOutputSet();
+
+        TiffOutputDirectory tod = outputSet.getRootDirectory();
+        if (tod == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        TiffOutputField tof = 
tod.findField(TiffTagConstants.TIFF_TAG_ORIENTATION);
+        if (tof == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        short imageOrientationVal = (short) 
exifMetadata.getFieldValue(TiffTagConstants.TIFF_TAG_ORIENTATION);
+
+        for (Orientation orientation : Orientation.values()) {
+            if(orientation.getVal() == imageOrientationVal) {
+                return orientation;
+            }
+        }
+
+        return Orientation.HORIZONTAL;
+    }
+
+    /**
+     * A method that sets a new value to the orientation field in the EXIF 
metadata of a JPEG file.
+     * @param orientation the value as a enum of the direction to set as the 
new EXIF orientation
+     *

Review Comment:
   Missing `@throws` here...



##########
src/main/java/org/apache/commons/imaging/formats/jpeg/exif/ExifOrientationRewriter.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg.exif;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.ImageWriteException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.common.bytesource.ByteSourceArray;
+import org.apache.commons.imaging.common.bytesource.ByteSourceFile;
+import org.apache.commons.imaging.formats.jpeg.JpegImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffContents;
+import org.apache.commons.imaging.formats.tiff.TiffHeader;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputDirectory;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputField;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputSet;
+
+public class ExifOrientationRewriter {
+
+    public enum Orientation {
+        HORIZONTAL((short)1),
+        MIRROR_HORIZONTAL((short)2),
+        ROTATE_180((short)3),
+        MIRROR_VERTICAL((short)4),
+        MIRROR_HORIZONTAL_AND_ROTATE_270((short)5),
+        ROTATE_90((short)6),
+        MIRROR_HORIZONTAL_AND_ROTATE_90((short)7),
+        ROTATE_270((short)8);
+
+        private short val;
+
+        Orientation(short orVal) {
+            this.val = orVal;
+        }
+
+        public short getVal() {
+            return val;
+        }
+    }
+
+    private ByteSource fileSrc;
+
+    public ExifOrientationRewriter(File imageFile) {
+        fileSrc = new ByteSourceFile(imageFile);
+    }
+    public ExifOrientationRewriter(byte[] byteArray) {
+        fileSrc = new ByteSourceArray(byteArray);
+    }
+    public ExifOrientationRewriter(ByteSource byteSource) {
+        fileSrc = byteSource;
+    }
+
+    /***
+     * Get the orientation (enum) of the current image
+     * Returns horizontal by default
+     * @return Orientation enum
+     * @throws IOException
+     * @throws ImageReadException
+     * @throws ImageWriteException
+     */
+    public Orientation getExifOrientation() throws IOException, 
ImageReadException, ImageWriteException {
+
+        final JpegImageMetadata metadata = (JpegImageMetadata) 
Imaging.getMetadata(this.fileSrc.getAll());
+
+        if (metadata == null) {
+            return Orientation.HORIZONTAL;
+        }

Review Comment:
   Isn't that dangerous to return horizontal? Wouldn't it be better to return 
`null` instead? This way you have the three valid states, 1) data has 
horizontal orientation, 2) data has vertical orientation, or 3) no metadata 
about orientation.



##########
src/main/java/org/apache/commons/imaging/formats/jpeg/exif/ExifOrientationRewriter.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg.exif;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.ImageWriteException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.common.bytesource.ByteSourceArray;
+import org.apache.commons.imaging.common.bytesource.ByteSourceFile;
+import org.apache.commons.imaging.formats.jpeg.JpegImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffContents;
+import org.apache.commons.imaging.formats.tiff.TiffHeader;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputDirectory;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputField;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputSet;
+
+public class ExifOrientationRewriter {
+
+    public enum Orientation {
+        HORIZONTAL((short)1),
+        MIRROR_HORIZONTAL((short)2),
+        ROTATE_180((short)3),
+        MIRROR_VERTICAL((short)4),
+        MIRROR_HORIZONTAL_AND_ROTATE_270((short)5),
+        ROTATE_90((short)6),
+        MIRROR_HORIZONTAL_AND_ROTATE_90((short)7),
+        ROTATE_270((short)8);
+
+        private short val;
+
+        Orientation(short orVal) {
+            this.val = orVal;
+        }
+
+        public short getVal() {
+            return val;
+        }
+    }
+
+    private ByteSource fileSrc;
+
+    public ExifOrientationRewriter(File imageFile) {
+        fileSrc = new ByteSourceFile(imageFile);
+    }
+    public ExifOrientationRewriter(byte[] byteArray) {
+        fileSrc = new ByteSourceArray(byteArray);
+    }
+    public ExifOrientationRewriter(ByteSource byteSource) {
+        fileSrc = byteSource;
+    }
+
+    /***
+     * Get the orientation (enum) of the current image
+     * Returns horizontal by default
+     * @return Orientation enum
+     * @throws IOException
+     * @throws ImageReadException
+     * @throws ImageWriteException
+     */
+    public Orientation getExifOrientation() throws IOException, 
ImageReadException, ImageWriteException {
+
+        final JpegImageMetadata metadata = (JpegImageMetadata) 
Imaging.getMetadata(this.fileSrc.getAll());
+
+        if (metadata == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        final TiffImageMetadata exifMetadata = metadata.getExif();
+
+        if (exifMetadata == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        final TiffOutputSet outputSet = exifMetadata.getOutputSet();
+
+        TiffOutputDirectory tod = outputSet.getRootDirectory();
+        if (tod == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        TiffOutputField tof = 
tod.findField(TiffTagConstants.TIFF_TAG_ORIENTATION);
+        if (tof == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        short imageOrientationVal = (short) 
exifMetadata.getFieldValue(TiffTagConstants.TIFF_TAG_ORIENTATION);
+
+        for (Orientation orientation : Orientation.values()) {
+            if(orientation.getVal() == imageOrientationVal) {
+                return orientation;
+            }
+        }
+
+        return Orientation.HORIZONTAL;
+    }
+
+    /**
+     * A method that sets a new value to the orientation field in the EXIF 
metadata of a JPEG file.
+     * @param orientation the value as a enum of the direction to set as the 
new EXIF orientation
+     *
+     */
+    public void setExifOrientation(Orientation orientation) throws 
ImageWriteException, IOException, ImageReadException {
+
+        JpegImageMetadata metadata = (JpegImageMetadata) 
Imaging.getMetadata(this.fileSrc.getAll());
+
+        if (metadata == null) {
+            metadata = new JpegImageMetadata(null, new TiffImageMetadata(new 
TiffContents(new TiffHeader(ByteOrder.BIG_ENDIAN, 0, 0), new ArrayList<>(), new 
ArrayList<>())));

Review Comment:
   I think `ExifRewriter` allows big and little endian. But it looks like 
`ExifOrientationRewriter` uses big endian if no metadata found? Wouldn't be 
better if we had the same behavior as in `ExifRewriter`? Letting the user 
choose the endianess.



##########
src/main/java/org/apache/commons/imaging/formats/jpeg/exif/ExifOrientationRewriter.java:
##########


Review Comment:
   Did you try to design the EXIF rotation as part of `ExifRewriter`, 
@snumlautoken ? What was the rationale for creating a new class instead? I 
guess that could mean that for other metadata rewriting/editing we would need 
other `Exif$MetadataRewriter`? Ditto for other metadata/formats? That sounds 
like it could become problematic in the long run, I think? WDYT?



##########
src/main/java/org/apache/commons/imaging/formats/jpeg/exif/ExifOrientationRewriter.java:
##########
@@ -0,0 +1,173 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements.  See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License.  You may obtain a copy of the License at
+ *
+ *      http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg.exif;
+
+import java.io.ByteArrayOutputStream;
+import java.io.File;
+import java.io.FileOutputStream;
+import java.io.IOException;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.ImageWriteException;
+import org.apache.commons.imaging.Imaging;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.common.bytesource.ByteSourceArray;
+import org.apache.commons.imaging.common.bytesource.ByteSourceFile;
+import org.apache.commons.imaging.formats.jpeg.JpegImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffContents;
+import org.apache.commons.imaging.formats.tiff.TiffHeader;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.constants.TiffTagConstants;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputDirectory;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputField;
+import org.apache.commons.imaging.formats.tiff.write.TiffOutputSet;
+
+public class ExifOrientationRewriter {
+
+    public enum Orientation {
+        HORIZONTAL((short)1),
+        MIRROR_HORIZONTAL((short)2),
+        ROTATE_180((short)3),
+        MIRROR_VERTICAL((short)4),
+        MIRROR_HORIZONTAL_AND_ROTATE_270((short)5),
+        ROTATE_90((short)6),
+        MIRROR_HORIZONTAL_AND_ROTATE_90((short)7),
+        ROTATE_270((short)8);
+
+        private short val;
+
+        Orientation(short orVal) {
+            this.val = orVal;
+        }
+
+        public short getVal() {
+            return val;
+        }
+    }
+
+    private ByteSource fileSrc;
+
+    public ExifOrientationRewriter(File imageFile) {
+        fileSrc = new ByteSourceFile(imageFile);
+    }
+    public ExifOrientationRewriter(byte[] byteArray) {
+        fileSrc = new ByteSourceArray(byteArray);
+    }
+    public ExifOrientationRewriter(ByteSource byteSource) {
+        fileSrc = byteSource;
+    }
+
+    /***
+     * Get the orientation (enum) of the current image
+     * Returns horizontal by default
+     * @return Orientation enum
+     * @throws IOException
+     * @throws ImageReadException
+     * @throws ImageWriteException
+     */
+    public Orientation getExifOrientation() throws IOException, 
ImageReadException, ImageWriteException {
+
+        final JpegImageMetadata metadata = (JpegImageMetadata) 
Imaging.getMetadata(this.fileSrc.getAll());
+
+        if (metadata == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        final TiffImageMetadata exifMetadata = metadata.getExif();
+
+        if (exifMetadata == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        final TiffOutputSet outputSet = exifMetadata.getOutputSet();
+
+        TiffOutputDirectory tod = outputSet.getRootDirectory();
+        if (tod == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        TiffOutputField tof = 
tod.findField(TiffTagConstants.TIFF_TAG_ORIENTATION);
+        if (tof == null) {
+            return Orientation.HORIZONTAL;
+        }
+
+        short imageOrientationVal = (short) 
exifMetadata.getFieldValue(TiffTagConstants.TIFF_TAG_ORIENTATION);
+
+        for (Orientation orientation : Orientation.values()) {
+            if(orientation.getVal() == imageOrientationVal) {
+                return orientation;
+            }
+        }
+
+        return Orientation.HORIZONTAL;
+    }
+
+    /**
+     * A method that sets a new value to the orientation field in the EXIF 
metadata of a JPEG file.
+     * @param orientation the value as a enum of the direction to set as the 
new EXIF orientation
+     *
+     */
+    public void setExifOrientation(Orientation orientation) throws 
ImageWriteException, IOException, ImageReadException {
+
+        JpegImageMetadata metadata = (JpegImageMetadata) 
Imaging.getMetadata(this.fileSrc.getAll());
+
+        if (metadata == null) {
+            metadata = new JpegImageMetadata(null, new TiffImageMetadata(new 
TiffContents(new TiffHeader(ByteOrder.BIG_ENDIAN, 0, 0), new ArrayList<>(), new 
ArrayList<>())));

Review Comment:
   s/new ArrayList()/Collections.emptyList()
   
   That should create that empty `EmptyList` instead of an `ArrayList`, and I 
think further down the pipe the `new ArrayList` will become an unmodifiable 
collection, so that should be fine.



-- 
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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


Reply via email to