From 893f867f36605caf06cbf6dfd1801602d04bca27 Mon Sep 17 00:00:00 2001 From: Michael Klimushyn Date: Tue, 9 Oct 2018 17:07:46 -0700 Subject: [PATCH] Add a ratio cap to decoded animated image frames (flutter/engine#6310) Provide a relative, per-image limit to the amount of memory that's used to cache decoded image frames. Adds an overridable default that developers can set to control how much memory images are allowed to use decoded vs undecoded. The cap is set in flutter/flutter#22452. Note that required frames are always cached regardless of the ratio cap, because they're currently necessary for the GIF to animate. Previously cached unessential frames are not cleared in response to the cache hitting or exceeding the cap. Addresses #20998 and #14344. --- engine/src/flutter/lib/ui/painting.dart | 33 ++++++-- engine/src/flutter/lib/ui/painting/codec.cc | 81 ++++++++++++++----- engine/src/flutter/lib/ui/painting/codec.h | 23 +++++- .../src/flutter/testing/dart/codec_test.dart | 23 ++++++ 4 files changed, 129 insertions(+), 31 deletions(-) diff --git a/engine/src/flutter/lib/ui/painting.dart b/engine/src/flutter/lib/ui/painting.dart index af408b4f75..5844565bbf 100644 --- a/engine/src/flutter/lib/ui/painting.dart +++ b/engine/src/flutter/lib/ui/painting.dart @@ -1606,22 +1606,32 @@ class Codec extends NativeFieldWrapperClass2 { /// Instantiates an image codec [Codec] object. /// /// [list] is the binary image data (e.g a PNG or GIF binary data). -/// The data can be for either static or animated images. +/// The data can be for either static or animated images. The following image +/// formats are supported: {@macro flutter.dart:ui.imageFormats} /// -/// The following image formats are supported: {@macro flutter.dart:ui.imageFormats} +/// The [decodedCacheRatioCap] is the default maximum multiple of the compressed +/// image size to cache when decoding animated image frames. For example, +/// setting this to `2.0` means that a 400KB GIF would be allowed at most to use +/// 800KB of memory caching unessential decoded frames. Caching decoded frames +/// saves CPU but can result in out-of-memory crashes when decoding large (or +/// multiple) animated images. Note that GIFs are highly compressed, and it's +/// unlikely that a factor that low will be sufficient to cache all decoded +/// frames. The default value is `25.0`. /// /// The returned future can complete with an error if the image decoding has /// failed. -Future instantiateImageCodec(Uint8List list) { +Future instantiateImageCodec(Uint8List list, { + double decodedCacheRatioCap = double.infinity, +}) { return _futurize( - (_Callback callback) => _instantiateImageCodec(list, callback, null) + (_Callback callback) => _instantiateImageCodec(list, callback, null, decodedCacheRatioCap), ); } /// Instantiates a [Codec] object for an image binary data. /// /// Returns an error message if the instantiation has failed, null otherwise. -String _instantiateImageCodec(Uint8List list, _Callback callback, _ImageInfo imageInfo) +String _instantiateImageCodec(Uint8List list, _Callback callback, _ImageInfo imageInfo, double decodedCacheRatioCap) native 'instantiateImageCodec'; /// Loads a single image frame from a byte array into an [Image] object. @@ -1646,17 +1656,26 @@ Future _decodeImageFromListAsync(Uint8List list, /// [rowBytes] is the number of bytes consumed by each row of pixels in the /// data buffer. If unspecified, it defaults to [width] multipled by the /// number of bytes per pixel in the provided [format]. +/// +/// The [decodedCacheRatioCap] is the default maximum multiple of the compressed +/// image size to cache when decoding animated image frames. For example, +/// setting this to `2.0` means that a 400KB GIF would be allowed at most to use +/// 800KB of memory caching unessential decoded frames. Caching decoded frames +/// saves CPU but can result in out-of-memory crashes when decoding large (or +/// multiple) animated images. Note that GIFs are highly compressed, and it's +/// unlikely that a factor that low will be sufficient to cache all decoded +/// frames. The default value is `25.0`. void decodeImageFromPixels( Uint8List pixels, int width, int height, PixelFormat format, ImageDecoderCallback callback, - {int rowBytes} + {int rowBytes, double decodedCacheRatioCap = double.infinity} ) { final _ImageInfo imageInfo = new _ImageInfo(width, height, format.index, rowBytes); final Future codecFuture = _futurize( - (_Callback callback) => _instantiateImageCodec(pixels, callback, imageInfo) + (_Callback callback) => _instantiateImageCodec(pixels, callback, imageInfo, decodedCacheRatioCap) ); codecFuture.then((Codec codec) => codec.getNextFrame()) .then((FrameInfo frameInfo) => callback(frameInfo.image)); diff --git a/engine/src/flutter/lib/ui/painting/codec.cc b/engine/src/flutter/lib/ui/painting/codec.cc index 6823350553..7287966c6b 100644 --- a/engine/src/flutter/lib/ui/painting/codec.cc +++ b/engine/src/flutter/lib/ui/painting/codec.cc @@ -86,6 +86,7 @@ static sk_sp DecodeImage(fml::WeakPtr context, fml::RefPtr InitCodec(fml::WeakPtr context, sk_sp buffer, fml::RefPtr unref_queue, + const float decodedCacheRatioCap, size_t trace_id) { TRACE_FLOW_STEP("flutter", kInitCodecTraceTag, trace_id); TRACE_EVENT0("blink", "InitCodec"); @@ -102,7 +103,8 @@ fml::RefPtr InitCodec(fml::WeakPtr context, return nullptr; } if (skCodec->getFrameCount() > 1) { - return fml::MakeRefCounted(std::move(skCodec)); + return fml::MakeRefCounted(std::move(skCodec), + decodedCacheRatioCap); } auto skImage = DecodeImage(context, buffer, trace_id); if (!skImage) { @@ -120,6 +122,7 @@ fml::RefPtr InitCodecUncompressed( sk_sp buffer, ImageInfo image_info, fml::RefPtr unref_queue, + const float decodedCacheRatioCap, size_t trace_id) { TRACE_FLOW_STEP("flutter", kInitCodecTraceTag, trace_id); TRACE_EVENT0("blink", "InitCodecUncompressed"); @@ -152,14 +155,16 @@ void InitCodecAndInvokeCodecCallback( std::unique_ptr callback, sk_sp buffer, std::unique_ptr image_info, + const float decodedCacheRatioCap, size_t trace_id) { fml::RefPtr codec; if (image_info) { codec = InitCodecUncompressed(context, std::move(buffer), *image_info, - std::move(unref_queue), trace_id); + std::move(unref_queue), decodedCacheRatioCap, + trace_id); } else { - codec = - InitCodec(context, std::move(buffer), std::move(unref_queue), trace_id); + codec = InitCodec(context, std::move(buffer), std::move(unref_queue), + decodedCacheRatioCap, trace_id); } ui_task_runner->PostTask( fml::MakeCopyable([callback = std::move(callback), @@ -277,6 +282,9 @@ void InstantiateImageCodec(Dart_NativeArguments args) { } } + const float decodedCacheRatioCap = + tonic::DartConverter::FromDart(Dart_GetNativeArgument(args, 3)); + auto buffer = SkData::MakeWithCopy(list.data(), list.num_elements()); auto dart_state = UIDartState::Current(); @@ -288,11 +296,12 @@ void InstantiateImageCodec(Dart_NativeArguments args) { buffer = std::move(buffer), trace_id, image_info = std::move(image_info), ui_task_runner = task_runners.GetUITaskRunner(), context = dart_state->GetResourceContext(), - queue = UIDartState::Current()->GetSkiaUnrefQueue()]() mutable { - InitCodecAndInvokeCodecCallback(std::move(ui_task_runner), context, - std::move(queue), std::move(callback), - std::move(buffer), - std::move(image_info), trace_id); + queue = UIDartState::Current()->GetSkiaUnrefQueue(), + decodedCacheRatioCap]() mutable { + InitCodecAndInvokeCodecCallback( + std::move(ui_task_runner), context, std::move(queue), + std::move(callback), std::move(buffer), std::move(image_info), + decodedCacheRatioCap, trace_id); })); } @@ -358,17 +367,36 @@ void Codec::dispose() { ClearDartWrapper(); } -MultiFrameCodec::MultiFrameCodec(std::unique_ptr codec) - : codec_(std::move(codec)) { +MultiFrameCodec::MultiFrameCodec(std::unique_ptr codec, + const float decodedCacheRatioCap) + : codec_(std::move(codec)), decodedCacheRatioCap_(decodedCacheRatioCap) { repetitionCount_ = codec_->getRepetitionCount(); frameInfos_ = codec_->getFrameInfo(); - frameBitmaps_.resize(frameInfos_.size()); + compressedSizeBytes_ = codec_->getInfo().computeMinByteSize(); + frameBitmaps_.clear(); + decodedCacheSize_ = 0; + // Initialize the frame cache, marking frames that are required for other + // dependent frames to render. + for (size_t frameIndex = 0; frameIndex < frameInfos_.size(); frameIndex++) { + const auto& frameInfo = frameInfos_[frameIndex]; + if (frameInfo.fRequiredFrame != SkCodec::kNoFrame) { + frameBitmaps_[frameInfo.fRequiredFrame] = + std::make_unique(/*required=*/true); + } + if (frameBitmaps_.count(frameIndex) < 1) { + frameBitmaps_[frameIndex] = + std::make_unique(/*required=*/false); + } + } nextFrameIndex_ = 0; } sk_sp MultiFrameCodec::GetNextFrameImage( fml::WeakPtr resourceContext) { - SkBitmap& bitmap = frameBitmaps_[nextFrameIndex_]; + // Populate this bitmap from the cache if it exists + DecodedFrame& cacheEntry = *frameBitmaps_[nextFrameIndex_]; + SkBitmap bitmap = + cacheEntry.bitmap_ != nullptr ? *cacheEntry.bitmap_ : SkBitmap(); if (!bitmap.getPixels()) { // We haven't decoded this frame yet const SkImageInfo info = codec_->getInfo().makeColorType(kN32_SkColorType); bitmap.allocPixels(info); @@ -377,17 +405,16 @@ sk_sp MultiFrameCodec::GetNextFrameImage( options.fFrameIndex = nextFrameIndex_; const int requiredFrame = frameInfos_[nextFrameIndex_].fRequiredFrame; if (requiredFrame != SkCodec::kNone) { - if (requiredFrame < 0 || - static_cast(requiredFrame) >= frameBitmaps_.size()) { + const SkBitmap* requiredBitmap = + frameBitmaps_[requiredFrame]->bitmap_.get(); + if (requiredBitmap == nullptr) { FML_LOG(ERROR) << "Frame " << nextFrameIndex_ << " depends on frame " - << requiredFrame << " which out of range (0," - << frameBitmaps_.size() << ")."; + << requiredFrame << " which has not been cached."; return NULL; } - SkBitmap& requiredBitmap = frameBitmaps_[requiredFrame]; - // For simplicity, do not try to cache old frames - if (requiredBitmap.getPixels() && - copy_to(&bitmap, requiredBitmap.colorType(), requiredBitmap)) { + + if (requiredBitmap->getPixels() && + copy_to(&bitmap, requiredBitmap->colorType(), *requiredBitmap)) { options.fPriorFrame = requiredFrame; } } @@ -397,6 +424,16 @@ sk_sp MultiFrameCodec::GetNextFrameImage( FML_LOG(ERROR) << "Could not getPixels for frame " << nextFrameIndex_; return NULL; } + + // Cache the bitmap if this is a required frame or if we're still under our + // ratio cap. + const size_t cachedFrameSize = bitmap.computeByteSize(); + if (cacheEntry.required_ || + ((decodedCacheSize_ + cachedFrameSize) / compressedSizeBytes_) <= + decodedCacheRatioCap_) { + cacheEntry.bitmap_ = std::make_unique(bitmap); + decodedCacheSize_ += cachedFrameSize; + } } if (resourceContext) { @@ -485,7 +522,7 @@ Dart_Handle SingleFrameCodec::getNextFrame(Dart_Handle callback_handle) { void Codec::RegisterNatives(tonic::DartLibraryNatives* natives) { natives->Register({ - {"instantiateImageCodec", InstantiateImageCodec, 3, true}, + {"instantiateImageCodec", InstantiateImageCodec, 4, true}, }); natives->Register({FOR_EACH_BINDING(DART_REGISTER_NATIVE)}); } diff --git a/engine/src/flutter/lib/ui/painting/codec.h b/engine/src/flutter/lib/ui/painting/codec.h index c66ee445f2..e171c4ca08 100644 --- a/engine/src/flutter/lib/ui/painting/codec.h +++ b/engine/src/flutter/lib/ui/painting/codec.h @@ -41,7 +41,8 @@ class MultiFrameCodec : public Codec { Dart_Handle getNextFrame(Dart_Handle args); private: - MultiFrameCodec(std::unique_ptr codec); + MultiFrameCodec(std::unique_ptr codec, + const float decodedCacheRatioCap); ~MultiFrameCodec() {} @@ -57,9 +58,27 @@ class MultiFrameCodec : public Codec { const std::unique_ptr codec_; int repetitionCount_; int nextFrameIndex_; + // The default max amount of memory to use for caching decoded animated image + // frames compared to total undecoded size. + const float decodedCacheRatioCap_; + size_t compressedSizeBytes_; + size_t decodedCacheSize_; std::vector frameInfos_; - std::vector frameBitmaps_; + // A struct linking the bitmap of a frame to whether it's required to render + // other dependent frames. + struct DecodedFrame { + std::unique_ptr bitmap_ = nullptr; + const bool required_; + + DecodedFrame(bool required) : required_(required) {} + }; + + // A cache of previously loaded bitmaps, indexed by the frame they belong to. + // Always holds at least the frames marked as required for reuse by + // [SkCodec::getFrameInfo()]. Will cache other non-essential frames until + // [decodedCacheSize_] : [compressedSize_] exceeds [decodedCacheRatioCap_]. + std::map> frameBitmaps_; FML_FRIEND_MAKE_REF_COUNTED(MultiFrameCodec); FML_FRIEND_REF_COUNTED_THREAD_SAFE(MultiFrameCodec); diff --git a/engine/src/flutter/testing/dart/codec_test.dart b/engine/src/flutter/testing/dart/codec_test.dart index 0e2f71cd48..b205aef539 100644 --- a/engine/src/flutter/testing/dart/codec_test.dart +++ b/engine/src/flutter/testing/dart/codec_test.dart @@ -55,6 +55,29 @@ void main() { ])); }); + test('decodedCacheRatioCap', () async { + // No real way to test the native layer, but a smoke test here to at least + // verify that animation is still consistent with caching disabled. + Uint8List data = await _getSkiaResource('test640x479.gif').readAsBytes(); + ui.Codec codec = await ui.instantiateImageCodec(data, decodedCacheRatioCap: 1.0); + List> decodedFrameInfos = []; + for (int i = 0; i < 5; i++) { + ui.FrameInfo frameInfo = await codec.getNextFrame(); + decodedFrameInfos.add([ + frameInfo.duration.inMilliseconds, + frameInfo.image.width, + frameInfo.image.height, + ]); + } + expect(decodedFrameInfos, equals([ + [200, 640, 479], + [200, 640, 479], + [200, 640, 479], + [200, 640, 479], + [200, 640, 479], + ])); + }); + test('non animated image', () async { Uint8List data = await _getSkiaResource('baby_tux.png').readAsBytes(); ui.Codec codec = await ui.instantiateImageCodec(data);