Make SkCodec support peek() and read()
- Update SkCodec's dox to point out that some of the data must be
read twice in order to decode
- Add an accessor that reports how much is needed for reading twice
- Make SkCodec default to use peek()
If an input stream supports peek()ing, peek() instead of reading.
This way the stream need not implement rewind()
- Make SkCodec use read() + rewind() as a backup
So that streams without peek() implemented can still function
properly (assuming they can rewind).
- read everything we may need to determine the format once
In SkCodec::NewFromStream, peek()/read() 14 bytes, which is enough
to read all of the types we support. Pass the buffer to each subtype,
which will have enough info to determine whether it is the right
type. This simplifies the code and results in less reading and
rewinding.
- NOTE: SkWbmpCodec needs the following number of bytes for the header
+ 1 (type)
+ 1 (reserved)
+ 3 (width - bytes needed to support up to 0xFFFF)
+ 3 (height - bytes needed to support up to 0xFFFF)
= 8
- in SkWebpCodec, support using read + rewind as a backup if peek does
not work.
A change in Android will add peek() to JavaInputStreamAdapter.
BUG=skia:3257
Review URL: https://codereview.chromium.org/1472123002
diff --git a/include/codec/SkCodec.h b/include/codec/SkCodec.h
index c0625ff..67ccdb2 100644
--- a/include/codec/SkCodec.h
+++ b/include/codec/SkCodec.h
@@ -26,9 +26,34 @@
class SkCodec : SkNoncopyable {
public:
/**
+ * Minimum number of bytes that must be buffered in SkStream input.
+ *
+ * An SkStream passed to NewFromStream must be able to use this many
+ * bytes to determine the image type. Then the same SkStream must be
+ * passed to the correct decoder to read from the beginning.
+ *
+ * This can be accomplished by implementing peek() to support peeking
+ * this many bytes, or by implementing rewind() to be able to rewind()
+ * after reading this many bytes.
+ */
+ static size_t MinBufferedBytesNeeded();
+
+ /**
* If this stream represents an encoded image that we know how to decode,
* return an SkCodec that can decode it. Otherwise return NULL.
*
+ * As stated above, this call must be able to peek or read
+ * MinBufferedBytesNeeded to determine the correct format, and then start
+ * reading from the beginning. First it will attempt to peek, and it
+ * assumes that if less than MinBufferedBytesNeeded bytes (but more than
+ * zero) are returned, this is because the stream is shorter than this,
+ * so falling back to reading would not provide more data. If peek()
+ * returns zero bytes, this call will instead attempt to read(). This
+ * will require that the stream can be rewind()ed.
+ *
+ * If SkPngChunkReader is not NULL, take a ref and pass it to libpng if
+ * the image is a png.
+ *
* If the SkPngChunkReader is not NULL then:
* If the image is not a PNG, the SkPngChunkReader will be ignored.
* If the image is a PNG, the SkPngChunkReader will be reffed.
diff --git a/src/codec/SkBmpCodec.cpp b/src/codec/SkBmpCodec.cpp
index 580ff25..3302e4f 100644
--- a/src/codec/SkBmpCodec.cpp
+++ b/src/codec/SkBmpCodec.cpp
@@ -56,12 +56,10 @@
/*
* Checks the start of the stream to see if the image is a bitmap
*/
-bool SkBmpCodec::IsBmp(SkStream* stream) {
+bool SkBmpCodec::IsBmp(const void* buffer, size_t bytesRead) {
// TODO: Support "IC", "PT", "CI", "CP", "BA"
const char bmpSig[] = { 'B', 'M' };
- char buffer[sizeof(bmpSig)];
- return stream->read(buffer, sizeof(bmpSig)) == sizeof(bmpSig) &&
- !memcmp(buffer, bmpSig, sizeof(bmpSig));
+ return bytesRead >= sizeof(bmpSig) && !memcmp(buffer, bmpSig, sizeof(bmpSig));
}
/*
diff --git a/src/codec/SkBmpCodec.h b/src/codec/SkBmpCodec.h
index 30523b0..5d77ca3 100644
--- a/src/codec/SkBmpCodec.h
+++ b/src/codec/SkBmpCodec.h
@@ -20,11 +20,7 @@
*/
class SkBmpCodec : public SkCodec {
public:
-
- /*
- * Checks the start of the stream to see if the image is a bmp
- */
- static bool IsBmp(SkStream*);
+ static bool IsBmp(const void*, size_t);
/*
* Assumes IsBmp was called and returned true
diff --git a/src/codec/SkCodec.cpp b/src/codec/SkCodec.cpp
index 11eb1f9..87a7ded 100644
--- a/src/codec/SkCodec.cpp
+++ b/src/codec/SkCodec.cpp
@@ -20,7 +20,7 @@
#include "SkWebpCodec.h"
struct DecoderProc {
- bool (*IsFormat)(SkStream*);
+ bool (*IsFormat)(const void*, size_t);
SkCodec* (*NewFromStream)(SkStream*);
};
@@ -35,6 +35,10 @@
{ SkWbmpCodec::IsWbmp, SkWbmpCodec::NewFromStream }
};
+size_t SkCodec::MinBufferedBytesNeeded() {
+ return WEBP_VP8_HEADER_SIZE;
+}
+
SkCodec* SkCodec::NewFromStream(SkStream* stream,
SkPngChunkReader* chunkReader) {
if (!stream) {
@@ -42,23 +46,41 @@
}
SkAutoTDelete<SkStream> streamDeleter(stream);
-
+
+ // 14 is enough to read all of the supported types.
+ const size_t bytesToRead = 14;
+ SkASSERT(bytesToRead <= MinBufferedBytesNeeded());
+
+ char buffer[bytesToRead];
+ size_t bytesRead = stream->peek(buffer, bytesToRead);
+
+ // It is also possible to have a complete image less than bytesToRead bytes
+ // (e.g. a 1 x 1 wbmp), meaning peek() would return less than bytesToRead.
+ // Assume that if bytesRead < bytesToRead, but > 0, the stream is shorter
+ // than bytesToRead, so pass that directly to the decoder.
+ // It also is possible the stream uses too small a buffer for peeking, but
+ // we trust the caller to use a large enough buffer.
+
+ if (0 == bytesRead) {
+ SkCodecPrintf("Could not peek!\n");
+ // It is possible the stream does not support peeking, but does support
+ // rewinding.
+ // Attempt to read() and pass the actual amount read to the decoder.
+ bytesRead = stream->read(buffer, bytesToRead);
+ if (!stream->rewind()) {
+ SkCodecPrintf("Could not rewind!\n");
+ return nullptr;
+ }
+ }
+
SkAutoTDelete<SkCodec> codec(nullptr);
// PNG is special, since we want to be able to supply an SkPngChunkReader.
// But this code follows the same pattern as the loop.
- const bool isPng = SkPngCodec::IsPng(stream);
- if (!stream->rewind()) {
- return NULL;
- }
- if (isPng) {
+ if (SkPngCodec::IsPng(buffer, bytesRead)) {
codec.reset(SkPngCodec::NewFromStream(streamDeleter.detach(), chunkReader));
} else {
for (DecoderProc proc : gDecoderProcs) {
- const bool correctFormat = proc.IsFormat(stream);
- if (!stream->rewind()) {
- return nullptr;
- }
- if (correctFormat) {
+ if (proc.IsFormat(buffer, bytesRead)) {
codec.reset(proc.NewFromStream(streamDeleter.detach()));
break;
}
diff --git a/src/codec/SkCodec_libgif.cpp b/src/codec/SkCodec_libgif.cpp
index e6a4016..40ed94d 100644
--- a/src/codec/SkCodec_libgif.cpp
+++ b/src/codec/SkCodec_libgif.cpp
@@ -16,9 +16,8 @@
/*
* Checks the start of the stream to see if the image is a gif
*/
-bool SkGifCodec::IsGif(SkStream* stream) {
- char buf[GIF_STAMP_LEN];
- if (stream->read(buf, GIF_STAMP_LEN) == GIF_STAMP_LEN) {
+bool SkGifCodec::IsGif(const void* buf, size_t bytesRead) {
+ if (bytesRead >= GIF_STAMP_LEN) {
if (memcmp(GIF_STAMP, buf, GIF_STAMP_LEN) == 0 ||
memcmp(GIF87_STAMP, buf, GIF_STAMP_LEN) == 0 ||
memcmp(GIF89_STAMP, buf, GIF_STAMP_LEN) == 0)
diff --git a/src/codec/SkCodec_libgif.h b/src/codec/SkCodec_libgif.h
index 200f176..ba48989 100644
--- a/src/codec/SkCodec_libgif.h
+++ b/src/codec/SkCodec_libgif.h
@@ -19,11 +19,7 @@
*/
class SkGifCodec : public SkCodec {
public:
-
- /*
- * Checks the start of the stream to see if the image is a gif
- */
- static bool IsGif(SkStream*);
+ static bool IsGif(const void*, size_t);
/*
* Assumes IsGif was called and returned true
diff --git a/src/codec/SkCodec_libico.cpp b/src/codec/SkCodec_libico.cpp
index e1314d7..660576a 100644
--- a/src/codec/SkCodec_libico.cpp
+++ b/src/codec/SkCodec_libico.cpp
@@ -55,11 +55,10 @@
/*
* Checks the start of the stream to see if the image is an Ico or Cur
*/
-bool SkIcoCodec::IsIco(SkStream* stream) {
+bool SkIcoCodec::IsIco(const void* buffer, size_t bytesRead) {
const char icoSig[] = { '\x00', '\x00', '\x01', '\x00' };
const char curSig[] = { '\x00', '\x00', '\x02', '\x00' };
- char buffer[sizeof(icoSig)];
- return stream->read(buffer, sizeof(icoSig)) == sizeof(icoSig) &&
+ return bytesRead >= sizeof(icoSig) &&
(!memcmp(buffer, icoSig, sizeof(icoSig)) ||
!memcmp(buffer, curSig, sizeof(curSig)));
}
@@ -176,10 +175,8 @@
bytesRead += size;
// Check if the embedded codec is bmp or png and create the codec
- const bool isPng = SkPngCodec::IsPng(embeddedStream);
- SkAssertResult(embeddedStream->rewind());
SkCodec* codec = nullptr;
- if (isPng) {
+ if (SkPngCodec::IsPng((const char*) data->bytes(), data->size())) {
codec = SkPngCodec::NewFromStream(embeddedStream.detach());
} else {
codec = SkBmpCodec::NewFromIco(embeddedStream.detach());
diff --git a/src/codec/SkCodec_libico.h b/src/codec/SkCodec_libico.h
index a1d99b8..9a3f248 100644
--- a/src/codec/SkCodec_libico.h
+++ b/src/codec/SkCodec_libico.h
@@ -15,11 +15,7 @@
*/
class SkIcoCodec : public SkCodec {
public:
-
- /*
- * Checks the start of the stream to see if the image is a Ico or Cur
- */
- static bool IsIco(SkStream*);
+ static bool IsIco(const void*, size_t);
/*
* Assumes IsIco was called and returned true
diff --git a/src/codec/SkCodec_libpng.cpp b/src/codec/SkCodec_libpng.cpp
index a611705..744bca4 100644
--- a/src/codec/SkCodec_libpng.cpp
+++ b/src/codec/SkCodec_libpng.cpp
@@ -206,17 +206,8 @@
// Creation
///////////////////////////////////////////////////////////////////////////////
-#define PNG_BYTES_TO_CHECK 4
-
-bool SkPngCodec::IsPng(SkStream* stream) {
- char buf[PNG_BYTES_TO_CHECK];
- if (stream->read(buf, PNG_BYTES_TO_CHECK) != PNG_BYTES_TO_CHECK) {
- return false;
- }
- if (png_sig_cmp((png_bytep) buf, (png_size_t)0, PNG_BYTES_TO_CHECK)) {
- return false;
- }
- return true;
+bool SkPngCodec::IsPng(const char* buf, size_t bytesRead) {
+ return !png_sig_cmp((png_bytep) buf, (png_size_t)0, bytesRead);
}
// Reads the header and initializes the output fields, if not NULL.
diff --git a/src/codec/SkCodec_libpng.h b/src/codec/SkCodec_libpng.h
index c2a5f4a..78f1c5d 100644
--- a/src/codec/SkCodec_libpng.h
+++ b/src/codec/SkCodec_libpng.h
@@ -19,7 +19,7 @@
class SkPngCodec : public SkCodec {
public:
- static bool IsPng(SkStream*);
+ static bool IsPng(const char*, size_t);
// Assume IsPng was called and returned true.
static SkCodec* NewFromStream(SkStream*, SkPngChunkReader* = NULL);
diff --git a/src/codec/SkCodec_wbmp.cpp b/src/codec/SkCodec_wbmp.cpp
index 8f8763f..d734ad3 100644
--- a/src/codec/SkCodec_wbmp.cpp
+++ b/src/codec/SkCodec_wbmp.cpp
@@ -9,6 +9,7 @@
#include "SkCodecPriv.h"
#include "SkColorPriv.h"
#include "SkColorTable.h"
+#include "SkData.h"
#include "SkStream.h"
#include "SkCodec_wbmp.h"
@@ -151,8 +152,10 @@
return kSuccess;
}
-bool SkWbmpCodec::IsWbmp(SkStream* stream) {
- return read_header(stream, nullptr);
+bool SkWbmpCodec::IsWbmp(const void* buffer, size_t bytesRead) {
+ SkAutoTUnref<SkData> data(SkData::NewWithoutCopy(buffer, bytesRead));
+ SkMemoryStream stream(data);
+ return read_header(&stream, nullptr);
}
SkCodec* SkWbmpCodec::NewFromStream(SkStream* stream) {
diff --git a/src/codec/SkCodec_wbmp.h b/src/codec/SkCodec_wbmp.h
index f54dd0f..fb062c9 100644
--- a/src/codec/SkCodec_wbmp.h
+++ b/src/codec/SkCodec_wbmp.h
@@ -13,7 +13,7 @@
class SkWbmpCodec final : public SkCodec {
public:
- static bool IsWbmp(SkStream*);
+ static bool IsWbmp(const void*, size_t);
/*
* Assumes IsWbmp was called and returned true
diff --git a/src/codec/SkJpegCodec.cpp b/src/codec/SkJpegCodec.cpp
index 6e2fc84..6ea13b7 100644
--- a/src/codec/SkJpegCodec.cpp
+++ b/src/codec/SkJpegCodec.cpp
@@ -23,11 +23,9 @@
#include "jpeglib.h"
}
-bool SkJpegCodec::IsJpeg(SkStream* stream) {
+bool SkJpegCodec::IsJpeg(const void* buffer, size_t bytesRead) {
static const uint8_t jpegSig[] = { 0xFF, 0xD8, 0xFF };
- char buffer[sizeof(jpegSig)];
- return stream->read(buffer, sizeof(jpegSig)) == sizeof(jpegSig) &&
- !memcmp(buffer, jpegSig, sizeof(jpegSig));
+ return bytesRead >= 3 && !memcmp(buffer, jpegSig, sizeof(jpegSig));
}
bool SkJpegCodec::ReadHeader(SkStream* stream, SkCodec** codecOut,
diff --git a/src/codec/SkJpegCodec.h b/src/codec/SkJpegCodec.h
index 687cf4b..ed94b61 100644
--- a/src/codec/SkJpegCodec.h
+++ b/src/codec/SkJpegCodec.h
@@ -25,12 +25,7 @@
*/
class SkJpegCodec : public SkCodec {
public:
-
- /*
- * Checks the start of the stream to see if the image is a jpeg
- * Does not take ownership of the stream
- */
- static bool IsJpeg(SkStream*);
+ static bool IsJpeg(const void*, size_t);
/*
* Assumes IsJpeg was called and returned true
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index 3c61b93..2137877 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -20,26 +20,26 @@
#include "webp/decode.h"
#include "webp/encode.h"
-bool SkWebpCodec::IsWebp(SkStream* stream) {
+bool SkWebpCodec::IsWebp(const void* buf, size_t bytesRead) {
// WEBP starts with the following:
// RIFFXXXXWEBPVP
// Where XXXX is unspecified.
- const char LENGTH = 14;
- char bytes[LENGTH];
- if (stream->read(&bytes, LENGTH) != LENGTH) {
- return false;
- }
- return !memcmp(bytes, "RIFF", 4) && !memcmp(&bytes[8], "WEBPVP", 6);
+ const char* bytes = static_cast<const char*>(buf);
+ return bytesRead >= 14 && !memcmp(bytes, "RIFF", 4) && !memcmp(&bytes[8], "WEBPVP", 6);
}
-static const size_t WEBP_VP8_HEADER_SIZE = 30;
-
// Parse headers of RIFF container, and check for valid Webp (VP8) content.
// NOTE: This calls peek instead of read, since onGetPixels will need these
// bytes again.
static bool webp_parse_header(SkStream* stream, SkImageInfo* info) {
unsigned char buffer[WEBP_VP8_HEADER_SIZE];
- if (!stream->peek(buffer, WEBP_VP8_HEADER_SIZE)) {
+ SkASSERT(WEBP_VP8_HEADER_SIZE <= SkCodec::MinBufferedBytesNeeded());
+
+ const size_t bytesPeeked = stream->peek(buffer, WEBP_VP8_HEADER_SIZE);
+ if (bytesPeeked != WEBP_VP8_HEADER_SIZE) {
+ // Use read + rewind as a backup
+ if (stream->read(buffer, WEBP_VP8_HEADER_SIZE) != WEBP_VP8_HEADER_SIZE
+ || !stream->rewind())
return false;
}
diff --git a/src/codec/SkWebpCodec.h b/src/codec/SkWebpCodec.h
index 97b3920..5c0f028 100644
--- a/src/codec/SkWebpCodec.h
+++ b/src/codec/SkWebpCodec.h
@@ -15,11 +15,13 @@
class SkStream;
+static const size_t WEBP_VP8_HEADER_SIZE = 30;
+
class SkWebpCodec final : public SkCodec {
public:
// Assumes IsWebp was called and returned true.
static SkCodec* NewFromStream(SkStream*);
- static bool IsWebp(SkStream*);
+ static bool IsWebp(const void*, size_t);
protected:
Result onGetPixels(const SkImageInfo&, void*, size_t, const Options&, SkPMColor*, int*, int*)
override;
diff --git a/tests/CodexTest.cpp b/tests/CodexTest.cpp
index 03dd16f..94eea65 100644
--- a/tests/CodexTest.cpp
+++ b/tests/CodexTest.cpp
@@ -848,6 +848,54 @@
}
#endif // PNG_READ_UNKNOWN_CHUNKS_SUPPORTED
+// Stream that can only peek up to a limit
+class LimitedPeekingMemStream : public SkStream {
+public:
+ LimitedPeekingMemStream(SkData* data, size_t limit)
+ : fStream(data)
+ , fLimit(limit) {}
+
+ size_t peek(void* buf, size_t bytes) const override {
+ return fStream.peek(buf, SkTMin(bytes, fLimit));
+ }
+ size_t read(void* buf, size_t bytes) override {
+ return fStream.read(buf, bytes);
+ }
+ bool rewind() override {
+ return fStream.rewind();
+ }
+ bool isAtEnd() const override {
+ return false;
+ }
+private:
+ SkMemoryStream fStream;
+ const size_t fLimit;
+};
+
+// Test that even if webp_parse_header fails to peek enough, it will fall back to read()
+// + rewind() and succeed.
+DEF_TEST(Codec_webp_peek, r) {
+ const char* path = "baby_tux.webp";
+ SkString fullPath(GetResourcePath(path));
+ SkAutoTUnref<SkData> data(SkData::NewFromFileName(fullPath.c_str()));
+ if (!data) {
+ SkDebugf("Missing resource '%s'\n", path);
+ return;
+ }
+
+ // The limit is less than webp needs to peek or read.
+ SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(new LimitedPeekingMemStream(data, 25)));
+ REPORTER_ASSERT(r, codec);
+
+ test_info(r, codec, codec->getInfo(), SkCodec::kSuccess, nullptr);
+
+ // Similarly, a stream which does not peek should still succeed.
+ codec.reset(SkCodec::NewFromStream(new LimitedPeekingMemStream(data, 0)));
+ REPORTER_ASSERT(r, codec);
+
+ test_info(r, codec, codec->getInfo(), SkCodec::kSuccess, nullptr);
+}
+
// SkCodec's wbmp decoder was initially more restrictive than SkImageDecoder.
// It required the second byte to be zero. But SkImageDecoder allowed a couple
// of bits to be 1 (so long as they do not overlap with 0x9F). Test that
@@ -877,3 +925,32 @@
}
test_info(r, codec, codec->getInfo(), SkCodec::kSuccess, nullptr);
}
+
+// wbmp images have a header that can be arbitrarily large, depending on the
+// size of the image. We cap the size at 65535, meaning we only need to look at
+// 8 bytes to determine whether we can read the image. This is important
+// because SkCodec only passes 14 bytes to SkWbmpCodec to determine whether the
+// image is a wbmp.
+DEF_TEST(Codec_wbmp_max_size, r) {
+ const unsigned char maxSizeWbmp[] = { 0x00, 0x00, // Header
+ 0x83, 0xFF, 0x7F, // W: 65535
+ 0x83, 0xFF, 0x7F }; // H: 65535
+ SkAutoTDelete<SkStream> stream(new SkMemoryStream(maxSizeWbmp, sizeof(maxSizeWbmp), false));
+ SkAutoTDelete<SkCodec> codec(SkCodec::NewFromStream(stream.detach()));
+
+ REPORTER_ASSERT(r, codec);
+ if (!codec) return;
+
+ REPORTER_ASSERT(r, codec->getInfo().width() == 65535);
+ REPORTER_ASSERT(r, codec->getInfo().height() == 65535);
+
+ // Now test an image which is too big. Any image with a larger header (i.e.
+ // has bigger width/height) is also too big.
+ const unsigned char tooBigWbmp[] = { 0x00, 0x00, // Header
+ 0x84, 0x80, 0x00, // W: 65536
+ 0x84, 0x80, 0x00 }; // H: 65536
+ stream.reset(new SkMemoryStream(tooBigWbmp, sizeof(tooBigWbmp), false));
+ codec.reset(SkCodec::NewFromStream(stream.detach()));
+
+ REPORTER_ASSERT(r, !codec);
+}