Pause Wuffs' getFrameCount in incremental decode

Bug: skia:8235
Bug: skia:8750
Change-Id: I7b8a6a1aba38ba2f16bd184253fbef2a37000cef
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/223016
Reviewed-by: Leon Scroggins <scroggo@google.com>
Commit-Queue: Leon Scroggins <scroggo@google.com>
This commit is contained in:
Nigel Tao 2019-06-22 17:35:38 +10:00 committed by Skia Commit-Bot
parent 64b858986b
commit 1f1cd1f449
2 changed files with 35 additions and 3 deletions

View File

@ -316,6 +316,9 @@ SkCodec::Result SkWuffsCodec::onStartIncrementalDecode(const SkImageInfo& d
void* dst,
size_t rowBytes,
const SkCodec::Options& options) {
if (!dst) {
return SkCodec::kInvalidParameters;
}
if (options.fSubset) {
return SkCodec::kUnimplemented;
}
@ -481,7 +484,37 @@ SkCodec::Result SkWuffsCodec::onIncrementalDecode(int* rowsDecoded) {
}
int SkWuffsCodec::onGetFrameCount() {
if (!fFramesComplete) {
// 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();
}

View File

@ -101,9 +101,8 @@ static void test_partial(skiatest::Reporter* r, const char* name, const sk_sp<Sk
while (true) {
// This imitates how Chromium calls getFrameCount before resuming a decode.
// Without this line, the test passes. With it, it fails when skia_use_wuffs
// is true.
partialCodec->getFrameCount();
const SkCodec::Result result = partialCodec->incrementalDecode();
if (result == SkCodec::kSuccess) {