You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@shindig.apache.org by lr...@apache.org on 2009/02/21 01:13:46 UTC

svn commit: r746420 - in /incubator/shindig/trunk/java/gadgets/src: main/java/org/apache/shindig/gadgets/rewrite/image/ test/java/org/apache/shindig/gadgets/rewrite/image/ test/resources/org/apache/shindig/gadgets/rewrite/image/

Author: lryan
Date: Sat Feb 21 00:13:45 2009
New Revision: 746420

URL: http://svn.apache.org/viewvc?rev=746420&view=rev
Log:
Fix various issues with image rewriting.
- Add more robust check for animated GIFs. Re-enabled test
- Add support for GIF serialization after palette recomputation using Sanselan
- Don't trust GIF palettes to correctly specify opacity.

Code is getting a little unwieldly so a refactoring is forthcoming but I wanted to address immediate issues

Modified:
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BMPOptimizer.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BaseOptimizer.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BasicImageRewriter.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizer.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JPEGOptimizer.java
    incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/PNGOptimizer.java
    incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizerTest.java
    incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/ImageRewriterTest.java
    incubator/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/large.gif

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BMPOptimizer.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BMPOptimizer.java?rev=746420&r1=746419&r2=746420&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BMPOptimizer.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BMPOptimizer.java Sat Feb 21 00:13:45 2009
@@ -23,6 +23,7 @@
 import java.io.IOException;
 
 import javax.imageio.ImageIO;
+import javax.imageio.ImageWriter;
 
 /**
  * Optimize BMP by converting to PNG
@@ -32,10 +33,11 @@
   public BMPOptimizer(OptimizerConfig config, HttpResponse original)
       throws IOException {
     super(config, original);
+    ImageWriter writer = ImageIO.getImageWritersByFormatName("png").next();
+    outputter = new ImageIOOutputter(writer, null);
   }
 
   protected void rewriteImpl(BufferedImage image) throws IOException {
-    writer = ImageIO.getImageWritersByFormatName("png").next();
     super.rewriteImpl(image);
   }
 

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BaseOptimizer.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BaseOptimizer.java?rev=746420&r1=746419&r2=746420&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BaseOptimizer.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BaseOptimizer.java Sat Feb 21 00:13:45 2009
@@ -17,14 +17,21 @@
  */
 package org.apache.shindig.gadgets.rewrite.image;
 
+import org.apache.sanselan.ImageFormat;
+import org.apache.sanselan.ImageWriteException;
+import org.apache.sanselan.Sanselan;
 import org.apache.shindig.gadgets.http.HttpResponse;
 import org.apache.shindig.gadgets.http.HttpResponseBuilder;
 
+import com.google.common.collect.ImmutableMap;
+import com.google.common.collect.Maps;
+
 import java.awt.image.BufferedImage;
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.util.Collections;
 import java.util.Iterator;
+import java.util.Map;
 
 import javax.imageio.IIOImage;
 import javax.imageio.ImageIO;
@@ -37,53 +44,53 @@
  * Base class for image optimizers
  */
 abstract class BaseOptimizer {
+
+  static final Map<String, ImageFormat> formatNameToImageFormat = ImmutableMap.of(
+      "png", ImageFormat.IMAGE_FORMAT_PNG,
+      "gif", ImageFormat.IMAGE_FORMAT_GIF,
+      "jpeg", ImageFormat.IMAGE_FORMAT_JPEG);
+
   final HttpResponse originalResponse;
   final OptimizerConfig config;
 
-  ImageWriter writer;
-
-  private ByteArrayOutputStream baos;
+  protected ImageOutputter outputter;
   protected byte[] minBytes;
   protected int minLength;
   int reductionPct;
 
+
   public BaseOptimizer(OptimizerConfig config, HttpResponse original)
       throws IOException {
     this.config = config;
-    Iterator<ImageWriter> writers = ImageIO.getImageWritersByFormatName(getOriginalFormatName());
-    if (writers.hasNext()) {
-      this.writer = writers.next();
-    }
     this.originalResponse = original;
     this.minLength = original.getContentLength();
+    this.outputter = getOutputter();
   }
 
-  /**
-   * Write the image using its default write parameters
-   */
-  protected void write(BufferedImage image) throws IOException {
-    write(image, null);
+  protected ImageOutputter getOutputter() {
+    Iterator<ImageWriter> writers = ImageIO.getImageWritersByFormatName(getOriginalFormatName());
+    if (writers.hasNext()) {
+      ImageWriter writer = writers.next();
+      ImageWriteParam param = writer.getDefaultWriteParam();
+      if (getOriginalFormatName().equals("jpeg")) {
+        param.setCompressionMode(ImageWriteParam.MODE_EXPLICIT);
+        param.setCompressionQuality(config.getJpegCompression());
+      }
+      return new ImageIOOutputter(writer, param);
+    }
+    return new SanselanOutputter(formatNameToImageFormat.get(getOriginalFormatName()));
   }
 
   /**
    * Write the image using a specified write param
    */
-  protected void write(BufferedImage image, ImageWriteParam writeParam) throws IOException {
+  protected void write(BufferedImage image) throws IOException {
     if (image == null) {
       return;
     }
-    ByteArrayOutputStream stream = getOutputStream();
-    writer.setOutput(ImageIO.createImageOutputStream(stream));
-    if (writeParam == null) {
-      writeParam = writer.getDefaultWriteParam();
-    }
-    // Create a new empty metadata set
-    IIOMetadata metadata = writer.getDefaultImageMetadata(
-        new ImageTypeSpecifier(image.getColorModel(), image.getSampleModel()),
-        writeParam);
-    writer.write(new IIOImage(image, Collections.<BufferedImage>emptyList(), metadata));
-    if (minLength > stream.size()) {
-      minBytes = stream.toByteArray();
+    byte[] bytes = outputter.toBytes(image);
+    if (minLength > bytes.length) {
+      minBytes = bytes;
       minLength = minBytes.length;
       reductionPct = ((originalResponse.getContentLength() - minLength) * 100) /
           originalResponse.getContentLength();
@@ -91,7 +98,7 @@
   }
 
   public HttpResponse rewrite(BufferedImage image) throws IOException {
-    if (writer == null) {
+    if (outputter == null) {
       return originalResponse;
     }
 
@@ -114,15 +121,6 @@
     return originalResponse;
   }
 
-  // Lazily initialize stream
-  private ByteArrayOutputStream getOutputStream() {
-    if (baos == null) {
-      baos = new ByteArrayOutputStream(originalResponse.getContentLength());
-    }
-    baos.reset();
-    return baos;
-  }
-
   /**
    * Get the rewritten image if available
    * @return
@@ -138,4 +136,70 @@
   protected abstract String getOriginalContentType();
 
   protected abstract String getOriginalFormatName();
+
+  /**
+   * Interface to allow for different serialization libraries to be used
+   */
+  public static interface ImageOutputter {
+    byte[] toBytes(BufferedImage image) throws IOException;
+  }
+
+  /**
+   * Standard ImageIO based image outputter
+   */
+  public static class ImageIOOutputter implements ImageOutputter {
+
+    ImageWriter writer;
+    ByteArrayOutputStream baos;
+    ImageWriteParam writeParam;
+    public ImageIOOutputter(ImageWriter writer, ImageWriteParam writeParam) {
+      this.writer = writer;
+      if (writeParam == null) {
+        this.writeParam = writer.getDefaultWriteParam();
+      } else {
+        this.writeParam = writeParam;
+      }
+    }
+
+    public byte[] toBytes(BufferedImage image) throws IOException {
+      if (baos == null) {
+        baos = new ByteArrayOutputStream();
+      } else {
+        baos.reset();
+      }
+      writer.setOutput(ImageIO.createImageOutputStream(baos));
+      // Create a new empty metadata set
+      IIOMetadata metadata = writer.getDefaultImageMetadata(
+          new ImageTypeSpecifier(image.getColorModel(), image.getSampleModel()),
+          writeParam);
+      writer.write(new IIOImage(image, Collections.<BufferedImage>emptyList(), metadata));
+      return baos.toByteArray();
+    }
+  }
+
+  /**
+   * Sanselan based image outputter
+   */
+  public static class SanselanOutputter implements ImageOutputter {
+
+    ImageFormat format;
+    ByteArrayOutputStream baos;
+    public SanselanOutputter(ImageFormat format) {
+      this.format = format;
+    }
+
+    public byte[] toBytes(BufferedImage image) throws IOException {
+      if (baos == null) {
+        baos = new ByteArrayOutputStream();
+      } else {
+        baos.reset();
+      }
+      try {
+        Sanselan.writeImage(image, baos, format, Maps.newHashMap());
+        return baos.toByteArray();
+      } catch (ImageWriteException iwe) {
+        throw new IOException(iwe.getMessage());
+      }
+    }
+  }
 }

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BasicImageRewriter.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BasicImageRewriter.java?rev=746420&r1=746419&r2=746420&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BasicImageRewriter.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/BasicImageRewriter.java Sat Feb 21 00:13:45 2009
@@ -94,6 +94,8 @@
       ImageInfo imageInfo = Sanselan.getImageInfo(response.getResponse(), uri.getPath());
 
       // Dont handle animations
+      // TODO: This doesnt work as current Sanselan doesnt accurately return image counts
+      // See animated gif detection below
       if (imageInfo.getNumberOfImages() > 1) {
         return response;
       }
@@ -111,8 +113,13 @@
       int originalBytes = response.getContentLength();
       originalImageBytes.addAndGet(originalBytes);
       if (imageFormat == ImageFormat.IMAGE_FORMAT_GIF) {
-        response = new GIFOptimizer(config, response)
-            .rewrite(Sanselan.getBufferedImage(response.getResponse()));
+        // Detecting the existence of the NETSCAPE2.0 extension by string comparison
+        // is not exactly clean but is good enough to determine if a GIF is animated
+        // Remove once Sanselan returns image count
+        if (!response.getResponseAsString().contains("NETSCAPE2.0")) {
+          response = new GIFOptimizer(config, response).rewrite(
+              Sanselan.getBufferedImage(response.getResponse()));
+        }
       } else if (imageFormat == ImageFormat.IMAGE_FORMAT_PNG) {
         response = new PNGOptimizer(config, response)
             .rewrite(Sanselan.getBufferedImage(response.getResponse()));

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizer.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizer.java?rev=746420&r1=746419&r2=746420&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizer.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizer.java Sat Feb 21 00:13:45 2009
@@ -19,8 +19,8 @@
 
 import org.apache.shindig.gadgets.http.HttpResponse;
 
-import java.awt.Transparency;
 import java.awt.image.BufferedImage;
+import java.awt.image.IndexColorModel;
 import java.io.IOException;
 
 import javax.imageio.ImageIO;
@@ -38,16 +38,15 @@
   }
 
   protected void rewriteImpl(BufferedImage image) throws IOException {
-    if (image.getColorModel().getTransparency() != Transparency.OPAQUE) {
+    if (!ImageUtils.isOpaque(image)) {
       // We can rewrite tranparent GIFs to PNG but for IE6 it requires the use of
       // the AlphaImageReader and some pain. Deferring this until that is proven to work
-      // Opacity check is valid as GIF always produces IndexColorModel
 
-      // Re-palettize and write to stip metadata
-      write(ImageUtils.palettize(image, config.getMaxPaletteSize()));
+      // Write to stip any metadata and re-compute the palette
+      write(ImageUtils.palettize(image, ((IndexColorModel)image.getColorModel()).getMapSize()));
     } else {
       usePng = true;
-      writer = ImageIO.getImageWritersByFormatName("png").next();
+      outputter = new ImageIOOutputter(ImageIO.getImageWritersByFormatName("png").next(), null);
       super.rewriteImpl(image);
     }
   }
@@ -56,4 +55,17 @@
   protected String getOriginalContentType() {
     return "image/gif";
   }
+
+  @Override
+  protected String getOutputContentType() {
+    if (usePng) {
+      return super.getOutputContentType();
+    }
+    return "image/gif";
+  }
+
+  @Override
+  protected String getOriginalFormatName() {
+    return "gif";
+  }
 }

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JPEGOptimizer.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JPEGOptimizer.java?rev=746420&r1=746419&r2=746420&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JPEGOptimizer.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/JPEGOptimizer.java Sat Feb 21 00:13:45 2009
@@ -23,8 +23,6 @@
 import java.awt.image.BufferedImage;
 import java.io.IOException;
 
-import javax.imageio.ImageWriteParam;
-
 /**
  * Optimize JPEG images by either converting them to PNGs or re-encoding them with a more
  * appropriate compression level.
@@ -57,10 +55,7 @@
     }
 
     // Write as standard JPEG using the configured default compression level
-    ImageWriteParam param = writer.getDefaultWriteParam();
-    param.setCompressionMode(ImageWriteParam.MODE_EXPLICIT);
-    param.setCompressionQuality(config.getJpegCompression());
-    write(image, param);
+    write(image);
 
     // JPEG did not beat PNG
     if (pngLength == minLength) {

Modified: incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/PNGOptimizer.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/PNGOptimizer.java?rev=746420&r1=746419&r2=746420&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/PNGOptimizer.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/main/java/org/apache/shindig/gadgets/rewrite/image/PNGOptimizer.java Sat Feb 21 00:13:45 2009
@@ -21,11 +21,11 @@
 
 import java.awt.image.BufferedImage;
 import java.awt.image.ColorModel;
-import java.awt.image.IndexColorModel;
 import java.io.IOException;
 
 import javax.imageio.ImageIO;
 import javax.imageio.ImageWriteParam;
+import javax.imageio.ImageWriter;
 
 /**
  * Optimize a PNG image and possibly convert it to a JPEG.
@@ -65,9 +65,6 @@
       boolean isOpaque;
       if (palettized != null){
         bufferedImage = palettized;
-      }
-      if (bufferedImage.getColorModel() instanceof IndexColorModel) {
-        // Only IndexColorModel faithfully reports transparency
         isOpaque = bufferedImage.getColorModel().getTransparency() == ColorModel.OPAQUE;
       } else {
         isOpaque = ImageUtils.isOpaque(bufferedImage);
@@ -76,11 +73,12 @@
       if (isOpaque) {
         byte[] lastBytes = minBytes;
         int prevReductionPct = reductionPct;
-        writer = ImageIO.getImageWritersByFormatName("jpeg").next();
+        ImageWriter writer = ImageIO.getImageWritersByFormatName("jpeg").next();
         ImageWriteParam param = writer.getDefaultWriteParam();
         param.setCompressionMode(ImageWriteParam.MODE_EXPLICIT);
         param.setCompressionQuality(config.getJpegCompression());
-        write(bufferedImage, param);
+        outputter = new ImageIOOutputter(writer, param);
+        write(bufferedImage);
         // Only use JPEG if it offers a significant reduction over other methods
         if (reductionPct > prevReductionPct + 20) {
           useJpeg = true;

Modified: incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizerTest.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizerTest.java?rev=746420&r1=746419&r2=746420&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizerTest.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/GIFOptimizerTest.java Sat Feb 21 00:13:45 2009
@@ -35,7 +35,12 @@
     assertSame(resp, httpResponse);
   }
 
-  public void testRewriteLargeGIFToPNG() throws Exception {
+  /**
+   * This is a GIF image with an palette that contains transparent entries but
+   * that has not pixels that map to them so it is Opaque.
+   * @throws Exception
+   */
+  public void testBadPaletteGIFToPNG() throws Exception {
     HttpResponse resp =
         createResponse("org/apache/shindig/gadgets/rewrite/image/large.gif", "image/gif");
     HttpResponse httpResponse = rewrite(resp);

Modified: incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/ImageRewriterTest.java
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/ImageRewriterTest.java?rev=746420&r1=746419&r2=746420&view=diff
==============================================================================
--- incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/ImageRewriterTest.java (original)
+++ incubator/shindig/trunk/java/gadgets/src/test/java/org/apache/shindig/gadgets/rewrite/image/ImageRewriterTest.java Sat Feb 21 00:13:45 2009
@@ -21,12 +21,13 @@
 import org.apache.shindig.common.uri.Uri;
 import org.apache.shindig.gadgets.http.HttpResponse;
 import org.apache.shindig.gadgets.http.HttpResponseBuilder;
-import static junit.framework.Assert.*;
 
-import static junit.framework.Assert.*;
+import static junit.framework.Assert.assertEquals;
+import static junit.framework.Assert.assertSame;
+import static junit.framework.Assert.assertTrue;
+
 import org.junit.Before;
 import org.junit.Test;
-import org.junit.Ignore;
 
 /**
  * Tests for ImageRewriter
@@ -96,7 +97,6 @@
   }
 
   @Test
-  @Ignore("This test is known failing with the current sanselan codebase as of Feb 20, 2009")
   public void testNoRewriteAnimatedGIF() throws Exception {
     byte[] bytes = IOUtils.toByteArray(getClass().getClassLoader().getResourceAsStream(
         "org/apache/shindig/gadgets/rewrite/image/animated.gif"));
@@ -108,5 +108,17 @@
     assertSame(rewriter.rewrite(Uri.parse("animated.gif"), original), original);
   }
 
+  @Test
+  public void testRewriteUnAnimatedGIF() throws Exception {
+    byte[] bytes = IOUtils.toByteArray(getClass().getClassLoader().getResourceAsStream(
+        "org/apache/shindig/gadgets/rewrite/image/large.gif"));
+    HttpResponse original = new HttpResponseBuilder()
+        .setHeader("Content-Type", "image/gif")
+        .setHttpStatusCode(HttpResponse.SC_OK)
+        .setResponse(bytes)
+        .create();
+    assertEquals(rewriter.rewrite(Uri.parse("large.gif"), original).getHeader("Content-Type"),
+        "image/png");
+  }
 }
  

Modified: incubator/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/large.gif
URL: http://svn.apache.org/viewvc/incubator/shindig/trunk/java/gadgets/src/test/resources/org/apache/shindig/gadgets/rewrite/image/large.gif?rev=746420&r1=746419&r2=746420&view=diff
==============================================================================
Binary files - no diff available.