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: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
