diff --git a/src/main/java/JP2ImageConverter/errors/CommandException.java b/src/main/java/JP2ImageConverter/errors/CommandException.java new file mode 100644 index 0000000..fa3e6a7 --- /dev/null +++ b/src/main/java/JP2ImageConverter/errors/CommandException.java @@ -0,0 +1,53 @@ +package JP2ImageConverter.errors; + +import java.util.List; + +/** + * @author bbpennel + */ +public class CommandException extends RuntimeException { + private static final long serialVersionUID = 1L; + private final int exitCode; + private final String output; + private final List command; + + public CommandException(String message, List command, String output, Throwable cause) { + this(message, command, output, -1, cause); + } + + public CommandException(String message, List command, String output, int exitCode) { + this(message, command, output, exitCode, null); + } + + public CommandException(String message, List command, String output, int exitCode, Throwable cause) { + super(message, cause); + this.exitCode = exitCode; + this.output = output; + this.command = command; + } + + @Override + public String getMessage() { + var message = super.getMessage() + + System.lineSeparator() + "Command: " + String.join(" ", getCommand()); + if (getExitCode() != -1) { + message += System.lineSeparator() + " with exit code: " + getExitCode(); + } + if (getOutput() != null) { + message += System.lineSeparator() + " with output: " + getOutput(); + } + return message; + } + + public int getExitCode() { + return exitCode; + } + + public String getOutput() { + return output; + } + + public List getCommand() { + return command; + } +} diff --git a/src/main/java/JP2ImageConverter/errors/CommandTimeoutException.java b/src/main/java/JP2ImageConverter/errors/CommandTimeoutException.java new file mode 100644 index 0000000..a3bb1c4 --- /dev/null +++ b/src/main/java/JP2ImageConverter/errors/CommandTimeoutException.java @@ -0,0 +1,14 @@ +package JP2ImageConverter.errors; + +import java.util.List; + +/** + * @author bbpennel + */ +public class CommandTimeoutException extends CommandException { + private static final long serialVersionUID = 1L; + + public CommandTimeoutException(String message, List command, String output) { + super(message, command, output, -1); + } +} diff --git a/src/main/java/JP2ImageConverter/services/ColorFieldsService.java b/src/main/java/JP2ImageConverter/services/ColorFieldsService.java index 08548c0..621a11f 100644 --- a/src/main/java/JP2ImageConverter/services/ColorFieldsService.java +++ b/src/main/java/JP2ImageConverter/services/ColorFieldsService.java @@ -1,5 +1,6 @@ package JP2ImageConverter.services; +import JP2ImageConverter.errors.CommandException; import JP2ImageConverter.util.CommandUtility; import com.drew.imaging.ImageMetadataReader; import com.drew.imaging.ImageProcessingException; @@ -180,11 +181,11 @@ public String identifyType(String fileName) { try { colorspace = CommandUtility.executeCommand(command); - } catch (Exception e) { + } catch (CommandException e) { log.warn("Colorspace not identified: {}", e.getMessage()); } - return colorspace; + return colorspace != null ? colorspace.trim() : null; } /** diff --git a/src/main/java/JP2ImageConverter/services/ImagePreproccessingService.java b/src/main/java/JP2ImageConverter/services/ImagePreproccessingService.java index d1acc52..c903a07 100644 --- a/src/main/java/JP2ImageConverter/services/ImagePreproccessingService.java +++ b/src/main/java/JP2ImageConverter/services/ImagePreproccessingService.java @@ -203,6 +203,21 @@ public String convertRw2(String fileName) throws Exception { return temporaryFile; } + /** + * Removes the alpha channel from the provided image + * @param fileName + * @return + * @throws Exception + */ + public String removeAlphaChannel(String fileName) throws Exception { + String temporaryFile = String.valueOf(prepareTempPath(fileName, ".tif")); + + List command = Arrays.asList(CONVERT, AUTO_ORIENT, "-alpha", "off", fileName, temporaryFile); + CommandUtility.executeCommand(command); + + return temporaryFile; + } + /** * Determine image format and preprocess if needed * for non-TIFF image formats: convert to temporary TIFF/PPM before kdu_compress diff --git a/src/main/java/JP2ImageConverter/services/KakaduService.java b/src/main/java/JP2ImageConverter/services/KakaduService.java index aa5f485..b4abf1d 100644 --- a/src/main/java/JP2ImageConverter/services/KakaduService.java +++ b/src/main/java/JP2ImageConverter/services/KakaduService.java @@ -1,5 +1,6 @@ package JP2ImageConverter.services; +import JP2ImageConverter.errors.CommandException; import JP2ImageConverter.util.CommandUtility; import org.apache.commons.io.FilenameUtils; import org.slf4j.Logger; @@ -16,7 +17,9 @@ import java.util.Iterator; import java.util.List; import java.util.Map; +import java.util.Objects; +import static JP2ImageConverter.services.ColorFieldsService.PHOTOMETRIC_INTERPRETATION; import static org.slf4j.LoggerFactory.getLogger; /** @@ -26,6 +29,9 @@ */ public class KakaduService { private static final Logger log = getLogger(KakaduService.class); + public static final String COLOR_SPACE = "ColorSpace"; + public static final String COLOR_TYPE = "Type"; + private final static Map SOURCE_FORMATS = new HashMap<>(); // accepted file types are listed in sourceFormats below static { @@ -78,7 +84,7 @@ public class KakaduService { * @param originalImage the original input image * @return colorSpace */ - public String getColorSpace(Map preprocessedImageMetadata, + public Map getColorInfo(Map preprocessedImageMetadata, Map originalImageMetadata, String originalImage) { String colorSpace; @@ -92,13 +98,13 @@ public String getColorSpace(Map preprocessedImageMetadata, colorSpace = "Gray"; } else if (preprocessedImageMetadata.get(ColorFieldsService.COLOR_SPACE) != null) { colorSpace = preprocessedImageMetadata.get(ColorFieldsService.COLOR_SPACE); - } else if (preprocessedImageMetadata.get(ColorFieldsService.PHOTOMETRIC_INTERPRETATION) != null) { - colorSpace = preprocessedImageMetadata.get(ColorFieldsService.PHOTOMETRIC_INTERPRETATION); + } else if (preprocessedImageMetadata.get(PHOTOMETRIC_INTERPRETATION) != null) { + colorSpace = preprocessedImageMetadata.get(PHOTOMETRIC_INTERPRETATION); } else if (originalImageMetadata.get(ColorFieldsService.COLOR_SPACE) != null) { colorSpace = originalImageMetadata.get(ColorFieldsService.COLOR_SPACE); - } else if (originalImageMetadata.get(ColorFieldsService.PHOTOMETRIC_INTERPRETATION) != null && - !originalImageMetadata.get(ColorFieldsService.PHOTOMETRIC_INTERPRETATION).equalsIgnoreCase("ycbcr")) { - colorSpace = originalImageMetadata.get(ColorFieldsService.PHOTOMETRIC_INTERPRETATION); + } else if (originalImageMetadata.get(PHOTOMETRIC_INTERPRETATION) != null && + !originalImageMetadata.get(PHOTOMETRIC_INTERPRETATION).equalsIgnoreCase("ycbcr")) { + colorSpace = originalImageMetadata.get(PHOTOMETRIC_INTERPRETATION); } else { colorSpace = "sRGB"; } @@ -114,7 +120,7 @@ public String getColorSpace(Map preprocessedImageMetadata, colorSpace = "aToB0"; } - return colorSpace; + return Map.of(COLOR_SPACE, colorSpace, COLOR_TYPE, imageType == null? "" : imageType); } /** @@ -194,9 +200,9 @@ public void kduCompress(String sourceFileName, Path outputPath, String sourceFor if (!fileName.equals(inputFile)) { preprocessedImageMetadata = extractMetadata(inputFile, ""); } - String colorSpace = getColorSpace(preprocessedImageMetadata, originalImageMetadata, fileName); - String orientation = originalImageMetadata.get(ColorFieldsService.ORIENTATION); - inputFile = correctInputImage(inputFile, fileName, sourceFormat, colorSpace, orientation, intermediateFiles); + var colorInfo = getColorInfo(preprocessedImageMetadata, originalImageMetadata, fileName); + var colorSpace = colorInfo.get(COLOR_SPACE); + inputFile = correctInputImage(inputFile, fileName, sourceFormat, colorInfo, preprocessedImageMetadata, intermediateFiles); List command = new ArrayList<>(Arrays.asList(kduCompress, input, inputFile, output, outputFile, clevels, clayers, cprecincts, stiles, corder, orggenplt, orgtparts, cblk, cusesop, cuseeph, @@ -230,11 +236,11 @@ private void performKakaduCommandWithRecovery(List command, List try { log.debug("Performing kakadu command: {}", command); CommandUtility.executeCommand(command); - } catch (Exception e) { - var message = e.getMessage(); + } catch (CommandException e) { + var output = e.getOutput(); if (retry) { - if (message.contains("ICC profile") && message.contains("reproduction curve appears to have been truncated")) { - log.warn("Invalid ICC profile, retrying without ICC profile: {}", message); + if (output.contains("ICC profile") && output.contains("reproduction curve appears to have been truncated")) { + log.warn("Invalid ICC profile, retrying without ICC profile: {}", e.getMessage()); var inputIndex = command.indexOf("-i") + 1; var modifiedTmpPath = imagePreproccessingService.handleIccProfile(command.get(inputIndex)); command.set(inputIndex, modifiedTmpPath); @@ -269,29 +275,39 @@ private String getSourceFormat(String fileName, String sourceFormat) { * @param fileName * @param sourceFormat * @param colorSpace - * @param orientation + * @param metadata extracted metadata from the source image * @param intermediateFiles * @return * @throws Exception */ - private String correctInputImage(String inputFile, String fileName, String sourceFormat, String colorSpace, - String orientation, List intermediateFiles) throws Exception { + private String correctInputImage(String inputFile, String fileName, String sourceFormat, Map colorInfo, + Map metadata, List intermediateFiles) throws Exception { var fileBeforeColorConversion = inputFile; + var orientation = metadata.get(ColorFieldsService.ORIENTATION); // for unusual color spaces (CMYK and YcbCr): convert to temporary TIFF before kduCompress - inputFile = imagePreproccessingService.convertColorSpaces(colorSpace, inputFile); - if (fileBeforeColorConversion.equals(inputFile)) { - // Create a temporary TIFF with the correct orientation if no color space conversion was done - // and the orientation is different from the default. - var format = sourceFormat != null && !sourceFormat.isEmpty() ? - sourceFormat : SOURCE_FORMATS.get(FilenameUtils.getExtension(fileName)); - if (orientation != null && format != null && format.equals("tiff") - && !ColorFieldsService.ORIENTATION_DEFAULT.equals(orientation)) { - inputFile = imagePreproccessingService.correctOrientation(fileName); - intermediateFiles.add(inputFile); - } - } else { + inputFile = imagePreproccessingService.convertColorSpaces(colorInfo.get(COLOR_SPACE), inputFile); + if (!fileBeforeColorConversion.equals(inputFile)) { + intermediateFiles.add(inputFile); + return inputFile; + } + // Strip alpha channel from grayscale images incorrectly identified as sRGB + if ((Objects.equals(metadata.get(PHOTOMETRIC_INTERPRETATION), "RGB") + || Objects.equals(metadata.get(ColorFieldsService.COLOR_SPACE), "RGB")) + && colorInfo.get(COLOR_TYPE).contains("GrayscaleAlpha")) { + inputFile = imagePreproccessingService.removeAlphaChannel(inputFile); + intermediateFiles.add(inputFile); + return inputFile; + } + // Create a temporary TIFF with the correct orientation if no color space conversion was done + // and the orientation is different from the default. + var format = sourceFormat != null && !sourceFormat.isEmpty() ? + sourceFormat : SOURCE_FORMATS.get(FilenameUtils.getExtension(fileName)); + if (orientation != null && format != null && format.equals("tiff") + && !ColorFieldsService.ORIENTATION_DEFAULT.equals(orientation)) { + inputFile = imagePreproccessingService.correctOrientation(fileName); intermediateFiles.add(inputFile); } + return inputFile; } @@ -322,6 +338,7 @@ public void fileListKduCompress(String fileName, Path outputPath, String sourceF public void deleteTinyGrayVoidImages(String outputFile) throws Exception { File output = new File(outputFile); if (output.length() < 10000 && colorFieldsService.identifyType(outputFile).contains("Gray")) { + log.warn("Deleting tiny gray image: {}", outputFile); Files.deleteIfExists(Path.of(outputFile)); } } diff --git a/src/main/java/JP2ImageConverter/util/CommandUtility.java b/src/main/java/JP2ImageConverter/util/CommandUtility.java index 12e075a..1a055fd 100644 --- a/src/main/java/JP2ImageConverter/util/CommandUtility.java +++ b/src/main/java/JP2ImageConverter/util/CommandUtility.java @@ -1,16 +1,28 @@ package JP2ImageConverter.util; +import JP2ImageConverter.errors.CommandException; +import JP2ImageConverter.errors.CommandTimeoutException; +import org.slf4j.Logger; + import java.io.BufferedReader; import java.io.File; +import java.io.IOException; import java.io.InputStream; import java.io.InputStreamReader; import java.util.List; +import java.util.concurrent.CompletableFuture; +import java.util.concurrent.TimeUnit; + +import static org.slf4j.LoggerFactory.getLogger; /** * Utility for executing commands * @author krwong */ public class CommandUtility { + private static final Logger log = getLogger(CommandUtility.class); + private static final int MAX_TIMEOUT_SECONDS = System.getProperty("jp24u.subcommand.timeout") != null ? + Integer.parseInt(System.getProperty("jp24u.subcommand.timeout")) : 60 * 5; private CommandUtility() { } @@ -20,41 +32,78 @@ private CommandUtility() { * @param command the command to be executed * @return command output */ - public static String executeCommand(List command) throws Exception { + public static String executeCommand(List command) { StringBuilder output = new StringBuilder(); try { ProcessBuilder builder = new ProcessBuilder(command); builder.redirectErrorStream(true); Process process = builder.start(); - InputStream is = process.getInputStream(); - BufferedReader br = new BufferedReader(new InputStreamReader(is)); - String line; - while ((line = br.readLine()) != null) { - output.append(line).append(System.lineSeparator()); - } - if (process.waitFor() != 0) { - throw new Exception("Command exited with status code " + process.waitFor() + ": " + output); + // Use a separate thread to read the output concurrently, to avoid deadlock in case command times out + var outputReaderTask = getOutputReaderTask(process.getInputStream(), output); + + waitForProcess(process, command, outputReaderTask, output); + // If any errors occurred while reading the output, they will be thrown here + outputReaderTask.join(); + if (process.exitValue() != 0) { + throw new CommandException("Command exited with errors", command, output.toString(), process.exitValue()); } - } catch (Exception e) { - throw new Exception("Command failed: " + command + System.lineSeparator() + "Output: " + output, e); + } catch (InterruptedException | IOException e) { + throw new CommandException("Command failed to execute", command, output.toString(), e); } return output.toString(); } - public static void executeCommandWriteToFile(List command, String temporaryFile) throws Exception { + /** + * Run a given command and write the output to a file + * @param command + * @param temporaryFile + */ + public static void executeCommandWriteToFile(List command, String temporaryFile) { + StringBuilder errorOutput = new StringBuilder(); try { ProcessBuilder builder = new ProcessBuilder(command); - builder.redirectErrorStream(true); builder.redirectOutput(new File(temporaryFile)); Process process = builder.start(); - if (process.waitFor() != 0) { - throw new Exception("Command exited with status code " + process.waitFor()); + // Use a separate thread to read the errors concurrently, to keep it separate from the main file output + var outputReaderTask = getOutputReaderTask(process.getErrorStream(), errorOutput); + + waitForProcess(process, command, outputReaderTask, errorOutput); + if (process.exitValue() != 0) { + throw new CommandException("Command failed", command, errorOutput.toString(), process.exitValue()); + } + } catch (InterruptedException | IOException e) { + throw new CommandException("Command failed to execute", command, errorOutput.toString(), e); + } + } + + private static CompletableFuture getOutputReaderTask(InputStream inputStream, StringBuilder output) { + return CompletableFuture.runAsync(() -> { + try (BufferedReader br = new BufferedReader(new InputStreamReader(inputStream))) { + String line; + while ((line = br.readLine()) != null) { + output.append(line).append(System.lineSeparator()); + } + } catch (IOException e) { + throw new CommandException("Error reading command output", null, output.toString(), e); + } + }); + } + + private static void waitForProcess(Process process, List command, CompletableFuture outputFuture, + StringBuilder output) + throws InterruptedException { + log.debug("Waiting for process for {} seconds: {}", MAX_TIMEOUT_SECONDS, command); + if (!process.waitFor(MAX_TIMEOUT_SECONDS, TimeUnit.SECONDS)) { + log.warn("Command timed out, attempting to end process: {}", command); + process.destroy(); + if (outputFuture != null) { + outputFuture.join(); } - } catch (Exception e) { - throw new Exception("Command failed: " + command, e); + throw new CommandTimeoutException("Command timed out after " + MAX_TIMEOUT_SECONDS + " seconds", + command, output.toString()); } } } diff --git a/src/test/java/JP2ImageConverter/services/ImagePreprocessingServiceTest.java b/src/test/java/JP2ImageConverter/services/ImagePreprocessingServiceTest.java index f430d75..12e744c 100644 --- a/src/test/java/JP2ImageConverter/services/ImagePreprocessingServiceTest.java +++ b/src/test/java/JP2ImageConverter/services/ImagePreprocessingServiceTest.java @@ -67,7 +67,7 @@ public void testConvertJpegtoPpm() throws Exception { @Test public void testConvertRw2ToPpm() throws Exception { - String testFile = "src/test/resources/test.rw2"; + String testFile = "src/test/resources/test.RW2"; var tempPpm = service.convertRw2(testFile); diff --git a/src/test/java/JP2ImageConverter/services/KakaduServiceTest.java b/src/test/java/JP2ImageConverter/services/KakaduServiceTest.java index 218568b..b29cff2 100644 --- a/src/test/java/JP2ImageConverter/services/KakaduServiceTest.java +++ b/src/test/java/JP2ImageConverter/services/KakaduServiceTest.java @@ -38,12 +38,13 @@ public void setup() throws Exception { } @Test - public void testRetrieveColorSpace() throws Exception { + public void testRetrieveColorInfo() throws Exception { // EXIF ColorSpace is null, EXIF PhotometricInterpretation is gray String testFile = "src/test/resources/P0024_0066.tif"; var originalImageMetadata = service.extractMetadata(testFile, "tiff"); - String colorSpace = service.getColorSpace(originalImageMetadata, originalImageMetadata, testFile); - assertEquals("Gray", colorSpace); + var info = service.getColorInfo(originalImageMetadata, originalImageMetadata, testFile); + assertEquals("Gray", info.get(KakaduService.COLOR_SPACE)); + assertEquals("Grayscale", info.get(KakaduService.COLOR_TYPE)); } @Test @@ -146,6 +147,15 @@ public void testKduCompressTifWithInvalidIccProfile() throws Exception { assertEquals(1, Files.list(tmpFolder).count()); } + @Test + public void testKduCompressTifWithSrgbAndTypeGray() throws Exception { + String testFile = "src/test/resources/obama_smoking.tiff"; + service.kduCompress(testFile, Paths.get(tmpFolder + "/obama_smoking"), ""); + + assertTrue(Files.exists(tmpFolder.resolve("obama_smoking.jp2"))); + assertEquals(1, Files.list(tmpFolder).count()); + } + @Test public void testKduCompressNef() throws Exception { String testFile = "src/test/resources/20170822_068.NEF"; diff --git a/src/test/resources/obama_smoking.tiff b/src/test/resources/obama_smoking.tiff new file mode 100644 index 0000000..c6d8fe2 Binary files /dev/null and b/src/test/resources/obama_smoking.tiff differ