[
https://issues.apache.org/jira/browse/IMAGING-340?focusedWorklogId=839270&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-839270
]
ASF GitHub Bot logged work on IMAGING-340:
------------------------------------------
Author: ASF GitHub Bot
Created on: 15/Jan/23 13:31
Start Date: 15/Jan/23 13:31
Worklog Time Spent: 10m
Work Description: kinow commented on code in PR #269:
URL: https://github.com/apache/commons-imaging/pull/269#discussion_r1070592972
##########
src/main/java/org/apache/commons/imaging/formats/png/package-info.java:
##########
@@ -16,7 +16,14 @@
*/
/**
- * The PNG image format.
+ * The PNG (Portable Network Graphics) image format.
+ * <p>
+ * The implementation is based on the
+ * <a href="http://www.libpng.org/pub/png/spec/1.2/";>PNG specification version
1.2</a>,
+ * and supports the following extensions:
+ * <ul>
+ * <li><a
href="http://ftp-osl.osuosl.org/pub/libpng/documents/pngext-1.5.0.html";>Extensions
to the PNG 1.2 Specification, Version 1.5.0</a></li>
+ * </ul>
Review Comment:
Thank you! :clap:
##########
src/main/java/org/apache/commons/imaging/formats/png/PngImageParser.java:
##########
@@ -282,21 +263,53 @@ public Dimension getImageSize(final ByteSource
byteSource, final PngImagingParam
@Override
public ImageMetadata getMetadata(final ByteSource byteSource, final
PngImagingParameters params)
throws ImageReadException, IOException {
- final List<PngChunk> chunks = readChunks(byteSource, new ChunkType[] {
ChunkType.tEXt, ChunkType.zTXt, ChunkType.iTXt }, false);
+ final ChunkType[] chunkTypes = { ChunkType.tEXt, ChunkType.zTXt,
ChunkType.iTXt, ChunkType.eXIf };
+ final List<PngChunk> chunks = readChunks(byteSource, chunkTypes,
false);
if (chunks.isEmpty()) {
return null;
}
- final GenericImageMetadata result = new GenericImageMetadata();
+ final GenericImageMetadata textual = new GenericImageMetadata();
+ TiffImageMetadata exif = null;
for (final PngChunk chunk : chunks) {
- final PngTextChunk textChunk = (PngTextChunk) chunk;
+ if (chunk instanceof PngTextChunk) {
+ final PngTextChunk textChunk = (PngTextChunk) chunk;
+ textual.add(textChunk.getKeyword(), textChunk.getText());
+ } else if (chunk.chunkType == ChunkType.eXIf.value) {
+ if (exif != null) {
+ throw new ImageReadException("Duplicate eXIf chunk");
+ }
+ exif = (TiffImageMetadata) new
TiffImageParser().getMetadata(chunk.getBytes());
+ }
Review Comment:
I think we should either log and/or raise an error for any other type here.
##########
src/main/java/org/apache/commons/imaging/formats/png/PngImageParser.java:
##########
@@ -187,29 +190,7 @@ private List<PngChunk> readChunks(final InputStream is,
final ChunkType[] chunkT
final int crc = read4Bytes("CRC", is, "Not a Valid PNG File",
getByteOrder());
if (keep) {
- if (chunkType == ChunkType.iCCP.value) {
- result.add(new PngChunkIccp(length, chunkType, crc,
bytes));
- } else if (chunkType == ChunkType.tEXt.value) {
- result.add(new PngChunkText(length, chunkType, crc,
bytes));
- } else if (chunkType == ChunkType.zTXt.value) {
- result.add(new PngChunkZtxt(length, chunkType, crc,
bytes));
- } else if (chunkType == ChunkType.IHDR.value) {
- result.add(new PngChunkIhdr(length, chunkType, crc,
bytes));
- } else if (chunkType == ChunkType.PLTE.value) {
- result.add(new PngChunkPlte(length, chunkType, crc,
bytes));
- } else if (chunkType == ChunkType.pHYs.value) {
- result.add(new PngChunkPhys(length, chunkType, crc,
bytes));
- } else if (chunkType == ChunkType.sCAL.value) {
- result.add(new PngChunkScal(length, chunkType, crc,
bytes));
- } else if (chunkType == ChunkType.IDAT.value) {
- result.add(new PngChunkIdat(length, chunkType, crc,
bytes));
- } else if (chunkType == ChunkType.gAMA.value) {
- result.add(new PngChunkGama(length, chunkType, crc,
bytes));
- } else if (chunkType == ChunkType.iTXt.value) {
- result.add(new PngChunkItxt(length, chunkType, crc,
bytes));
- } else {
- result.add(new PngChunk(length, chunkType, crc, bytes));
- }
+ result.add(ChunkType.makeChunk(length, chunkType, crc, bytes));
Review Comment:
:ok_man: :clap: bravo, @Glavo
##########
src/main/java/org/apache/commons/imaging/formats/png/PngImageMetadata.java:
##########
@@ -0,0 +1,95 @@
+/*
+ * 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.png;
+
+import org.apache.commons.imaging.common.ImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.internal.Debug;
+
+import java.util.ArrayList;
+import java.util.List;
+import java.util.Objects;
+
+public class PngImageMetadata implements ImageMetadata {
Review Comment:
Can you add javadocs to the new public classes/methods/fields/etc, please?
Doesn't need to be long, but we need a `@since
$current-snapshot-version-minus-snapshot` here and in the public members added
to existing code.
##########
src/main/java/org/apache/commons/imaging/formats/png/PngImageParser.java:
##########
@@ -282,21 +263,53 @@ public Dimension getImageSize(final ByteSource
byteSource, final PngImagingParam
@Override
public ImageMetadata getMetadata(final ByteSource byteSource, final
PngImagingParameters params)
throws ImageReadException, IOException {
- final List<PngChunk> chunks = readChunks(byteSource, new ChunkType[] {
ChunkType.tEXt, ChunkType.zTXt, ChunkType.iTXt }, false);
+ final ChunkType[] chunkTypes = { ChunkType.tEXt, ChunkType.zTXt,
ChunkType.iTXt, ChunkType.eXIf };
+ final List<PngChunk> chunks = readChunks(byteSource, chunkTypes,
false);
if (chunks.isEmpty()) {
return null;
}
- final GenericImageMetadata result = new GenericImageMetadata();
+ final GenericImageMetadata textual = new GenericImageMetadata();
+ TiffImageMetadata exif = null;
for (final PngChunk chunk : chunks) {
- final PngTextChunk textChunk = (PngTextChunk) chunk;
+ if (chunk instanceof PngTextChunk) {
+ final PngTextChunk textChunk = (PngTextChunk) chunk;
+ textual.add(textChunk.getKeyword(), textChunk.getText());
+ } else if (chunk.chunkType == ChunkType.eXIf.value) {
+ if (exif != null) {
+ throw new ImageReadException("Duplicate eXIf chunk");
+ }
+ exif = (TiffImageMetadata) new
TiffImageParser().getMetadata(chunk.getBytes());
+ }
+ }
- result.add(textChunk.getKeyword(), textChunk.getText());
+ return new PngImageMetadata(textual, exif);
+ }
+
+ public TiffImageMetadata getExifMetadata(final ByteSource byteSource,
TiffImagingParameters params)
+ throws ImageReadException, IOException {
+ final byte[] bytes = getExifRawData(byteSource);
+ if (null == bytes) {
+ return null;
}
- return result;
+ if (params == null) {
+ params = new TiffImagingParameters();
+ }
+
+ return (TiffImageMetadata) new TiffImageParser().getMetadata(bytes,
params);
+ }
+
+ public byte[] getExifRawData(final ByteSource byteSource) throws
ImageReadException, IOException {
Review Comment:
Docs for new public members with the `@since` tag, please :+1:
Issue Time Tracking
-------------------
Worklog Id: (was: 839270)
Time Spent: 0.5h (was: 20m)
> Support PNG extension
> ---------------------
>
> Key: IMAGING-340
> URL: https://issues.apache.org/jira/browse/IMAGING-340
> Project: Commons Imaging
> Issue Type: Improvement
> Components: Format: PNG
> Reporter: Glavo
> Priority: Minor
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> Support [Extensions to the PNG 1.2 Specification, Version
> 1.5.0|http://ftp-osl.osuosl.org/pub/libpng/documents/pngext-1.5.0.html].
--
This message was sent by Atlassian Jira
(v8.20.10#820010)
