From a4983f3fde28063ab3f652921cc69539a4fbca73 Mon Sep 17 00:00:00 2001 From: Andrew Khoury Date: Sat, 8 Jul 2023 15:56:36 -0700 Subject: [PATCH 1/4] IMAGING-355 - Large animated GIF takes too much heap memory in getMetadata --- .../apache/commons/imaging/formats/gif/GifImageParser.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/main/java/org/apache/commons/imaging/formats/gif/GifImageParser.java b/src/main/java/org/apache/commons/imaging/formats/gif/GifImageParser.java index 7e48e656bc..fa3785208a 100644 --- a/src/main/java/org/apache/commons/imaging/formats/gif/GifImageParser.java +++ b/src/main/java/org/apache/commons/imaging/formats/gif/GifImageParser.java @@ -407,7 +407,7 @@ public byte[] getIccProfileBytes(final ByteSource byteSource, final GifImagingPa @Override public ImageInfo getImageInfo(final ByteSource byteSource, final GifImagingParameters params) throws ImagingException, IOException { - final GifImageContents blocks = readFile(byteSource, false); + final GifImageContents blocks = readFile(byteSource, true); final GifHeaderInfo bhi = blocks.gifHeaderInfo; if (bhi == null) { @@ -482,7 +482,7 @@ public Dimension getImageSize(final ByteSource byteSource, final GifImagingParam @Override public ImageMetadata getMetadata(final ByteSource byteSource, final GifImagingParameters params) throws ImagingException, IOException { - final GifImageContents imageContents = readFile(byteSource, false); + final GifImageContents imageContents = readFile(byteSource, true); final GifHeaderInfo bhi = imageContents.gifHeaderInfo; if (bhi == null) { From 52a6eead16be47480c55821801b67d7f403afd1f Mon Sep 17 00:00:00 2001 From: Andrew Khoury Date: Wed, 9 Aug 2023 12:31:50 -0700 Subject: [PATCH 2/4] IMAGING-355 - Large animated GIF takes too much heap memory in getMetadata * Add stopReadingBeforeImageData to GifImageParameters * Add unit test for IMAGING-355 --- .../commons/imaging/formats/gif/GifImageParser.java | 12 ++++++++++-- .../imaging/formats/gif/GifImagingParameters.java | 10 +++++++++- .../commons/imaging/formats/gif/GifReadTest.java | 11 ++++++++--- 3 files changed, 27 insertions(+), 6 deletions(-) diff --git a/src/main/java/org/apache/commons/imaging/formats/gif/GifImageParser.java b/src/main/java/org/apache/commons/imaging/formats/gif/GifImageParser.java index fa3785208a..211b114552 100644 --- a/src/main/java/org/apache/commons/imaging/formats/gif/GifImageParser.java +++ b/src/main/java/org/apache/commons/imaging/formats/gif/GifImageParser.java @@ -407,7 +407,11 @@ public byte[] getIccProfileBytes(final ByteSource byteSource, final GifImagingPa @Override public ImageInfo getImageInfo(final ByteSource byteSource, final GifImagingParameters params) throws ImagingException, IOException { - final GifImageContents blocks = readFile(byteSource, true); + boolean stopReadingBeforeImageData = true; + if (params != null) { + stopReadingBeforeImageData = params.getStopReadingBeforeImageData(); + } + final GifImageContents blocks = readFile(byteSource, stopReadingBeforeImageData); final GifHeaderInfo bhi = blocks.gifHeaderInfo; if (bhi == null) { @@ -482,7 +486,11 @@ public Dimension getImageSize(final ByteSource byteSource, final GifImagingParam @Override public ImageMetadata getMetadata(final ByteSource byteSource, final GifImagingParameters params) throws ImagingException, IOException { - final GifImageContents imageContents = readFile(byteSource, true); + boolean stopReadingBeforeImageData = true; + if (params != null) { + stopReadingBeforeImageData = params.getStopReadingBeforeImageData(); + } + final GifImageContents imageContents = readFile(byteSource, stopReadingBeforeImageData); final GifHeaderInfo bhi = imageContents.gifHeaderInfo; if (bhi == null) { diff --git a/src/main/java/org/apache/commons/imaging/formats/gif/GifImagingParameters.java b/src/main/java/org/apache/commons/imaging/formats/gif/GifImagingParameters.java index 720dfd8ccb..80ed4ef5dc 100644 --- a/src/main/java/org/apache/commons/imaging/formats/gif/GifImagingParameters.java +++ b/src/main/java/org/apache/commons/imaging/formats/gif/GifImagingParameters.java @@ -25,5 +25,13 @@ * @since 1.0-alpha3 */ public class GifImagingParameters extends XmpImagingParameters { - // empty + private boolean stopReadingBeforeImageData; + + public void setStopReadingBeforeImageData(boolean stopReadingBeforeImageData) { + this.stopReadingBeforeImageData = stopReadingBeforeImageData; + } + + public boolean getStopReadingBeforeImageData() { + return stopReadingBeforeImageData; + } } diff --git a/src/test/java/org/apache/commons/imaging/formats/gif/GifReadTest.java b/src/test/java/org/apache/commons/imaging/formats/gif/GifReadTest.java index 617dc22c75..1f1fc3900e 100644 --- a/src/test/java/org/apache/commons/imaging/formats/gif/GifReadTest.java +++ b/src/test/java/org/apache/commons/imaging/formats/gif/GifReadTest.java @@ -28,6 +28,7 @@ import java.util.List; import java.util.stream.Stream; +import org.apache.commons.imaging.common.ImageMetadata; import org.apache.commons.imaging.ImageInfo; import org.apache.commons.imaging.Imaging; import org.apache.commons.imaging.ImagingException; @@ -139,11 +140,15 @@ public void testImageInfo(final File imageFile) throws Exception { // TODO assert more } - @Disabled(value = "RoundtripTest has to be fixed before implementation can throw UnsupportedOperationException") @ParameterizedTest @MethodSource("data") - public void testMetadata(final File imageFile) { - Assertions.assertThrows(UnsupportedOperationException.class, () -> Imaging.getMetadata(imageFile)); + public void testMetadata(final File imageFile) throws IOException { + ImageMetadata metadata = Imaging.getMetadata(imageFile); + assertNotNull(metadata); + assertTrue(metadata instanceof GifImageMetadata); + assertTrue(((GifImageMetadata)metadata).getWidth() > 0); + assertTrue(((GifImageMetadata)metadata).getHeight() > 0); + assertNotNull(metadata.getItems()); } /** From 2d267534e59faa1fe2b281d8fac80421be6e2807 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Sat, 23 Dec 2023 10:30:03 -0500 Subject: [PATCH 3/4] Use final and format tweak --- .../org/apache/commons/imaging/formats/gif/GifReadTest.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/test/java/org/apache/commons/imaging/formats/gif/GifReadTest.java b/src/test/java/org/apache/commons/imaging/formats/gif/GifReadTest.java index 3069a55914..8b75d96234 100644 --- a/src/test/java/org/apache/commons/imaging/formats/gif/GifReadTest.java +++ b/src/test/java/org/apache/commons/imaging/formats/gif/GifReadTest.java @@ -117,7 +117,7 @@ public void testImageDimensions(final File imageFile) throws Exception { int width = 0; int height = 0; - for(int i = 0; i < images.size(); i++) { + for (int i = 0; i < images.size(); i++) { final BufferedImage image = images.get(i); final GifImageMetadataItem metadataItem = metadata.getItems().get(i); final int xOffset = metadataItem.getLeftPosition(); @@ -143,7 +143,7 @@ public void testImageInfo(final File imageFile) throws Exception { @ParameterizedTest @MethodSource("data") public void testMetadata(final File imageFile) throws IOException { - ImageMetadata metadata = Imaging.getMetadata(imageFile); + final ImageMetadata metadata = Imaging.getMetadata(imageFile); assertNotNull(metadata); assertTrue(metadata instanceof GifImageMetadata); assertTrue(((GifImageMetadata)metadata).getWidth() > 0); From c0c98466358cedbd78ba91b238bb2cc1a4718f53 Mon Sep 17 00:00:00 2001 From: Gary Gregory Date: Sat, 23 Dec 2023 10:32:01 -0500 Subject: [PATCH 4/4] Use final and sort methods --- .../imaging/formats/gif/GifImagingParameters.java | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/src/main/java/org/apache/commons/imaging/formats/gif/GifImagingParameters.java b/src/main/java/org/apache/commons/imaging/formats/gif/GifImagingParameters.java index 80ed4ef5dc..1bca589be2 100644 --- a/src/main/java/org/apache/commons/imaging/formats/gif/GifImagingParameters.java +++ b/src/main/java/org/apache/commons/imaging/formats/gif/GifImagingParameters.java @@ -27,11 +27,12 @@ public class GifImagingParameters extends XmpImagingParameters { private boolean stopReadingBeforeImageData; - public void setStopReadingBeforeImageData(boolean stopReadingBeforeImageData) { - this.stopReadingBeforeImageData = stopReadingBeforeImageData; - } - public boolean getStopReadingBeforeImageData() { return stopReadingBeforeImageData; } + + public void setStopReadingBeforeImageData(final boolean stopReadingBeforeImageData) { + this.stopReadingBeforeImageData = stopReadingBeforeImageData; + } + }