Cap memory usage in webp catch up

third_party/WebKit/Source/platform/image-decoders/webp/WEBPImageDecoder.cpp

 /*
  * Copyright (C) 2010 Google Inc. All rights reserved.
  *
  * Redistribution and use in source and binary forms, with or without
  * modification, are permitted provided that the following conditions
  * are met:
  *
  * 1.  Redistributions of source code must retain the above copyright
  *     notice, this list of conditions and the following disclaimer.
  * 2.  Redistributions in binary form must reproduce the above copyright
@@ skipping 113 matching lines @@
 WEBPImageDecoder::WEBPImageDecoder(AlphaOption alphaOption, GammaAndColorProfileOption colorOptions, size_t maxDecodedBytes)
     : ImageDecoder(alphaOption, colorOptions, maxDecodedBytes)
     , m_decoder(0)
     , m_formatFlags(0)
     , m_frameBackgroundHasAlpha(false)
     , m_demux(0)
     , m_demuxState(WEBP_DEMUX_PARSING_HEADER)
     , m_haveAlreadyParsedThisData(false)
     , m_repetitionCount(cAnimationLoopOnce)
     , m_decodedHeight(0)
+    , m_purgeAggressively(false)
 {
     m_blendFunction = (alphaOption == AlphaPremultiplied) ? alphaBlendPremultiplied : alphaBlendNonPremultiplied;
 }
 
 WEBPImageDecoder::~WEBPImageDecoder()
 {
     clear();
 }
 
 void WEBPImageDecoder::clear()
@@ skipping 261 matching lines @@
     buffer->setAlphaBlendSource(animatedFrame.blend_method == WEBP_MUX_BLEND ? ImageFrame::BlendAtopPreviousFrame : ImageFrame::BlendAtopBgcolor);
     buffer->setRequiredPreviousFrameIndex(findRequiredPreviousFrame(index, !animatedFrame.has_alpha));
     WebPDemuxReleaseIterator(&animatedFrame);
 }
 
 void WEBPImageDecoder::decode(size_t index)
 {
     if (failed())
         return;
 
+    updateAggressivePurging(index);

[urvang, 2016/10/03 21:07:23]

You only need to call this method when number of frames (seen so far) changes,
right?
Then, you can do this only at the end of UpdateDemuxer() at line#240.

[cblume, 2016/10/10 21:18:38]

Sounds good.

It looks like updateDemuxer() is called from frameCount(), which is called from
a handful of locations. And updateDemuxer() exits early after it is called for
each setData(). So presumably then updateDemuxer() is run once per incoming data
set (as data arrives). Is my understanding correct?

[urvang, 2016/10/10 21:30:50]

Yes, updateDemuxer() updates the demuxer and related metadata (like frame count)
whenever we have received some new data (since the last update).

+
     Vector<size_t> framesToDecode;
     size_t frameToDecode = index;
     do {
         framesToDecode.append(frameToDecode);
         frameToDecode = m_frameBufferCache[frameToDecode].requiredPreviousFrameIndex();
     } while (frameToDecode != kNotFound && m_frameBufferCache[frameToDecode].getStatus() != ImageFrame::FrameComplete);
 
     ASSERT(m_demux);
     for (auto i = framesToDecode.rbegin(); i != framesToDecode.rend(); ++i) {
         if ((m_formatFlags & ANIMATION_FLAG) && !initFrameBuffer(*i))
             return;
         WebPIterator webpFrame;
         if (!WebPDemuxGetFrame(m_demux, *i + 1, &webpFrame)) {
             setFailed();
         } else {
             decodeSingleFrame(webpFrame.fragment.bytes, webpFrame.fragment.size, *i);
             WebPDemuxReleaseIterator(&webpFrame);
         }
         if (failed())
             return;
 
+        if (m_purgeAggressively)
+            clearCacheExceptFrame(*i);

[urvang, 2016/09/26 18:37:05]

I think you need to purge after line#444.
Otherwise, the decoding of 'i'th frame may NOT have completed yet, which means
that the frame at (i-1) may still be needed (to decode rest of the pixels of ith
frame).

[cblume, 2016/10/01 11:06:23]

Done.
I should do this on the GIF decoder as well.
I updated the GIFImageDecoder version over at https://crrev.com/2382143004

[urvang, 2016/10/03 19:12:36]

I don't see this change yet. Forgot to upload?

[cblume, 2016/10/10 21:18:38]

Oh, yeah. Sorry.

+
         // We need more data to continue decoding.
         if (m_frameBufferCache[*i].getStatus() != ImageFrame::FrameComplete)
             break;
     }
 
     // It is also a fatal error if all data is received and we have decoded all
     // frames available but the file is truncated.
     if (index >= m_frameBufferCache.size() - 1 && isAllDataReceived() && m_demux && m_demuxState != WEBP_DEMUX_DONE)
         setFailed();
 }
@@ skipping 52 matching lines @@
             applyPostProcessing(frameIndex);
             return false;
         }
         // FALLTHROUGH
     default:
         clear();
         return setFailed();
     }
 }
 
+void WEBPImageDecoder::updateAggressivePurging(size_t index)

[scroggo_chromium, 2016/09/26 12:04:48]

This looks to be the exact same method as in GIFImageDecoder (after you update
it in crrev.com/2371563002). Once APNG is in, we may want the same thing there.
I think this method belongs in ImageDecoder.

[cblume, 2016/10/01 11:06:23]

Good call.
I've moved this into ImageDecoder in https://crrev.com/2376033005

+{
+    if (m_purgeAggressively)
+        return;
+
+    // We don't want to cache so much that we cause a memory issue.
+    //
+    // If we used a LRU cache we would fill it and then on next animation loop
+    // we would need to decode all the frames again -- the LRU would give no
+    // benefit and would consume more memory.
+    // So instead, simply purge unused frames if caching all of the frames of
+    // the image would use more memory than the image decoder is allowed
+    // (m_maxDecodedBytes) or would overflow 32 bits..
+    //
+    // As we decode we will learn the total number of frames, and thus total
+    // possible image memory used.
+
+    const uint64_t frameArea = decodedSize().area();
+    const uint64_t frameMemoryUsage = frameArea * 4; // 4 bytes per pixel
+    if (frameMemoryUsage / 4 != frameArea) { // overflow occurred
+        m_purgeAggressively = true;
+        return;
+    }
+
+    const uint64_t totalMemoryUsage = frameMemoryUsage * index;

[skal, 2016/09/26 12:22:51]

this multiply is still even more likely to overflow that then one at 532.

You should also check that the presentation if the result is exact
(before comparing to m_maxDecodedBytes), with something like:
if (totalMemoryUsage / frameMemoryUsage != index) { // overflow occurred
...
}


(sorry i misplaced the line of my previous similar comment)

[cblume, 2016/10/01 11:06:23]

Ah, got ya. Fixed.
I've moved this into ImageDecoder in https://crrev.com/2376033005

+    if (totalMemoryUsage > m_maxDecodedBytes) {
+        m_purgeAggressively = true;
+    }
+}
 } // namespace blink