[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##
@@ -42,76 +45,105 @@
  * }
  *
  * @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
 File Format
- *
  * @since 1.0-alpha4
  */
-public final class WebPChunkVP8X extends WebPChunk {
-private final boolean hasICC;
+public final class WebPChunkVp8x extends WebPChunk {
+private final boolean hasIcc;
 private final boolean hasAlpha;
 private final boolean hasExif;
 private final boolean hasXmp;
-private final boolean isAnimation;
+private final boolean hasAnimation;
 private final int canvasWidth;
 private final int canvasHeight;
 
-public WebPChunkVP8X(int type, int size, byte[] bytes) throws 
ImagingException {
+/**
+ * Create a VP8x chunk.
+ *
+ * @param type  VP8X chunk type
+ * @param size  VP8X chunk size
+ * @param bytes VP8X chunk data
+ * @throws ImagingException if the chunk data and the size provided do not 
match,
+ *  or if the other parameters provided are 
invalid.
+ */
+public WebPChunkVp8x(int type, int size, byte[] bytes) throws 
ImagingException {
 super(type, size, bytes);
 
 if (size != 10) {
 throw new ImagingException("VP8X chunk size must be 10");
 }
 
 int mark = bytes[0] & 0xFF;
-this.hasICC = (mark & 0b0010_) != 0;
+this.hasIcc = (mark & 0b0010_) != 0;
 this.hasAlpha = (mark & 0b0001_) != 0;
 this.hasExif = (mark & 0b_1000) != 0;
 this.hasXmp = (mark & 0b_0100) != 0;
-this.isAnimation = (mark & 0b_0010) != 0;
+this.hasAnimation = (mark & 0b_0010) != 0;
 
-this.canvasWidth = (bytes[4] & 0xFF) + ((bytes[5] & 0xFF) << 8) + 
((bytes[6] & 0xFF) << 16) + 1;
-this.canvasHeight = (bytes[7] & 0xFF) + ((bytes[8] & 0xFF) << 8) + 
((bytes[9] & 0xFF) << 16) + 1;
+this.canvasWidth = SafeOperations.add((bytes[4] & 0xFF), ((bytes[5] & 
0xFF) << 8), ((bytes[6] & 0xFF) << 16), 1);
+this.canvasHeight = SafeOperations.add((bytes[7] & 0xFF), ((bytes[8] & 
0xFF) << 8), ((bytes[9] & 0xFF) << 16), 1);

Review Comment:
   Thanks @Glavo ! Will update it over next day(s) :+1: 



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##
@@ -29,30 +30,50 @@
  * used by the parser.
  *
  * @see https://developers.google.com/speed/webp/docs/riff_container;>WebP 
Container Specification
- *
  * @since 1.0-alpha4
  */
 public abstract class WebPChunk extends BinaryFileParser {
 private final int type;
 private final int size;
 protected final byte[] bytes;
+private int chunkSize;

Review Comment:
   Good catch! Done.



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,351 @@
+/*
+ * 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.webp;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.common.XmpEmbeddable;
+import org.apache.commons.imaging.common.XmpImagingParameters;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANIM;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANMF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkEXIF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkICCP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8L;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8X;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXMP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXYZW;
+
+import java.awt.Dimension;
+import java.awt.image.BufferedImage;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
+
+public class WebPImageParser extends ImageParser 
implements XmpEmbeddable {
+
+private static final String DEFAULT_EXTENSION = 
ImageFormats.WEBP.getDefaultExtension();
+private static final String[] ACCEPTED_EXTENSIONS = 
ImageFormats.WEBP.getExtensions();
+
+@Override
+public WebPImagingParameters getDefaultParameters() {
+return new WebPImagingParameters();
+}
+
+@Override
+public String getName() {
+return "WebP-Custom";
+}
+
+@Override
+public String getDefaultExtension() {
+return DEFAULT_EXTENSION;
+}
+
+@Override
+protected String[] getAcceptedExtensions() {
+return ACCEPTED_EXTENSIONS;
+}
+
+@Override
+protected ImageFormat[] getAcceptedTypes() {
+return new ImageFormat[]{ImageFormats.WEBP};
+}
+
+static int readFileHeader(InputStream is) throws IOException, 
ImageReadException {
+byte[] buffer = new byte[4];
+if (is.read(buffer) < 4 || 
!WebPConstants.RIFF_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+int fileSize = read4Bytes("File Size", is, "Not a Valid WebP File", 
ByteOrder.LITTLE_ENDIAN);
+if (fileSize < 0) {
+throw new ImageReadException("File Size is too long:" + fileSize);
+}
+
+if (is.read(buffer) < 4 || 
!WebPConstants.WEBP_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");

Review Comment:
   All good over here. The Imaging classes called by users throw 
`ImagingException`. The `BinaryFunctions` and other classes may raise 
`IOException`, but that would be either wrapped in an ImagingException or 
raised directly to the user. At least that was the original design of the 
component code, if I recall correctly, so we just follow it (more or less). 
Marking as resolved :+1: 



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,351 @@
+/*
+ * 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.webp;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.common.XmpEmbeddable;
+import org.apache.commons.imaging.common.XmpImagingParameters;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANIM;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANMF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkEXIF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkICCP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8L;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8X;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXMP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXYZW;
+
+import java.awt.Dimension;
+import java.awt.image.BufferedImage;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
+
+public class WebPImageParser extends ImageParser 
implements XmpEmbeddable {
+
+private static final String DEFAULT_EXTENSION = 
ImageFormats.WEBP.getDefaultExtension();
+private static final String[] ACCEPTED_EXTENSIONS = 
ImageFormats.WEBP.getExtensions();
+
+@Override
+public WebPImagingParameters getDefaultParameters() {
+return new WebPImagingParameters();
+}
+
+@Override
+public String getName() {
+return "WebP-Custom";
+}
+
+@Override
+public String getDefaultExtension() {
+return DEFAULT_EXTENSION;
+}
+
+@Override
+protected String[] getAcceptedExtensions() {
+return ACCEPTED_EXTENSIONS;
+}
+
+@Override
+protected ImageFormat[] getAcceptedTypes() {
+return new ImageFormat[]{ImageFormats.WEBP};
+}
+
+static int readFileHeader(InputStream is) throws IOException, 
ImageReadException {
+byte[] buffer = new byte[4];
+if (is.read(buffer) < 4 || 
!WebPConstants.RIFF_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+int fileSize = read4Bytes("File Size", is, "Not a Valid WebP File", 
ByteOrder.LITTLE_ENDIAN);
+if (fileSize < 0) {
+throw new ImageReadException("File Size is too long:" + fileSize);
+}
+
+if (is.read(buffer) < 4 || 
!WebPConstants.WEBP_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+return fileSize;
+}
+
+@Override
+public WebPImageMetadata getMetadata(ByteSource byteSource, 
WebPImagingParameters params) throws ImageReadException, IOException {
+try (ChunksReader reader = new ChunksReader(byteSource, 
WebPChunkXMP.TYPE_EXIF)) {
+WebPChunk chunk = reader.readChunk();
+return chunk == null ? null : new 
WebPImageMetadata((TiffImageMetadata) new 
TiffImageParser().getMetadata(chunk.getBytes()));
+}
+}
+
+@Override
+public String getXmpXml(ByteSource byteSource, XmpImagingParameters 
params) throws ImageReadException, IOException {
+try (ChunksReader reader =

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,351 @@
+/*
+ * 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.webp;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.common.XmpEmbeddable;
+import org.apache.commons.imaging.common.XmpImagingParameters;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANIM;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANMF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkEXIF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkICCP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8L;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8X;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXMP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXYZW;
+
+import java.awt.Dimension;
+import java.awt.image.BufferedImage;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
+
+public class WebPImageParser extends ImageParser 
implements XmpEmbeddable {
+
+private static final String DEFAULT_EXTENSION = 
ImageFormats.WEBP.getDefaultExtension();
+private static final String[] ACCEPTED_EXTENSIONS = 
ImageFormats.WEBP.getExtensions();
+
+@Override
+public WebPImagingParameters getDefaultParameters() {
+return new WebPImagingParameters();
+}
+
+@Override
+public String getName() {
+return "WebP-Custom";
+}
+
+@Override
+public String getDefaultExtension() {
+return DEFAULT_EXTENSION;
+}
+
+@Override
+protected String[] getAcceptedExtensions() {
+return ACCEPTED_EXTENSIONS;
+}
+
+@Override
+protected ImageFormat[] getAcceptedTypes() {
+return new ImageFormat[]{ImageFormats.WEBP};
+}
+
+static int readFileHeader(InputStream is) throws IOException, 
ImageReadException {
+byte[] buffer = new byte[4];
+if (is.read(buffer) < 4 || 
!WebPConstants.RIFF_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+int fileSize = read4Bytes("File Size", is, "Not a Valid WebP File", 
ByteOrder.LITTLE_ENDIAN);
+if (fileSize < 0) {
+throw new ImageReadException("File Size is too long:" + fileSize);
+}
+
+if (is.read(buffer) < 4 || 
!WebPConstants.WEBP_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+return fileSize;
+}
+
+@Override
+public WebPImageMetadata getMetadata(ByteSource byteSource, 
WebPImagingParameters params) throws ImageReadException, IOException {
+try (ChunksReader reader = new ChunksReader(byteSource, 
WebPChunkXMP.TYPE_EXIF)) {
+WebPChunk chunk = reader.readChunk();
+return chunk == null ? null : new 
WebPImageMetadata((TiffImageMetadata) new 
TiffImageParser().getMetadata(chunk.getBytes()));
+}
+}
+
+@Override
+public String getXmpXml(ByteSource byteSource, XmpImagingParameters 
params) throws ImageReadException, IOException {
+try (ChunksReader reader =

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,351 @@
+/*
+ * 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.webp;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.common.XmpEmbeddable;
+import org.apache.commons.imaging.common.XmpImagingParameters;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANIM;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANMF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkEXIF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkICCP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8L;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8X;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXMP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXYZW;
+
+import java.awt.Dimension;
+import java.awt.image.BufferedImage;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
+
+public class WebPImageParser extends ImageParser 
implements XmpEmbeddable {
+
+private static final String DEFAULT_EXTENSION = 
ImageFormats.WEBP.getDefaultExtension();
+private static final String[] ACCEPTED_EXTENSIONS = 
ImageFormats.WEBP.getExtensions();
+
+@Override
+public WebPImagingParameters getDefaultParameters() {
+return new WebPImagingParameters();
+}
+
+@Override
+public String getName() {
+return "WebP-Custom";
+}
+
+@Override
+public String getDefaultExtension() {
+return DEFAULT_EXTENSION;
+}
+
+@Override
+protected String[] getAcceptedExtensions() {
+return ACCEPTED_EXTENSIONS;
+}
+
+@Override
+protected ImageFormat[] getAcceptedTypes() {
+return new ImageFormat[]{ImageFormats.WEBP};
+}
+
+static int readFileHeader(InputStream is) throws IOException, 
ImageReadException {
+byte[] buffer = new byte[4];
+if (is.read(buffer) < 4 || 
!WebPConstants.RIFF_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+int fileSize = read4Bytes("File Size", is, "Not a Valid WebP File", 
ByteOrder.LITTLE_ENDIAN);
+if (fileSize < 0) {
+throw new ImageReadException("File Size is too long:" + fileSize);
+}
+
+if (is.read(buffer) < 4 || 
!WebPConstants.WEBP_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+return fileSize;
+}
+
+@Override
+public WebPImageMetadata getMetadata(ByteSource byteSource, 
WebPImagingParameters params) throws ImageReadException, IOException {
+try (ChunksReader reader = new ChunksReader(byteSource, 
WebPChunkXMP.TYPE_EXIF)) {
+WebPChunk chunk = reader.readChunk();
+return chunk == null ? null : new 
WebPImageMetadata((TiffImageMetadata) new 
TiffImageParser().getMetadata(chunk.getBytes()));
+}
+}
+
+@Override
+public String getXmpXml(ByteSource byteSource, XmpImagingParameters 
params) throws ImageReadException, IOException {
+try (ChunksReader reader =

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageMetadata.java:
##
@@ -0,0 +1,53 @@
+/*
+ * 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.webp;
+
+import org.apache.commons.imaging.common.GenericImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+
+import java.util.ArrayList;
+import java.util.Collections;
+import java.util.List;
+
+/**
+ * @since 1.0-alpha4

Review Comment:
   Done.



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,326 @@
+/*
+ * 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.webp;
+
+import org.apache.commons.imaging.AbstractImageParser;
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImagingException;
+import org.apache.commons.imaging.bytesource.ByteSource;
+import org.apache.commons.imaging.common.XmpEmbeddable;
+import org.apache.commons.imaging.common.XmpImagingParameters;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8L;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8X;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXMP;
+
+import java.awt.Dimension;
+import java.awt.image.BufferedImage;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
+
+/**
+ * @since 1.0-alpha4
+ */
+public class WebPImageParser extends 
AbstractImageParser implements 
XmpEmbeddable {
+
+private static final String DEFAULT_EXTENSION = 
ImageFormats.WEBP.getDefaultExtension();
+private static final String[] ACCEPTED_EXTENSIONS = 
ImageFormats.WEBP.getExtensions();
+
+@Override
+public WebPImagingParameters getDefaultParameters() {
+return new WebPImagingParameters();
+}
+
+@Override
+public String getName() {
+return "WebP-Custom";
+}
+
+@Override
+public String getDefaultExtension() {
+return DEFAULT_EXTENSION;
+}
+
+@Override
+protected String[] getAcceptedExtensions() {
+return ACCEPTED_EXTENSIONS;
+}
+
+@Override
+protected ImageFormat[] getAcceptedTypes() {
+return new ImageFormat[]{ImageFormats.WEBP};
+}
+
+/**
+ * Read the file header of WebP file.
+ *
+ * @return file size in file header (including the WebP signature,
+ * excluding the TIFF signature and the file size field).
+ */
+private static int readFileHeader(InputStream is) throws IOException, 
ImagingException {
+byte[] buffer = new byte[4];
+if (is.read(buffer) < 4 || 
!WebPConstants.RIFF_SIGNATURE.equals(buffer)) {
+throw new ImagingException("Not a Valid WebP File");
+}
+
+int fileSize = read4Bytes("File Size", is, "Not a Valid WebP File", 
ByteOrder.LITTLE_ENDIAN);
+if (fileSize < 0) {
+throw new ImagingException("File Size is too long:" + fileSize);
+}
+
+if (is.read(buffer) < 4 || 
!WebPConstants.WEBP_SIGNATURE.equals(buffer)) {
+throw new ImagingException("Not a Valid WebP File");

Review Comment:
   No, maybe there was, but it's doing exact same thing as other parsers. 
Resolving this comment.



##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,326 @@
+/*
+ * 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

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/test/java/org/apache/commons/imaging/formats/webp/WebPBaseTest.java:
##
@@ -0,0 +1,39 @@
+/*
+ * 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.webp;
+
+import org.apache.commons.imaging.*;
+
+import java.io.File;
+import java.io.IOException;
+import java.util.List;
+
+public abstract class WebPBaseTest extends AbstractImagingTest {

Review Comment:
   No we did not. Resolving this comment.



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVP8X.java:
##
@@ -0,0 +1,118 @@
+/*
+ * 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.webp.chunks;
+
+import org.apache.commons.imaging.ImagingException;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+/**
+ * {@code
+ *  0   1   2   3
+ *  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |   |
+ * |   WebP file header (12 bytes) |
+ * |   |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |  ChunkHeader('VP8X')  |
+ * |   |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |Rsv|I|L|E|X|A|R|   Reserved|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |  Canvas Width Minus One   | ...
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * ...  Canvas Height Minus One|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * }
+ *
+ * @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
 File Format
+ *
+ * @since 1.0-alpha4
+ */
+public final class WebPChunkVP8X extends WebPChunk {
+private final boolean hasICC;
+private final boolean hasAlpha;
+private final boolean hasExif;
+private final boolean hasXmp;
+private final boolean isAnimation;
+private final int canvasWidth;
+private final int canvasHeight;
+
+public WebPChunkVP8X(int type, int size, byte[] bytes) throws 
ImagingException {
+super(type, size, bytes);
+
+if (size != 10) {
+throw new ImagingException("VP8X chunk size must be 10");
+}
+
+int mark = bytes[0] & 0xFF;
+this.hasICC = (mark & 0b0010_) != 0;
+this.hasAlpha = (mark & 0b0001_) != 0;
+this.hasExif = (mark & 0b_1000) != 0;
+this.hasXmp = (mark & 0b_0100) != 0;
+this.isAnimation = (mark & 0b_0010) != 0;
+
+this.canvasWidth = (bytes[4] & 0xFF) + ((bytes[5] & 0xFF) << 8) + 
((bytes[6] & 0xFF) << 16) + 1;
+this.canvasHeight = (bytes[7] & 0xFF) + ((bytes[8] & 0xFF) << 8) + 
((bytes[9] & 0xFF) << 16) + 1;
+
+if (canvasWidth < 0 || canvasHeight < 0 || canvasWidth * canvasHeight 
< 0) {
+throw new ImagingException("Illegal canvas size");
+}
+}
+
+public boolean isHasICC() {
+return hasICC;
+}
+
+public boolean isHasAlpha() {

Review Comment:
   Done! Maintaining the variable as `hasAlpha`.



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVP8X.java:
##
@@ -0,0 +1,118 @@
+/*
+ * 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.webp.chunks;
+
+import org.apache.commons.imaging.ImagingException;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+/**
+ * {@code
+ *  0   1   2   3
+ *  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |   |
+ * |   WebP file header (12 bytes) |
+ * |   |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |  ChunkHeader('VP8X')  |
+ * |   |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |Rsv|I|L|E|X|A|R|   Reserved|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |  Canvas Width Minus One   | ...
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * ...  Canvas Height Minus One|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * }
+ *
+ * @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
 File Format
+ *
+ * @since 1.0-alpha4
+ */
+public final class WebPChunkVP8X extends WebPChunk {
+private final boolean hasICC;
+private final boolean hasAlpha;
+private final boolean hasExif;
+private final boolean hasXmp;
+private final boolean isAnimation;
+private final int canvasWidth;
+private final int canvasHeight;
+
+public WebPChunkVP8X(int type, int size, byte[] bytes) throws 
ImagingException {
+super(type, size, bytes);
+
+if (size != 10) {
+throw new ImagingException("VP8X chunk size must be 10");
+}
+
+int mark = bytes[0] & 0xFF;
+this.hasICC = (mark & 0b0010_) != 0;
+this.hasAlpha = (mark & 0b0001_) != 0;
+this.hasExif = (mark & 0b_1000) != 0;
+this.hasXmp = (mark & 0b_0100) != 0;
+this.isAnimation = (mark & 0b_0010) != 0;
+
+this.canvasWidth = (bytes[4] & 0xFF) + ((bytes[5] & 0xFF) << 8) + 
((bytes[6] & 0xFF) << 16) + 1;
+this.canvasHeight = (bytes[7] & 0xFF) + ((bytes[8] & 0xFF) << 8) + 
((bytes[9] & 0xFF) << 16) + 1;
+
+if (canvasWidth < 0 || canvasHeight < 0 || canvasWidth * canvasHeight 
< 0) {

Review Comment:
   Not needed, resolving it.



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/internal/SafeOperations.java:
##
@@ -0,0 +1,49 @@
+/*
+ * 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.internal;
+
+import java.util.Arrays;
+
+/**
+ * Provides safe arithmetic operations to avoid, for example,
+ * numeric overflows.
+ *
+ * @since 1.0-alpha4
+ */
+public class SafeOperations {
+
+/**
+ * Applies {@link Math#addExact(int, int)} to a variable
+ * length array of integers.
+ *
+ * @param values variable length array of integers.
+ * @return the values safely added.
+ */
+public static int add(int... values) {
+if (values == null || values.length < 2) {
+throw new IllegalArgumentException("You must provide at least two 
elements to be added");
+}
+return Arrays

Review Comment:
   I didn't think too much about this class yet. Wrote the first version that 
came to my mind without worrying about optimizing it. Maybe we can check if 
that becomes an issue and fix it if needed?



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##
@@ -61,22 +74,41 @@ public String getTypeDescription() {
 (byte) ((type >> 24) & 0xff)}, StandardCharsets.UTF_8);
 }
 
+/**
+ * @return the payload size.
+ */
 public int getPayloadSize() {
 return size;
 }
 
+/**
+ * @return the chunk size.
+ */
 public int getChunkSize() {
 // if chunk size is odd, a single padding byte is added
 int padding = (size % 2) != 0 ? 1 : 0;
 
 // Chunk FourCC (4 bytes) + Chunk Size (4 bytes) + Chunk Payload (n 
bytes) + Padding
-return 4 + 4 + size + padding;
+int chunkFourCCandSize = 4 + 4;
+int paddedSize = Math.addExact(size, padding);
+return Math.addExact(chunkFourCCandSize, paddedSize);

Review Comment:
   Have a look if that's what you had in mind :+1: thanks!



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -277,13 +279,14 @@ public void close() throws IOException {
 
 WebPChunk readChunk() throws ImagingException, IOException {
 while (sizeCount < fileSize) {
-int type = read4Bytes("Chunk Type", is, "Not a Valid WebP 
File", ByteOrder.LITTLE_ENDIAN);
-int payloadSize = read4Bytes("Chunk Size", is, "Not a Valid 
WebP File", ByteOrder.LITTLE_ENDIAN);
+int type = read4Bytes("Chunk Type", is, "Not a valid WebP 
file", ByteOrder.LITTLE_ENDIAN);
+int payloadSize = read4Bytes("Chunk Size", is, "Not a valid 
WebP file", ByteOrder.LITTLE_ENDIAN);
 if (payloadSize < 0) {
 throw new ImagingException("Chunk Payload is too long:" + 
payloadSize);
 }
 boolean padding = (payloadSize % 2) != 0;
-int chunkSize = payloadSize + 8 + (padding ? 1 : 0);
+int extraPadding = Math.addExact(8, (padding ? 1 : 0));

Review Comment:
   >The maximum value of extraPadding is 9, so there is no risk of overflow.
   
   `9` as per specification? Or is there a constant somewhere?
   
   Asking as we had security issues that had to be fixed (with some urgency, 
which is no fun) where users crafted images with custom metadata, many times 
invalid image formats.
   
   So we just need to confirm if `9` is the maximum value, or if the user can 
somehow manipulate it.
   
   >Furthermore, the name extraPadding is somewhat misleading, since padding 
only refers to the padding at the end of the chunk.
   
   Sorry, I just changed that, pushing a new version now... it uses a function 
to avoid having to delcare variables.



##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##
@@ -61,22 +74,41 @@ public String getTypeDescription() {
 (byte) ((type >> 24) & 0xff)}, StandardCharsets.UTF_8);
 }
 
+/**
+ * @return the payload size.
+ */
 public int getPayloadSize() {
 return size;
 }
 
+/**
+ * @return the chunk size.
+ */
 public int getChunkSize() {
 // if chunk size is odd, a single padding byte is added
 int padding = (size % 2) != 0 ? 1 : 0;
 
 // Chunk FourCC (4 bytes) + Chunk Size (4 bytes) + Chunk Payload (n 
bytes) + Padding
-return 4 + 4 + size + padding;
+int chunkFourCCandSize = 4 + 4;
+int paddedSize = Math.addExact(size, padding);
+return Math.addExact(chunkFourCCandSize, paddedSize);

Review Comment:
   Good idea. Let me try that instead.



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-10-01 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##
@@ -42,76 +44,105 @@
  * }
  *
  * @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
 File Format
- *
  * @since 1.0-alpha4
  */
-public final class WebPChunkVP8X extends WebPChunk {
-private final boolean hasICC;
-private final boolean hasAlpha;
-private final boolean hasExif;
-private final boolean hasXmp;
-private final boolean isAnimation;
+public final class WebPChunkVp8x extends WebPChunk {
+private final boolean icc;

Review Comment:
   No strong opinion here, let me revert it.



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-09-30 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##
@@ -0,0 +1,83 @@
+/*
+ * 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.webp.chunks;
+
+import org.apache.commons.imaging.ImagingException;
+import org.apache.commons.imaging.common.BinaryFileParser;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.nio.charset.StandardCharsets;
+
+/**
+ * A WebP image is composed of several chunks. This is the base class for the 
chunks,
+ * used by the parser.
+ *
+ * @see https://developers.google.com/speed/webp/docs/riff_container;>WebP 
Container Specification

Review Comment:
   If the others are not doing that, then no worries @Glabo :) I spoke without 
looking at the other parsers (more a note as I did a first pass without the 
IDE). Thanks for replying it!



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-09-30 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVP8X.java:
##
@@ -0,0 +1,118 @@
+/*
+ * 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.webp.chunks;
+
+import org.apache.commons.imaging.ImagingException;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+/**
+ * {@code
+ *  0   1   2   3
+ *  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |   |
+ * |   WebP file header (12 bytes) |
+ * |   |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |  ChunkHeader('VP8X')  |
+ * |   |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |Rsv|I|L|E|X|A|R|   Reserved|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |  Canvas Width Minus One   | ...
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * ...  Canvas Height Minus One|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * }
+ *
+ * @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
 File Format
+ *
+ * @since 1.0-alpha4
+ */
+public final class WebPChunkVP8X extends WebPChunk {
+private final boolean hasICC;
+private final boolean hasAlpha;
+private final boolean hasExif;
+private final boolean hasXmp;
+private final boolean isAnimation;
+private final int canvasWidth;
+private final int canvasHeight;
+
+public WebPChunkVP8X(int type, int size, byte[] bytes) throws 
ImagingException {
+super(type, size, bytes);
+
+if (size != 10) {
+throw new ImagingException("VP8X chunk size must be 10");
+}
+
+int mark = bytes[0] & 0xFF;
+this.hasICC = (mark & 0b0010_) != 0;
+this.hasAlpha = (mark & 0b0001_) != 0;
+this.hasExif = (mark & 0b_1000) != 0;
+this.hasXmp = (mark & 0b_0100) != 0;
+this.isAnimation = (mark & 0b_0010) != 0;
+
+this.canvasWidth = (bytes[4] & 0xFF) + ((bytes[5] & 0xFF) << 8) + 
((bytes[6] & 0xFF) << 16) + 1;
+this.canvasHeight = (bytes[7] & 0xFF) + ((bytes[8] & 0xFF) << 8) + 
((bytes[9] & 0xFF) << 16) + 1;
+
+if (canvasWidth < 0 || canvasHeight < 0 || canvasWidth * canvasHeight 
< 0) {

Review Comment:
   I'll remove the `canvasWidth < 0 || canvasHeight < 0` from the if statement. 
IntelliJ had a code inspection warning here, and after some napkin bit math, I 
think `canvasWidth` and `canvasHeight` can not be negative. They can, however, 
be quite large, causing the multiplication to give a negative number.
   
   Will see whether we should leave the `canvasWidth * canvasHeight < 0` and/or 
if it can be replaced by one of those JVM safe methods like `multiplyExact`.



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-09-30 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,351 @@
+/*
+ * 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.webp;
+
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImageParser;
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.common.XmpEmbeddable;
+import org.apache.commons.imaging.common.XmpImagingParameters;
+import org.apache.commons.imaging.common.bytesource.ByteSource;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANIM;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkANMF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkEXIF;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkICCP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8L;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8X;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXMP;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXYZW;
+
+import java.awt.Dimension;
+import java.awt.image.BufferedImage;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
+
+public class WebPImageParser extends ImageParser 
implements XmpEmbeddable {
+
+private static final String DEFAULT_EXTENSION = 
ImageFormats.WEBP.getDefaultExtension();
+private static final String[] ACCEPTED_EXTENSIONS = 
ImageFormats.WEBP.getExtensions();
+
+@Override
+public WebPImagingParameters getDefaultParameters() {
+return new WebPImagingParameters();
+}
+
+@Override
+public String getName() {
+return "WebP-Custom";
+}
+
+@Override
+public String getDefaultExtension() {
+return DEFAULT_EXTENSION;
+}
+
+@Override
+protected String[] getAcceptedExtensions() {
+return ACCEPTED_EXTENSIONS;
+}
+
+@Override
+protected ImageFormat[] getAcceptedTypes() {
+return new ImageFormat[]{ImageFormats.WEBP};
+}
+
+static int readFileHeader(InputStream is) throws IOException, 
ImageReadException {
+byte[] buffer = new byte[4];
+if (is.read(buffer) < 4 || 
!WebPConstants.RIFF_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");
+}
+
+int fileSize = read4Bytes("File Size", is, "Not a Valid WebP File", 
ByteOrder.LITTLE_ENDIAN);
+if (fileSize < 0) {
+throw new ImageReadException("File Size is too long:" + fileSize);
+}
+
+if (is.read(buffer) < 4 || 
!WebPConstants.WEBP_SIGNATURE.equals(buffer)) {
+throw new IOException("Not a Valid WebP File");

Review Comment:
   (will look at this one later, don't worry :+1: )



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-09-30 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVP8X.java:
##
@@ -0,0 +1,118 @@
+/*
+ * 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.webp.chunks;
+
+import org.apache.commons.imaging.ImagingException;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+
+/**
+ * {@code
+ *  0   1   2   3
+ *  0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1 2 3 4 5 6 7 8 9 0 1
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |   |
+ * |   WebP file header (12 bytes) |
+ * |   |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |  ChunkHeader('VP8X')  |
+ * |   |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |Rsv|I|L|E|X|A|R|   Reserved|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |  Canvas Width Minus One   | ...
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * ...  Canvas Height Minus One|
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * }
+ *
+ * @see https://developers.google.com/speed/webp/docs/riff_container#extended_file_format;>Extended
 File Format
+ *
+ * @since 1.0-alpha4
+ */
+public final class WebPChunkVP8X extends WebPChunk {
+private final boolean hasICC;
+private final boolean hasAlpha;
+private final boolean hasExif;
+private final boolean hasXmp;
+private final boolean isAnimation;
+private final int canvasWidth;
+private final int canvasHeight;
+
+public WebPChunkVP8X(int type, int size, byte[] bytes) throws 
ImagingException {
+super(type, size, bytes);
+
+if (size != 10) {
+throw new ImagingException("VP8X chunk size must be 10");
+}
+
+int mark = bytes[0] & 0xFF;
+this.hasICC = (mark & 0b0010_) != 0;
+this.hasAlpha = (mark & 0b0001_) != 0;
+this.hasExif = (mark & 0b_1000) != 0;
+this.hasXmp = (mark & 0b_0100) != 0;
+this.isAnimation = (mark & 0b_0010) != 0;
+
+this.canvasWidth = (bytes[4] & 0xFF) + ((bytes[5] & 0xFF) << 8) + 
((bytes[6] & 0xFF) << 16) + 1;
+this.canvasHeight = (bytes[7] & 0xFF) + ((bytes[8] & 0xFF) << 8) + 
((bytes[9] & 0xFF) << 16) + 1;
+
+if (canvasWidth < 0 || canvasHeight < 0 || canvasWidth * canvasHeight 
< 0) {
+throw new ImagingException("Illegal canvas size");
+}
+}
+
+public boolean isHasICC() {
+return hasICC;
+}
+
+public boolean isHasAlpha() {

Review Comment:
   @Glavo would it OK to call this method "hasAlpha()" instead of "isHasAlpha" 
? Ditto for the other methods? (I can batch rename them as I'm modifying the 
code, writing Javadocs, etc)



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-09-27 Thread via GitHub 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##
@@ -0,0 +1,83 @@
+/*
+ * 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.webp.chunks;
+
+import org.apache.commons.imaging.ImagingException;
+import org.apache.commons.imaging.common.BinaryFileParser;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.nio.charset.StandardCharsets;
+
+/**
+ * A WebP image is composed of several chunks. This is the base class for the 
chunks,
+ * used by the parser.
+ *
+ * @see https://developers.google.com/speed/webp/docs/riff_container;>WebP 
Container Specification

Review Comment:
   Maybe this should be documented in the parser as well.



##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##
@@ -0,0 +1,326 @@
+/*
+ * 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.webp;
+
+import org.apache.commons.imaging.AbstractImageParser;
+import org.apache.commons.imaging.ImageFormat;
+import org.apache.commons.imaging.ImageFormats;
+import org.apache.commons.imaging.ImageInfo;
+import org.apache.commons.imaging.ImagingException;
+import org.apache.commons.imaging.bytesource.ByteSource;
+import org.apache.commons.imaging.common.XmpEmbeddable;
+import org.apache.commons.imaging.common.XmpImagingParameters;
+import org.apache.commons.imaging.formats.tiff.TiffImageMetadata;
+import org.apache.commons.imaging.formats.tiff.TiffImageParser;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8L;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkVP8X;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXMP;
+
+import java.awt.Dimension;
+import java.awt.image.BufferedImage;
+import java.io.Closeable;
+import java.io.IOException;
+import java.io.InputStream;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.util.ArrayList;
+
+import static org.apache.commons.imaging.common.BinaryFunctions.read4Bytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.readBytes;
+import static org.apache.commons.imaging.common.BinaryFunctions.skipBytes;
+
+/**
+ * @since 1.0-alpha4

Review Comment:
   Maybe the description here could be improved, perhaps including a link to 
where this implementation came from (spec, docs, another library, etc.)



##
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageMetadata.java:
##
@@ -0,0 +1,53 @@
+/*
+ * 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

[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support

2023-01-12 Thread GitBox 


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


##
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##
@@ -0,0 +1,80 @@
+/*
+ * 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.webp.chunks;
+
+import org.apache.commons.imaging.ImageReadException;
+import org.apache.commons.imaging.common.BinaryFileParser;
+
+import java.io.IOException;
+import java.io.PrintWriter;
+import java.nio.ByteOrder;
+import java.nio.charset.StandardCharsets;
+
+public abstract class WebPChunk extends BinaryFileParser {
+public static final int TYPE_VP8 = 0x20385056;
+public static final int TYPE_VP8L = 0x4C385056;
+public static final int TYPE_VP8X = 0x58385056;
+public static final int TYPE_ANIM = 0x4D494E41;
+public static final int TYPE_ANMF = 0x464D4E41;
+public static final int TYPE_ICCP = 0x50434349;
+public static final int TYPE_EXIF = 0x46495845;
+public static final int TYPE_XMP = 0x20504D58;
+
+private final int type;
+private final int size;
+protected final byte[] bytes;
+
+WebPChunk(int type, int size, byte[] bytes) throws ImageReadException {
+super(ByteOrder.LITTLE_ENDIAN);
+
+if (size != bytes.length) {
+throw new AssertionError("Chunk size must match bytes length");

Review Comment:
   I think it'd be important to be consistent, so users are not surprised when 
using the API. Have a look at the other parsers, @Glavo , and see what 
exceptions are being used. If you believe we must change it to another 
exception type, then we can create a follow up issue to discuss and plan how to 
do that in all other parsers :+1: 



-- 
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: issues-unsubscr...@commons.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org