AuroraMiddleware/skia2
1
0
Fork 0
Code Issues Pull Requests Projects Releases Wiki Activity

Revert "Reland "Respect canvas size and frame offset in webp decoder""

Browse Source 
This reverts commit 604971e39a.

Reason for revert: Strange vk failures

Original change's description:
> Reland "Respect canvas size and frame offset in webp decoder"
> 
> Original Change Reviewed At:
> https://skia-review.googlesource.com/c/7800
> 
> CQ_INCLUDE_TRYBOTS=skia.primary:Test-Ubuntu-Clang-GCE-CPU-AVX2-x86_64-Debug-MSAN
> 
> BUG=skia:6185
> 
> Change-Id: I92baa9070e15ef3c62dd347c08c906c2715dda10
> Reviewed-on: https://skia-review.googlesource.com/8050
> Reviewed-by: Matt Sarett <msarett@google.com>
> Commit-Queue: Matt Sarett <msarett@google.com>
> 

TBR=msarett@google.com,reviews@skia.org
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=skia:6185
CQ_INCLUDE_TRYBOTS=skia.primary:Test-Ubuntu-Clang-GCE-CPU-AVX2-x86_64-Debug-MSAN

Change-Id: Ice93b62c55ea13fce83140567be16225ff0e2fdb
Reviewed-on: https://skia-review.googlesource.com/8123
Commit-Queue: Matt Sarett <msarett@google.com>
Reviewed-by: Matt Sarett <msarett@google.com>
This commit is contained in:
Matt Sarett 2017-02-07 16:39:23 +00:00 committed by Skia Commit-Bot
parent 4f77bd0505
commit 2bb94e8147
5 changed files with 275 additions and 306 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

2
infra/bots/assets/skimage/VERSION
View File

@ -1 +1 @@
20 19

378
infra/bots/tasks.json
View File

File diff suppressed because it is too large Load Diff

BIN
resources/invalid_images/invalid-offset.webp
View File

Binary file not shown.
Side by Side

Before

Width:  |  Height:  |  Size: 374 B

199
src/codec/SkWebpCodec.cpp
View File

@ -7,10 +7,9 @@
#include "SkCodecPriv.h" #include "SkCodecPriv.h"
#include "SkColorSpaceXform.h" #include "SkColorSpaceXform.h"
#include "SkSampler.h" #include "SkWebpCodec.h"
#include "SkStreamPriv.h" #include "SkStreamPriv.h"
#include "SkTemplates.h" #include "SkTemplates.h"
#include "SkWebpCodec.h"
// A WebP decoder on top of (subset of) libwebp // A WebP decoder on top of (subset of) libwebp
// For more information on WebP image format, and libwebp library, see: // For more information on WebP image format, and libwebp library, see:
@ -60,21 +59,6 @@ SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) {
return nullptr; return nullptr;
} }
const int width = WebPDemuxGetI(demux, WEBP_FF_CANVAS_WIDTH);
const int height = WebPDemuxGetI(demux, WEBP_FF_CANVAS_HEIGHT);
// Sanity check for image size that's about to be decoded.
{
const int64_t size = sk_64_mul(width, height);
if (!sk_64_isS32(size)) {
return nullptr;
}
// now check that if we are 4-bytes per pixel, we also don't overflow
if (sk_64_asS32(size) > (0x7FFFFFFF >> 2)) {
return nullptr;
}
}
WebPChunkIterator chunkIterator; WebPChunkIterator chunkIterator;
SkAutoTCallVProc<WebPChunkIterator, WebPDemuxReleaseChunkIterator> autoCI(&chunkIterator); SkAutoTCallVProc<WebPChunkIterator, WebPDemuxReleaseChunkIterator> autoCI(&chunkIterator);
sk_sp<SkColorSpace> colorSpace = nullptr; sk_sp<SkColorSpace> colorSpace = nullptr;
@ -89,15 +73,36 @@ SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) {
colorSpace = SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named); colorSpace = SkColorSpace::MakeNamed(SkColorSpace::kSRGB_Named);
} }
// Get the first frame and its "features" to determine the color and alpha types. // Since we do not yet support animation, we get the |width|, |height|, |color|, and |alpha|
// Since we do not yet support animated webp, this is the only frame that we will // from the first frame. It's the only frame we will decode.
// decode. //
// TODO:
// When we support animation, we'll want to report the canvas width and canvas height instead.
// We can get these from the |demux| directly.
// What |color| and |alpha| will we want to report though? WebP allows different frames
// to be encoded in different ways, making the encoded format difficult to describe.
WebPIterator frame; WebPIterator frame;
SkAutoTCallVProc<WebPIterator, WebPDemuxReleaseIterator> autoFrame(&frame); SkAutoTCallVProc<WebPIterator, WebPDemuxReleaseIterator> autoFrame(&frame);
if (!WebPDemuxGetFrame(demux, 1, &frame)) { if (!WebPDemuxGetFrame(demux, 1, &frame)) {
return nullptr; return nullptr;
} }
// Sanity check for image size that's about to be decoded.
{
const int64_t size = sk_64_mul(frame.width, frame.height);
if (!sk_64_isS32(size)) {
return nullptr;
}
// now check that if we are 4-bytes per pixel, we also don't overflow
if (sk_64_asS32(size) > (0x7FFFFFFF >> 2)) {
return nullptr;
}
}
// TODO:
// The only reason we actually need to call WebPGetFeatures() is to get the |features.format|.
// This call actually re-reads the frame header. Should we suggest that libwebp expose
// the format on the |frame|?
WebPBitstreamFeatures features; WebPBitstreamFeatures features;
VP8StatusCode status = WebPGetFeatures(frame.fragment.bytes, frame.fragment.size, &features); VP8StatusCode status = WebPGetFeatures(frame.fragment.bytes, frame.fragment.size, &features);
if (VP8_STATUS_OK != status) { if (VP8_STATUS_OK != status) {
@ -108,8 +113,8 @@ SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) {
SkEncodedInfo::Alpha alpha; SkEncodedInfo::Alpha alpha;
switch (features.format) { switch (features.format) {
case 0: case 0:
// This indicates a "mixed" format. We could see this for // This indicates a "mixed" format. We would see this for
// animated webps (multiple fragments). // animated webps or for webps encoded in multiple fragments.
// I believe that this is a rare case. // I believe that this is a rare case.
// We could also guess kYUV here, but I think it makes more // We could also guess kYUV here, but I think it makes more
// sense to guess kBGRA which is likely closer to the final // sense to guess kBGRA which is likely closer to the final
@ -120,7 +125,7 @@ SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) {
break; break;
case 1: case 1:
// This is the lossy format (YUV). // This is the lossy format (YUV).
if (SkToBool(features.has_alpha) || frame.width != width || frame.height != height) { if (SkToBool(features.has_alpha)) {
color = SkEncodedInfo::kYUVA_Color; color = SkEncodedInfo::kYUVA_Color;
alpha = SkEncodedInfo::kUnpremul_Alpha; alpha = SkEncodedInfo::kUnpremul_Alpha;
} else { } else {
@ -138,9 +143,9 @@ SkCodec* SkWebpCodec::NewFromStream(SkStream* stream) {
} }
SkEncodedInfo info = SkEncodedInfo::Make(color, alpha, 8); SkEncodedInfo info = SkEncodedInfo::Make(color, alpha, 8);
SkWebpCodec* codecOut = new SkWebpCodec(width, height, info, std::move(colorSpace), SkWebpCodec* codecOut = new SkWebpCodec(features.width, features.height, info,
streamDeleter.release(), demux.release(), std::move(colorSpace), streamDeleter.release(),
std::move(data)); demux.release(), std::move(data));
codecOut->setUnsupportedICC(unsupportedICC); codecOut->setUnsupportedICC(unsupportedICC);
return codecOut; return codecOut;
} }
@ -212,85 +217,48 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
// Free any memory associated with the buffer. Must be called last, so we declare it first. // Free any memory associated with the buffer. Must be called last, so we declare it first.
SkAutoTCallVProc<WebPDecBuffer, WebPFreeDecBuffer> autoFree(&(config.output)); SkAutoTCallVProc<WebPDecBuffer, WebPFreeDecBuffer> autoFree(&(config.output));
WebPIterator frame; SkIRect bounds = SkIRect::MakeSize(this->getInfo().dimensions());
SkAutoTCallVProc<WebPIterator, WebPDemuxReleaseIterator> autoFrame(&frame);
// If this succeeded in NewFromStream(), it should succeed again here.
SkAssertResult(WebPDemuxGetFrame(fDemux, 1, &frame));
// Get the frameRect. libwebp will have already signaled an error if this is not fully
// contained by the canvas.
auto frameRect = SkIRect::MakeXYWH(frame.x_offset, frame.y_offset, frame.width, frame.height);
SkASSERT(this->getInfo().bounds().contains(frameRect));
bool frameIsSubset = frameRect.size() != this->getInfo().dimensions();
if (frameIsSubset) {
SkSampler::Fill(dstInfo, dst, rowBytes, 0, options.fZeroInitialized);
}
int dstX = frameRect.x();
int dstY = frameRect.y();
int subsetWidth = frameRect.width();
int subsetHeight = frameRect.height();
if (options.fSubset) { if (options.fSubset) {
SkIRect subset = *options.fSubset; // Caller is requesting a subset.
SkASSERT(this->getInfo().bounds().contains(subset)); if (!bounds.contains(*options.fSubset)) {
SkASSERT(SkIsAlign2(subset.fLeft) && SkIsAlign2(subset.fTop)); // The subset is out of bounds.
SkASSERT(this->getValidSubset(&subset) && subset == *options.fSubset); return kInvalidParameters;
if (!SkIRect::IntersectsNoEmptyCheck(subset, frameRect)) {
return kSuccess;
} }
int minXOffset = SkTMin(dstX, subset.x()); bounds = *options.fSubset;
int minYOffset = SkTMin(dstY, subset.y());
dstX -= minXOffset;
dstY -= minYOffset;
frameRect.offset(-minXOffset, -minYOffset);
subset.offset(-minXOffset, -minYOffset);
// Just like we require that the requested subset x and y offset are even, libwebp // This is tricky. libwebp snaps the top and left to even values. We could let libwebp
// guarantees that the frame x and y offset are even (it's actually impossible to specify // do the snap, and return a subset which is a different one than requested. The problem
// an odd frame offset). So we can still guarantee that the adjusted offsets are even. // with that approach is that the caller may try to stitch subsets together, and if we
SkASSERT(SkIsAlign2(subset.fLeft) && SkIsAlign2(subset.fTop)); // returned different subsets than requested, there would be artifacts at the boundaries.
// Instead, we report that we cannot support odd values for top and left..
if (!SkIsAlign2(bounds.fLeft) || !SkIsAlign2(bounds.fTop)) {
return kInvalidParameters;
}
SkIRect intersection; #ifdef SK_DEBUG
SkAssertResult(intersection.intersect(frameRect, subset)); {
subsetWidth = intersection.width(); // Make a copy, since getValidSubset can change its input.
subsetHeight = intersection.height(); SkIRect subset(bounds);
// That said, getValidSubset should *not* change its input, in this case; otherwise
// getValidSubset does not match the actual subsets we can do.
SkASSERT(this->getValidSubset(&subset) && subset == bounds);
}
#endif
config.options.use_cropping = 1; config.options.use_cropping = 1;
config.options.crop_left = subset.x(); config.options.crop_left = bounds.fLeft;
config.options.crop_top = subset.y(); config.options.crop_top = bounds.fTop;
config.options.crop_width = subsetWidth; config.options.crop_width = bounds.width();
config.options.crop_height = subsetHeight; config.options.crop_height = bounds.height();
} }
// Ignore the frame size and offset when determining if scaling is necessary. SkISize dstDimensions = dstInfo.dimensions();
int scaledWidth = subsetWidth; if (bounds.size() != dstDimensions) {
int scaledHeight = subsetHeight; // Caller is requesting scaling.
SkISize srcSize = options.fSubset ? options.fSubset->size() : this->getInfo().dimensions();
if (srcSize != dstInfo.dimensions()) {
config.options.use_scaling = 1; config.options.use_scaling = 1;
config.options.scaled_width = dstDimensions.width();
if (frameIsSubset) { config.options.scaled_height = dstDimensions.height();
float scaleX = ((float) dstInfo.width()) / srcSize.width();
float scaleY = ((float) dstInfo.height()) / srcSize.height();
// We need to be conservative here and floor rather than round.
// Otherwise, we may find ourselves decoding off the end of memory.
dstX = scaleX * dstX;
scaledWidth = scaleX * scaledWidth;
dstY = scaleY * dstY;
scaledHeight = scaleY * scaledHeight;
if (0 == scaledWidth || 0 == scaledHeight) {
return kSuccess;
}
} else {
scaledWidth = dstInfo.width();
scaledHeight = dstInfo.height();
}
config.options.scaled_width = scaledWidth;
config.options.scaled_height = scaledHeight;
} }
// Swizzling between RGBA and BGRA is zero cost in a color transform. So when we have a // Swizzling between RGBA and BGRA is zero cost in a color transform. So when we have a
@ -306,17 +274,21 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
// does not provide a row-by-row API. This is a shame particularly in the F16 case, // does not provide a row-by-row API. This is a shame particularly in the F16 case,
// where we need to allocate an extra image-sized buffer. // where we need to allocate an extra image-sized buffer.
SkAutoTMalloc<uint32_t> pixels; SkAutoTMalloc<uint32_t> pixels;
bool isF16 = kRGBA_F16_SkColorType == dstInfo.colorType(); if (kRGBA_F16_SkColorType == dstInfo.colorType()) {
void* webpDst = isF16 ? pixels.reset(dstInfo.width() * dstInfo.height()) : dst; pixels.reset(dstDimensions.width() * dstDimensions.height());
size_t webpRowBytes = isF16 ? dstInfo.width() * sizeof(uint32_t) : rowBytes; config.output.u.RGBA.rgba = (uint8_t*) pixels.get();
size_t totalBytes = isF16 ? webpRowBytes * dstInfo.height() : dstInfo.getSafeSize(webpRowBytes); config.output.u.RGBA.stride = (int) dstDimensions.width() * sizeof(uint32_t);
size_t dstBpp = SkColorTypeBytesPerPixel(dstInfo.colorType()); config.output.u.RGBA.size = config.output.u.RGBA.stride * dstDimensions.height();
size_t webpBpp = isF16 ? sizeof(uint32_t) : dstBpp; } else {
config.output.u.RGBA.rgba = (uint8_t*) dst;
config.output.u.RGBA.stride = (int) rowBytes;
config.output.u.RGBA.size = dstInfo.getSafeSize(rowBytes);
}
size_t offset = dstX * webpBpp + dstY * webpRowBytes; WebPIterator frame;
config.output.u.RGBA.rgba = SkTAddOffset<uint8_t>(webpDst, offset); SkAutoTCallVProc<WebPIterator, WebPDemuxReleaseIterator> autoFrame(&frame);
config.output.u.RGBA.stride = (int) webpRowBytes; // If this succeeded in NewFromStream(), it should succeed again here.
config.output.u.RGBA.size = totalBytes - offset; SkAssertResult(WebPDemuxGetFrame(fDemux, 1, &frame));
SkAutoTCallVProc<WebPIDecoder, WebPIDelete> idec(WebPIDecode(nullptr, 0, &config)); SkAutoTCallVProc<WebPIDecoder, WebPIDelete> idec(WebPIDecode(nullptr, 0, &config));
if (!idec) { if (!idec) {
@ -327,12 +299,12 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
SkCodec::Result result; SkCodec::Result result;
switch (WebPIUpdate(idec, frame.fragment.bytes, frame.fragment.size)) { switch (WebPIUpdate(idec, frame.fragment.bytes, frame.fragment.size)) {
case VP8_STATUS_OK: case VP8_STATUS_OK:
rowsDecoded = scaledHeight; rowsDecoded = dstInfo.height();
result = kSuccess; result = kSuccess;
break; break;
case VP8_STATUS_SUSPENDED: case VP8_STATUS_SUSPENDED:
WebPIDecGetRGB(idec, &rowsDecoded, nullptr, nullptr, nullptr); WebPIDecGetRGB(idec, rowsDecodedPtr, nullptr, nullptr, nullptr);
*rowsDecodedPtr = rowsDecoded + dstY; rowsDecoded = *rowsDecodedPtr;
result = kIncompleteInput; result = kIncompleteInput;
break; break;
default: default:
@ -344,15 +316,14 @@ SkCodec::Result SkWebpCodec::onGetPixels(const SkImageInfo& dstInfo, void* dst,
SkAlphaType xformAlphaType = select_xform_alpha(dstInfo.alphaType(), SkAlphaType xformAlphaType = select_xform_alpha(dstInfo.alphaType(),
this->getInfo().alphaType()); this->getInfo().alphaType());
uint32_t* xformSrc = (uint32_t*) config.output.u.RGBA.rgba; uint32_t* src = (uint32_t*) config.output.u.RGBA.rgba;
void* xformDst = SkTAddOffset<void>(dst, dstBpp * dstX + rowBytes * dstY);
size_t srcRowBytes = config.output.u.RGBA.stride; size_t srcRowBytes = config.output.u.RGBA.stride;
for (int y = 0; y < rowsDecoded; y++) { for (int y = 0; y < rowsDecoded; y++) {
SkAssertResult(this->colorXform()->apply(dstColorFormat, xformDst, SkAssertResult(this->colorXform()->apply(dstColorFormat, dst,
SkColorSpaceXform::kBGRA_8888_ColorFormat, xformSrc, scaledWidth, SkColorSpaceXform::kBGRA_8888_ColorFormat, src, dstInfo.width(),
xformAlphaType)); xformAlphaType));
xformDst = SkTAddOffset<void>(xformDst, rowBytes); dst = SkTAddOffset<void>(dst, rowBytes);
xformSrc = SkTAddOffset<uint32_t>(xformSrc, srcRowBytes); src = SkTAddOffset<uint32_t>(src, srcRowBytes);
} }
} }

2
tests/CodecTest.cpp
View File

@ -662,8 +662,6 @@ DEF_TEST(Codec_Empty, r) {
test_invalid(r, "empty_images/zero-height.wbmp"); test_invalid(r, "empty_images/zero-height.wbmp");
// This image is an ico with an embedded mask-bmp. This is illegal. // This image is an ico with an embedded mask-bmp. This is illegal.
test_invalid(r, "invalid_images/mask-bmp-ico.ico"); test_invalid(r, "invalid_images/mask-bmp-ico.ico");
// It is illegal for a webp frame to not be fully contained by the canvas.
test_invalid(r, "invalid_images/invalid-offset.webp");
#if defined(SK_CODEC_DECODES_RAW) && (!defined(_WIN32)) #if defined(SK_CODEC_DECODES_RAW) && (!defined(_WIN32))
test_invalid(r, "empty_images/zero_height.tiff"); test_invalid(r, "empty_images/zero_height.tiff");
#endif #endif