darkma773r commented on a change in pull request #116:
URL: https://github.com/apache/commons-imaging/pull/116#discussion_r762471491
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormats.java
##########
@@ -16,40 +16,68 @@
*/
package org.apache.commons.imaging;
+import org.apache.commons.imaging.formats.bmp.BmpImagingParameters;
+import org.apache.commons.imaging.formats.gif.GifImagingParameters;
+import org.apache.commons.imaging.formats.icns.IcnsImagingParameters;
+import org.apache.commons.imaging.formats.ico.IcoImagingParameters;
+import org.apache.commons.imaging.formats.jpeg.JpegImagingParameters;
+import org.apache.commons.imaging.formats.pcx.PcxImagingParameters;
+import org.apache.commons.imaging.formats.png.PngImagingParameters;
+import org.apache.commons.imaging.formats.pnm.PnmImagingParameters;
+import org.apache.commons.imaging.formats.psd.PsdImagingParameters;
+import org.apache.commons.imaging.formats.rgbe.RgbeImagingParameters;
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+import org.apache.commons.imaging.formats.wbmp.WbmpImagingParameters;
+import org.apache.commons.imaging.formats.xbm.XbmImagingParameters;
+import org.apache.commons.imaging.formats.xpm.XpmImagingParameters;
+
+import java.util.function.Supplier;
+
/**
* Enum of known image formats.
*/
public enum ImageFormats implements ImageFormat {
- UNKNOWN,
- BMP,
- DCX,
- GIF,
- ICNS,
- ICO,
- JBIG2,
- JPEG,
- PAM,
- PSD,
- PBM,
- PGM,
- PNM,
- PPM,
- PCX,
- PNG,
- RGBE,
- TGA,
- TIFF,
- WBMP,
- XBM,
- XPM;
+ UNKNOWN(null),
+ BMP(BmpImagingParameters::new, "bmp", "dib"),
+ DCX(PcxImagingParameters::new, "dcx"),
+ GIF(GifImagingParameters::new, "gif"),
+ ICNS(IcnsImagingParameters::new, "icns"),
+ ICO(IcoImagingParameters::new, "ico"),
+ JBIG2(null),
+ JPEG(JpegImagingParameters::new, "jpg", "jpeg"),
+ PAM(PnmImagingParameters::new, "pam"),
+ PSD(PsdImagingParameters::new, "psd"),
+ PBM(PnmImagingParameters::new, "pbm"),
+ PGM(PnmImagingParameters::new, "pgm"),
+ PNM(PnmImagingParameters::new, "pnm"),
+ PPM(PnmImagingParameters::new, "ppm"),
+ PCX(PcxImagingParameters::new, "pcx", "pcc"),
+ PNG(PngImagingParameters::new, "png"),
+ RGBE(RgbeImagingParameters::new, "rgbe"),
+ TGA(null),
+ TIFF(TiffImagingParameters::new, "tif", "tiff"),
+ WBMP(WbmpImagingParameters::new, "wbmp"),
+ XBM(XbmImagingParameters::new, "xbm"),
+ XPM(XpmImagingParameters::new, "xpm");
+
+ private final String[] extensions;
+
+ ImageFormats(Supplier<? extends ImagingParameters> factory, String
...extensions) {
+ this.extensions = extensions;
+ }
@Override
public String getName() {
return name();
}
@Override
- public String getExtension() {
- return name();
+ public String[] getExtensions() {
+ return this.extensions.clone();
+ }
+
+ @Override
+ public String getDefaultExtension() {
+ return this.extensions[0];
Review comment:
`ImageFormats.UNKNOWN.getDefaultExtension()` throws index out of bounds.
##########
File path: src/main/java/org/apache/commons/imaging/Imaging.java
##########
@@ -16,9 +16,6 @@
*/
package org.apache.commons.imaging;
-import static org.apache.commons.imaging.ImagingConstants.PARAM_KEY_FILENAME;
-import static org.apache.commons.imaging.ImagingConstants.PARAM_KEY_FORMAT;
Review comment:
Woo hoo!
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormat.java
##########
@@ -33,6 +34,7 @@
*
* @return String extension
*/
- String getExtension();
+ String[] getExtensions();
Review comment:
Use `List<String>` instead of `String[]`? I generally find lists easier
to work with, especially for streams.
##########
File path: src/main/java/org/apache/commons/imaging/ImageFormats.java
##########
@@ -16,40 +16,68 @@
*/
package org.apache.commons.imaging;
+import org.apache.commons.imaging.formats.bmp.BmpImagingParameters;
+import org.apache.commons.imaging.formats.gif.GifImagingParameters;
+import org.apache.commons.imaging.formats.icns.IcnsImagingParameters;
+import org.apache.commons.imaging.formats.ico.IcoImagingParameters;
+import org.apache.commons.imaging.formats.jpeg.JpegImagingParameters;
+import org.apache.commons.imaging.formats.pcx.PcxImagingParameters;
+import org.apache.commons.imaging.formats.png.PngImagingParameters;
+import org.apache.commons.imaging.formats.pnm.PnmImagingParameters;
+import org.apache.commons.imaging.formats.psd.PsdImagingParameters;
+import org.apache.commons.imaging.formats.rgbe.RgbeImagingParameters;
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+import org.apache.commons.imaging.formats.wbmp.WbmpImagingParameters;
+import org.apache.commons.imaging.formats.xbm.XbmImagingParameters;
+import org.apache.commons.imaging.formats.xpm.XpmImagingParameters;
+
+import java.util.function.Supplier;
+
/**
* Enum of known image formats.
*/
public enum ImageFormats implements ImageFormat {
- UNKNOWN,
- BMP,
- DCX,
- GIF,
- ICNS,
- ICO,
- JBIG2,
- JPEG,
- PAM,
- PSD,
- PBM,
- PGM,
- PNM,
- PPM,
- PCX,
- PNG,
- RGBE,
- TGA,
- TIFF,
- WBMP,
- XBM,
- XPM;
+ UNKNOWN(null),
+ BMP(BmpImagingParameters::new, "bmp", "dib"),
Review comment:
These constructor references don't seem to be used anymore. Remove?
##########
File path: src/main/java/org/apache/commons/imaging/Imaging.java
##########
@@ -857,20 +837,19 @@ public static Dimension getImageSize(final File file)
throws ImageReadException,
*
* @param file
* File containing image data.
- * @param params
- * Map of optional parameters, defined in ImagingConstants.
+ * @param params optional parameters.
* @return The width and height of the image.
* @throws ImageReadException if it fails to parse the image
* @throws IOException if it fails to read the image data
*/
- public static Dimension getImageSize(final File file, final Map<String,
Object> params)
+ public static Dimension getImageSize(final File file, final
ImagingParameters params)
Review comment:
Not sure about this API. Callers need to know the type of the image
being read in order to avoid a `ClassCastException`. Possible options:
1. Make it possible to "upgrade" the parameters object internally and copy
the values into an instance of the correct type.
2. Remove this method (and others that accept `ImagingParameters`)
altogether and only allow parameters to be passed using the methods on the
parser. We could make the parsers more easily accessible by making the
`getImageParser` method public.
##########
File path:
src/main/java/org/apache/commons/imaging/formats/jpeg/JpegImagingParameters.java
##########
@@ -0,0 +1,24 @@
+/*
+ * Licensed 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.
+ * under the License.
+ */
+
+package org.apache.commons.imaging.formats.jpeg;
+
+import org.apache.commons.imaging.formats.tiff.TiffImagingParameters;
+
+/**
+ * Jpeg format parameters.
+ * @since 1.0-alpha3
+ */
+public class JpegImagingParameters extends TiffImagingParameters {}
Review comment:
Do all the properties on `TiffImagingParameters` apply to JPEGs? (ex,
`setExif(TiffOutputSet exif)`)
--
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]
