Skip to content

Commit 027f89b

Browse files
Nigel TaoSkia Commit-Bot
authored andcommitted
Allow one-pass SkWuffsCodec decoding
Prior to this commit, SkWuffsCodec always used what this commit calls two pass decoding. After this commit, it can sometimes use one pass decoding, which means we don't have to allocate that intermediate pixel buffer. For the "droids.gif" example mentioned in a recent commit, 39da10b "Optimize SkWuffsCodec pixbuf zero-initialization", this means that we allocate 19 MB less than we used to (2560 width x 1920 height x 4 bytes per pixel = 19660800 bytes). Continuing with "droids.gif" and the image_decode_bench program from 39da10b, the time taken per decode drops from 35.627ms to 28.655ms. Bug: chromium:1023191 Bug: chromium:1023129 Bug: skia:8235 Change-Id: Ic4fd6bd2856493eaf777416326f39198aa8d97c1 Reviewed-on: https://skia-review.googlesource.com/c/skia/+/255956 Commit-Queue: Leon Scroggins <[email protected]> Reviewed-by: Leon Scroggins <[email protected]>
1 parent 46e2d8d commit 027f89b

1 file changed

Lines changed: 160 additions & 64 deletions

File tree

src/codec/SkWuffsCodec.cpp

Lines changed: 160 additions & 64 deletions
Original file line numberDiff line numberDiff line change
@@ -177,12 +177,9 @@ class SkWuffsCodec final : public SkScalingCodec {
177177
SkWuffsCodec(SkEncodedInfo&& encodedInfo,
178178
std::unique_ptr<SkStream> stream,
179179
std::unique_ptr<wuffs_gif__decoder, decltype(&sk_free)> dec,
180-
std::unique_ptr<uint8_t, decltype(&sk_free)> pixbuf_ptr,
181-
bool pixbuf_zeroed,
182180
std::unique_ptr<uint8_t, decltype(&sk_free)> workbuf_ptr,
183181
size_t workbuf_len,
184182
wuffs_base__image_config imgcfg,
185-
wuffs_base__pixel_buffer pixbuf,
186183
wuffs_base__io_buffer iobuf);
187184

188185
const SkWuffsFrame* frame(int i) const;
@@ -232,44 +229,64 @@ class SkWuffsCodec final : public SkScalingCodec {
232229
bool onGetFrameInfo(int, FrameInfo*) const override;
233230
int onGetRepetitionCount() override;
234231

235-
Result seekFrame(WhichDecoder which, int frameIndex);
236-
237-
void onGetFrameCountInternal();
238-
Result onIncrementalDecodeInternal(int* rowsDecoded);
239-
232+
// Two separate implementations of onStartIncrementalDecode and
233+
// onIncrementalDecode, named "one pass" and "two pass" decoding. One pass
234+
// decoding writes directly from the Wuffs image decoder to the dst buffer
235+
// (the dst argument to onStartIncrementalDecode). Two pass decoding first
236+
// writes into an intermediate buffer, and then composites and transforms
237+
// the intermediate buffer into the dst buffer.
238+
//
239+
// In the general case, we need the two pass decoder, because of Skia API
240+
// features that Wuffs doesn't support (e.g. color correction, scaling,
241+
// RGB565). But as an optimization, we use one pass decoding (it's faster
242+
// and uses less memory) if applicable (see the assignment to
243+
// fIncrDecOnePass that calculates when we can do so).
244+
Result onStartIncrementalDecodeOnePass(const SkImageInfo& dstInfo,
245+
uint8_t* dst,
246+
size_t rowBytes,
247+
const SkCodec::Options& options,
248+
wuffs_base__pixel_format pixelFormat,
249+
size_t bytesPerPixel);
250+
Result onStartIncrementalDecodeTwoPass();
251+
Result onIncrementalDecodeOnePass();
252+
Result onIncrementalDecodeTwoPass();
253+
254+
void onGetFrameCountInternal();
255+
Result seekFrame(WhichDecoder which, int frameIndex);
240256
Result resetDecoder(WhichDecoder which);
241257
const char* decodeFrameConfig(WhichDecoder which);
242258
const char* decodeFrame(WhichDecoder which);
243259
void updateNumFullyReceivedFrames(WhichDecoder which);
244260

245261
SkWuffsFrameHolder fFrameHolder;
246262
std::unique_ptr<SkStream> fStream;
247-
std::unique_ptr<uint8_t, decltype(&sk_free)> fPixbufPtr;
248263
std::unique_ptr<uint8_t, decltype(&sk_free)> fWorkbufPtr;
249264
size_t fWorkbufLen;
250265

251266
std::unique_ptr<wuffs_gif__decoder, decltype(&sk_free)> fDecoders[WhichDecoder::kNumDecoders];
252267

253268
const uint64_t fFirstFrameIOPosition;
254269
wuffs_base__frame_config fFrameConfigs[WhichDecoder::kNumDecoders];
270+
wuffs_base__pixel_config fPixelConfig;
255271
wuffs_base__pixel_buffer fPixelBuffer;
256272
wuffs_base__io_buffer fIOBuffer;
257273

258274
// Incremental decoding state.
259275
uint8_t* fIncrDecDst;
260276
uint64_t fIncrDecReaderIOPosition;
261277
size_t fIncrDecRowBytes;
278+
bool fIncrDecOnePass;
262279
bool fFirstCallToIncrementalDecode;
263280

281+
// Lazily allocated intermediate pixel buffer, for two pass decoding.
282+
std::unique_ptr<uint8_t, decltype(&sk_free)> fTwoPassPixbufPtr;
283+
size_t fTwoPassPixbufLen;
284+
264285
uint64_t fFrameCountReaderIOPosition;
265286
uint64_t fNumFullyReceivedFrames;
266287
std::vector<SkWuffsFrame> fFrames;
267288
bool fFramesComplete;
268289

269-
// True if fPixelBuffer's contents are known to be already zeroed. This is
270-
// conservative, and may be false even if the buffer is zeroed.
271-
bool fPixbufZeroed;
272-
273290
// If calling an fDecoders[which] method returns an incomplete status, then
274291
// fDecoders[which] is suspended in a coroutine (i.e. waiting on I/O or
275292
// halted on a non-recoverable error). To keep its internal proof-of-safety
@@ -336,12 +353,9 @@ const SkFrame* SkWuffsFrameHolder::onGetFrame(int i) const {
336353
SkWuffsCodec::SkWuffsCodec(SkEncodedInfo&& encodedInfo,
337354
std::unique_ptr<SkStream> stream,
338355
std::unique_ptr<wuffs_gif__decoder, decltype(&sk_free)> dec,
339-
std::unique_ptr<uint8_t, decltype(&sk_free)> pixbuf_ptr,
340-
bool pixbuf_zeroed,
341356
std::unique_ptr<uint8_t, decltype(&sk_free)> workbuf_ptr,
342357
size_t workbuf_len,
343358
wuffs_base__image_config imgcfg,
344-
wuffs_base__pixel_buffer pixbuf,
345359
wuffs_base__io_buffer iobuf)
346360
: INHERITED(std::move(encodedInfo),
347361
skcms_PixelFormat_RGBA_8888,
@@ -351,7 +365,6 @@ SkWuffsCodec::SkWuffsCodec(SkEncodedInfo&&
351365
nullptr),
352366
fFrameHolder(),
353367
fStream(std::move(stream)),
354-
fPixbufPtr(std::move(pixbuf_ptr)),
355368
fWorkbufPtr(std::move(workbuf_ptr)),
356369
fWorkbufLen(workbuf_len),
357370
fDecoders{
@@ -363,16 +376,19 @@ SkWuffsCodec::SkWuffsCodec(SkEncodedInfo&&
363376
wuffs_base__null_frame_config(),
364377
wuffs_base__null_frame_config(),
365378
},
366-
fPixelBuffer(pixbuf),
379+
fPixelConfig(imgcfg.pixcfg),
380+
fPixelBuffer(wuffs_base__null_pixel_buffer()),
367381
fIOBuffer(wuffs_base__empty_io_buffer()),
368382
fIncrDecDst(nullptr),
369383
fIncrDecReaderIOPosition(0),
370384
fIncrDecRowBytes(0),
385+
fIncrDecOnePass(false),
371386
fFirstCallToIncrementalDecode(false),
387+
fTwoPassPixbufPtr(nullptr, &sk_free),
388+
fTwoPassPixbufLen(0),
372389
fFrameCountReaderIOPosition(0),
373390
fNumFullyReceivedFrames(0),
374391
fFramesComplete(false),
375-
fPixbufZeroed(pixbuf_zeroed),
376392
fDecoderIsSuspended{
377393
false,
378394
false,
@@ -441,15 +457,110 @@ SkCodec::Result SkWuffsCodec::onStartIncrementalDecode(const SkImageInfo& d
441457
return SkCodec::kErrorInInput;
442458
}
443459

444-
uint32_t src_bits_per_pixel =
445-
wuffs_base__pixel_format__bits_per_pixel(fPixelBuffer.pixcfg.pixel_format());
446-
if ((src_bits_per_pixel == 0) || (src_bits_per_pixel % 8 != 0)) {
460+
wuffs_base__pixel_format pixelFormat = WUFFS_BASE__PIXEL_FORMAT__INVALID;
461+
size_t bytesPerPixel = 0;
462+
463+
switch (dstInfo.colorType()) {
464+
case kBGRA_8888_SkColorType:
465+
pixelFormat = WUFFS_BASE__PIXEL_FORMAT__BGRA_NONPREMUL;
466+
bytesPerPixel = 4;
467+
break;
468+
case kRGBA_8888_SkColorType:
469+
pixelFormat = WUFFS_BASE__PIXEL_FORMAT__RGBA_NONPREMUL;
470+
bytesPerPixel = 4;
471+
break;
472+
default:
473+
break;
474+
}
475+
476+
// We can use "one pass" decoding if we have a Skia pixel format that Wuffs
477+
// supports...
478+
fIncrDecOnePass =
479+
(pixelFormat != WUFFS_BASE__PIXEL_FORMAT__INVALID) &&
480+
// ...and no color profile (as Wuffs does not support them)...
481+
(!getEncodedInfo().profile()) &&
482+
// ...and we have an independent frame (as Wuffs does not support the
483+
// equivalent of SkBlendMode::kSrcOver)...
484+
((options.fFrameIndex == 0) ||
485+
(this->frame(options.fFrameIndex)->getRequiredFrame() == SkCodec::kNoFrame)) &&
486+
// ...and we use the identity transform (as Wuffs does not support
487+
// scaling).
488+
(this->dimensions() == dstInfo.dimensions());
489+
490+
result = fIncrDecOnePass ? this->onStartIncrementalDecodeOnePass(
491+
dstInfo, static_cast<uint8_t*>(dst), rowBytes, options,
492+
pixelFormat, bytesPerPixel)
493+
: this->onStartIncrementalDecodeTwoPass();
494+
if (result != SkCodec::kSuccess) {
495+
return result;
496+
}
497+
498+
fIncrDecDst = static_cast<uint8_t*>(dst);
499+
fIncrDecReaderIOPosition = fIOBuffer.reader_io_position();
500+
fIncrDecRowBytes = rowBytes;
501+
fFirstCallToIncrementalDecode = true;
502+
return SkCodec::kSuccess;
503+
}
504+
505+
SkCodec::Result SkWuffsCodec::onStartIncrementalDecodeOnePass(const SkImageInfo& dstInfo,
506+
uint8_t* dst,
507+
size_t rowBytes,
508+
const SkCodec::Options& options,
509+
wuffs_base__pixel_format pixelFormat,
510+
size_t bytesPerPixel) {
511+
wuffs_base__pixel_config pixelConfig;
512+
pixelConfig.set(pixelFormat, WUFFS_BASE__PIXEL_SUBSAMPLING__NONE, dstInfo.width(),
513+
dstInfo.height());
514+
515+
wuffs_base__table_u8 table;
516+
table.ptr = dst;
517+
table.width = static_cast<size_t>(dstInfo.width()) * bytesPerPixel;
518+
table.height = dstInfo.height();
519+
table.stride = rowBytes;
520+
521+
const char* status = fPixelBuffer.set_from_table(&pixelConfig, table);
522+
if (status != nullptr) {
523+
SkCodecPrintf("set_from_table: %s", status);
447524
return SkCodec::kInternalError;
448525
}
449-
size_t src_bytes_per_pixel = src_bits_per_pixel / 8;
450526

451-
// Zero-initialize Wuffs' buffer covering the frame rect.
452-
if (!fPixbufZeroed) {
527+
SkSampler::Fill(dstInfo, dst, rowBytes, options.fZeroInitialized);
528+
return SkCodec::kSuccess;
529+
}
530+
531+
SkCodec::Result SkWuffsCodec::onStartIncrementalDecodeTwoPass() {
532+
// Either re-use the previously allocated "two pass" pixel buffer (and
533+
// memset to zero), or allocate (and zero initialize) a new one.
534+
bool already_zeroed = false;
535+
536+
if (!fTwoPassPixbufPtr) {
537+
uint64_t pixbuf_len = fPixelConfig.pixbuf_len();
538+
void* pixbuf_ptr_raw = (pixbuf_len <= SIZE_MAX)
539+
? sk_malloc_flags(pixbuf_len, SK_MALLOC_ZERO_INITIALIZE)
540+
: nullptr;
541+
if (!pixbuf_ptr_raw) {
542+
return SkCodec::kInternalError;
543+
}
544+
fTwoPassPixbufPtr.reset(reinterpret_cast<uint8_t*>(pixbuf_ptr_raw));
545+
fTwoPassPixbufLen = SkToSizeT(pixbuf_len);
546+
already_zeroed = true;
547+
}
548+
549+
const char* status = fPixelBuffer.set_from_slice(
550+
&fPixelConfig, wuffs_base__make_slice_u8(fTwoPassPixbufPtr.get(), fTwoPassPixbufLen));
551+
if (status != nullptr) {
552+
SkCodecPrintf("set_from_slice: %s", status);
553+
return SkCodec::kInternalError;
554+
}
555+
556+
if (!already_zeroed) {
557+
uint32_t src_bits_per_pixel =
558+
wuffs_base__pixel_format__bits_per_pixel(fPixelConfig.pixel_format());
559+
if ((src_bits_per_pixel == 0) || (src_bits_per_pixel % 8 != 0)) {
560+
return SkCodec::kInternalError;
561+
}
562+
size_t src_bytes_per_pixel = src_bits_per_pixel / 8;
563+
453564
wuffs_base__rect_ie_u32 frame_rect = fFrameConfigs[WhichDecoder::kIncrDecode].bounds();
454565
wuffs_base__table_u8 pixels = fPixelBuffer.plane(0);
455566

@@ -468,16 +579,7 @@ SkCodec::Result SkWuffsCodec::onStartIncrementalDecode(const SkImageInfo& d
468579
}
469580
}
470581
}
471-
// The buffer is zeroed now, but this onStartIncrementalDecode call will
472-
// almost certainly be followed by some onIncrementalDecode calls that can
473-
// modify fPixelBuffer's contents. We set fPixbufZeroed to false so that
474-
// the next onStartIncrementalDecode call will zero-initialize the buffer.
475-
fPixbufZeroed = false;
476582

477-
fIncrDecDst = static_cast<uint8_t*>(dst);
478-
fIncrDecReaderIOPosition = fIOBuffer.reader_io_position();
479-
fIncrDecRowBytes = rowBytes;
480-
fFirstCallToIncrementalDecode = true;
481583
return SkCodec::kSuccess;
482584
}
483585

@@ -497,18 +599,37 @@ SkCodec::Result SkWuffsCodec::onIncrementalDecode(int* rowsDecoded) {
497599
return SkCodec::kInternalError;
498600
}
499601

500-
SkCodec::Result result = this->onIncrementalDecodeInternal(rowsDecoded);
602+
if (rowsDecoded) {
603+
*rowsDecoded = dstInfo().height();
604+
}
605+
606+
SkCodec::Result result =
607+
fIncrDecOnePass ? this->onIncrementalDecodeOnePass() : this->onIncrementalDecodeTwoPass();
501608
if (result == SkCodec::kSuccess) {
502609
fIncrDecDst = nullptr;
503610
fIncrDecReaderIOPosition = 0;
504611
fIncrDecRowBytes = 0;
612+
fIncrDecOnePass = false;
505613
} else {
506614
fIncrDecReaderIOPosition = fIOBuffer.reader_io_position();
507615
}
508616
return result;
509617
}
510618

511-
SkCodec::Result SkWuffsCodec::onIncrementalDecodeInternal(int* rowsDecoded) {
619+
SkCodec::Result SkWuffsCodec::onIncrementalDecodeOnePass() {
620+
const char* status = this->decodeFrame(WhichDecoder::kIncrDecode);
621+
if (status != nullptr) {
622+
if (status == wuffs_base__suspension__short_read) {
623+
return SkCodec::kIncompleteInput;
624+
} else {
625+
SkCodecPrintf("decodeFrame: %s", status);
626+
return SkCodec::kErrorInInput;
627+
}
628+
}
629+
return SkCodec::kSuccess;
630+
}
631+
632+
SkCodec::Result SkWuffsCodec::onIncrementalDecodeTwoPass() {
512633
SkCodec::Result result = SkCodec::kSuccess;
513634
const char* status = this->decodeFrame(WhichDecoder::kIncrDecode);
514635
bool independent;
@@ -571,10 +692,6 @@ SkCodec::Result SkWuffsCodec::onIncrementalDecodeInternal(int* rowsDecoded) {
571692
SkASSERT(index == 0);
572693
}
573694

574-
if (rowsDecoded) {
575-
*rowsDecoded = dstInfo().height();
576-
}
577-
578695
// If the frame's dirty rect is empty, no need to swizzle.
579696
wuffs_base__rect_ie_u32 dirty_rect = fDecoders[WhichDecoder::kIncrDecode]->frame_dirty_rect();
580697
if (!dirty_rect.is_empty()) {
@@ -910,26 +1027,6 @@ std::unique_ptr<SkCodec> SkWuffsCodec_MakeFromStream(std::unique_ptr<SkStream> s
9101027
std::unique_ptr<uint8_t, decltype(&sk_free)> workbuf_ptr(
9111028
reinterpret_cast<uint8_t*>(workbuf_ptr_raw), &sk_free);
9121029

913-
constexpr int pixbuf_sk_malloc_flags = SK_MALLOC_ZERO_INITIALIZE;
914-
uint64_t pixbuf_len = imgcfg.pixcfg.pixbuf_len();
915-
void* pixbuf_ptr_raw =
916-
pixbuf_len <= SIZE_MAX ? sk_malloc_flags(pixbuf_len, pixbuf_sk_malloc_flags) : nullptr;
917-
if (!pixbuf_ptr_raw) {
918-
*result = SkCodec::kInternalError;
919-
return nullptr;
920-
}
921-
std::unique_ptr<uint8_t, decltype(&sk_free)> pixbuf_ptr(
922-
reinterpret_cast<uint8_t*>(pixbuf_ptr_raw), &sk_free);
923-
wuffs_base__pixel_buffer pixbuf = wuffs_base__null_pixel_buffer();
924-
925-
const char* status = pixbuf.set_from_slice(
926-
&imgcfg.pixcfg, wuffs_base__make_slice_u8(pixbuf_ptr.get(), SkToSizeT(pixbuf_len)));
927-
if (status != nullptr) {
928-
SkCodecPrintf("set_from_slice: %s", status);
929-
*result = SkCodec::kInternalError;
930-
return nullptr;
931-
}
932-
9331030
SkEncodedInfo::Color color =
9341031
(imgcfg.pixcfg.pixel_format() == WUFFS_BASE__PIXEL_FORMAT__BGRA_NONPREMUL)
9351032
? SkEncodedInfo::kBGRA_Color
@@ -943,8 +1040,7 @@ std::unique_ptr<SkCodec> SkWuffsCodec_MakeFromStream(std::unique_ptr<SkStream> s
9431040
SkEncodedInfo encodedInfo = SkEncodedInfo::Make(width, height, color, alpha, 8);
9441041

9451042
*result = SkCodec::kSuccess;
946-
return std::unique_ptr<SkCodec>(new SkWuffsCodec(
947-
std::move(encodedInfo), std::move(stream), std::move(decoder), std::move(pixbuf_ptr),
948-
(pixbuf_sk_malloc_flags & SK_MALLOC_ZERO_INITIALIZE) != 0, std::move(workbuf_ptr),
949-
workbuf_len, imgcfg, pixbuf, iobuf));
1043+
return std::unique_ptr<SkCodec>(new SkWuffsCodec(std::move(encodedInfo), std::move(stream),
1044+
std::move(decoder), std::move(workbuf_ptr),
1045+
workbuf_len, imgcfg, iobuf));
9501046
}

0 commit comments

Comments
 (0)