Add support for multiple frames in SkCodec

dm/DMSrcSink.cpp

 /*
  * Copyright 2015 Google Inc.
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "DMSrcSink.h"
 #include "Resources.h"
 #include "SkAndroidCodec.h"
@@ skipping 392 matching lines @@
 
     // Visually inspecting very small output images is not necessary.  We will
     // cover these cases in unit testing.
     if ((size.width() <= 10 || size.height() <= 10) && 1.0f != fScale) {
         return Error::Nonfatal("Scaling very small images is uninteresting.");
     }
     decodeInfo = decodeInfo.makeWH(size.width(), size.height());
 
     const int bpp = SkColorTypeBytesPerPixel(decodeInfo.colorType());
     const size_t rowBytes = size.width() * bpp;
-    SkAutoMalloc pixels(decodeInfo.getSafeSize(rowBytes));
+    const size_t safeSize = decodeInfo.getSafeSize(rowBytes);
+    SkAutoMalloc pixels(safeSize);
     SkPMColor colorPtr[256];
     int colorCount = 256;
 
     SkCodec::Options options;
     if (kCodecZeroInit_Mode == fMode) {
         memset(pixels.get(), 0, size.height() * rowBytes);
         options.fZeroInitialized = SkCodec::kYes_ZeroInitialized;
     }
 
     SkImageInfo bitmapInfo = decodeInfo;
     if (kRGBA_8888_SkColorType == decodeInfo.colorType() ||
             kBGRA_8888_SkColorType == decodeInfo.colorType()) {
         bitmapInfo = bitmapInfo.makeColorType(kN32_SkColorType);
     }
 
     switch (fMode) {
+        case kAnimated_Mode: {
+            std::vector<SkCodec::FrameInfo> frameInfos = codec->getFrameInfo();
+            if (frameInfos.size() <= 1) {
+                return SkStringPrintf("%s is not an animated image.", fPath.c_str());
+            }
+
+            SkAutoCanvasRestore acr(canvas, true);
+            // Used to cache a frame that future frames will depend on.
+            SkAutoMalloc priorFramePixels;
+            size_t cachedFrame = SkCodec::kIndependentFrame;
+            SkCodec::MultiFrameOptions multiOptions;
+            options.fFrameOptions = &multiOptions;
+            for (size_t i = 0; i < frameInfos.size(); i++) {
+                multiOptions.fIndex = i;
+                // Check for a prior frame
+                const size_t reqFrame = frameInfos[i].fRequiredFrame;
+                if (reqFrame != SkCodec::kIndependentFrame && reqFrame == cachedFrame

[cblume, 2016/10/14 16:52:35]

I wonder if we can find a better name than kIndependentFrame.

I understand it is an invalid frame index which tells us that there is no
required frame. But it is *this* frame which is independent, not the required
frame. So this ends up reading in a bit of a strange way.

This might sound silly but I think it would be easier to read as:
if (reqFrame != SkCodec::kInvalidFrameIndex

We could even provide:
bool isIndependentFrame(size_t index) {
    const size_t reqFrame = frameInfos[i].fRequiredFrame;
    return reqFrame == SkCodec::kInvalidFrameIndex;
}

[scroggo_chromium, 2016/10/14 19:55:59]

I understand your confusion, and I'm not opposed to the name kInvalidFrameIndex
(maybe something simpler like kNone? Blink uses kNotFound, which doesn't quite
sound right to me). I've added a FIXME for now, until we agree on a name. reed@?

[cblume, 2016/10/18 21:57:32]

I agree that I'm not a fan of kNotFound. It wasn't "not found". It doesn't
exist.

I'm cool with kNone. That is short and sweet and clear.
if (reqFrame != SkCodec::kNone
easy

[scroggo_chromium, 2016/10/20 17:57:12]

Done.

+                        && priorFramePixels.get()) {
+                    // Copy into pixels
+                    memcpy(pixels.get(), priorFramePixels.get(), safeSize);
+                    multiOptions.fHasPriorFrame = true;
+                } else {
+                    multiOptions.fHasPriorFrame = false;
+                }
+                const SkCodec::Result result = codec->getPixels(decodeInfo, pixels.get(),
+                                                                rowBytes, &options,
+                                                                colorPtr, &colorCount);
+                switch (result) {
+                    case SkCodec::kSuccess:
+                    case SkCodec::kIncompleteInput:
+                        draw_to_canvas(canvas, bitmapInfo, pixels.get(), rowBytes,
+                                       colorPtr, colorCount, fDstColorType);
+                        if (result == SkCodec::kIncompleteInput) {
+                            return "";
+                        }
+                        break;
+                    default:
+                        return SkStringPrintf("Couldn't getPixels for frame %i in %s.",
+                                              i, fPath.c_str());
+                }
+
+                // If a future frame depends on this one, store it in priorFrame.
+                // (Note that if i+1 does *not* depend on i, then no future frame can.)
+                if (i+1 < frameInfos.size() && frameInfos[i+1].fRequiredFrame == i) {
+                    memcpy(priorFramePixels.reset(safeSize), pixels.get(), safeSize);
+                    cachedFrame = i;
+                }

[cblume, 2016/10/14 16:52:35]

I like this way of dealing with DisposeOverwritePrevious.

+
+                canvas->translate(SkIntToScalar(0), SkIntToScalar(decodeInfo.height()));
+            }
+            break;
+        }
         case kCodecZeroInit_Mode:
         case kCodec_Mode: {
             switch (codec->getPixels(decodeInfo, pixels.get(), rowBytes, &options,
                     colorPtr, &colorCount)) {
                 case SkCodec::kSuccess:
                     // We consider incomplete to be valid, since we should still decode what is
                     // available.
                 case SkCodec::kIncompleteInput:
                     break;
                 default:
                     // Everything else is considered a failure.
                     return SkStringPrintf("Couldn't getPixels %s.", fPath.c_str());
             }
 
             draw_to_canvas(canvas, bitmapInfo, pixels.get(), rowBytes, colorPtr, colorCount,
                            fDstColorType);
             break;
         }
         case kScanline_Mode: {
             void* dst = pixels.get();
             uint32_t height = decodeInfo.height();
-            const bool png = fPath.endsWith("png");
+            const bool useIncremental = fPath.endsWith("png") || fPath.endsWith("gif") || fPath.endsWith("GIF");

[msarett, 2016/10/14 14:58:50]

nit: 100 chars
nit: Should we defensively add PNG here as well?

[cblume, 2016/10/14 16:52:35]

or just toLower

[scroggo_chromium, 2016/10/14 19:55:59]

done
done

+            // ico may use the old scanline method or the new one, depending on whether it
+            // internally holds a bmp or a png.
             const bool ico = fPath.endsWith("ico");
-            bool useOldScanlineMethod = !png && !ico;
-            if (png || ico) {
+            bool useOldScanlineMethod = !useIncremental && !ico;
+            if (useIncremental || ico) {
                 if (SkCodec::kSuccess == codec->startIncrementalDecode(decodeInfo, dst,
                         rowBytes, nullptr, colorPtr, &colorCount)) {
                     int rowsDecoded;
                     if (SkCodec::kIncompleteInput == codec->incrementalDecode(&rowsDecoded)) {
                         codec->fillIncompleteImage(decodeInfo, dst, rowBytes,
                                                    SkCodec::kNo_ZeroInitialized, height,
                                                    rowsDecoded);
                     }
                 } else {
-                    if (png) {
-                        // Error: PNG should support incremental decode.
+                    if (useIncremental) {
+                        // Error: These should support incremental decode.
                         return "Could not start incremental decode";
                     }
                     // Otherwise, this is an ICO. Since incremental failed, it must contain a BMP,
                     // which should work via startScanlineDecode
                     useOldScanlineMethod = true;
                 }
             }
 
             if (useOldScanlineMethod) {
                 if (SkCodec::kSuccess != codec->startScanlineDecode(decodeInfo, NULL, colorPtr,
                                                                     &colorCount)) {
                     return "Could not start scanline decoder";
                 }
 
                 switch (codec->getScanlineOrder()) {
                     case SkCodec::kTopDown_SkScanlineOrder:
                     case SkCodec::kBottomUp_SkScanlineOrder:
                         // We do not need to check the return value.  On an incomplete
                         // image, memory will be filled with a default value.
                         codec->getScanlines(dst, height, rowBytes);
                         break;
-                    case SkCodec::kOutOfOrder_SkScanlineOrder: {
-                        for (int y = 0; y < decodeInfo.height(); y++) {
-                            int dstY = codec->outputScanline(y);
-                            void* dstPtr = SkTAddOffset<void>(dst, rowBytes * dstY);
-                            // We complete the loop, even if this call begins to fail
-                            // due to an incomplete image.  This ensures any uninitialized
-                            // memory will be filled with the proper value.
-                            codec->getScanlines(dstPtr, 1, rowBytes);
-                        }
-                        break;
-                    }
                 }
             }
 
             draw_to_canvas(canvas, bitmapInfo, dst, rowBytes, colorPtr, colorCount, fDstColorType);
             break;
         }
         case kStripe_Mode: {
             const int height = decodeInfo.height();
             // This value is chosen arbitrarily.  We exercise more cases by choosing a value that
             // does not align with image blocks.
@@ skipping 145 matching lines @@
     }
     return "";
 }
 
 SkISize CodecSrc::size() const {
     sk_sp<SkData> encoded(SkData::MakeFromFileName(fPath.c_str()));
     SkAutoTDelete<SkCodec> codec(SkCodec::NewFromData(encoded));
     if (nullptr == codec) {
         return SkISize::Make(0, 0);
     }
-    return codec->getScaledDimensions(fScale);
+
+    auto imageSize = codec->getScaledDimensions(fScale);
+    if (fMode == kAnimated_Mode) {
+        // We'll draw one of each frame, so make it big enough to hold them all.
+        const size_t count = codec->getFrameInfo().size();
+        imageSize.fHeight = imageSize.fHeight * count;
+    }
+    return imageSize;
 }
 
 Name CodecSrc::name() const {
     if (1.0f == fScale) {
         return SkOSPath::Basename(fPath.c_str());
     }
     return get_scaled_name(fPath, fScale);
 }
 
 /*~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~*/
@@ skipping 1034 matching lines @@
         Error err = src.draw(&rec);
         if (!err.isEmpty()) {
             return err;
         }
         dl->draw(canvas);
         return check_against_reference(bitmap, src, fSink);
     });
 }
 
 }  // namespace DM