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

JPEG metadata always read-only #668

Open
ojaha065 opened this issue Mar 1, 2022 · 9 comments
Open

JPEG metadata always read-only #668

ojaha065 opened this issue Mar 1, 2022 · 9 comments

Comments

@ojaha065
Copy link

ojaha065 commented Mar 1, 2022

Greetings. I'm trying to read and modify metadata of JPEG images. Let's look at the following (heavily simplified) code:

try (final Closeable imageInputStream = ImageIO.createImageInputStream(new File("D:\\testImage.jpg"))) {
  final ImageReader reader = ImageIO.getImageReaders(imageInputStream).next();
  reader.setInput(imageInputStream);

  final IIOImage imageAndMeta = reader.readAll(0, reader.getDefaultReadParam());
  reader.dispose();

  final IIOMetadata metadata = imageAndMeta.getMetadata();

  final Node root = new IIOMetadataNode("javax_imageio_jpeg_image_1.0");
  root.appendChild(new IIOMetadataNode("JPEGvariety"));
  root.appendChild(new IIOMetadataNode("markerSequence"));
  //root.appendChild(...somethingElse...)

  metadata.mergeTree("javax_imageio_jpeg_image_1.0", root);
} catch (final Exception error) {
  error.printStackTrace();
}

It works fine without TwelveMonkeys, but throws an error (only) when TwelveMonkeys is installed:

java.lang.IllegalStateException: Metadata is read-only
	at com.twelvemonkeys.imageio.AbstractMetadata.assertMutable(AbstractMetadata.java:117) ~[imageio-core-3.8.1.jar:3.8.1]
	at com.twelvemonkeys.imageio.AbstractMetadata.mergeTree(AbstractMetadata.java:96) ~[imageio-core-3.8.1.jar:3.8.1]
	...

It seems that the metadata is always read-only and thus mergeTree function does not work. I've looked into this with debugger and it seems that in TwelveMonkeys' JPEGImage10Metadata.java isReadOnly() always returns true while in the original com.sun.imageio.plugins.jpeg.JPEGMetadata it's always false. Any idea why is this and how could I get around this?

@haraldk
Copy link
Owner

haraldk commented Mar 1, 2022

Hi Jani,

It would help my understanding if you describe what the use case is here, ie. what will you use the merged metadata for. 😀

In any case, the metadata is read-only for now, as there hasn't been any need to merge. It's non-trivial, but probably not too hard to implement mergeTree, if you would like to have a go.

Without actually making the metadata mutable and implementing mergeTree, one option could be to simply create an empty instance of com.sun.imageio.plugins.jpeg.JPEGMetadata and use setFromTree. But keep in mind that I created the JPEGImage10Metadata class because the standard class does not support a lot of in-the-wild JPEGs, so it may not work for all cases...

@ojaha065
Copy link
Author

ojaha065 commented Mar 2, 2022

Hi and thanks for the prompt response.

I totally understand why the use case for this might be a little hard to understand. So let me include some more details. Basically, our application has the following workflow:

  • First, a user uploads an image file. The original file gets saved to disk without any modifications. The significant thing to note is that the original image might contain a lot of EXIF data.

  • Later, we need to generate two to three different sized preview images from the original image. However, one of our customers has a very specific requirement: They would like the generated previews have to same EXIF data than the original image except that DPI is always set 300 regardless of what the original image reports at it's DPI.

To achieve this, our application first reads the metadata from the original image just like shown in the code snippet in my first message. Then we set the required hardcoded metadata like so:

final Node root = new IIOMetadataNode("javax_imageio_jpeg_image_1.0");
final Node jpegVariety = new IIOMetadataNode("JPEGvariety");

final Element app0JFIF = new IIOMetadataNode("app0JFIF");
app0JFIF.setAttribute("Xdensity", "300");
app0JFIF.setAttribute("Ydensity", "300");
//app0JFIF.setAttribute(...);

jpegVariety.appendChild(app0JFIF);
root.appendChild(jpegVariety);

and then we merge our custom metadata (root in the example above) to the orignal metadata with mergeTree. Finally we write the generated preview image like this

imageWriter.write(null, new IIOImage(bufferedImage, null, metadata), iwParam);

This approach worked fine for us for a long time. Recently we noticed ImageIO threw various errors with some images our users uploaded. Adding TwelveMonkeys to the project resolved those issues (good job, btw) but as a side effect of metadata being read-only it broke our metadata handler.

Your suggestion of using a instance of com.sun.imageio.plugins.jpeg.JPEGMetadata seems to the best solution for us right now. From itinial testing it seems to work and it's not a end of world for us if the metadata handling fails for some of the "in-the-wild" JPEGs. It would still be nice if TwelveMonkeys didn't change the behaviour is this regard, but I totally understand if it's not feasible.

@davidekholm
Copy link

We're also facing problems with this in jAlbum 29 since we moved from using v3.5 to v3.8.1 of your lib. In our case it's the setFromTree() call that triggers this error. We're also including the original EXIF data with generated image as a user option, but this no longer works. The reason why we call setFromTree is that we have to modify the tree a bit for some JPEG images in order to avoid errors when including the metadata for some images. I've attached one such image, it triggers an "javax.imageio.metadata.IIOInvalidTreeException: Invalid DHT node" exception if we simply include the original IIOMetadata object when writing the new scaled file.

priscilla-du-preez

@haraldk
Copy link
Owner

haraldk commented Oct 10, 2022

@davidekholm I'm not able to reproduce the problem with the JPEG you provided. It does not seem to be a problem with the DHT node, and I don't find any Exif data either... Can you double check that the file is the correct one, and that GitHub upload does not "fix" the file for us? 😉

If the file is as intended, can you please send me the code you use? I think I should be able to provide a workaround in any case.

PS: I fixed an issue related to the "Invalid DHT node" issue in #559, that is in the 3.8.2 release. I verified that the image indeed has this problem under 3.8.1. Can you please upgrade to 3.8.3.
PPS: I see now that there is actually an Exif segment there, but it's completely empty...

@davidekholm
Copy link

Yes, the image might not have EXIF data, still I simply want to pass along whatever metadata it has. If you can show how to only pass along EXIF data, I appreciate if you can show an example.

Anyway, here's a sample code that throws "Invalid DHT node" when writing an image, passing along metadata from the image I attached earlier. This code is close to pseudocode, but you can actually execute it within jAlbum's system console. Just drop the sample image onto jAlbum, then select it and open jAlbum's system console window, set the script language to "Groovy" and execute it:

// Read image metadata and image
import javax.imageio.*;
iis = ImageIO.createImageInputStream(selected.file);
reader = ImageIO.getImageReadersBySuffix("jpeg").next();
reader.setInput(iis, true, true);
meta = reader.getImageMetadata(reader.getMinIndex());
image = reader.read(reader.getMinIndex())

// Write image along with metadata
outputFile = new File("/tmp/generated-image.jpeg");
writer = ImageIO.getImageWritersByFormatName("jpeg").next();
ios = ImageIO.createImageOutputStream(outputFile);
writer.setOutput(ios);
writer.write(null, new IIOImage(image, null, meta), writer.getDefaultWriteParam());
writer.dispose();
ios.close();

@davidekholm
Copy link

Hi Harald. I've updated to v3.8.3 and the "Invalid DHT node" error is indeed gone now. Thank you.

@haraldk
Copy link
Owner

haraldk commented Oct 11, 2022

One possible workaround:

try (ImageInputStream input = ImageIO.createImageInputStream(...)) {
    ImageReader reader = ImageIO.getImageReaders(input).next();
    reader.setInput(input);

    IIOImage image = reader.readAll(0, null);

    IIOMetadataNode root = (IIOMetadataNode) image.getMetadata().getAsTree("javax_imageio_jpeg_image_1.0");

    // Manipulate metadata from input as you like...

    try (ImageOutputStream output = ImageIO.createImageOutputStream(...))) {
        ImageWriter writer = ImageIO.getImageWriter(reader);
        writer.setOutput(output);

        ImageWriteParam param = writer.getDefaultWriteParam();

        // Get default metadata from writer (to obtain a mutable com.sun.imageio.plugins.jpeg.JPEGMetadata instance)
        IIOMetadata metadata = writer.getDefaultImageMetadata(ImageTypeSpecifiers.createFromRenderedImage(image.getRenderedImage()), param);

        // Populate with manipulated metadata from original
        metadata.setFromTree("javax_imageio_jpeg_image_1.0", root);

        image.setMetadata(metadata);
        writer.write(null, image, param);
    }
}

@davidekholm
Copy link

Thanks Harald, so this will copy the EXIF data only?

@haraldk
Copy link
Owner

haraldk commented Oct 12, 2022

No, it gives you a mutable JPEG metadata, so you can copy whatever you like (what this issue is about).


Copying only Exif is (probably) possible, but tedious..

To copy only the Exif segment, you need to look at the "markerSequence", get all the "unknown" nodes, and get the ones with "MarkerTag" value "225" (APP1/0xFFE1). There might be other APP1 markers beside Exif, so you probably need to look into the userObject, which for all "unknown" segments will contain a byte array. For Exif, this array will start with the string Exif\0\0 (and be followed by the TIFF structure).

Sadly, there is a bug in the JDK metadata, that inserts multiple "unknown" markers when you merge... 😕
Workaround for this is to merge the nodes first, then use setFromTree. Again, unnecessary hard work for something that should be simple.

I'd love to fix it, but proposing PRs to OpenJDK often takes several times that of a workaround... If only there was more time.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants