WIP: Implement APNG

third_party/WebKit/Source/platform/image-decoders/png/PNGImageReader.cpp

+/*
+ * Copyright (C) 2006 Apple Computer, Inc.
+ * Copyright (C) Research In Motion Limited 2009-2010. All rights reserved.
+ *
+ * Portions are Copyright (C) 2001 mozilla.org
+ *
+ * Other contributors:
+ *   Stuart Parmenter <stuart@mozilla.com>
+ *
+ * This library is free software; you can redistribute it and/or
+ * modify it under the terms of the GNU Lesser General Public
+ * License as published by the Free Software Foundation; either
+ * version 2.1 of the License, or (at your option) any later version.
+ *
+ * This library is distributed in the hope that it will be useful,
+ * but WITHOUT ANY WARRANTY; without even the implied warranty of
+ * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the GNU
+ * Lesser General Public License for more details.
+ *
+ * You should have received a copy of the GNU Lesser General Public
+ * License along with this library; if not, write to the Free Software
+ * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA  02110-1301  USA
+ *
+ * Alternatively, the contents of this file may be used under the terms
+ * of either the Mozilla Public License Version 1.1, found at
+ * http://www.mozilla.org/MPL/ (the "MPL") or the GNU General Public
+ * License Version 2.0, found at http://www.fsf.org/copyleft/gpl.html
+ * (the "GPL"), in which case the provisions of the MPL or the GPL are
+ * applicable instead of those above.  If you wish to allow use of your
+ * version of this file only under the terms of one of those two
+ * licenses (the MPL or the GPL) and not to allow others to use your
+ * version of this file under the LGPL, indicate your decision by
+ * deletingthe provisions above and replace them with the notice and
+ * other provisions required by the MPL or the GPL, as the case may be.
+ * If you do not delete the provisions above, a recipient may use your
+ * version of this file under any of the LGPL, the MPL or the GPL.
+ */
+
+#include "platform/image-decoders/png/PNGImageReader.h"
+
+#include "platform/image-decoders/png/PNGImageDecoder.h"
+#include "platform/image-decoders/FastSharedBufferReader.h"
+#include "png.h"
+#include "wtf/PtrUtil.h"
+#include <memory>
+
+#if !defined(PNG_LIBPNG_VER_MAJOR) || !defined(PNG_LIBPNG_VER_MINOR)
+#error version error: compile against a versioned libpng.
+#endif
+#if USE(QCMSLIB)
+#include "qcms.h"
+#endif
+
+#if PNG_LIBPNG_VER_MAJOR > 1 || (PNG_LIBPNG_VER_MAJOR == 1 && PNG_LIBPNG_VER_MINOR >= 4)
+#define JMPBUF(png_ptr) png_jmpbuf(png_ptr)
+#else
+#define JMPBUF(png_ptr) png_ptr->jmpbuf
+#endif
+
+namespace {
+
+inline blink::PNGImageDecoder* imageDecoder(png_structp png)
+{
+    return static_cast<blink::PNGImageDecoder*>(png_get_progressive_ptr(png));
+}
+
+void PNGAPI pngHeaderAvailable(png_structp png, png_infop)
+{
+    imageDecoder(png)->headerAvailable();
+}
+
+void PNGAPI pngRowAvailable(png_structp png, png_bytep row,
+                            png_uint_32 rowIndex, int state)
+{
+    imageDecoder(png)->rowAvailable(row, rowIndex, state);
+}
+
+void PNGAPI pngComplete(png_structp png, png_infop)
+{
+    imageDecoder(png)->complete();
+}
+
+void PNGAPI pngFailed(png_structp png, png_const_charp err)
+{
+    longjmp(JMPBUF(png), 1);
+}
+
+} // namespace
+
+namespace blink {
+
+// This is the callback function for unknown PNG chunks, which is used to
+// extract the animation chunks.
+static int readAnimationChunk(png_structp png_ptr, png_unknown_chunkp chunk)
+{
+    PNGImageReader* reader = (PNGImageReader*) png_get_user_chunk_ptr(png_ptr);
+    reader->parseAnimationChunk((const char*) chunk->name, chunk->data,
+                                chunk->size);
+    return 1;
+}
+
+PNGImageReader::PNGImageReader(PNGImageDecoder* decoder, size_t initialOffset)
+    : m_decoder(decoder)
+    , m_initialOffset(initialOffset)
+    , m_readOffset(initialOffset)
+    , m_decodeOffset(0)
+    , m_idatOffset(0)
+    , m_hasAlpha(false)
+    , m_idatIsPartOfAnimation(false)
+    , m_isAnimated(false)
+    , m_parsedSignature(false)
+#if USE(QCMSLIB)
+    , m_rowBuffer()
+#endif
+{
+    m_png = png_create_read_struct(PNG_LIBPNG_VER_STRING, 0, pngFailed, 0);
+    m_info = png_create_info_struct(m_png);
+    png_set_progressive_read_fn(m_png, m_decoder, pngHeaderAvailable,
+                                pngRowAvailable, pngComplete);
+
+    // Keep the chunks which are of interest for APNG. We don't need to keep
+    // the fdAT chunks, since they are converted to IDAT's by the frame decoder.
+    png_byte apngChunks[] = {"acTL\0fcTL\0"};
+    png_set_keep_unknown_chunks(m_png, PNG_HANDLE_CHUNK_NEVER, apngChunks, 2);
+    png_set_read_user_chunk_fn(m_png, (png_voidp) this, readAnimationChunk);
+}
+
+PNGImageReader::~PNGImageReader()
+{
+    png_destroy_read_struct(m_png ? &m_png : 0, m_info ? &m_info : 0, 0);
+    ASSERT(!m_png && !m_info);
+}
+
+// This method reads from the FastSharedBufferReader, starting at offset,
+// and returns |length| bytes in the form of a pointer to a const png_byte*.
+// This function is used to make it easy to access data from the reader in a
+// png friendly way, and pass it to libpng for decoding.
+//
+// Pre-conditions before using this:
+// - |reader|.size() >= |readOffset| + |length|
+// - |buffer|.size() >= |length|
+// - |length| <= |kBufferSize|
+//
+// The reason for the last two precondition is that currently the png signature
+// plus IHDR chunk (8B + 25B = 33B) is the largest chunk that is read using this
+// method. If the data is not consecutive, it is stored in |buffer|, which must
+// have the size of (at least) |length|, but there's no need for it to be larger
+// than |kBufferSize|.
+static constexpr size_t kBufferSize = 33;
+const png_byte* readAsConstPngBytep(const FastSharedBufferReader& reader,
+                                    size_t readOffset, size_t length,
+                                    char* buffer)
+{
+    ASSERT(length <= kBufferSize);
+    return reinterpret_cast<const png_byte*>(
+            reader.getConsecutiveData(readOffset, length, buffer));
+}
+
+// This is used as a value for the byteLength of a frameInfo struct to
+// indicate that it is the first frame, and we still need to set byteLength
+// to the correct value as soon as the parser knows it. 1 is a safe value
+// since the byteLength field of a frame is at least 12, in the case of an
+// empty fdAT or IDAT chunk.
+static constexpr size_t kFirstFrameIndicator = 1;
+
+void PNGImageReader::decode(SegmentReader& data, size_t index)
+{
+    if (index >= m_frameInfo.size())
+        return;
+
+    // When decoding by libpng fails in either the non-animated branch or the
+    // animated branch, the decoder needs to be set to the failed state.
+    if (setjmp(JMPBUF(m_png))) {
+        m_decoder->setFailed();
+        return;
+    }
+    
+    // For non animated PNG's, we don't want to waste CPU time with recreating

[scroggo_chromium, 2016/10/28 14:20:33]

More importantly, if we are resuming the first frame, recreating the struct
would mean we start the decode over again. Unless we start processing from the
beginning of the IDAT (which would be wasteful), we'd either make the decode
fail (i.e. report failure) or decode the wrong thing.

[joostouwerling, 2016/10/28 18:41:25]

Done.

+    // the png struct. It suffices to continue parsing where we left off.
+    if (!m_isAnimated) {
+        m_decodeOffset += processData(

[scroggo_chromium, 2016/10/28 14:20:33]

Why not continue to use m_frameInfo[0].readOffset?

That's more clear to me. I find it confusing that m_decodeOffset only applies in
some cases. (But maybe there's a good reason to separate them, which I still
don't understand?)

[joostouwerling, 2016/10/28 18:41:25]

I use m_decodeOffset to keep track how far the first frame is decoded
progressively. I did not create the variable before since I can reuse the frame
offset for non animated images, but since it was necessary to have it for
animated images, I decided to use it here for consistency as well. 

I will rename it to m_progressiveDecodeOffset to make its usage more clear.

[scroggo_chromium, 2016/10/31 13:35:11]

I figured out why I'm confused. PNGImageReader::m_readOffset is how far we've
read into the input data - it changes when we read more data. But
FrameInfo::readOffset is only set once (or I think twice - once just to
initialize it to zero as a sentinel) - it means the start of the data. How about
changing FrameInfo::readOffset to something that better reflects that, like
initialOffset? (I'm not a huge fan of that name, but I cannot think of anything
better, and that's at least analogous to m_initialOffset, which means something
similar.) And please add a comment explaining it.

[joostouwerling, 2016/10/31 18:40:19]

How about startOffset? When glancing over the code, I guess initialOffset could
also cause confusion if it is interpreted as the start of the PNG stream. 

[scroggo_chromium, 2016/10/31 19:34:06]

sgtm

+            data, m_frameInfo[0].readOffset + m_decodeOffset, 0);
+        return;
+    }
+
+    // Progressive decoding is only done if:

[scroggo_chromium, 2016/10/28 14:20:33]

I find the term "progressive decoding" a little bit confusing. We're still using
libpng's progressive read function. The difference is we'll never have to resume
in the case of later frames.

It occurs to me that there's a potential downside to only decoding later frames
when they're complete (i.e. have all the data they report to have). Let's say we
supported progressively decoding later frames. The client could begin decoding
frame i (where i > 0) in a background thread or if it has spare cycles as soon
as some data becomes available, even though they won't use that frame yet (our
client already handles this, for the case of an animated gif). Since we don't
allow this, when the frame is actually complete, the client will have to wait
longer for the decode when they need it.

That doesn't necessarily mean it's the wrong decision, but we should consider
that. (As I stated, the GIF decoder made a different decision, but I think WEBP
made the same decision as we have for APNG.) cblume@, any thoughts?

[cblume, 2016/10/28 17:29:17]

If it isn't too much trouble, I would love the ability to decode in advance.

Right now Chrome does lazy decoding when it urgently needs a frame. But with
things like big.LITTLE I would have preferred to have been decoding in advance
on a core that takes less energy overall.

It would take us a while to implement this in Chrome. But when I step back, that
is definitely something I want.

Because we won't be able to use it for a while, and because it may make landing
this CL harder, I'm fine with putting it off until later.

[joostouwerling, 2016/10/28 18:41:26]

Right. But I think we still need to use the progressive read functions to work
with the APNG chunks, or at least, if we don't want to copy the image data,
since we need to modify small parts the data stream before it is fed to libpng
(i.e the fdAT chunks).

[joostouwerling, 2016/10/28 18:41:26]

It would not be too difficult to implement progressive decoding for each frame,
I guess. We can report a frame when the frame control is seen, instead of all
data, and we need to keep track of which frame we were decoding before and how
far in the stream we've came. But the infrastructure for the actual decoding is
not different than for frame 0, as far as I can see it now.

But I am fine with postponing it to make the landing process smoother. 

[scroggo_chromium, 2016/10/31 13:35:11]

FWIW, I think it will make the landing process (or more to the point - the
reviewing process) easier if we decode all frames progressively. It means that
more code will be shared and we can remove some code around deciding which path
to take.

[scroggo_chromium, 2016/10/31 13:35:12]

Oh I'm not suggesting that we shouldn't use the progressive read functions. I'm
suggesting that we should think of some better nomenclature.

[joostouwerling, 2016/10/31 18:40:19]

As agreed upon in person: for now, only the first frame is decoded
progressively, and later frames are decoded in one call. We can revisit this
before trying to land this CL.

Another thing that sprung to mind is that progressively decoding because we have
some cycles left is something different that progressively decoding because not
all data has arrived yet. AFAIK there is nothing from stopping the client to
decode a frame in advance, before it needs to be shown.

[scroggo_chromium, 2016/10/31 19:34:06]

Sure, it's just that allowing progressive decoding for later frames means that
the client can decode a frame in advance, even if it hasn't been fully received
yet.

+    // - It is the first frame, thus |index| == 0

[scroggo_chromium, 2016/10/28 14:20:33]

I think you're saying that both of these statements have to be true? Maybe end
this line with AND (or "both" in the line above)? I initially read this as an
OR.

[joostouwerling, 2016/10/28 18:41:26]

Done.

+    // - The byteLength of the first frame is not yet known, *or* it is known
+    //   but we're only halfway in a progressive decode, started earlier.

[scroggo_chromium, 2016/10/28 14:20:33]

I don't think you mean precisely halfway. partway?

Another way of saying this (that I think is more clear) is that we skip
progressive decoding (is there a word for non-progressive decoding?) if we have
all the data for the first frame and have not already started decoding it.

[joostouwerling, 2016/10/28 18:41:26]

Ack. Good question about the word. The antonyms don't really work on this one. 

+    bool firstFrameIncomplete = m_frameInfo[0].byteLength == kFirstFrameIndicator;

[scroggo_chromium, 2016/10/28 14:20:33]

I find the word "Incomplete" here confusing, since we have
FrameStatus::FrameComplete.

I started to suggest something like

  bool firstFrameNotFullyReceived

(kind of long and hard to read...)

or

  bool firstFrameFullyReceived = byteLength != kFirstFrameInd

that's a little easier to read, but non-intuitive since you'll later say
!firstFrameFullyReceived. Even worse, though, I think that's not even true. It
doesn't tell us whether we have all the data for the first frame. It just tells
whether we *know* that we have all the data for the first frame. (Right?)

[joostouwerling, 2016/10/28 18:41:25]

Right, if the client would have supplied more data without having called
parse(PNGMetaDataQuery) we would have all the data but don't know about it. I
choose firstFrameLengthKnown since it implies what we know at this point.

[scroggo_chromium, 2016/10/31 13:35:11]

sgtm

+    bool progressiveDecodingAlreadyStarted = m_decodeOffset > 0;
+    bool progressiveDecode = (index == 0
+            && (firstFrameIncomplete || progressiveDecodingAlreadyStarted));
+    
+    // Initialize a new png struct for this frame. For a progressive decode of
+    // the first frame, we only need to do this once.
+    if (!progressiveDecode || m_decodeOffset == 0)

[scroggo_chromium, 2016/10/28 14:20:33]

I think this can be

if (!progressiveDecode || !progressiveDecodingAlreadyStarted)

[joostouwerling, 2016/10/28 18:41:26]

Done.

+        startFrameDecoding(data, index);

[scroggo_chromium, 2016/10/28 14:20:33]

If the first frame fills the image size, can we skip recreating the png struct?

[joostouwerling, 2016/10/28 18:41:26]

We could, but there needs to be a check that this is the original png struct,
and not one used for a different frame. That could happen if this frame was
decoded a second time. 

[scroggo_chromium, 2016/10/31 13:35:12]

I think the common case is that it *is* the original png struct. I don't think
it would be too hard to check for, but perhaps at least add a FIXME?

[joostouwerling, 2016/10/31 18:40:19]

Done.

+    
+    // By default, a frame will be considered to be decoded completely, unless

[scroggo_chromium, 2016/10/28 14:20:33]

Try to keep comments focused on why, rather than what. I don't think this
comment really explains anything, but it tells us what the next few lines say.
How about the following:

bool decodedFrameCompletely;
if (progressiveDecode)
  decodedFrameCompletely = progressivelyDecodeFirstFrame(data)
else {
  decodeFrame(data, index);
  // For a non-progressive decode, we already have all the data we are going to
get,
  // so consider the frame complete.
  decodedFrameCompletely = true;
}

Bigger picture issue, though - if frame 2 is incomplete, in the sense that it
only has half the data that it should, should we indicate that in some way to
the client? Maybe they don't want to bother with decoding frame 3 in that case?
I think this will result in marking frame 2 as FrameComplete, so they'll assume
that frame 3 will composite correctly with it.

[joostouwerling, 2016/10/28 18:41:26]

Acknowledged.
You mean a frame that says it has |n| rows of |m| columns, but provides less
data than that? I commented elsewhere on this as well, but my take is that the
decoder can take this into account if it receives the complete() call before it
has received all the data it anticipated.

[scroggo_chromium, 2016/10/31 13:35:11]

Yes.
i.e. in PNGImageDecoder::complete()? It looks like the decoder does not
currently do this?

[joostouwerling, 2016/10/31 18:40:19]

I added a todo, which depends on the result of the investigation in libpng's
behavior I mentioned at PGNImageDecoderTest [1].

[1]:
https://codereview.chromium.org/2386453003/diff/160001/third_party/WebKit/Sou...

+    // the progressive decoding of the first frame returns false, which
+    // indicates not all data was available yet.
+    bool decodedFrameCompletely = true;
+    if (progressiveDecode)
+        decodedFrameCompletely = progressivelyDecodeFirstFrame(data);
+    else
+        decodeFrame(data, index);
+
+    // Finish decoding by sending the IEND chunk, but only if the frame was

[scroggo_chromium, 2016/10/28 14:20:33]

nit: I find this a little wordy. How about:

// Send the IEND chunk if the frame is completely decoded.

(Maybe this comment is unnecessary, since it doesn't explain why. The
interesting info in the comment is that endFrameDecoding sends the IEND chunk.
But maybe that's not interesting at this level.)

[joostouwerling, 2016/10/28 18:41:26]

I'd say it is interesting to know that this will call the complete() method in
|m_decoder|. Updated it. 

+    // completely decoded.
+    if (decodedFrameCompletely)
+        endFrameDecoding();
+

[scroggo_chromium, 2016/10/28 14:20:33]

nit: No need for blank line

[joostouwerling, 2016/10/28 18:41:26]

Done.

+}
+
+void PNGImageReader::startFrameDecoding(SegmentReader& data, size_t index)
+{
+    // Each frame is processed as if it were a complete, single frame png image.
+    // To accomplish this, destroy the current |m_png| and |m_info| structs and
+    // create new ones. CRC errors are ignored, so fdAT chunks can be processed
+    // as IDATs without recalculating the CRC value. 
+    png_destroy_read_struct(m_png ? &m_png : 0, m_info ? &m_info : 0, 0);
+    m_png = png_create_read_struct(PNG_LIBPNG_VER_STRING, 0, pngFailed, 0);
+    m_info = png_create_info_struct(m_png);
+    png_set_crc_action(m_png, PNG_CRC_QUIET_USE, PNG_CRC_QUIET_USE);
+    png_set_progressive_read_fn(m_png, m_decoder, pngHeaderAvailable,
+                                pngRowAvailable, pngComplete);
+   
+    // If the frame is the size of the whole image, we don't need to modify any
+    // data in the IHDR chunk. This means it suffices to re-process all header
+    // data up to the first frame, for mimicking a png image.
+    const IntRect& frameRect = m_frameInfo[index].frameRect;
+    if (frameRect.location() == IntPoint()
+        && frameRect.size() == m_decoder->size()) {
+        processData(data, m_initialOffset, m_idatOffset); 
+        return;
+    }
+
+    // Process the IHDR chunk, but change the width and height so it reflects
+    // the frame's width and height. Image Decoder will apply the x,y offset.
+    // This step is omitted if the width and height are equal to the image size,
+    // which is done in the block above.
+    FastSharedBufferReader reader(&data);
+    char readBuffer[kBufferSize];
+
+    // |headerSize| is equal to |kBufferSize|, but adds more semantic insight.
+    constexpr size_t headerSize = 33;
+    png_byte header[headerSize];
+    const png_byte* chunk = readAsConstPngBytep(reader, m_initialOffset,
+                                                headerSize, readBuffer);
+    memcpy(header, chunk, headerSize);
+    
+    // Write the unclipped width and height. Clipping happens in the decoder.
+    png_save_uint_32(header + 16, frameRect.width());
+    png_save_uint_32(header + 20, frameRect.height());
+    png_process_data(m_png, m_info, header, headerSize);
+
+    // Process the rest of the header chunks. Start after the PNG signature and
+    // IHDR chunk, 33B, and process up to the first data chunk. The number of 
+    // bytes up to the first data chunk is stored in |m_idatOffset|.
+    processData(data, m_initialOffset + headerSize, m_idatOffset - headerSize);
+}
+
+bool PNGImageReader::progressivelyDecodeFirstFrame(SegmentReader& data)
+{
+    FastSharedBufferReader reader(&data);
+    char readBuffer[8]; // large enough to identify a chunk.
+    size_t offset = m_frameInfo[0].readOffset;
+
+    // Loop while there is enough data to do progressive decoding.
+    while (data.size() >= offset + 8) {
+
+        // At the beginning of each loop, the offset is at the start of a chunk.
+        const png_byte* chunk = readAsConstPngBytep(reader, offset, 8,
+                                                    readBuffer);
+        const png_uint_32 length = png_get_uint_32(chunk);
+
+        // When an fcTL or IEND chunk is encountered, the frame data has ended.
+        // Return true, since all frame data is decoded.
+        if (memcmp(chunk, "fcTL", 4) == 0 || memcmp(chunk, "IEND", 0)) {
+            m_decodeOffset = 0;

[scroggo_chromium, 2016/10/28 14:20:34]

Maybe I don't understand this variable. We're done progressively decoding the
first frame, so why do we need to reset it? Is this in case the first frame gets
cleared and then we need to redecode it later? (In that case, we won't need the
progressive case, since we know we already have all the data, right?)

[joostouwerling, 2016/10/28 18:41:26]

I apparently removed a comment here by accident, which explained this. In the
case the frame is cleared, and is decoded for a second time, |m_decodeOffset| >
0 will set |progressiveDecodingAlreadyStarted| to true. This is undesired since
it will try to continue progressive decoding from the end of the frame data,
instead of treating it as any other fully seen frame.

Now that I think about this, I'd say a better place to do this is in decode(),
if this method returns true, so it is closer to the code where it makes a
difference.

+            return true;
+        }
+        
+        // If this chunk was already decoded, move on to the next.
+        if (m_decodeOffset >= offset + length + 12) {
+            offset += length + 12;
+            continue;
+        }
+
+        // At this point, three scenario's are possible:

[scroggo_chromium, 2016/10/28 14:20:34]

nit: scenarios*

[joostouwerling, 2016/10/28 18:41:25]

Done.

+        // 1) Some bytes of this chunk are already decoded in a previous call,

[scroggo_chromium, 2016/10/28 14:20:34]

nit: are -> were

[joostouwerling, 2016/10/28 18:41:25]

Done.

+        //    so we need to continue from there.
+        // 2) This is an fdAT chunk, so we need to convert it to an IDAT chunk
+        //    before we can decode it.
+        // 3) This is any other chunk, most likely an IDAT chunk.
+        //
+        // In each scenario, we want to decode as much data as possible. In each
+        // one, do the scenario specific work and set |offset| to where decoding
+        // needs to continue. From there, decode until the end of the chunk, if
+        // possible. If the whole chunk is decoded, continue to the next loop.
+        // Otherwise, store how far we've came in |m_decodeOffset| and return
+        // false to indicate to the caller that the frame is partially decoded.
+
+        size_t endOffsetChunk = offset + length + 12;
+
+        // Scenario 1: |m_decodeOffset| is ahead of the chunk tag.

[scroggo_chromium, 2016/10/28 14:20:34]

Why does this happen? I guess we didn't update readOffset to account for having
read further? Why not?

[joostouwerling, 2016/10/28 18:41:26]

This can happen if a chunk is partially decoded. We can't set |offset| to where
we stopped in the previous call since we won't know how many bytes are left in
this chunk. That is necessary to know, to make sure we (possibly) convert the
next fdAT chunk to an IDAT chunk. A downside of this method is that we need to
skip over the chunks that are already decoded before we get to the required
offset. The advantage is that we don't need to store something like
|m_bytesLeftInChunkForProgressiveDecode| and can omit more complex logic in this
method. The nice thing about this structure is that we're always at the
beginning of a chunk in each loop.

For APNG we can't use m_frameInfo[0].readOffset to keep track of how far we've
decoded the frame, since it may be necessary to re-decode the frame when it has
been cleared.

[scroggo_chromium, 2016/10/31 13:35:12]

I have a comment about this elsewhere - I misunderstood FrameInfo::readOffset
entirely. I think giving it a name that doesn't sound like
PNGImageReader::m_readOffset will provide some clarity.

[joostouwerling, 2016/10/31 18:40:19]

Done.

+        if (m_decodeOffset >= offset + 8) {
+            offset = m_decodeOffset;
+        
+        // Scenario 2: we need to convert the fdAT to an IDAT chunk. For an
+        // explanation of the numbers, see the comments in decodeFrame().
+        } else if (memcmp(chunk, "fdAT", 4) == 0) {
+            png_byte chunkIDAT[] = {0, 0, 0, 0, 'I', 'D', 'A', 'T'};
+            png_save_uint_32(chunkIDAT, length - 4);
+            png_process_data(m_png, m_info, chunkIDAT, 8);
+            // Skip the sequence number
+            offset += 12;
+        
+        // Scenario 3: for any other chunk type, process the first 8 bytes.
+        } else {
+            png_process_data(m_png, m_info, const_cast<png_byte*>(chunk), 8);
+            offset += 8;
+        }
+
+        size_t bytesLeftInChunk = endOffsetChunk - offset;
+        size_t bytesDecoded = processData(data, offset, bytesLeftInChunk);
+        m_decodeOffset = offset + bytesDecoded;
+        if (bytesDecoded < bytesLeftInChunk)
+            return false;
+        offset += bytesDecoded;
+    }        
+
+    return false;
+}
+
+void PNGImageReader::decodeFrame(SegmentReader& data, size_t index)
+{
+    // From the frame info that was gathered during parsing, it is known at
+    // what offset the frame data starts and how many bytes are in the stream
+    // before the frame ends. Using this, we process all chunks that fall in
+    // this interval. We catch every fdAT chunk and transform it to an IDAT
+    // chunk, so libpng will decode it like a non-animated PNG image.
+    size_t offset = m_frameInfo[index].readOffset;
+    size_t endOffset = offset + m_frameInfo[index].byteLength;
+    char readBuffer[8];
+    FastSharedBufferReader reader(&data);
+
+    while (offset < endOffset) {
+        const png_byte* chunk = readAsConstPngBytep(reader, offset, 8, readBuffer);
+        const png_uint_32 length = png_get_uint_32(chunk);
+        if (memcmp(chunk + 4, "fdAT", 4) == 0) {
+            // An fdAT chunk is build up as follows: 
+            // - |length| (4B)
+            // - fdAT tag (4B)
+            // - sequence number (4B) 
+            // - frame data (|length| - 4B)
+            // - CRC (4B)
+            // Thus, to reformat this into an IDAT chunk, we need to:
+            // - write |length| - 4 as the new length, since the sequence number
+            //   must be removed.
+            // - change the tag to IDAT.
+            // - omit the sequence number from the data part of the chunk.
+            png_byte chunkIDAT[] = {0, 0, 0, 0, 'I', 'D', 'A', 'T'};
+            png_save_uint_32(chunkIDAT, length - 4);
+            png_process_data(m_png, m_info, chunkIDAT, 8);
+            // The frame data and the CRC span |length| bytes, so skip the
+            // sequence number and process |length| bytes to decode the frame.
+            processData(data, offset + 12, length);
+        } else {
+            png_process_data(m_png, m_info, const_cast<png_byte*>(chunk), 8);
+            processData(data, offset + 8, length + 4);
+        }
+        offset += 12 + length; 
+    }
+}
+
+void PNGImageReader::endFrameDecoding()
+{
+    png_byte IEND[12] = {0, 0, 0, 0, 'I', 'E', 'N', 'D', 174, 66, 96, 130};
+    png_process_data(m_png, m_info, IEND, 12);
+}
+
+bool PNGImageReader::parse(SegmentReader& data,
+                           PNGImageDecoder::PNGParseQuery query)
+{
+    if (setjmp(JMPBUF(m_png)))
+        return m_decoder->setFailed();
+
+    // If the size has not been parsed, do that first, since it's necessary
+    // for both the Size and MetaData query. If parseSize returns false,
+    // it failed because of a lack of data so we can return false at this point.
+    if (!m_decoder->isDecodedSizeAvailable() && !parseSize(data))
+        return false;
+    
+    if (query == PNGImageDecoder::PNGParseQuery::PNGSizeQuery)
+        return m_decoder->isDecodedSizeAvailable();
+    
+    // For non animated images (identified by no acTL chunk before the IDAT), 
+    // we create one frame. This saves some processing time since we don't need
+    // to go over the stream to find chunks.
+    if (!m_isAnimated) {
+        if (m_frameInfo.isEmpty()) {
+            FrameInfo frame;
+            // This needs to be plus 8 since the first 8 bytes of the IDAT chunk
+            // are already processed in parseSize().
+            frame.readOffset = m_readOffset + 8;
+            frame.frameRect = IntRect(IntPoint(), m_decoder->size());
+            frame.duration = 0;
+            frame.alphaBlend = ImageFrame::AlphaBlendSource::BlendAtopBgcolor;
+            frame.disposalMethod = ImageFrame::DisposalMethod::DisposeNotSpecified;
+            m_frameInfo.append(frame);
+            m_decoder->setMetaDataDecoded();
+        }
+        return true;
+    }
+
+    FastSharedBufferReader reader(&data);
+    char readBuffer[kBufferSize];
+
+    // At this point, the query is FrameMetaDataQuery. Loop over the data and
+    // register all frames we can find. A frame is registered on the next fcTL
+    // chunk or when the IEND chunk is found. This ensures that only complete
+    // frames are reported, unless there is an error in the stream.
+    while (reader.size() >= m_readOffset + 8) {
+        const png_byte* chunk = readAsConstPngBytep(reader, m_readOffset, 8,
+                                                    readBuffer);
+        const size_t length = png_get_uint_32(chunk);
+        const bool isFCTLChunk = memcmp(chunk + 4, "fcTL", 4) == 0;
+        const bool isIENDChunk = memcmp(chunk + 4, "IEND", 4) == 0;
+        
+        // When we find an IDAT chunk (when the IDAT is part of the animation),
+        // or an fdAT chunk, and the readOffset field of the newFrame is 0,
+        // we have found the beginning of a new block of frame data.
+        const bool isFrameData = memcmp(chunk + 4, "fdAT", 4) == 0
+            || (memcmp(chunk + 4, "IDAT", 4) == 0 && m_idatIsPartOfAnimation);
+        if (m_newFrame.readOffset == 0 && isFrameData) {
+            m_newFrame.readOffset = m_readOffset;
+            
+            // When the |frameInfo| vector is empty, the first frame needs to be
+            // reported as soon as possible, even before all frame data is in
+            // |data|, so the first frame can be decoded progressively. 
+            if (m_frameInfo.isEmpty()) {
+                m_newFrame.byteLength = kFirstFrameIndicator;
+                m_frameInfo.append(m_newFrame); 
+            }
+
+        // An fcTL or IEND marks the end of the previous frame. Thus, the
+        // FrameInfo data in m_newFrame is submitted to the m_frameInfo vector.
+        //
+        // Furthermore, an fcTL chunk indicates a new frame is coming,
+        // so the m_newFrame variable is prepared accordingly by setting the 
+        // readOffset field to 0, which indicates that the frame control info
+        // is available but that we haven't seen any frame data yet.
+        } else if (isFCTLChunk || isIENDChunk) {
+            if (m_newFrame.readOffset != 0) {
+                m_newFrame.byteLength = m_readOffset - m_newFrame.readOffset;
+                if (m_frameInfo[0].byteLength == kFirstFrameIndicator)

[scroggo_chromium, 2016/10/28 14:20:34]

What would happen if we had a broken APNG with the IEND or fcTL chunk before any
fdAT or IDAT chunks? I'm guessing that m_frameInfo would be empty, so this call
would crash. (We might be safe in the case of IEND before IDAT, because I think
our attempt to parseSize would have failed before we reach here. And maybe the
readOffset will be zero in the other cases, so we won't reach here. But it's not
obvious without making sure I really understand the code, so I think this is a
good area to have tests.)

[joostouwerling, 2016/10/28 18:41:25]

The IEND case is indeed an invalid APNG file, but like you said, it would fail
in in parseSize. I'll add a test for this, and for more cases of interchanged or
missing chunks for that matter.

When an fcTL chunk appears before the IDAT chunk it indicates that the IDAT is
part of the animation. That is a valid PNG encoding. This is already tested with
the png-animated-idat-part-of-animation image. 

+                    m_frameInfo[0].byteLength = m_newFrame.byteLength;
+                else
+                    m_frameInfo.append(m_newFrame);
+
+                m_newFrame.readOffset = 0;
+            }
+
+            if (reader.size() < m_readOffset + 12 + length)
+                return false;
+            
+            if (isIENDChunk) {
+                // Let the decoder know we've parsed all data, so it does not 
+                // need to query again.
+                m_decoder->setMetaDataDecoded();
+                return true;
+            }
+
+            // At this point, we're dealing with an fcTL chunk, since the above
+            // statement already returns on IEND chunks.
+            
+            // If the fcTL chunk is not 26 bytes long, we can't process it.
+            if (length != 26)
+                return m_decoder->setFailed();
+
+            chunk = readAsConstPngBytep(reader, m_readOffset + 8, length,
+                                        readBuffer);
+            parseFrameInfo(chunk);
+
+        }
+        m_readOffset += 12 + length;
+    }
+    return false; 
+}
+
+// If |length| == 0, read until the stream ends.
+// @return: number of bytes processed.
+size_t PNGImageReader::processData(SegmentReader& data, size_t offset,
+                                   size_t length)
+{
+    const char* segment;
+    size_t totalProcessedBytes = 0;
+    while (size_t segmentLength = data.getSomeData(segment, offset)) {
+        if (length > 0 && segmentLength + totalProcessedBytes > length)
+            segmentLength = length - totalProcessedBytes;
+        png_process_data(m_png, m_info,
+                         reinterpret_cast<png_byte*>(const_cast<char*>(segment)),
+                         segmentLength);
+        offset += segmentLength;
+        totalProcessedBytes += segmentLength;
+        if (totalProcessedBytes == length)
+            return length;
+    }
+    return totalProcessedBytes;
+}
+
+// This methods reads through the stream until it has parsed the image size.
+// @return true when it succeeds in parsing the size.
+//         false when:
+//         A) not enough data is provided
+//         B) decoding by libpng fails. In the this case, it will also call
+//            setFailed on m_decoder. 
+bool PNGImageReader::parseSize(SegmentReader &data)
+{
+    FastSharedBufferReader reader(&data);
+    char readBuffer[kBufferSize];
+
+    // Process the PNG signature and the IHDR with libpng, such that this code 
+    // does not need to be bothered with parsing the contents. This also enables
+    // the reader to use the existing headerAvailable callback in the decoder.
+    //
+    // When we already have decoded the signature, we don't need to do it again.
+    // By setting a flag for this we allow for byte by byte parsing.
+    if (!m_parsedSignature) {
+        if (reader.size() < m_readOffset + 8)
+            return false;
+        const png_byte* chunk = readAsConstPngBytep(reader, m_readOffset, 8,
+                                                    readBuffer);
+        png_process_data(m_png, m_info, const_cast<png_byte*>(chunk), 8);
+        m_readOffset += 8;
+        m_parsedSignature = true;
+        // Initialize the newFrame by setting the readOffset to 0.
+        m_newFrame.readOffset = 0;
+    }
+
+    // This loop peeks at the chunk tag until the IDAT chunk is found. When
+    // a different tag is encountered, pass it on to libpng for general parsing.
+    // We can peek at chunks by looking at the first 8 bytes, which contain the
+    // length and the chunk tag.
+    //
+    // When an fcTL (frame control) is encountered before the IDAT, the frame
+    // data in the IDAT chunk is part of the animation. This case is flagged
+    // and the frame info is stored by parsing the fcTL chunk.
+    while (reader.size() >= m_readOffset + 8) {
+        const png_byte* chunk = readAsConstPngBytep(reader, m_readOffset, 8,
+                                                    readBuffer);
+        const png_uint_32 length = png_get_uint_32(chunk);
+
+        // If we encounter the IDAT chunk, we're done with the png header
+        // chunks. Indicate this to libpng by sending the beginning of the IDAT
+        // chunk, which will trigger libpng to call the headerAvailable
+        // callback on m_decoder. This provides the size to the decoder.
+        if (memcmp(chunk + 4, "IDAT", 4) == 0) {
+            m_idatOffset = m_readOffset;
+            png_process_data(m_png, m_info, const_cast<png_byte*>(chunk), 8);
+            return true;
+        }
+
+        // Consider the PNG image animated if an acTL chunk of the correct
+        // length is present. Parsing the acTL content is done by 
+        // parseAnimationControl, called by libpng's png_process_data.
+        if (memcmp(chunk + 4, "acTL", 4) == 0 && length == 8)
+            m_isAnimated = true;
+
+        // We don't need to check for |length| here, because the decoder will
+        // fail later on for invalid fcTL chunks.
+        if (memcmp(chunk + 4, "fcTL", 4) == 0)
+            m_idatIsPartOfAnimation = true;
+        
+        // 12 is the length, tag and crc part of the chunk, which are all 4B.
+        if (reader.size() < m_readOffset + length + 12)
+            break;  
+
+        png_process_data(m_png, m_info, const_cast<png_byte*>(chunk), 8);
+        processData(data, m_readOffset + 8, length + 4);
+        m_readOffset += length + 12;
+    }
+
+    // If we end up here, not enough data was available for the IDAT chunk
+    // So libpng would not have called headerAvailable yet.
+    return false;
+} 
+
+
+void PNGImageReader::parseAnimationChunk(const char tag[], const void* data_chunk, size_t length)
+{
+    const png_byte* data = static_cast<const png_byte*>(data_chunk);
+
+    // The number of frames as indicated in the animation control chunk (acTL) 
+    // is ignored, and the number of frames that are actually present is used.
+    // For now, when the number of indicated frames is different from the
+    // number of supplied frames, the number of supplied frames is what is
+    // provided to the decoder. Therefore, it does not add any benefit of
+    // looking at the value of the indicated framecount. A note here is that
+    // there may be optimisations available, for example, prescaling vectors.
+    if (strcmp(tag, "acTL") == 0 && length == 8) {
+        png_uint_32 repetitionCount = png_get_uint_32(data + 4);
+        m_decoder->setRepetitionCount(repetitionCount);
+    
+    // For fcTL, decoding fails if it does not have the correct length. It is
+    // impossible to make a guess about the frame if not all data is available.
+    // Use longjmp to get back to parse(), which is necessary since this method
+    // is called by a libpng callback.
+    } else if (strcmp(tag, "fcTL") == 0) {
+        if (length != 26)
+            longjmp(JMPBUF(m_png), 1);
+        parseFrameInfo(data);
+    }
+
+}
+
+size_t PNGImageReader::frameCount() const
+{
+    return m_frameInfo.size();
+}
+
+const PNGImageReader::FrameInfo& PNGImageReader::frameInfo(size_t index) const
+{
+    ASSERT(index < m_frameInfo.size());
+    return m_frameInfo[index];
+}
+
+// Extract the frame control info and store it in m_newFrame. The length check
+// on the data chunk has been done in parseAnimationChunk.
+// The fcTL specification used can be found at:
+// https://wiki.mozilla.org/APNG_Specification#.60fcTL.60:_The_Frame_Control_Chunk
+void PNGImageReader::parseFrameInfo(const png_byte* data)
+{
+    png_uint_32 width, height, xOffset, yOffset;
+    png_uint_16 delayNumerator, delayDenominator;
+    width               = png_get_uint_32(data + 4);
+    height              = png_get_uint_32(data + 8);
+    xOffset             = png_get_uint_32(data + 12);
+    yOffset             = png_get_uint_32(data + 16);
+    delayNumerator      = png_get_uint_16(data + 20);
+    delayDenominator    = png_get_uint_16(data + 22);
+
+    m_newFrame.duration         = (delayDenominator == 0) ? delayNumerator * 10
+                                    : delayNumerator * 1000 / delayDenominator;
+    m_newFrame.frameRect        = IntRect(xOffset, yOffset, width, height);
+    m_newFrame.disposalMethod   = data[24];
+    m_newFrame.alphaBlend       = data[25];
+    
+}
+
+}; // namespace blink