Have SkWuffsCodec use two Wuffs decoders

This allows onGetFrameCount to advance the stream (and return a higher
frame count), even when in the middle of a onIncrementalDecode sequence.

Such behavior is expected by Chromium's
TestResumePartialDecodeAfterClearFrameBufferCache:

https://cs.chromium.org/chromium/src/third_party/blink/renderer/platform/image-decoders/image_decoder_test_helpers.cc?l=365&rcl=23787ba147959ebf4ad168c595d5ec87919fdbd2

Bug: skia:8235
Change-Id: Iba7267468b02ce73455b362e50d8878f53b0ff88
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/250937
Commit-Queue: Nigel Tao <nigeltao@google.com>
Commit-Queue: Mike Klein <mtklein@google.com>
Reviewed-by: Leon Scroggins <scroggo@google.com>
This commit is contained in:
Nigel Tao 2019-10-29 11:10:25 +11:00 committed by Skia Commit-Bot
parent e662695654
commit 2777cd37bf
2 changed files with 167 additions and 111 deletions

View File

@ -83,6 +83,11 @@ static SkCodecAnimation::DisposalMethod wuffs_disposal_to_skia_disposal(
}
}
static SkCodec::Result reset_and_decode_image_config(wuffs_gif__decoder* decoder,
wuffs_base__image_config* imgcfg,
wuffs_base__io_buffer* b,
SkStream* s);
// -------------------------------- Class definitions
class SkWuffsCodec;
@ -138,6 +143,37 @@ public:
const SkWuffsFrame* frame(int i) const;
private:
// It is valid, in terms of the SkCodec API, to call SkCodec::getFrameCount
// while in an incremental decode (after onStartIncrementalDecode returns
// and before the rest of the image is decoded). Some Skia users expect
// getFrameCount to increase, and the SkStream to advance, when given more
// data.
//
// On the other hand, while in an incremental decode, the underlying Wuffs
// object is suspended in a coroutine. To keep its internal proof-of-safety
// invariants consistent, there's only two things you can safely do with a
// suspended Wuffs object: resume the coroutine, or reset all state (memset
// to zero and start again).
//
// The Wuffs API provides a limited, optional form of seeking, to the start
// of an animation frame's data, but does not provide arbitrary save and
// load of its internal state whilst in the middle of an animation frame.
//
// SkWuffsCodec therefore uses two Wuffs decoders: a primary decoder
// (kIncrDecode) to support startIncrementalDecode / incrementalDecode, and
// a secondary decoder (kFrameCount) to support getFrameCount. The two
// decoders' states can change independently.
//
// As of Wuffs version 0.2, both of these decoders have the same type. A
// future Wuffs version might let us use a different type for kFrameCount,
// one that is much lighter weight (in terms of memory requirements), as it
// doesn't have to handle decompressing pixel data.
enum WhichDecoder {
kIncrDecode,
kFrameCount,
kNumDecoders,
};
// SkCodec overrides.
SkEncodedImageFormat onGetEncodedFormat() const override;
Result onGetPixels(const SkImageInfo&, void*, size_t, const Options&, int*) override;
@ -152,27 +188,32 @@ private:
int onGetRepetitionCount() override;
void readFrames();
Result seekFrame(int frameIndex);
Result seekFrame(WhichDecoder which, int frameIndex);
Result resetDecoder();
const char* decodeFrameConfig();
const char* decodeFrame();
void updateNumFullyReceivedFrames();
void onGetFrameCountInternal();
Result onIncrementalDecodeInternal(int* rowsDecoded);
SkWuffsFrameHolder fFrameHolder;
std::unique_ptr<SkStream> fStream;
std::unique_ptr<wuffs_gif__decoder, decltype(&sk_free)> fDecoder;
std::unique_ptr<uint8_t, decltype(&sk_free)> fPixbufPtr;
std::unique_ptr<uint8_t, decltype(&sk_free)> fWorkbufPtr;
size_t fWorkbufLen;
Result resetDecoder(WhichDecoder which);
const char* decodeFrameConfig(WhichDecoder which);
const char* decodeFrame(WhichDecoder which);
void updateNumFullyReceivedFrames(WhichDecoder which);
SkWuffsFrameHolder fFrameHolder;
std::unique_ptr<SkStream> fStream;
std::unique_ptr<uint8_t, decltype(&sk_free)> fPixbufPtr;
std::unique_ptr<uint8_t, decltype(&sk_free)> fWorkbufPtr;
size_t fWorkbufLen;
std::unique_ptr<wuffs_gif__decoder, decltype(&sk_free)> fDecoders[WhichDecoder::kNumDecoders];
const uint64_t fFirstFrameIOPosition;
wuffs_base__frame_config fFrameConfig;
wuffs_base__frame_config fFrameConfigs[WhichDecoder::kNumDecoders];
wuffs_base__pixel_buffer fPixelBuffer;
wuffs_base__io_buffer fIOBuffer;
// Incremental decoding state.
uint8_t* fIncrDecDst;
uint64_t fIncrDecReaderIOPosition;
size_t fIncrDecRowBytes;
bool fFirstCallToIncrementalDecode;
@ -180,17 +221,17 @@ private:
std::vector<SkWuffsFrame> fFrames;
bool fFramesComplete;
// If calling an fDecoder method returns an incomplete status, then
// fDecoder is suspended in a coroutine (i.e. waiting on I/O or halted on a
// non-recoverable error). To keep its internal proof-of-safety invariants
// consistent, there's only two things you can safely do with a suspended
// Wuffs object: resume the coroutine, or reset all state (memset to zero
// and start again).
// If calling an fDecoders[which] method returns an incomplete status, then
// fDecoders[which] is suspended in a coroutine (i.e. waiting on I/O or
// halted on a non-recoverable error). To keep its internal proof-of-safety
// invariants consistent, there's only two things you can safely do with a
// suspended Wuffs object: resume the coroutine, or reset all state (memset
// to zero and start again).
//
// If fDecoderIsSuspended, and we aren't sure that we're going to resume
// the coroutine, then we will need to call this->resetDecoder before
// calling other fDecoder methods.
bool fDecoderIsSuspended;
// If fDecoderIsSuspended[which], and we aren't sure that we're going to
// resume the coroutine, then we will need to call this->resetDecoder
// before calling other fDecoders[which] methods.
bool fDecoderIsSuspended[WhichDecoder::kNumDecoders];
uint8_t fBuffer[SK_WUFFS_CODEC_BUFFER_SIZE];
@ -260,20 +301,30 @@ SkWuffsCodec::SkWuffsCodec(SkEncodedInfo&&
nullptr),
fFrameHolder(),
fStream(std::move(stream)),
fDecoder(std::move(dec)),
fPixbufPtr(std::move(pixbuf_ptr)),
fWorkbufPtr(std::move(workbuf_ptr)),
fWorkbufLen(workbuf_len),
fDecoders{
std::move(dec),
std::unique_ptr<wuffs_gif__decoder, decltype(&sk_free)>(nullptr, sk_free),
},
fFirstFrameIOPosition(imgcfg.first_frame_io_position()),
fFrameConfig(wuffs_base__null_frame_config()),
fFrameConfigs{
wuffs_base__null_frame_config(),
wuffs_base__null_frame_config(),
},
fPixelBuffer(pixbuf),
fIOBuffer(wuffs_base__empty_io_buffer()),
fIncrDecDst(nullptr),
fIncrDecReaderIOPosition(0),
fIncrDecRowBytes(0),
fFirstCallToIncrementalDecode(false),
fNumFullyReceivedFrames(0),
fFramesComplete(false),
fDecoderIsSuspended(false) {
fDecoderIsSuspended{
false,
false,
} {
fFrameHolder.init(this, imgcfg.pixcfg.width(), imgcfg.pixcfg.height());
// Initialize fIOBuffer's fields, copying any outstanding data from iobuf to
@ -325,12 +376,12 @@ SkCodec::Result SkWuffsCodec::onStartIncrementalDecode(const SkImageInfo& d
if (options.fFrameIndex > 0 && SkColorTypeIsAlwaysOpaque(dstInfo.colorType())) {
return SkCodec::kInvalidConversion;
}
SkCodec::Result result = this->seekFrame(options.fFrameIndex);
SkCodec::Result result = this->seekFrame(WhichDecoder::kIncrDecode, options.fFrameIndex);
if (result != SkCodec::kSuccess) {
return result;
}
const char* status = this->decodeFrameConfig();
const char* status = this->decodeFrameConfig(WhichDecoder::kIncrDecode);
if (status == wuffs_base__suspension__short_read) {
return SkCodec::kIncompleteInput;
} else if (status != nullptr) {
@ -346,7 +397,7 @@ SkCodec::Result SkWuffsCodec::onStartIncrementalDecode(const SkImageInfo& d
size_t src_bytes_per_pixel = src_bits_per_pixel / 8;
// Zero-initialize Wuffs' buffer covering the frame rect.
wuffs_base__rect_ie_u32 frame_rect = fFrameConfig.bounds();
wuffs_base__rect_ie_u32 frame_rect = fFrameConfigs[WhichDecoder::kIncrDecode].bounds();
wuffs_base__table_u8 pixels = fPixelBuffer.plane(0);
for (uint32_t y = frame_rect.min_incl_y; y < frame_rect.max_excl_y; y++) {
sk_bzero(pixels.ptr + (y * pixels.stride) + (frame_rect.min_incl_x * src_bytes_per_pixel),
@ -354,6 +405,7 @@ SkCodec::Result SkWuffsCodec::onStartIncrementalDecode(const SkImageInfo& d
}
fIncrDecDst = static_cast<uint8_t*>(dst);
fIncrDecReaderIOPosition = fIOBuffer.reader_io_position();
fIncrDecRowBytes = rowBytes;
fFirstCallToIncrementalDecode = true;
return SkCodec::kSuccess;
@ -368,11 +420,34 @@ SkCodec::Result SkWuffsCodec::onIncrementalDecode(int* rowsDecoded) {
return SkCodec::kInternalError;
}
// If multiple SkCodec::incrementalDecode calls are made consecutively (or
// if SkCodec::incrementalDecode is called immediately after
// SkCodec::startIncrementalDecode), then this seek should be a no-op.
// However, it is possible to interleave SkCodec::getFrameCount calls in
// between SkCodec::incrementalDecode calls, and those other calls may
// advance the stream. This seek restores the stream to where the last
// SkCodec::startIncrementalDecode or SkCodec::incrementalDecode stopped.
if (!seek_buffer(&fIOBuffer, fStream.get(), fIncrDecReaderIOPosition)) {
return SkCodec::kInternalError;
}
SkCodec::Result result = this->onIncrementalDecodeInternal(rowsDecoded);
if (result == SkCodec::kSuccess) {
fIncrDecDst = nullptr;
fIncrDecReaderIOPosition = 0;
fIncrDecRowBytes = 0;
} else {
fIncrDecReaderIOPosition = fIOBuffer.reader_io_position();
}
return result;
}
SkCodec::Result SkWuffsCodec::onIncrementalDecodeInternal(int* rowsDecoded) {
SkCodec::Result result = SkCodec::kSuccess;
const char* status = this->decodeFrame();
bool independent;
SkAlphaType alphaType;
const int index = options().fFrameIndex;
const char* status = this->decodeFrame(WhichDecoder::kIncrDecode);
bool independent;
SkAlphaType alphaType;
const int index = options().fFrameIndex;
if (index == 0) {
independent = true;
alphaType = to_alpha_type(getEncodedInfo().opaque());
@ -403,7 +478,7 @@ SkCodec::Result SkWuffsCodec::onIncrementalDecode(int* rowsDecoded) {
}
size_t src_bytes_per_pixel = src_bits_per_pixel / 8;
wuffs_base__rect_ie_u32 frame_rect = fFrameConfig.bounds();
wuffs_base__rect_ie_u32 frame_rect = fFrameConfigs[WhichDecoder::kIncrDecode].bounds();
if (fFirstCallToIncrementalDecode) {
if (frame_rect.width() > (SIZE_MAX / src_bytes_per_pixel)) {
return SkCodec::kInternalError;
@ -415,8 +490,7 @@ SkCodec::Result SkWuffsCodec::onIncrementalDecode(int* rowsDecoded) {
// If the frame rect does not fill the output, ensure that those pixels are not
// left uninitialized.
if (independent && (bounds != this->bounds() || result != kSuccess)) {
SkSampler::Fill(dstInfo(), fIncrDecDst, fIncrDecRowBytes,
options().fZeroInitialized);
SkSampler::Fill(dstInfo(), fIncrDecDst, fIncrDecRowBytes, options().fZeroInitialized);
}
fFirstCallToIncrementalDecode = false;
} else {
@ -436,7 +510,7 @@ SkCodec::Result SkWuffsCodec::onIncrementalDecode(int* rowsDecoded) {
}
// If the frame's dirty rect is empty, no need to swizzle.
wuffs_base__rect_ie_u32 dirty_rect = fDecoder->frame_dirty_rect();
wuffs_base__rect_ie_u32 dirty_rect = fDecoders[WhichDecoder::kIncrDecode]->frame_dirty_rect();
if (!dirty_rect.is_empty()) {
wuffs_base__table_u8 pixels = fPixelBuffer.plane(0);
@ -446,16 +520,16 @@ SkCodec::Result SkWuffsCodec::onIncrementalDecode(int* rowsDecoded) {
//
// To get from the start (in the X-direction) of the image to the start
// of the dirty_rect, we adjust s by (dirty_rect.min_incl_x * src_bytes_per_pixel).
uint8_t* s = pixels.ptr + (dirty_rect.min_incl_y * pixels.stride)
+ (dirty_rect.min_incl_x * src_bytes_per_pixel);
uint8_t* s = pixels.ptr + (dirty_rect.min_incl_y * pixels.stride) +
(dirty_rect.min_incl_x * src_bytes_per_pixel);
// Currently, this is only used for GIF, which will never have an ICC profile. When it is
// used for other formats that might have one, we will need to transform from profiles that
// do not have corresponding SkColorSpaces.
SkASSERT(!getEncodedInfo().profile());
auto srcInfo = getInfo().makeWH(dirty_rect.width(), dirty_rect.height())
.makeAlphaType(alphaType);
auto srcInfo =
getInfo().makeWH(dirty_rect.width(), dirty_rect.height()).makeAlphaType(alphaType);
SkBitmap src;
src.installPixels(srcInfo, s, pixels.stride);
SkPaint paint;
@ -475,52 +549,34 @@ SkCodec::Result SkWuffsCodec::onIncrementalDecode(int* rowsDecoded) {
SkMatrix translate = SkMatrix::MakeTrans(dirty_rect.min_incl_x, dirty_rect.min_incl_y);
draw.drawBitmap(src, translate, nullptr, paint);
}
if (result == SkCodec::kSuccess) {
fIncrDecDst = nullptr;
fIncrDecRowBytes = 0;
}
return result;
}
int SkWuffsCodec::onGetFrameCount() {
// It is valid, in terms of the SkCodec API, to call SkCodec::getFrameCount
// while in an incremental decode (after onStartIncrementalDecode returns
// and before onIncrementalDecode returns kSuccess).
//
// We should not advance the SkWuffsCodec' stream while doing so, even
// though other SkCodec implementations can return increasing values from
// onGetFrameCount when given more data. If we tried to do so, the
// subsequent resume of the incremental decode would continue reading from
// a different position in the I/O stream, leading to an incorrect error.
//
// Other SkCodec implementations can move the stream forward during
// onGetFrameCount because they assume that the stream is rewindable /
// seekable. For example, an alternative GIF implementation may choose to
// store, for each frame walked past when merely counting the number of
// frames, the I/O position of each of the frame's GIF data blocks. (A GIF
// frame's compressed data can have multiple data blocks, each at most 255
// bytes in length). Obviously, this can require O(numberOfFrames) extra
// memory to store these I/O positions. The constant factor is small, but
// it's still O(N), not O(1).
//
// Wuffs and SkWuffsCodec tries to minimize relying on the rewindable /
// seekable assumption. By design, Wuffs per se aims for O(1) memory use
// (after any pixel buffers are allocated) instead of O(N), and its I/O
// type, wuffs_base__io_buffer, is not necessarily rewindable or seekable.
//
// The Wuffs API provides a limited, optional form of seeking, to the start
// of an animation frame's data, but does not provide arbitrary save and
// load of its internal state whilst in the middle of an animation frame.
bool incrementalDecodeIsInProgress = fIncrDecDst != nullptr;
if (!fFramesComplete && !incrementalDecodeIsInProgress) {
this->readFrames();
this->updateNumFullyReceivedFrames();
if (!fFramesComplete) {
this->onGetFrameCountInternal();
}
return fFrames.size();
}
void SkWuffsCodec::onGetFrameCountInternal() {
if (!seek_buffer(&fIOBuffer, fStream.get(), 0)) {
return;
}
if (!fDecoders[WhichDecoder::kFrameCount]) {
void* decoder_raw = sk_malloc_canfail(sizeof__wuffs_gif__decoder());
if (!decoder_raw) {
return;
}
std::unique_ptr<wuffs_gif__decoder, decltype(&sk_free)> decoder(
reinterpret_cast<wuffs_gif__decoder*>(decoder_raw), &sk_free);
fDecoders[WhichDecoder::kFrameCount] = std::move(decoder);
}
reset_and_decode_image_config(fDecoders[WhichDecoder::kFrameCount].get(), nullptr, &fIOBuffer,
fStream.get());
this->readFrames();
}
bool SkWuffsCodec::onGetFrameInfo(int i, SkCodec::FrameInfo* frameInfo) const {
const SkWuffsFrame* f = this->frame(i);
if (!f) {
@ -537,7 +593,7 @@ int SkWuffsCodec::onGetRepetitionCount() {
// number is how many times to play the loop. Skia's int number is how many
// times to play the loop *after the first play*. Wuffs and Skia use 0 and
// kRepetitionCountInfinite respectively to mean loop forever.
uint32_t n = fDecoder->num_animation_loops();
uint32_t n = fDecoders[WhichDecoder::kIncrDecode]->num_animation_loops();
if (n == 0) {
return SkCodec::kRepetitionCountInfinite;
}
@ -548,14 +604,14 @@ int SkWuffsCodec::onGetRepetitionCount() {
void SkWuffsCodec::readFrames() {
size_t n = fFrames.size();
int i = n ? n - 1 : 0;
if (this->seekFrame(i) != SkCodec::kSuccess) {
if (this->seekFrame(WhichDecoder::kFrameCount, i) != SkCodec::kSuccess) {
return;
}
// Iterate through the frames, converting from Wuffs'
// wuffs_base__frame_config type to Skia's SkWuffsFrame type.
for (; i < INT_MAX; i++) {
const char* status = this->decodeFrameConfig();
const char* status = this->decodeFrameConfig(WhichDecoder::kFrameCount);
if (status == nullptr) {
// No-op.
} else if (status == wuffs_base__warning__end_of_data) {
@ -567,7 +623,7 @@ void SkWuffsCodec::readFrames() {
if (static_cast<size_t>(i) < fFrames.size()) {
continue;
}
fFrames.emplace_back(&fFrameConfig);
fFrames.emplace_back(&fFrameConfigs[WhichDecoder::kFrameCount]);
SkWuffsFrame* f = &fFrames[fFrames.size() - 1];
fFrameHolder.setAlphaAndRequiredFrame(f);
}
@ -575,9 +631,9 @@ void SkWuffsCodec::readFrames() {
fFramesComplete = true;
}
SkCodec::Result SkWuffsCodec::seekFrame(int frameIndex) {
if (fDecoderIsSuspended) {
SkCodec::Result res = this->resetDecoder();
SkCodec::Result SkWuffsCodec::seekFrame(WhichDecoder which, int frameIndex) {
if (fDecoderIsSuspended[which]) {
SkCodec::Result res = this->resetDecoder(which);
if (res != SkCodec::kSuccess) {
return res;
}
@ -597,7 +653,8 @@ SkCodec::Result SkWuffsCodec::seekFrame(int frameIndex) {
if (!seek_buffer(&fIOBuffer, fStream.get(), pos)) {
return SkCodec::kInternalError;
}
const char* status = fDecoder->restart_frame(frameIndex, fIOBuffer.reader_io_position());
const char* status =
fDecoders[which]->restart_frame(frameIndex, fIOBuffer.reader_io_position());
if (status != nullptr) {
return SkCodec::kInternalError;
}
@ -708,57 +765,58 @@ static SkCodec::Result reset_and_decode_image_config(wuffs_gif__decoder* d
return SkCodec::kSuccess;
}
SkCodec::Result SkWuffsCodec::resetDecoder() {
SkCodec::Result SkWuffsCodec::resetDecoder(WhichDecoder which) {
if (!fStream->rewind()) {
return SkCodec::kInternalError;
}
fIOBuffer.meta = wuffs_base__empty_io_buffer_meta();
SkCodec::Result result =
reset_and_decode_image_config(fDecoder.get(), nullptr, &fIOBuffer, fStream.get());
reset_and_decode_image_config(fDecoders[which].get(), nullptr, &fIOBuffer, fStream.get());
if (result == SkCodec::kIncompleteInput) {
return SkCodec::kInternalError;
} else if (result != SkCodec::kSuccess) {
return result;
}
fDecoderIsSuspended = false;
fDecoderIsSuspended[which] = false;
return SkCodec::kSuccess;
}
const char* SkWuffsCodec::decodeFrameConfig() {
while (true) {
const char* status = fDecoder->decode_frame_config(&fFrameConfig, &fIOBuffer);
if ((status == wuffs_base__suspension__short_read) &&
fill_buffer(&fIOBuffer, fStream.get())) {
continue;
}
fDecoderIsSuspended = !wuffs_base__status__is_complete(status);
this->updateNumFullyReceivedFrames();
return status;
}
}
const char* SkWuffsCodec::decodeFrame() {
const char* SkWuffsCodec::decodeFrameConfig(WhichDecoder which) {
while (true) {
const char* status =
fDecoder->decode_frame(&fPixelBuffer, &fIOBuffer,
wuffs_base__make_slice_u8(fWorkbufPtr.get(), fWorkbufLen), NULL);
fDecoders[which]->decode_frame_config(&fFrameConfigs[which], &fIOBuffer);
if ((status == wuffs_base__suspension__short_read) &&
fill_buffer(&fIOBuffer, fStream.get())) {
continue;
}
fDecoderIsSuspended = !wuffs_base__status__is_complete(status);
this->updateNumFullyReceivedFrames();
fDecoderIsSuspended[which] = !wuffs_base__status__is_complete(status);
this->updateNumFullyReceivedFrames(which);
return status;
}
}
void SkWuffsCodec::updateNumFullyReceivedFrames() {
const char* SkWuffsCodec::decodeFrame(WhichDecoder which) {
while (true) {
const char* status = fDecoders[which]->decode_frame(
&fPixelBuffer, &fIOBuffer, wuffs_base__make_slice_u8(fWorkbufPtr.get(), fWorkbufLen),
NULL);
if ((status == wuffs_base__suspension__short_read) &&
fill_buffer(&fIOBuffer, fStream.get())) {
continue;
}
fDecoderIsSuspended[which] = !wuffs_base__status__is_complete(status);
this->updateNumFullyReceivedFrames(which);
return status;
}
}
void SkWuffsCodec::updateNumFullyReceivedFrames(WhichDecoder which) {
// num_decoded_frames's return value, n, can change over time, both up and
// down, as we seek back and forth in the underlying stream.
// fNumFullyReceivedFrames is the highest n we've seen.
uint64_t n = fDecoder->num_decoded_frames();
uint64_t n = fDecoders[which]->num_decoded_frames();
if (fNumFullyReceivedFrames < n) {
fNumFullyReceivedFrames = n;
}

View File

@ -170,7 +170,6 @@ DEF_TEST(Codec_partialWuffs, r) {
}
}
#ifndef SK_HAS_WUFFS_LIBRARY
DEF_TEST(Codec_frameCountUpdatesInIncrementalDecode, r) {
sk_sp<SkData> file = GetResourceAsData("images/colorTables.gif");
size_t fileSize = file->size();
@ -198,7 +197,6 @@ DEF_TEST(Codec_frameCountUpdatesInIncrementalDecode, r) {
stream->addNewData(fileSize - n);
REPORTER_ASSERT(r, partialCodec->getFrameCount() == 2);
}
#endif
// Verify that when decoding an animated gif byte by byte we report the correct
// fRequiredFrame as soon as getFrameInfo reports the frame.