Chromium Code Reviews
chromiumcodereview-hr@appspot.gserviceaccount.com (chromiumcodereview-hr) | Please choose your nickname with Settings | Help | Chromium Project | Gerrit Changes | Sign out
(74)

Unified Diff: src/codec/SkWebpCodec.cpp

Issue 2311793004: Use demux API in SkWebpCodec (Closed)
Patch Set: Forward declare Created 4 years, 3 months ago
Use n/p to move between diff chunks; N/P to move between comments. Draft comments are only viewable by you.
Jump to:
View side-by-side diff with in-line comments
Download patch
« no previous file with comments | « src/codec/SkWebpCodec.h ('k') | no next file » | no next file with comments »
Expand Comments ('e') | Collapse Comments ('c') | Show Comments Hide Comments ('s')
Index: src/codec/SkWebpCodec.cpp
diff --git a/src/codec/SkWebpCodec.cpp b/src/codec/SkWebpCodec.cpp
index c28d077bb329826750ebca4bb92ac2f005e2ac02..77af3dce39d29112bb1884ae93b1742a1f14bf6f 100644
--- a/src/codec/SkWebpCodec.cpp
+++ b/src/codec/SkWebpCodec.cpp
@@ -36,24 +36,61 @@ bool SkWebpCodec::IsWebp(const void* buf, size_t bytesRead) {
SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) {
SkAutoTDelete<SkStream> streamDeleter(stream);
- unsigned char 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())
+ // Webp demux needs a contiguous data buffer.
+ sk_sp<SkData> data = nullptr;
+ if (stream->getMemoryBase()) {
scroggo 2016/09/07 14:20:51 I think we have some common code to handle this ca
msarett 2016/09/07 16:47:10 Ahh yes. The common code always performs a copy,
+ data = SkData::MakeWithoutCopy(stream->getMemoryBase(), stream->getLength());
scroggo 2016/09/07 14:20:51 Maybe add a comment here that this is safe because
msarett 2016/09/07 16:47:10 SGTM
+ } else if (stream->hasLength()) {
+ size_t size = stream->getLength();
+ data = SkData::MakeUninitialized(size);
+ stream->read(data->writable_data(), size);
+
+ // Go ahead and delete the stream, we don't need it anymore.
+ streamDeleter.reset(nullptr);
+ } else {
+ SkDynamicMemoryWStream writeStream;
+ static constexpr size_t kChunkSize = 8192; // Arbitrary
+ uint8_t buffer[kChunkSize];
+ size_t bytesRead;
+ do {
+ bytesRead = stream->read(buffer, kChunkSize);
+ writeStream.write(buffer, bytesRead);
+ } while (bytesRead == kChunkSize);
+ data.reset(writeStream.copyToData());
+
+ streamDeleter.reset(nullptr);
+ }
+
+ WebPData webpData = { data->bytes(), data->size() };
+ SkAutoTCallVProc<WebPDemuxer, WebPDemuxDelete> demux(WebPDemuxPartial(&webpData, nullptr));
scroggo 2016/09/07 14:20:51 I think this is OK (in fact I think it's what chro
msarett 2016/09/07 16:47:10 Agree that this is weird... Don't think it's nice
+ if (nullptr == demux) {
+ return nullptr;
+ }
+
+ WebPChunkIterator chunkIterator;
+ SkAutoTCallVProc<WebPChunkIterator, WebPDemuxReleaseChunkIterator> autoCI(&chunkIterator);
+ sk_sp<SkColorSpace> colorSpace = nullptr;
+ if (WebPDemuxGetChunk(demux, "ICCP", 1, &chunkIterator)) {
+ colorSpace = SkColorSpace::NewICC(chunkIterator.chunk.bytes, chunkIterator.chunk.size);
+ }
+
+ if (!colorSpace) {
+ colorSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
+ }
+
+ WebPIterator frame;
+ SkAutoTCallVProc<WebPIterator, WebPDemuxReleaseIterator> autoFrame(&frame);
+ if (!WebPDemuxGetFrame(demux, 1, &frame)) {
scroggo 2016/09/07 14:20:51 I forget, does this decode the frame? I guess not,
msarett 2016/09/07 16:47:10 Yes, this won't decode the image - just sets us up
return nullptr;
}
WebPBitstreamFeatures features;
- VP8StatusCode status = WebPGetFeatures(buffer, WEBP_VP8_HEADER_SIZE, &features);
+ VP8StatusCode status = WebPGetFeatures(frame.fragment.bytes, frame.fragment.size, &features);
if (VP8_STATUS_OK != status) {
- return nullptr; // Invalid WebP file.
+ return nullptr;
}
- // sanity check for image size that's about to be decoded.
+ // Sanity check for image size that's about to be decoded.
{
const int64_t size = sk_64_mul(features.width, features.height);
if (!sk_64_isS32(size)) {
@@ -98,30 +135,9 @@ SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) {
return nullptr;
}
- // FIXME (msarett):
- // Temporary strategy for getting ICC profiles from webps. Once the incremental decoding
- // API lands, we will use the WebPDemuxer to manage the entire decode.
- sk_sp<SkColorSpace> colorSpace = nullptr;
- const void* memory = stream->getMemoryBase();
- if (memory) {
- WebPData data = { (const uint8_t*) memory, stream->getLength() };
- WebPDemuxState state;
- SkAutoTCallVProc<WebPDemuxer, WebPDemuxDelete> demux(WebPDemuxPartial(&data, &state));
-
- WebPChunkIterator chunkIterator;
- SkAutoTCallVProc<WebPChunkIterator, WebPDemuxReleaseChunkIterator> autoCI(&chunkIterator);
- if (demux && WebPDemuxGetChunk(demux, "ICCP", 1, &chunkIterator)) {
- colorSpace = SkColorSpace::NewICC(chunkIterator.chunk.bytes, chunkIterator.chunk.size);
- }
- }
-
- if (!colorSpace) {
- colorSpace = SkColorSpace::NewNamed(SkColorSpace::kSRGB_Named);
- }
-
SkEncodedInfo info = SkEncodedInfo::Make(color, alpha, 8);
- return new SkWebpCodec(features.width, features.height, info, colorSpace,
- streamDeleter.release());
+ return new SkWebpCodec(features.width, features.height, info, std::move(colorSpace),
+ streamDeleter.release(), demux.release(), std::move(data));
scroggo 2016/09/07 14:20:51 Sometimes streamDeleter will hold null. I forgot,
msarett 2016/09/07 16:47:10 I hadn't thought about this, but now that I look c
}
// This version is slightly different from SkCodecPriv's version of conversion_possible. It
@@ -158,7 +174,6 @@ bool SkWebpCodec::onDimensionsSupported(const SkISize& dim) {
&& dim.height() >= 1 && dim.height() <= info.height();
}
-
static WEBP_CSP_MODE webp_decode_mode(SkColorType ct, bool premultiply) {
switch (ct) {
case kBGRA_8888_SkColorType:
@@ -172,11 +187,6 @@ static WEBP_CSP_MODE webp_decode_mode(SkColorType ct, bool premultiply) {
}
}
-// The WebP decoding API allows us to incrementally pass chunks of bytes as we receive them to the
-// decoder with WebPIAppend. In order to do so, we need to read chunks from the SkStream. This size
-// is arbitrary.
-static const size_t BUFFER_SIZE = 4096;
-
bool SkWebpCodec::onGetValidSubset(SkIRect* desiredSubset) const {
if (!desiredSubset) {
return false;
@@ -263,33 +273,31 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
config.output.u.RGBA.size = dstInfo.getSafeSize(rowBytes);
config.output.is_external_memory = 1;
+ WebPIterator frame;
+ SkAutoTCallVProc<WebPIterator, WebPDemuxReleaseIterator> autoFrame(&frame);
+ // If this succeeded in NewFromStream(), it should succeed again here.
+ SkAssertResult(WebPDemuxGetFrame(fDemux, 1, &frame));
+
SkAutoTCallVProc<WebPIDecoder, WebPIDelete> idec(WebPIDecode(nullptr, 0, &config));
if (!idec) {
return kInvalidInput;
}
- SkAutoTMalloc<uint8_t> storage(BUFFER_SIZE);
- uint8_t* buffer = storage.get();
- while (true) {
- const size_t bytesRead = stream()->read(buffer, BUFFER_SIZE);
- if (0 == bytesRead) {
- WebPIDecGetRGB(idec, rowsDecoded, NULL, NULL, NULL);
+ switch (WebPIUpdate(idec, frame.fragment.bytes, frame.fragment.size)) {
msarett 2016/09/06 23:47:11 It's kind of ugly to create the WebPIDecoder with
scroggo 2016/09/07 14:20:51 It's the same as we did before though, right?
msarett 2016/09/07 16:47:10 Yes, true. Before it seemed to make a little more
+ case VP8_STATUS_OK:
+ return kSuccess;
+ case VP8_STATUS_SUSPENDED:
+ WebPIDecGetRGB(idec, rowsDecoded, nullptr, nullptr, nullptr);
return kIncompleteInput;
- }
-
- switch (WebPIAppend(idec, buffer, bytesRead)) {
- case VP8_STATUS_OK:
- return kSuccess;
- case VP8_STATUS_SUSPENDED:
- // Break out of the switch statement. Continue the loop.
- break;
- default:
- return kInvalidInput;
- }
+ default:
+ return kInvalidInput;
}
}
SkWebpCodec::SkWebpCodec(int width, int height, const SkEncodedInfo& info,
- sk_sp<SkColorSpace> colorSpace, SkStream* stream)
- : INHERITED(width, height, info, stream, colorSpace)
+ sk_sp<SkColorSpace> colorSpace, SkStream* stream, WebPDemuxer* demux,
+ sk_sp<SkData> data)
+ : INHERITED(width, height, info, stream, std::move(colorSpace))
+ , fDemux(demux)
+ , fData(std::move(data))
{}
« no previous file with comments | « src/codec/SkWebpCodec.h ('k') | no next file » | no next file with comments »

Powered by Google App Engine
This is Rietveld 408576698