You are viewing a plain text version of this content. The canonical link for it is here.
Posted to issues@commons.apache.org by GitBox <gi...@apache.org> on 2022/12/14 15:55:26 UTC
[GitHub] [commons-imaging] Glavo opened a new pull request, #254: [WIP] Basic WebP Support
Glavo opened a new pull request, #254:
URL: https://github.com/apache/commons-imaging/pull/254
This PR is designed to parse WebP meta information.
Task List:
- [x] Add WebP support for `Imaging.guessFormat`;
- [x] Read all of webp chunks;
- [x] Implementing `ImageParser::getImageSize` for webp files;
- [ ] Implementing `ImageParser::getMetadata` for webp files;
- [ ] Implementing `ImageParser::getImageInfo` for webp files.
Non-Goals:
* Read image from webp file;
* Write to webp files.
Although I hope to implement complete WebP support, it is very difficult for me at present.
Therefore, the main goal of this PR is to read metadata from webp files.
A more complete implementation may require me to have a deeper understanding of vp8 format, or let others complete 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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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<WebPImagingParameters> 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 = new ChunksReader(byteSource, WebPChunkXMP.TYPE_XMP)) {
+ WebPChunkXMP chunk = (WebPChunkXMP) reader.readChunk();
+ return chunk == null ? null : chunk.getXml();
+ }
+ }
+
+ @Override
+ public ImageInfo getImageInfo(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ try (ChunksReader reader = new ChunksReader(byteSource,
+ WebPChunk.TYPE_VP8, WebPChunk.TYPE_VP8L, WebPChunk.TYPE_VP8X, WebPChunk.TYPE_ANMF)) {
+ String formatDetails;
+ int width;
+ int height;
+ int numberOfImages;
+ boolean hasAlpha = false;
+ ImageInfo.ColorType colorType = ImageInfo.ColorType.RGB;
+
+ WebPChunk chunk = reader.readChunk();
+ if (chunk instanceof WebPChunkVP8) {
+ formatDetails = "WebP/Lossy";
+ numberOfImages = 1;
+
+ WebPChunkVP8 vp8 = (WebPChunkVP8) chunk;
+ width = vp8.getWidth();
+ height = vp8.getHeight();
+ colorType = ImageInfo.ColorType.YCbCr;
+ } else if (chunk instanceof WebPChunkVP8L) {
+ formatDetails = "WebP/Lossless";
+ numberOfImages = 1;
+
+ WebPChunkVP8L vp8l = (WebPChunkVP8L) chunk;
+ width = vp8l.getImageWidth();
+ height = vp8l.getImageHeight();
+ } else if (chunk instanceof WebPChunkVP8X) {
+ WebPChunkVP8X vp8x = (WebPChunkVP8X) chunk;
+ width = vp8x.getCanvasWidth();
+ height = vp8x.getCanvasHeight();
+ hasAlpha = ((WebPChunkVP8X) chunk).isHasAlpha();
+
+ if (vp8x.isAnimation()) {
+ formatDetails = "WebP/Animation";
+
+ numberOfImages = 0;
+ while ((chunk = reader.readChunk()) != null) {
+ if (chunk.getType() == WebPChunk.TYPE_ANMF) {
+ numberOfImages++;
+ }
+ }
+
+ } else {
+ numberOfImages = 1;
+ chunk = reader.readChunk();
+
+ if (chunk == null) {
+ throw new ImageReadException("Image has no content");
+ }
+
+ if (chunk.getType() == WebPChunk.TYPE_ANMF) {
+ throw new ImageReadException("Non animated image should not contain ANMF chunks");
+ }
+
+ if (chunk.getType() == WebPChunk.TYPE_VP8) {
+ formatDetails = "WebP/Lossy (Extended)";
+ colorType = ImageInfo.ColorType.YCbCr;
+ } else if (chunk.getType() == WebPChunk.TYPE_VP8L) {
+ formatDetails = "WebP/Lossless (Extended)";
+ } else {
+ throw new ImageReadException("Unknown webp chunk type: " + chunk);
+ }
+ }
+ } else {
+ throw new ImageReadException("Unknown webp chunk type: " + chunk);
+ }
+
+ return new ImageInfo(
+ formatDetails, 32, new ArrayList<>(), ImageFormats.WEBP,
+ "webp", height, "image/webp", numberOfImages,
+ -1, -1, -1, -1,
+ width, false, hasAlpha, false,
+ colorType, ImageInfo.CompressionAlgorithm.UNKNOWN
+ );
+ }
+ }
+
+ @Override
+ public Dimension getImageSize(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ try (ChunksReader reader = new ChunksReader(byteSource)) {
+ WebPChunk chunk = reader.readChunk();
+ if (chunk instanceof WebPChunkVP8) {
+ WebPChunkVP8 vp8 = (WebPChunkVP8) chunk;
+ return new Dimension(vp8.getWidth(), vp8.getHeight());
+ } else if (chunk instanceof WebPChunkVP8L) {
+ WebPChunkVP8L vp8l = (WebPChunkVP8L) chunk;
+ return new Dimension(vp8l.getImageWidth(), vp8l.getImageHeight());
+ } else if (chunk instanceof WebPChunkVP8X) {
+ WebPChunkVP8X vp8x = (WebPChunkVP8X) chunk;
+ return new Dimension(vp8x.getCanvasWidth(), vp8x.getCanvasHeight());
+ } else {
+ throw new ImageReadException("Unknown webp chunk type: " + chunk);
+ }
+ }
+ }
+
+ @Override
+ public byte[] getICCProfileBytes(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ try (ChunksReader reader = new ChunksReader(byteSource, WebPChunkXMP.TYPE_ICCP)) {
+ WebPChunk chunk = reader.readChunk();
+ return chunk == null ? null : chunk.getBytes();
+ }
+ }
+
+ @Override
+ public BufferedImage getBufferedImage(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ throw new ImageReadException("Reading WebP files is currently not supported");
+ }
+
+ @Override
+ public boolean dumpImageFile(PrintWriter pw, ByteSource byteSource) throws ImageReadException, IOException {
+ pw.println("webp.dumpImageFile");
+ try (ChunksReader reader = new ChunksReader(byteSource)) {
+ int offset = reader.getOffset();
+ WebPChunk chunk = reader.readChunk();
+ if (chunk == null) {
+ throw new ImageReadException("No WebP chunks found");
+ }
+
+ do {
+ chunk.dump(pw, offset);
+
+ offset = reader.getOffset();
+ chunk = reader.readChunk();
+ } while (chunk != null);
Review Comment:
Adding a `TODO` comment. Thought a little about it, and I think we are safe from DOS attacks here.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IMAGING-339] Basic WebP Support [commons-imaging]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1752023058
Thank you @Glavo ! Let's wait a while to see if Gary has time to have a quick look just to see if I missed anything important. Then it should be ready to be merged. 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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1068648410
##########
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:
@kinow I think `AssertionError` should be used here.
I think `ImageReadException` should be thrown only when there is no error in commons-imaging itself, e.g. we encountered illegal input at runtime.
However, here, illegal input data will not cause this problem. An exception will be thrown only if commons-imaging itself has logical errors, so I use `AssertionError` here.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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;
+
+/**
+ * <pre>{@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 |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * }</pre>
+ *
+ * @see <a href="https://developers.google.com/speed/webp/docs/riff_container#extended_file_format">Extended File Format</a>
+ *
+ * @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_0000) != 0;
+ this.hasAlpha = (mark & 0b0001_0000) != 0;
+ this.hasExif = (mark & 0b0000_1000) != 0;
+ this.hasXmp = (mark & 0b0000_0100) != 0;
+ this.isAnimation = (mark & 0b0000_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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192570
##########
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?
Maybe. It's really not that noticeable compared to the I/O overhead.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] garydgregory commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1068685156
##########
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:
Which leads to consider if we should present the smallest API surface for this new code, IOW the minimal elements that should be public or protected. Once public or protected, it's set in stone within a major release line.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1358061052
@kinow Can you open this issue for me?
I'm sorry to reply you so late, because I'm not feeling well recently (probably infected with COVID-19), and I need a rest for a while.
Besides, I'm not good at English. You can better describe this problem than let me use Google Translate myself.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1161772193
##########
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<WebPImagingParameters> 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:
@kinow Hey?
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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 @@
* }</pre>
*
* @see <a href="https://developers.google.com/speed/webp/docs/riff_container#extended_file_format">Extended File Format</a>
- *
* @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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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<WebPImagingParameters> implements XmpEmbeddable<WebPImagingParameters> {
+
+ 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 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:
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192400
##########
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##########
@@ -29,30 +30,50 @@
* used by the parser.
*
* @see <a href="https://developers.google.com/speed/webp/docs/riff_container">WebP Container Specification</a>
- *
* @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:
The `final` modifier should be added to 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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1352258560
@kinow Will the next version be 1.0 beta? Do you have any ideas about when to make AWT as optional dependency? I think it is better to solve this problem in the alpha phase, because this change may break the user's code.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] garydgregory commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1068651074
##########
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:
You must be thinking of IllegalStateException or IllegalArgumentException we should not throw Errors.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1692931533
Sorry I am overseas visiting family until the end of September. Not sure if I will be able to review it until then.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1737924058
> Considering the libwebp CVE the industry is currently dealing with, would love to see this PR's conflicts resolved and this merged in sooner than later so I can move away from using a library that relies on JNI. 🙏
@Glavo can you rebase and fix conflicts, please? I returned from my overseas trip last Friday. Just going through the backlog ($work/emails/etc.), but I should be able to review this pretty soon. Or if you are busy (and you trust us) just say so and we can try to fix the conflicts and review it. 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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on pull request #254: [IMAGING-339] Basic WebP Support
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1742114264
I probably just named them arbitrarily, so feel free to rename them if necessary.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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<WebPImagingParameters> 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 = new ChunksReader(byteSource, WebPChunkXMP.TYPE_XMP)) {
+ WebPChunkXMP chunk = (WebPChunkXMP) reader.readChunk();
+ return chunk == null ? null : chunk.getXml();
+ }
+ }
+
+ @Override
+ public ImageInfo getImageInfo(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ try (ChunksReader reader = new ChunksReader(byteSource,
+ WebPChunk.TYPE_VP8, WebPChunk.TYPE_VP8L, WebPChunk.TYPE_VP8X, WebPChunk.TYPE_ANMF)) {
+ String formatDetails;
+ int width;
+ int height;
+ int numberOfImages;
+ boolean hasAlpha = false;
+ ImageInfo.ColorType colorType = ImageInfo.ColorType.RGB;
+
+ WebPChunk chunk = reader.readChunk();
+ if (chunk instanceof WebPChunkVP8) {
+ formatDetails = "WebP/Lossy";
+ numberOfImages = 1;
+
+ WebPChunkVP8 vp8 = (WebPChunkVP8) chunk;
+ width = vp8.getWidth();
+ height = vp8.getHeight();
+ colorType = ImageInfo.ColorType.YCbCr;
+ } else if (chunk instanceof WebPChunkVP8L) {
+ formatDetails = "WebP/Lossless";
+ numberOfImages = 1;
+
+ WebPChunkVP8L vp8l = (WebPChunkVP8L) chunk;
+ width = vp8l.getImageWidth();
+ height = vp8l.getImageHeight();
+ } else if (chunk instanceof WebPChunkVP8X) {
+ WebPChunkVP8X vp8x = (WebPChunkVP8X) chunk;
+ width = vp8x.getCanvasWidth();
+ height = vp8x.getCanvasHeight();
+ hasAlpha = ((WebPChunkVP8X) chunk).isHasAlpha();
+
+ if (vp8x.isAnimation()) {
+ formatDetails = "WebP/Animation";
Review Comment:
I think these could be constants in the class, like the extensions. Will move them as it's harmless :man_shrugging:
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IMAGING-339] Basic WebP Support [commons-imaging]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1347905661
##########
src/main/java/org/apache/commons/imaging/formats/webp/WebPChunkType.java:
##########
@@ -0,0 +1,119 @@
+/*
+ * 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.ImagingException;
+import org.apache.commons.imaging.common.BinaryFunctions;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunk;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkAlph;
+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.WebPChunkXml;
+import org.apache.commons.imaging.formats.webp.chunks.WebPChunkXyzw;
+
+import java.io.IOException;
+
+/**
+ * WebP chunk type.
+ *
+ * @since 1.0-alpha4
+ */
+public enum WebPChunkType {
+
+ /**
+ * @see WebPChunkAlph
+ */
+ ALPH(WebPChunkAlph::new),
Review Comment:
Added `ALPH` that was not listed here before.
##########
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunk.java:
##########
@@ -0,0 +1,122 @@
+/*
+ * 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 org.apache.commons.imaging.internal.SafeOperations;
+
+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 <a href="https://developers.google.com/speed/webp/docs/riff_container">WebP Container Specification</a>
+ * @since 1.0-alpha4
+ */
+public abstract class WebPChunk extends BinaryFileParser {
+ private final int type;
+ private final int size;
+ protected final byte[] bytes;
+ private final int chunkSize;
+
+ /**
+ * Create a new WebP chunk.
+ *
+ * @param type chunk type.
+ * @param size chunk size.
+ * @param bytes chunk data.
+ * @throws ImagingException if the chunk data and the size provided do not match.
+ */
+ WebPChunk(int type, int size, byte[] bytes) throws ImagingException {
+ super(ByteOrder.LITTLE_ENDIAN);
+
+ if (size != bytes.length) {
+ throw new ImagingException("Chunk size must match bytes length");
+ }
+
+ this.type = type;
+ this.size = size;
+ this.bytes = bytes;
+
+ // 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
+ this.chunkSize = SafeOperations.add(4, 4, size, padding);
+ }
+
+ /**
+ * @return the chunk type.
+ */
+ public int getType() {
+ return type;
+ }
+
+ /**
+ * @return the description of the chunk type.
+ */
+ public String getTypeDescription() {
+ return new String(new byte[]{
+ (byte) (type & 0xff),
+ (byte) ((type >> 8) & 0xff),
+ (byte) ((type >> 16) & 0xff),
+ (byte) ((type >> 24) & 0xff)}, StandardCharsets.UTF_8);
+ }
+
+ /**
+ * @return the payload size.
+ */
+ public int getPayloadSize() {
+ return size;
+ }
+
+ /**
+ * @return the chunk size.
+ */
+ public int getChunkSize() {
+ return chunkSize;
+ }
+
+ /**
+ * @return a copy of the chunk data as bytes.
+ */
+ public byte[] getBytes() {
+ return bytes.clone();
+ }
+
+ /**
+ * Print the chunk to the given stream.
+ *
+ * @param pw a stream to write to.
+ * @param offset chunk offset.
+ * @throws ImagingException if the image is invalid.
+ * @throws IOException if it fails to write to the given stream.
+ */
+ public void dump(PrintWriter pw, int offset) throws ImagingException, IOException {
+ pw.printf(
+ "Chunk %s at offset %s, length %d%n, payload size %d%n",
+ getTypeDescription(),
+ offset >= 0 ? String.valueOf(offset) : "unknown",
+ getChunkSize(),
+ getPayloadSize());
Review Comment:
Modified the `dump` methods to call the getters/setters, this way we stress every method in the tests.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IMAGING-339] Basic WebP Support [commons-imaging]
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1752021660
I re-checked this PR and it looks good.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1049312581
##########
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<WebPImagingParameters> 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:
@kinow I don't understand the real difference between `IOException` and `ImageReadException`.
I use `IOException` here to imitate the behavior of `BinaryFunctions.read4Bytes`, because it will also throw `IOException` instead of `ImageReadException`.
The subtle difference between them confused me. Should we let `ImagingException` extends `IOException`?
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1068659362
##########
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:
> You must be thinking of IllegalStateException, we should not throw Errors.
In my understanding, here I am asserting that throwing an exception is an unreachable path.
But I thought again, maybe it is also feasible to throw `IllegalArgumentException`, because Chunk may also be constructed by users themselves.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342192570
##########
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?
Okay. It's really not that noticeable compared to the I/O overhead, so we don't have to rush to deal with it
##########
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?
Okay. It's really not that noticeable compared to the I/O overhead, so we don't have to rush to deal with 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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1742114767
> I probably just named them arbitrarily, so feel free to rename them if necessary.
Thanks @Glavo !
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IMAGING-339] Basic WebP Support [commons-imaging]
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1752022542
> VP8X doesn't have a version number, and I couldn't find in the developer docs anything saying that it should have, but other chunks have it... I guess that chunk doesn't need it?
I implemented it in full compliance with the developer documentation. I'm not sure why there's no version number, but I guess it's not needed.
> ALPH chunk was never used. I added it to the enum of chunk types... or was it intentional to not add that one?
I don't remember the reason for this, so it should just be an oversight on my part. Thanks for the fix.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1049316355
##########
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<WebPImagingParameters> 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 = new ChunksReader(byteSource, WebPChunkXMP.TYPE_XMP)) {
+ WebPChunkXMP chunk = (WebPChunkXMP) reader.readChunk();
+ return chunk == null ? null : chunk.getXml();
+ }
+ }
+
+ @Override
+ public ImageInfo getImageInfo(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ try (ChunksReader reader = new ChunksReader(byteSource,
+ WebPChunk.TYPE_VP8, WebPChunk.TYPE_VP8L, WebPChunk.TYPE_VP8X, WebPChunk.TYPE_ANMF)) {
+ String formatDetails;
+ int width;
+ int height;
+ int numberOfImages;
+ boolean hasAlpha = false;
+ ImageInfo.ColorType colorType = ImageInfo.ColorType.RGB;
+
+ WebPChunk chunk = reader.readChunk();
+ if (chunk instanceof WebPChunkVP8) {
+ formatDetails = "WebP/Lossy";
+ numberOfImages = 1;
+
+ WebPChunkVP8 vp8 = (WebPChunkVP8) chunk;
+ width = vp8.getWidth();
+ height = vp8.getHeight();
+ colorType = ImageInfo.ColorType.YCbCr;
+ } else if (chunk instanceof WebPChunkVP8L) {
+ formatDetails = "WebP/Lossless";
+ numberOfImages = 1;
+
+ WebPChunkVP8L vp8l = (WebPChunkVP8L) chunk;
+ width = vp8l.getImageWidth();
+ height = vp8l.getImageHeight();
+ } else if (chunk instanceof WebPChunkVP8X) {
+ WebPChunkVP8X vp8x = (WebPChunkVP8X) chunk;
+ width = vp8x.getCanvasWidth();
+ height = vp8x.getCanvasHeight();
+ hasAlpha = ((WebPChunkVP8X) chunk).isHasAlpha();
+
+ if (vp8x.isAnimation()) {
+ formatDetails = "WebP/Animation";
Review Comment:
> Worth moving these strings to constants?
I checked the implementation of other image formats. Format details seems to be an implementation specific internal representation, so I don't think it needs to be placed elsewhere.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
kinow commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1352724001
>In my understanding, the alpha phase allows large API changes, but after entering the beta phase, the API tends to be stable, and there can be some minor damage, but usually it should not change as much as the alpha phase.
Correct. That's why it's important to decide on whether it will be an alpha3 or beta1. I didn't have plans to add anything to the public API before 1.0, but I think WebP support could go in any non-final release as long as we take care with what is added to the public API, and add tests/docs.
>However, even if this problem is solved, I think the commons-imaging API still relies too much on AWT.
Agreed.
>I don't think so many BufferedImage based APIs should be provided, nor should the implementation of each format be based on AWT/Swing.
>
>Maybe we can provide a general abstraction to hide these details. The implementation of the format should only use a common interface, and behind the interface may be AWT/Swing, JavaFX, Android, or even other file formats writter.
>
>This is my rough idea. Do you have any opinion on it?
Hmmm, this sounds interesting!
I just remembered that when I went looking for ways to support Android, I found someone that had a project with the `library-something.jar`, with the interfaces, and `library-something-awt.jar` and `library-something-android.jar`. But I found that solution harder to implement & maintain.
But let's give it another try before the 1.0. I don't mind postponing this release in order to have more alphas/betas. This is the issue for Android, we can rename/use it for AWT: https://issues.apache.org/jira/browse/IMAGING-208 (if you do not have an account, let me know and I'll prepare the sign up process for you, ASF blocked new sign ups due to Spam).
Thanks!!!
-Bruno
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
kinow commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1358092800
> @kinow Can you open this issue for me?
>
> I'm sorry to reply you so late, because I'm not feeling well recently (probably infected with COVID-19), and I need a rest for a while.
>
> Besides, I'm not good at English. You can better describe this problem than let me use Google Translate myself.
Hi @Glavo , your English is excellent for me (I am not a native speaker either) , but don't worry I left this notification unread on my GitHub list of notifications, and I will update it later with a JIRA issue.
Get some rest, and I hope you feel better soon.
Thanks
Bruno
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1738133122
> We don't need to use since tags IMO since we don't have a 1.0 yet.
I think there are other since tags (can't recall if only for 1.0-alphaX or for 0.98 (sanselan) as well)
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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<WebPImagingParameters> 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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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 <a href="https://developers.google.com/speed/webp/docs/riff_container">WebP Container Specification</a>
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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<WebPImagingParameters> 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 = new ChunksReader(byteSource, WebPChunkXMP.TYPE_XMP)) {
+ WebPChunkXMP chunk = (WebPChunkXMP) reader.readChunk();
+ return chunk == null ? null : chunk.getXml();
+ }
+ }
+
+ @Override
+ public ImageInfo getImageInfo(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ try (ChunksReader reader = new ChunksReader(byteSource,
+ WebPChunk.TYPE_VP8, WebPChunk.TYPE_VP8L, WebPChunk.TYPE_VP8X, WebPChunk.TYPE_ANMF)) {
+ String formatDetails;
+ int width;
+ int height;
+ int numberOfImages;
+ boolean hasAlpha = false;
+ ImageInfo.ColorType colorType = ImageInfo.ColorType.RGB;
+
+ WebPChunk chunk = reader.readChunk();
+ if (chunk instanceof WebPChunkVP8) {
+ formatDetails = "WebP/Lossy";
+ numberOfImages = 1;
+
+ WebPChunkVP8 vp8 = (WebPChunkVP8) chunk;
+ width = vp8.getWidth();
+ height = vp8.getHeight();
+ colorType = ImageInfo.ColorType.YCbCr;
+ } else if (chunk instanceof WebPChunkVP8L) {
+ formatDetails = "WebP/Lossless";
+ numberOfImages = 1;
+
+ WebPChunkVP8L vp8l = (WebPChunkVP8L) chunk;
+ width = vp8l.getImageWidth();
+ height = vp8l.getImageHeight();
+ } else if (chunk instanceof WebPChunkVP8X) {
+ WebPChunkVP8X vp8x = (WebPChunkVP8X) chunk;
+ width = vp8x.getCanvasWidth();
+ height = vp8x.getCanvasHeight();
+ hasAlpha = ((WebPChunkVP8X) chunk).isHasAlpha();
+
+ if (vp8x.isAnimation()) {
+ formatDetails = "WebP/Animation";
Review Comment:
Oh, we could even use an enum for the formats... but now I see they are being used once, and the performance benefit would be minimal. We can worry about that later. Leaving as-is, 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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
kinow commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1352242618
>@kinow By adding exclusion rules, I no longer get warnings when running mvn -V --no-transfer-progress locally. Can you trigger the CI again?
Done!
>Basic support for WebP has been completed. Now I can successfully read the basic information of the lossy/lossless compressed webp file.
>
>I also implemented dumpFile for WebP files. Now it can print the contents of webp chunks in a similar way to the webpinfo tool.
Thakn you!
>It is difficult for me to implement the function of reading/writing image content. I try to read the source code of libwebp, but it is not easy for me. So I can't make more contributions to WebP support in the near future.
I never read the format docs, so no idea how hard it would be. But one option would be to include it in our upcoming 1.0beta release (one beta before the final release) and document what it does and what it doesn't. It's better to be able to at least read metadata/information, then not being able to read WebP at all.
Thank you @Glavo !
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1352708022
> > @kinow Will the next version be 1.0 beta?
>
> Yes. We could release an 1.0-alpha3 if the API is not stable enough. I was actually thinking in releasing 1.0 final, but there was some internal discussion about it, and it was decided to release at least one beta version after these alpha releases, to a) make sure we have a good public API (i.e. err on having less than what's needed, not more, so that we can maintain it for a while) and b) it has enough features for users.
>
> Do you have any suggestion on alpha/beta/final?
>
> > Do you have any ideas about when to make AWT as optional dependency? I think it is better to solve this problem in the alpha phase, because this change may break the user's code.
>
> I think it is more a matter of deciding the how first. If it is not too complicated, we can include it in the next release.
>
> I can't recall what classes we used from AWT (Dimension maybe?).
>
> We had a user that contributed with a lot of suggestions and by testing pull requests, and his application was being deployed on Android. I think he was the first one to request it, from what I recall, as for the time being he had to do some patching on his code in order to use Imaging.
>
> If you have any suggestions, feel free to let us know, please 🙂
>
> BTW, it would be great if you could create a JIRA issue for this issue too, as we need one to track changes in our `changes.xml` (that's updated by committers, no need to worry about that).
>
> Thanks heaps!
>
> EDIT: BufferedImage too from awt, d'oh
In my understanding, the alpha phase allows large API changes, but after entering the beta phase, the API tends to be stable, and there can be some minor damage, but usually it should not change as much as the alpha phase.
> I can't recall what classes we used from AWT (Dimension maybe?).
Yes, if we provide an alternative to `Dimension`, we can use commons-imaging in more environments without AWT.
However, even if this problem is solved, I think the commons-imaging API still relies too much on AWT.
I don't think so many `BufferedImage` based APIs should be provided, nor should the implementation of each format be based on AWT/Swing.
Maybe we can provide a general abstraction to hide these details. The implementation of the format should only use a common interface, and behind the interface may be AWT/Swing, JavaFX, Android, or even other file formats writter.
This is my rough idea. Do you have any opinion on 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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] garydgregory commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by GitBox <gi...@apache.org>.
garydgregory commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1068651074
##########
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:
You must be thinking of IllegalStateException, we should not throw Errors.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] garydgregory commented on pull request #254: [IMAGING-339] Basic WebP Support
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1738088877
We don't need to use since tags IMO since we don't have a 1.0 yet.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IMAGING-339] Basic WebP Support [commons-imaging]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1747644753
Well, this is as much as I could get today.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342187578
##########
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##########
@@ -42,76 +44,105 @@
* }</pre>
*
* @see <a href="https://developers.google.com/speed/webp/docs/riff_container#extended_file_format">Extended File Format</a>
- *
* @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:
Should these field names be renamed? Referring to `PamFileInfo`, perhaps it would be better for us to keep the 'has' prefix in the field name.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342191305
##########
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:
It doesn't seem to be that easy to optimize. Maybe we should add a few overloads?
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342195072
##########
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##########
@@ -42,76 +45,105 @@
* }</pre>
*
* @see <a href="https://developers.google.com/speed/webp/docs/riff_container#extended_file_format">Extended File Format</a>
- *
* @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_0000) != 0;
+ this.hasIcc = (mark & 0b0010_0000) != 0;
this.hasAlpha = (mark & 0b0001_0000) != 0;
this.hasExif = (mark & 0b0000_1000) != 0;
this.hasXmp = (mark & 0b0000_0100) != 0;
- this.isAnimation = (mark & 0b0000_0010) != 0;
+ this.hasAnimation = (mark & 0b0000_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:
The other changes look good. I’m going to take a break too, and I’ll revisit the entire PR later to see what I haven’t forgotten.
##########
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##########
@@ -42,76 +45,105 @@
* }</pre>
*
* @see <a href="https://developers.google.com/speed/webp/docs/riff_container#extended_file_format">Extended File Format</a>
- *
* @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_0000) != 0;
+ this.hasIcc = (mark & 0b0010_0000) != 0;
this.hasAlpha = (mark & 0b0001_0000) != 0;
this.hasExif = (mark & 0b0000_1000) != 0;
this.hasXmp = (mark & 0b0000_0100) != 0;
- this.isAnimation = (mark & 0b0000_0010) != 0;
+ this.hasAnimation = (mark & 0b0000_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:
The other changes look good. I’m going to take a break too, and I’ll revisit the entire PR later to see what I haven’t forgotten.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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 <a href="https://developers.google.com/speed/webp/docs/riff_container">WebP Container Specification</a>
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 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:
Maybe the description here could be improved.
##########
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkALPH.java:
##########
@@ -0,0 +1,41 @@
+/*
+ * 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;
+
+/**
+ * <pre>{@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
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * | ChunkHeader('ALPH') |
+ * | |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * |Rsv| P | F | C | Alpha Bitstream... |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * }</pre>
Review Comment:
:ok_man: Good docs! Thanks!
##########
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<WebPImagingParameters> implements XmpEmbeddable<WebPImagingParameters> {
+
+ 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:
Been too long since I worked on the code, but I **think** there is an `ImageReadException`. Have to confirm if other parsers raise that or this error, then try to keep it consistent.
##########
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageParser.java:
##########
Review Comment:
I went through the other files and they all look OK. Added some minor comments. This is the only class that seems it will take a bit longer to review (which is normal, the parsers are normally the most used classes, with more functions).
##########
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:
Note to self: did we use (or started using) the since tag in tests too? Or was it in another component?
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] codecov-commenter commented on pull request #254: [WIP] Basic WebP Support
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1351697280
# [Codecov](https://codecov.io/gh/apache/commons-imaging/pull/254?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#254](https://codecov.io/gh/apache/commons-imaging/pull/254?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (4fb4592) into [master](https://codecov.io/gh/apache/commons-imaging/commit/da9c65942c3e47d91b2d9cba1307a389e3a86c68?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (da9c659) will **decrease** coverage by `0.79%`.
> The diff coverage is `2.01%`.
```diff
@@ Coverage Diff @@
## master #254 +/- ##
============================================
- Coverage 70.76% 69.96% -0.80%
Complexity 3362 3362
============================================
Files 332 346 +14
Lines 16958 17157 +199
Branches 2652 2685 +33
============================================
+ Hits 12000 12004 +4
- Misses 3909 4103 +194
- Partials 1049 1050 +1
```
| [Impacted Files](https://codecov.io/gh/apache/commons-imaging/pull/254?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...he/commons/imaging/formats/webp/WebPConstants.java](https://codecov.io/gh/apache/commons-imaging/pull/254/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW1hZ2luZy9mb3JtYXRzL3dlYnAvV2ViUENvbnN0YW50cy5qYXZh) | `0.00% <0.00%> (ø)` | |
| [.../commons/imaging/formats/webp/WebPImageParser.java](https://codecov.io/gh/apache/commons-imaging/pull/254/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW1hZ2luZy9mb3JtYXRzL3dlYnAvV2ViUEltYWdlUGFyc2VyLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...ns/imaging/formats/webp/WebPImagingParameters.java](https://codecov.io/gh/apache/commons-imaging/pull/254/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW1hZ2luZy9mb3JtYXRzL3dlYnAvV2ViUEltYWdpbmdQYXJhbWV0ZXJzLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...commons/imaging/formats/webp/chunks/WebPChunk.java](https://codecov.io/gh/apache/commons-imaging/pull/254/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW1hZ2luZy9mb3JtYXRzL3dlYnAvY2h1bmtzL1dlYlBDaHVuay5qYXZh) | `0.00% <0.00%> (ø)` | |
| [...ons/imaging/formats/webp/chunks/WebPChunkALPH.java](https://codecov.io/gh/apache/commons-imaging/pull/254/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW1hZ2luZy9mb3JtYXRzL3dlYnAvY2h1bmtzL1dlYlBDaHVua0FMUEguamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ons/imaging/formats/webp/chunks/WebPChunkANIM.java](https://codecov.io/gh/apache/commons-imaging/pull/254/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW1hZ2luZy9mb3JtYXRzL3dlYnAvY2h1bmtzL1dlYlBDaHVua0FOSU0uamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ons/imaging/formats/webp/chunks/WebPChunkANMF.java](https://codecov.io/gh/apache/commons-imaging/pull/254/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW1hZ2luZy9mb3JtYXRzL3dlYnAvY2h1bmtzL1dlYlBDaHVua0FOTUYuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ons/imaging/formats/webp/chunks/WebPChunkEXIF.java](https://codecov.io/gh/apache/commons-imaging/pull/254/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW1hZ2luZy9mb3JtYXRzL3dlYnAvY2h1bmtzL1dlYlBDaHVua0VYSUYuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...ons/imaging/formats/webp/chunks/WebPChunkICCP.java](https://codecov.io/gh/apache/commons-imaging/pull/254/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW1hZ2luZy9mb3JtYXRzL3dlYnAvY2h1bmtzL1dlYlBDaHVua0lDQ1AuamF2YQ==) | `0.00% <0.00%> (ø)` | |
| [...mons/imaging/formats/webp/chunks/WebPChunkVP8.java](https://codecov.io/gh/apache/commons-imaging/pull/254/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c3JjL21haW4vamF2YS9vcmcvYXBhY2hlL2NvbW1vbnMvaW1hZ2luZy9mb3JtYXRzL3dlYnAvY2h1bmtzL1dlYlBDaHVua1ZQOC5qYXZh) | `0.00% <0.00%> (ø)` | |
| ... and [6 more](https://codecov.io/gh/apache/commons-imaging/pull/254/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
:mega: We’re building smart automated test selection to slash your CI/CD build times. [Learn more](https://about.codecov.io/iterative-testing/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1352148173
It seems that there are some strict checkstyle rules here. I try to solve them.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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;
+
+/**
+ * <pre>{@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 |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * }</pre>
+ *
+ * @see <a href="https://developers.google.com/speed/webp/docs/riff_container#extended_file_format">Extended File Format</a>
+ *
+ * @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_0000) != 0;
+ this.hasAlpha = (mark & 0b0001_0000) != 0;
+ this.hasExif = (mark & 0b0000_1000) != 0;
+ this.hasXmp = (mark & 0b0000_0100) != 0;
+ this.isAnimation = (mark & 0b0000_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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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 <a href="https://developers.google.com/speed/webp/docs/riff_container">WebP Container Specification</a>
- *
* @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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342188466
##########
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.
Furthermore, the name `extraPadding ` is somewhat misleading, since `padding` only refers to the padding at the end of the chunk.
##########
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:
I think we should check chunk size in constructor instead of here, this method should not throw exception.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IMAGING-339] Basic WebP Support [commons-imaging]
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1346186993
##########
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##########
@@ -42,76 +45,105 @@
* }</pre>
*
* @see <a href="https://developers.google.com/speed/webp/docs/riff_container#extended_file_format">Extended File Format</a>
- *
* @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_0000) != 0;
+ this.hasIcc = (mark & 0b0010_0000) != 0;
this.hasAlpha = (mark & 0b0001_0000) != 0;
this.hasExif = (mark & 0b0000_1000) != 0;
this.hasXmp = (mark & 0b0000_0100) != 0;
- this.isAnimation = (mark & 0b0000_0010) != 0;
+ this.hasAnimation = (mark & 0b0000_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:
You were right @glavo, reverted the change :+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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IMAGING-339] Basic WebP Support [commons-imaging]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory merged PR #254:
URL: https://github.com/apache/commons-imaging/pull/254
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1049312581
##########
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<WebPImagingParameters> 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:
@kinow I don't understand the real difference between `IOException` and `ImageReadException`.
I use `IOException` here to imitate the behavior of `BinaryFunctions.read4Bytes`, because it will also throw `IOException` instead of `ImageReadException`.
The subtle difference between them confused me. Maybe we should make `ImagingException` extends `IOException`?
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1056979055
##########
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVP8.java:
##########
@@ -0,0 +1,98 @@
+/*
+ * 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 java.io.IOException;
+import java.io.PrintWriter;
+
+public final class WebPChunkVP8 extends WebPChunk {
+ private final int versionNumber;
+ private final int width;
+ private final int height;
+ private final int horizontalScale;
+ private final int verticalScale;
+
+ public WebPChunkVP8(int type, int size, byte[] bytes) throws ImageReadException {
+ super(type, size, bytes);
+
+ if (size < 10) {
+ throw new ImageReadException("Invalid VP8 chunk");
+ }
+
+ int b0 = bytes[0] & 0xFF;
+ // int b1 = bytes[1] & 0xFF;
+ // int b2 = bytes[2] & 0xFF;
Review Comment:
I like that these two are commented out, meaning that it was not a mistake to skip these. But we still need a comment, so other devs are aware of why this was done, please.
##########
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");
+ }
+
+ this.type = type;
+ this.size = size;
+ this.bytes = bytes;
+ }
+
+ public int getType() {
+ return type;
+ }
+
+ public String getTypeDescription() {
+ return new String(new byte[]{
+ (byte) (type & 0xff),
+ (byte) ((type >> 8) & 0xff),
+ (byte) ((type >> 16) & 0xff),
+ (byte) ((type >> 24) & 0xff)}, StandardCharsets.UTF_8);
+ }
+
+ public int getPayloadSize() {
+ return size;
+ }
+
+ public int getChunkSize() {
+ return size + 8 + ((size % 2) == 0 ? 0 : 1);
Review Comment:
This is a good example of something that's not too complicated, but would be nicer to have a short explanation why we are doing it. If it's in the spec, you can just say so :+1:
##########
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:
Instead of `AssertionError` I think it would be best to throw the `ImageReadException`
##########
src/main/java/org/apache/commons/imaging/formats/webp/WebPImagingParameters.java:
##########
@@ -0,0 +1,22 @@
+/*
+ * 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.XmpImagingParameters;
+
+public class WebPImagingParameters extends XmpImagingParameters {
Review Comment:
Even small public methods and classes need a javadoc. It normally must include
- brief comment
- @since tag
- iff it's useful, tell users whether it's thread-safe or not
- if it contains complicated code, you can have a longer comment, include code examples, links to spec, etc.
##########
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkXYZW.java:
##########
@@ -0,0 +1,25 @@
+/*
+ * 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;
+
+public final class WebPChunkXYZW extends WebPChunk {
+ public WebPChunkXYZW(int type, int size, byte[] bytes) throws ImageReadException {
Review Comment:
We need javadocs for public methods at least. If there are internal/private methods, that are not easy to understand (e.g. complex logic, lots of byte handling) it's also recommended to have some javadocs & code comments.
##########
src/main/java/org/apache/commons/imaging/formats/webp/WebPImagingParameters.java:
##########
@@ -0,0 +1,22 @@
+/*
+ * 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.XmpImagingParameters;
+
+public class WebPImagingParameters extends XmpImagingParameters {
Review Comment:
e.g. https://github.com/apache/commons-imaging/blob/cb0b52e5b82321cac1d6b4c97db7d58375322733/src/main/java/org/apache/commons/imaging/formats/png/PngImagingParameters.java#L23-L26
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1352212147
@kinow By adding exclusion rules, I no longer get warnings when running `mvn -V --no-transfer-progress` locally. Can you trigger the CI again?
Basic support for WebP has been completed. Now I can successfully read the basic information of the lossy/lossless compressed webp file.
I also implemented dumpFile for WebP files. Now it can print the contents of webp chunks in a similar way to the `webpinfo` tool.
It is difficult for me to implement the function of reading/writing image content. I try to read the source code of libwebp, but it is not easy for me. So I can't make more contributions to WebP support in the near future.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on pull request #254: [IMAGING-339] Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1381023692
@kinow I went back to this work and completed the missing documents.
Can you take a look at my question here at https://github.com/apache/commons-imaging/pull/254/files/08d6a0dff607eefa633c89d231a01623e3ad90c6#r1049312581?
I feel that the usage of `ImageReadException` and `IOException` is often confused, so I don't know which one to choose.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by GitBox <gi...@apache.org>.
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
kinow commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1737998055
Thank you @Glavo ! Hopefully we can solve simple issues and we won't need any major refactoring for the review :crossed_fingers: Coming back to this one soon... (allowed GH Actions to run)
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1339491986
##########
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 <a href="https://developers.google.com/speed/webp/docs/riff_container">WebP Container Specification</a>
Review Comment:
None of the parsers for the other formats have javadoc, so I'm confused as to where to put the documentation.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1342194529
##########
src/main/java/org/apache/commons/imaging/formats/webp/chunks/WebPChunkVp8x.java:
##########
@@ -42,76 +45,105 @@
* }</pre>
*
* @see <a href="https://developers.google.com/speed/webp/docs/riff_container#extended_file_format">Extended File Format</a>
- *
* @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_0000) != 0;
+ this.hasIcc = (mark & 0b0010_0000) != 0;
this.hasAlpha = (mark & 0b0001_0000) != 0;
this.hasExif = (mark & 0b0000_1000) != 0;
this.hasXmp = (mark & 0b0000_0100) != 0;
- this.isAnimation = (mark & 0b0000_0010) != 0;
+ this.hasAnimation = (mark & 0b0000_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:
There is no need to use `SafeOperations` here. Both `canvasWidth ` and `canvasHeight ` are 24-bit integers, so overflow is not possible.
I forget why I want to check that they are non-negative, but I guess using `SafeOperations` would mislead the reader that an overflow might occur here, so it might be better to 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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1068661686
##########
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<WebPImagingParameters> 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:
@kinow Can you take a look at this question?
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on pull request #254: [IMAGING-339] Basic WebP Support
Posted by GitBox <gi...@apache.org>.
Glavo commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1383319953
I updated this PR, added the `@since` tag and more detailed javadoc.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
kinow commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1352317094
> @kinow Will the next version be 1.0 beta?
Yes. We could release an 1.0-alpha3 if the API is not stable enough. I was actually thinking in releasing 1.0 final, but there was some internal discussion about it, and it was decided to release at least one beta version after these alpha releases, to a) make sure we have a good public API (i.e. err on having less than what's needed, not more, so that we can maintain it for a while) and b) it has enough features for users.
Do you have any suggestion on alpha/beta/final?
> Do you have any ideas about when to make AWT as optional dependency? I think it is better to solve this problem in the alpha phase, because this change may break the user's code.
I think it is more a matter of deciding the how first. If it is not too complicated, we can include it in the next release.
I can't recall what classes we used from AWT (Dimension maybe?).
We had a user that contributed with a lot of suggestions and by testing pull requests, and his application was being deployed on Android. I think he was the first one to request it, from what I recall, as for the time being he had to do some patching on his code in order to use Imaging.
If you have any suggestions, feel free to let us know, please :slightly_smiling_face:
BTW, it would be great if you could create a JIRA issue for this issue too, as we need one to track changes in our `changes.xml` (that's updated by committers, no need to worry about that).
Thanks heaps!
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: Basic WebP Support
Posted by GitBox <gi...@apache.org>.
kinow commented on code in PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#discussion_r1049042869
##########
src/main/java/org/apache/commons/imaging/formats/webp/WebPConstants.java:
##########
@@ -0,0 +1,29 @@
+/*
+ * 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.BinaryConstant;
+
+public final class WebPConstants {
+
+ public static final BinaryConstant RIFF_SIGNATURE = new BinaryConstant(new byte[]{'R', 'I', 'F', 'F'});
+ public static final BinaryConstant WEBP_SIGNATURE = new BinaryConstant(new byte[]{'W', 'E', 'B', 'P'});
Review Comment:
Would it be possible include Javadocs for these public fields & for the class, please?
##########
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<WebPImagingParameters> 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 = new ChunksReader(byteSource, WebPChunkXMP.TYPE_XMP)) {
+ WebPChunkXMP chunk = (WebPChunkXMP) reader.readChunk();
+ return chunk == null ? null : chunk.getXml();
+ }
+ }
+
+ @Override
+ public ImageInfo getImageInfo(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ try (ChunksReader reader = new ChunksReader(byteSource,
+ WebPChunk.TYPE_VP8, WebPChunk.TYPE_VP8L, WebPChunk.TYPE_VP8X, WebPChunk.TYPE_ANMF)) {
+ String formatDetails;
+ int width;
+ int height;
+ int numberOfImages;
+ boolean hasAlpha = false;
+ ImageInfo.ColorType colorType = ImageInfo.ColorType.RGB;
+
+ WebPChunk chunk = reader.readChunk();
+ if (chunk instanceof WebPChunkVP8) {
+ formatDetails = "WebP/Lossy";
+ numberOfImages = 1;
+
+ WebPChunkVP8 vp8 = (WebPChunkVP8) chunk;
+ width = vp8.getWidth();
+ height = vp8.getHeight();
+ colorType = ImageInfo.ColorType.YCbCr;
+ } else if (chunk instanceof WebPChunkVP8L) {
+ formatDetails = "WebP/Lossless";
+ numberOfImages = 1;
+
+ WebPChunkVP8L vp8l = (WebPChunkVP8L) chunk;
+ width = vp8l.getImageWidth();
+ height = vp8l.getImageHeight();
+ } else if (chunk instanceof WebPChunkVP8X) {
+ WebPChunkVP8X vp8x = (WebPChunkVP8X) chunk;
+ width = vp8x.getCanvasWidth();
+ height = vp8x.getCanvasHeight();
+ hasAlpha = ((WebPChunkVP8X) chunk).isHasAlpha();
+
+ if (vp8x.isAnimation()) {
+ formatDetails = "WebP/Animation";
Review Comment:
Worth moving these strings to constants?
##########
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<WebPImagingParameters> 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 = new ChunksReader(byteSource, WebPChunkXMP.TYPE_XMP)) {
+ WebPChunkXMP chunk = (WebPChunkXMP) reader.readChunk();
+ return chunk == null ? null : chunk.getXml();
+ }
+ }
+
+ @Override
+ public ImageInfo getImageInfo(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ try (ChunksReader reader = new ChunksReader(byteSource,
+ WebPChunk.TYPE_VP8, WebPChunk.TYPE_VP8L, WebPChunk.TYPE_VP8X, WebPChunk.TYPE_ANMF)) {
+ String formatDetails;
+ int width;
+ int height;
+ int numberOfImages;
+ boolean hasAlpha = false;
+ ImageInfo.ColorType colorType = ImageInfo.ColorType.RGB;
+
+ WebPChunk chunk = reader.readChunk();
+ if (chunk instanceof WebPChunkVP8) {
+ formatDetails = "WebP/Lossy";
+ numberOfImages = 1;
+
+ WebPChunkVP8 vp8 = (WebPChunkVP8) chunk;
+ width = vp8.getWidth();
+ height = vp8.getHeight();
+ colorType = ImageInfo.ColorType.YCbCr;
+ } else if (chunk instanceof WebPChunkVP8L) {
+ formatDetails = "WebP/Lossless";
+ numberOfImages = 1;
+
+ WebPChunkVP8L vp8l = (WebPChunkVP8L) chunk;
+ width = vp8l.getImageWidth();
+ height = vp8l.getImageHeight();
+ } else if (chunk instanceof WebPChunkVP8X) {
+ WebPChunkVP8X vp8x = (WebPChunkVP8X) chunk;
+ width = vp8x.getCanvasWidth();
+ height = vp8x.getCanvasHeight();
+ hasAlpha = ((WebPChunkVP8X) chunk).isHasAlpha();
+
+ if (vp8x.isAnimation()) {
+ formatDetails = "WebP/Animation";
+
+ numberOfImages = 0;
+ while ((chunk = reader.readChunk()) != null) {
+ if (chunk.getType() == WebPChunk.TYPE_ANMF) {
+ numberOfImages++;
+ }
+ }
+
+ } else {
+ numberOfImages = 1;
+ chunk = reader.readChunk();
+
+ if (chunk == null) {
+ throw new ImageReadException("Image has no content");
+ }
+
+ if (chunk.getType() == WebPChunk.TYPE_ANMF) {
+ throw new ImageReadException("Non animated image should not contain ANMF chunks");
+ }
+
+ if (chunk.getType() == WebPChunk.TYPE_VP8) {
+ formatDetails = "WebP/Lossy (Extended)";
+ colorType = ImageInfo.ColorType.YCbCr;
+ } else if (chunk.getType() == WebPChunk.TYPE_VP8L) {
+ formatDetails = "WebP/Lossless (Extended)";
+ } else {
+ throw new ImageReadException("Unknown webp chunk type: " + chunk);
+ }
+ }
+ } else {
+ throw new ImageReadException("Unknown webp chunk type: " + chunk);
+ }
+
+ return new ImageInfo(
+ formatDetails, 32, new ArrayList<>(), ImageFormats.WEBP,
+ "webp", height, "image/webp", numberOfImages,
+ -1, -1, -1, -1,
+ width, false, hasAlpha, false,
+ colorType, ImageInfo.CompressionAlgorithm.UNKNOWN
+ );
+ }
+ }
+
+ @Override
+ public Dimension getImageSize(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ try (ChunksReader reader = new ChunksReader(byteSource)) {
+ WebPChunk chunk = reader.readChunk();
+ if (chunk instanceof WebPChunkVP8) {
+ WebPChunkVP8 vp8 = (WebPChunkVP8) chunk;
+ return new Dimension(vp8.getWidth(), vp8.getHeight());
+ } else if (chunk instanceof WebPChunkVP8L) {
+ WebPChunkVP8L vp8l = (WebPChunkVP8L) chunk;
+ return new Dimension(vp8l.getImageWidth(), vp8l.getImageHeight());
+ } else if (chunk instanceof WebPChunkVP8X) {
+ WebPChunkVP8X vp8x = (WebPChunkVP8X) chunk;
+ return new Dimension(vp8x.getCanvasWidth(), vp8x.getCanvasHeight());
+ } else {
+ throw new ImageReadException("Unknown webp chunk type: " + chunk);
Review Comment:
webp & WebP, to be consistent in the error messages.
##########
src/main/java/org/apache/commons/imaging/formats/webp/WebPImageMetadata.java:
##########
@@ -0,0 +1,45 @@
+/*
+ * 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;
+
+public final class WebPImageMetadata extends GenericImageMetadata {
Review Comment:
Ditto here, javadocs for public members.
##########
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<WebPImagingParameters> 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 {
Review Comment:
For these internal methods, Javadocs are not mandatory, but still recommended, even if just a short description, to document it for other devs / posterity / etc. :+1:
##########
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<WebPImagingParameters> 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:
Was there some reason for throwing both `IOException` and `ImageReadException` here?
I think the other parsers should be probably throwing `ImageReadException`'s when the bytes read are not what it was expected (I think IPTC is an exception). Other classes that deal with byte reading/processing I think throw `IOException`... or when we are reading a stream.
##########
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<WebPImagingParameters> 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 = new ChunksReader(byteSource, WebPChunkXMP.TYPE_XMP)) {
+ WebPChunkXMP chunk = (WebPChunkXMP) reader.readChunk();
+ return chunk == null ? null : chunk.getXml();
+ }
+ }
+
+ @Override
+ public ImageInfo getImageInfo(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ try (ChunksReader reader = new ChunksReader(byteSource,
+ WebPChunk.TYPE_VP8, WebPChunk.TYPE_VP8L, WebPChunk.TYPE_VP8X, WebPChunk.TYPE_ANMF)) {
+ String formatDetails;
+ int width;
+ int height;
+ int numberOfImages;
+ boolean hasAlpha = false;
+ ImageInfo.ColorType colorType = ImageInfo.ColorType.RGB;
+
+ WebPChunk chunk = reader.readChunk();
+ if (chunk instanceof WebPChunkVP8) {
+ formatDetails = "WebP/Lossy";
+ numberOfImages = 1;
+
+ WebPChunkVP8 vp8 = (WebPChunkVP8) chunk;
+ width = vp8.getWidth();
+ height = vp8.getHeight();
+ colorType = ImageInfo.ColorType.YCbCr;
+ } else if (chunk instanceof WebPChunkVP8L) {
+ formatDetails = "WebP/Lossless";
+ numberOfImages = 1;
+
+ WebPChunkVP8L vp8l = (WebPChunkVP8L) chunk;
+ width = vp8l.getImageWidth();
+ height = vp8l.getImageHeight();
+ } else if (chunk instanceof WebPChunkVP8X) {
+ WebPChunkVP8X vp8x = (WebPChunkVP8X) chunk;
+ width = vp8x.getCanvasWidth();
+ height = vp8x.getCanvasHeight();
+ hasAlpha = ((WebPChunkVP8X) chunk).isHasAlpha();
+
+ if (vp8x.isAnimation()) {
+ formatDetails = "WebP/Animation";
+
+ numberOfImages = 0;
+ while ((chunk = reader.readChunk()) != null) {
+ if (chunk.getType() == WebPChunk.TYPE_ANMF) {
+ numberOfImages++;
+ }
+ }
+
+ } else {
+ numberOfImages = 1;
+ chunk = reader.readChunk();
+
+ if (chunk == null) {
+ throw new ImageReadException("Image has no content");
+ }
+
+ if (chunk.getType() == WebPChunk.TYPE_ANMF) {
+ throw new ImageReadException("Non animated image should not contain ANMF chunks");
+ }
+
+ if (chunk.getType() == WebPChunk.TYPE_VP8) {
+ formatDetails = "WebP/Lossy (Extended)";
+ colorType = ImageInfo.ColorType.YCbCr;
+ } else if (chunk.getType() == WebPChunk.TYPE_VP8L) {
+ formatDetails = "WebP/Lossless (Extended)";
+ } else {
+ throw new ImageReadException("Unknown webp chunk type: " + chunk);
+ }
+ }
+ } else {
+ throw new ImageReadException("Unknown webp chunk type: " + chunk);
+ }
+
+ return new ImageInfo(
+ formatDetails, 32, new ArrayList<>(), ImageFormats.WEBP,
+ "webp", height, "image/webp", numberOfImages,
+ -1, -1, -1, -1,
+ width, false, hasAlpha, false,
+ colorType, ImageInfo.CompressionAlgorithm.UNKNOWN
+ );
+ }
+ }
+
+ @Override
+ public Dimension getImageSize(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ try (ChunksReader reader = new ChunksReader(byteSource)) {
+ WebPChunk chunk = reader.readChunk();
+ if (chunk instanceof WebPChunkVP8) {
+ WebPChunkVP8 vp8 = (WebPChunkVP8) chunk;
+ return new Dimension(vp8.getWidth(), vp8.getHeight());
+ } else if (chunk instanceof WebPChunkVP8L) {
+ WebPChunkVP8L vp8l = (WebPChunkVP8L) chunk;
+ return new Dimension(vp8l.getImageWidth(), vp8l.getImageHeight());
+ } else if (chunk instanceof WebPChunkVP8X) {
+ WebPChunkVP8X vp8x = (WebPChunkVP8X) chunk;
+ return new Dimension(vp8x.getCanvasWidth(), vp8x.getCanvasHeight());
+ } else {
+ throw new ImageReadException("Unknown webp chunk type: " + chunk);
+ }
+ }
+ }
+
+ @Override
+ public byte[] getICCProfileBytes(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ try (ChunksReader reader = new ChunksReader(byteSource, WebPChunkXMP.TYPE_ICCP)) {
+ WebPChunk chunk = reader.readChunk();
+ return chunk == null ? null : chunk.getBytes();
+ }
+ }
+
+ @Override
+ public BufferedImage getBufferedImage(ByteSource byteSource, WebPImagingParameters params) throws ImageReadException, IOException {
+ throw new ImageReadException("Reading WebP files is currently not supported");
+ }
+
+ @Override
+ public boolean dumpImageFile(PrintWriter pw, ByteSource byteSource) throws ImageReadException, IOException {
+ pw.println("webp.dumpImageFile");
+ try (ChunksReader reader = new ChunksReader(byteSource)) {
+ int offset = reader.getOffset();
+ WebPChunk chunk = reader.readChunk();
+ if (chunk == null) {
+ throw new ImageReadException("No WebP chunks found");
+ }
+
+ do {
+ chunk.dump(pw, offset);
+
+ offset = reader.getOffset();
+ chunk = reader.readChunk();
+ } while (chunk != null);
Review Comment:
Adding a note to self: check for possible loops & add/multiply that could be made more defensive/safe to avoid CVE's (we had a few found by users & by the oss-fuzzer)
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] Glavo commented on pull request #254: [IMAGING-339] Basic WebP Support
Posted by "Glavo (via GitHub)" <gi...@apache.org>.
Glavo commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1737971692
I've rebased and resolved conflicts.
I noticed that the project had changed a lot in my time away, so I simply updated this PR so that it would compile and pass tests. If there is anything else to update, it will take me some time to get back to this work.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] spencerhakim commented on pull request #254: [IMAGING-339] Basic WebP Support
Posted by "spencerhakim (via GitHub)" <gi...@apache.org>.
spencerhakim commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1737605199
Considering the libwebp CVE the industry is currently dealing with, would love to see this PR's conflicts resolved and this merged in sooner than later so I can move away from using a library that relies on JNI. 🙏
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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;
+
+/**
+ * <pre>{@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 |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * }</pre>
+ *
+ * @see <a href="https://developers.google.com/speed/webp/docs/riff_container#extended_file_format">Extended File Format</a>
+ *
+ * @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_0000) != 0;
+ this.hasAlpha = (mark & 0b0001_0000) != 0;
+ this.hasExif = (mark & 0b0000_1000) != 0;
+ this.hasXmp = (mark & 0b0000_0100) != 0;
+ this.isAnimation = (mark & 0b0000_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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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<WebPImagingParameters> 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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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;
+
+/**
+ * <pre>{@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 |
+ * +-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+
+ * }</pre>
+ *
+ * @see <a href="https://developers.google.com/speed/webp/docs/riff_container#extended_file_format">Extended File Format</a>
+ *
+ * @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_0000) != 0;
+ this.hasAlpha = (mark & 0b0001_0000) != 0;
+ this.hasExif = (mark & 0b0000_1000) != 0;
+ this.hasXmp = (mark & 0b0000_0100) != 0;
+ this.isAnimation = (mark & 0b0000_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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [commons-imaging] kinow commented on a diff in pull request #254: [IMAGING-339] Basic WebP Support
Posted by "kinow (via GitHub)" <gi...@apache.org>.
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 @@
* }</pre>
*
* @see <a href="https://developers.google.com/speed/webp/docs/riff_container#extended_file_format">Extended File Format</a>
- *
* @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_0000) != 0;
+ this.hasIcc = (mark & 0b0010_0000) != 0;
this.hasAlpha = (mark & 0b0001_0000) != 0;
this.hasExif = (mark & 0b0000_1000) != 0;
this.hasXmp = (mark & 0b0000_0100) != 0;
- this.isAnimation = (mark & 0b0000_0010) != 0;
+ this.hasAnimation = (mark & 0b0000_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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] [IMAGING-339] Basic WebP Support [commons-imaging]
Posted by "garydgregory (via GitHub)" <gi...@apache.org>.
garydgregory commented on PR #254:
URL: https://github.com/apache/commons-imaging/pull/254#issuecomment-1747365679
I hope we can get coverage above 90%.
--
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-unsubscribe@commons.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org