Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[IMAGING-105] Add methods for editing EXIF-value for rotation for jpeg-files #283

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Copy link
Member

Choose a reason for hiding this comment

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

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?

Original file line number Diff line number Diff line change
@@ -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 {
Copy link
Member

Choose a reason for hiding this comment

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

Missing javadoc with @since tag (see other classes).


public enum Orientation {
Copy link
Member

Choose a reason for hiding this comment

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

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

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
Copy link
Member

Choose a reason for hiding this comment

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

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.).

* Returns horizontal by default
* @return Orientation enum
* @throws IOException
* @throws ImageReadException
* @throws ImageWriteException
Copy link
Member

Choose a reason for hiding this comment

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

These also need a comment explaining the exceptions.

*/
public Orientation getExifOrientation() throws IOException, ImageReadException, ImageWriteException {

final JpegImageMetadata metadata = (JpegImageMetadata) Imaging.getMetadata(this.fileSrc.getAll());

if (metadata == null) {
return Orientation.HORIZONTAL;
}
Copy link
Member

Choose a reason for hiding this comment

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

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.


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
*
Copy link
Member

Choose a reason for hiding this comment

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

Missing @throws here...

*/
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<>())));
Copy link
Member

Choose a reason for hiding this comment

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

I think 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.

Copy link
Member

Choose a reason for hiding this comment

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

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.

}

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
Copy link
Member

Choose a reason for hiding this comment

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

Extra space before Writes, and s/Bytesource/ByteSource

* @param path String of the path in which the file is written
* @throws IOException
*/
public void getOutput(String path)
Copy link
Member

Choose a reason for hiding this comment

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

I think this is never tested?

throws IOException {
final File tempFile = new File(path);
try (FileOutputStream outputStream = new FileOutputStream(tempFile)) {
outputStream.write(fileSrc.getAll());
}
}

}
Original file line number Diff line number Diff line change
@@ -0,0 +1,185 @@
/*
* 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 static org.junit.jupiter.api.Assertions.assertDoesNotThrow;
import static org.junit.jupiter.api.Assertions.assertEquals;
import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull;

import java.io.ByteArrayOutputStream;
import java.io.File;
import java.io.IOException;
import java.nio.file.Files;
import java.util.List;

import org.apache.commons.imaging.Imaging;
import org.apache.commons.imaging.ImageReadException;
import org.apache.commons.imaging.ImageWriteException;
import org.apache.commons.imaging.common.bytesource.ByteSource;
import org.apache.commons.imaging.common.bytesource.ByteSourceFile;
import org.apache.commons.imaging.formats.jpeg.JpegImageMetadata;
import org.apache.commons.imaging.formats.jpeg.exif.ExifOrientationRewriter.Orientation;
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.TiffOutputSet;
import org.apache.commons.io.FileUtils;
import org.junit.jupiter.api.Test;

public class ExifOrientationRewriterTest extends ExifBaseTest {

@Test
public void originalOrientationMatchesRewriterGet()
throws ImageReadException, IOException, ImageWriteException {

final List<File> images = getImagesWithExifData();
for (final File imageFile : images) {
final JpegImageMetadata originalMetadata = (JpegImageMetadata) Imaging.getMetadata(imageFile);
assertNotNull(originalMetadata);

final TiffImageMetadata originalExifMetadata = originalMetadata.getExif();
assertNotNull(originalExifMetadata);

short originalOrt = (short) originalExifMetadata.getFieldValue(TiffTagConstants.TIFF_TAG_ORIENTATION);

ExifOrientationRewriter rewriter = new ExifOrientationRewriter(imageFile);
short ort = rewriter.getExifOrientation().getVal();

assertEquals(ort, originalOrt);
}

}

@Test
public void rewrittenOrientationMatchesRewriterGet()
throws ImageReadException, IOException, ImageWriteException {

final List<File> images = getImagesWithExifData();
for (final File imageFile : images) {
final ByteSource byteSource = new ByteSourceFile(imageFile);

final JpegImageMetadata originalMetadata = (JpegImageMetadata) Imaging.getMetadata(imageFile);
assertNotNull(originalMetadata);

final TiffImageMetadata originalExifMetadata = originalMetadata.getExif();
assertNotNull(originalExifMetadata);

final TiffOutputSet outputSet = originalExifMetadata.getOutputSet();

outputSet.getOrCreateRootDirectory().removeField(TiffTagConstants.TIFF_TAG_ORIENTATION);
outputSet.getOrCreateRootDirectory().add(TiffTagConstants.TIFF_TAG_ORIENTATION, (short) TiffTagConstants.ORIENTATION_VALUE_ROTATE_270_CW);

final ByteArrayOutputStream baos = new ByteArrayOutputStream();

new ExifRewriter().updateExifMetadataLossy(byteSource.getAll(), baos,
outputSet);

final byte[] bytes = baos.toByteArray();
final File tempFile = Files.createTempFile("inserted" + "_", ".jpg").toFile();

FileUtils.writeByteArrayToFile(tempFile, bytes);

final ExifOrientationRewriter rewriter = new ExifOrientationRewriter(tempFile);
short ort = rewriter.getExifOrientation().getVal();

assertEquals(ort, (short) TiffTagConstants.ORIENTATION_VALUE_ROTATE_270_CW);
}

}

@Test
public void getWithNullExifReturnsHorizontal()
throws ImageReadException, IOException, ImageWriteException {

final List<File> images = getImagesWithExifData();
for (final File imageFile : images) {
final ByteSource byteSource = new ByteSourceFile(imageFile);
final JpegImageMetadata originalMetadata = (JpegImageMetadata) Imaging.getMetadata(imageFile);
assertNotNull(originalMetadata);

final TiffImageMetadata originalExifMetadata = originalMetadata.getExif();
assertNotNull(originalExifMetadata);

final ByteArrayOutputStream baos = new ByteArrayOutputStream();
new ExifRewriter().removeExifMetadata(byteSource, baos);
final byte[] bytes = baos.toByteArray();
final File tempFile = Files.createTempFile("removed", ".jpg").toFile();
FileUtils.writeByteArrayToFile(tempFile, bytes);

assertFalse(hasExifData(tempFile));

final ExifOrientationRewriter rewriter = new ExifOrientationRewriter(tempFile);
short ort = rewriter.getExifOrientation().getVal();

assertEquals(ort, Orientation.HORIZONTAL.getVal());
}

}

@Test
void testSetExifOrientation()
throws IOException, ImageReadException, ImageWriteException {

final List<File> images = getImagesWithExifData();
for (final File imageFile : images) {
ExifOrientationRewriter eor = new ExifOrientationRewriter(imageFile);
ExifOrientationRewriter.Orientation eo = ExifOrientationRewriter.Orientation.ROTATE_180;
eor.setExifOrientation(eo);

ByteSource bs = eor.getOutput();
final JpegImageMetadata newMetadata = (JpegImageMetadata) Imaging.getMetadata(bs.getAll());
final TiffImageMetadata newExifMetadata = newMetadata.getExif();

assertNotNull(newExifMetadata, "The new EXIF metadata is null");
assertEquals(newExifMetadata.getFieldValue(TiffTagConstants.TIFF_TAG_ORIENTATION), Short.valueOf(eo.getVal()), "The orientation field in the EXIF metadata is not set correctly");
}
}

@Test
public void setWithNullExifDoesNotThrow()
throws ImageReadException, IOException, ImageWriteException {

final List<File> images = getImagesWithExifData();
for (final File imageFile : images) {
final ByteSource byteSource = new ByteSourceFile(imageFile);
final JpegImageMetadata originalMetadata = (JpegImageMetadata) Imaging.getMetadata(imageFile);
assertNotNull(originalMetadata);

final TiffImageMetadata originalExifMetadata = originalMetadata.getExif();
assertNotNull(originalExifMetadata);

final ByteArrayOutputStream baos = new ByteArrayOutputStream();
new ExifRewriter().removeExifMetadata(byteSource, baos);
final byte[] bytes = baos.toByteArray();
final File tempFile = Files.createTempFile("removed", ".jpg").toFile();
FileUtils.writeByteArrayToFile(tempFile, bytes);

assertFalse(hasExifData(tempFile));

final ExifOrientationRewriter rewriter = new ExifOrientationRewriter(tempFile);
assertDoesNotThrow(() -> rewriter.setExifOrientation(Orientation.ROTATE_180));

ByteSource bs = rewriter.getOutput();
final JpegImageMetadata newMetadata = (JpegImageMetadata) Imaging.getMetadata(bs.getAll());
final TiffImageMetadata newExifMetadata = newMetadata.getExif();

assertNotNull(newExifMetadata, "The new EXIF metadata is null");
assertEquals(newExifMetadata.getFieldValue(TiffTagConstants.TIFF_TAG_ORIENTATION), Orientation.ROTATE_180.getVal(), "The orientation field in the EXIF metadata is not set correctly");
}
}
}