From 2325501f3fcc6450afe207f540dd29598b7a1dd3 Mon Sep 17 00:00:00 2001 From: Thibault Deckers Date: Tue, 4 Mar 2025 00:58:00 +0100 Subject: [PATCH] decoding fixes --- .../channel/AvesByteSendingMethodCodec.kt | 27 +++++++------- .../aves/channel/calls/AppAdapterHandler.kt | 5 +-- .../aves/channel/calls/EmbeddedDataHandler.kt | 5 +-- .../channel/calls/fetchers/RegionFetcher.kt | 3 +- .../calls/fetchers/SvgRegionFetcher.kt | 3 +- .../calls/fetchers/ThumbnailFetcher.kt | 6 ++-- .../calls/fetchers/TiffRegionFetcher.kt | 7 ++-- .../channel/streams/ImageByteStreamHandler.kt | 18 +++++----- .../thibault/aves/decoder/TiffGlideModule.kt | 2 +- .../thibault/aves/utils/BitmapUtils.kt | 35 +++++++++++-------- lib/image_providers/uri_image_provider.dart | 11 ++---- lib/services/media/media_fetch_service.dart | 7 ++-- 12 files changed, 69 insertions(+), 60 deletions(-) diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/channel/AvesByteSendingMethodCodec.kt b/android/app/src/main/kotlin/deckers/thibault/aves/channel/AvesByteSendingMethodCodec.kt index 9ae286aff..724b8d2ef 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/channel/AvesByteSendingMethodCodec.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/channel/AvesByteSendingMethodCodec.kt @@ -21,27 +21,28 @@ class AvesByteSendingMethodCodec private constructor() : MethodCodec { return STANDARD.encodeMethodCall(methodCall) } + override fun encodeErrorEnvelope(errorCode: String, errorMessage: String?, errorDetails: Any?): ByteBuffer { + return STANDARD.encodeErrorEnvelope(errorCode, errorMessage, errorDetails) + } + + override fun encodeErrorEnvelopeWithStacktrace(errorCode: String, errorMessage: String?, errorDetails: Any?, errorStacktrace: String?): ByteBuffer { + return STANDARD.encodeErrorEnvelopeWithStacktrace(errorCode, errorMessage, errorDetails, errorStacktrace) + } + + // `StandardMethodCodec` writes the result to a `ByteArrayOutputStream`, then writes the stream to a `ByteBuffer`. + // Here we only handle `ByteArray` results, but we avoid the intermediate stream. override fun encodeSuccessEnvelope(result: Any?): ByteBuffer { if (result is ByteArray) { - val size = result.size - return ByteBuffer.allocateDirect(4 + size).apply { + return ByteBuffer.allocateDirect(1 + result.size).apply { + // following `StandardMethodCodec`: + // First byte is zero in success case, and non-zero otherwise. put(0) put(result) } } Log.e(LOG_TAG, "encodeSuccessEnvelope failed with result=$result") - return ByteBuffer.allocateDirect(0) - } - - override fun encodeErrorEnvelope(errorCode: String, errorMessage: String?, errorDetails: Any?): ByteBuffer { - Log.e(LOG_TAG, "encodeErrorEnvelope failed with errorCode=$errorCode, errorMessage=$errorMessage, errorDetails=$errorDetails") - return ByteBuffer.allocateDirect(0) - } - - override fun encodeErrorEnvelopeWithStacktrace(errorCode: String, errorMessage: String?, errorDetails: Any?, errorStacktrace: String?): ByteBuffer { - Log.e(LOG_TAG, "encodeErrorEnvelopeWithStacktrace failed with errorCode=$errorCode, errorMessage=$errorMessage, errorDetails=$errorDetails, errorStacktrace=$errorStacktrace") - return ByteBuffer.allocateDirect(0) + return encodeErrorEnvelope("invalid-result-type", "Called success with a result which is not a `ByteArray`, type=${result?.javaClass}", null) } companion object { diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/AppAdapterHandler.kt b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/AppAdapterHandler.kt index c0660be05..6d17bc331 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/AppAdapterHandler.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/AppAdapterHandler.kt @@ -38,7 +38,6 @@ import deckers.thibault.aves.channel.calls.Coresult.Companion.safe import deckers.thibault.aves.channel.calls.Coresult.Companion.safeSuspend import deckers.thibault.aves.model.FieldMap import deckers.thibault.aves.utils.BitmapUtils -import deckers.thibault.aves.utils.BitmapUtils.getRawBytes import deckers.thibault.aves.utils.LogUtils import deckers.thibault.aves.utils.anyCauseIs import deckers.thibault.aves.utils.getApplicationInfoCompat @@ -175,7 +174,9 @@ class AppAdapterHandler(private val context: Context) : MethodCallHandler { try { val bitmap = withContext(Dispatchers.IO) { target.get() } - bytes = bitmap?.getRawBytes(recycle = false) + // do not recycle bitmaps fetched from `ContentResolver` as their lifecycle is unknown + val recycle = false + bytes = BitmapUtils.getRawBytes(bitmap, recycle = recycle) } catch (e: Exception) { Log.w(LOG_TAG, "failed to decode app icon for packageName=$packageName", e) } diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/EmbeddedDataHandler.kt b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/EmbeddedDataHandler.kt index ac839727a..58c07fbf4 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/EmbeddedDataHandler.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/EmbeddedDataHandler.kt @@ -22,7 +22,6 @@ import deckers.thibault.aves.model.FieldMap import deckers.thibault.aves.model.provider.ImageProvider import deckers.thibault.aves.model.provider.ImageProviderFactory.getProvider import deckers.thibault.aves.utils.BitmapUtils -import deckers.thibault.aves.utils.BitmapUtils.getRawBytes import deckers.thibault.aves.utils.FileUtils.transferFrom import deckers.thibault.aves.utils.LogUtils import deckers.thibault.aves.utils.MimeTypes @@ -74,7 +73,9 @@ class EmbeddedDataHandler(private val context: Context) : MethodCallHandler { val orientation = exif.getAttributeInt(ExifInterface.TAG_ORIENTATION, ExifInterface.ORIENTATION_NORMAL) exif.thumbnailBitmap?.let { bitmap -> TransformationUtils.rotateImageExif(BitmapUtils.getBitmapPool(context), bitmap, orientation)?.let { - it.getRawBytes(recycle = false)?.let { bytes -> thumbnails.add(bytes) } + // do not recycle bitmaps fetched from `ExifInterface` as their lifecycle is unknown + val recycle = false + BitmapUtils.getRawBytes(it, recycle = recycle)?.let { bytes -> thumbnails.add(bytes) } } } } diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/RegionFetcher.kt b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/RegionFetcher.kt index b4ce08252..73fad7469 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/RegionFetcher.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/RegionFetcher.kt @@ -14,7 +14,6 @@ import deckers.thibault.aves.decoder.AvesAppGlideModule import deckers.thibault.aves.decoder.MultiPageImage import deckers.thibault.aves.utils.BitmapRegionDecoderCompat import deckers.thibault.aves.utils.BitmapUtils -import deckers.thibault.aves.utils.BitmapUtils.getRawBytes import deckers.thibault.aves.utils.LogUtils import deckers.thibault.aves.utils.MathUtils import deckers.thibault.aves.utils.MemoryUtils @@ -132,7 +131,7 @@ class RegionFetcher internal constructor( bitmap = decoder.decodeRegion(effectiveRect, options) } - val bytes = bitmap?.getRawBytes(recycle = true) + val bytes = BitmapUtils.getRawBytes(bitmap, recycle = true) if (bytes != null) { result.success(bytes) } else { diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/SvgRegionFetcher.kt b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/SvgRegionFetcher.kt index 59fb3b25d..afcae1453 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/SvgRegionFetcher.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/SvgRegionFetcher.kt @@ -14,7 +14,6 @@ import com.caverock.androidsvg.SVGParseException import deckers.thibault.aves.metadata.SVGParserBufferedInputStream import deckers.thibault.aves.metadata.SvgHelper.normalizeSize import deckers.thibault.aves.utils.BitmapUtils -import deckers.thibault.aves.utils.BitmapUtils.getRawBytes import deckers.thibault.aves.utils.MemoryUtils import deckers.thibault.aves.utils.StorageUtils import io.flutter.plugin.common.MethodChannel @@ -109,7 +108,7 @@ class SvgRegionFetcher internal constructor( svg.renderToCanvas(canvas, renderOptions) bitmap = Bitmap.createBitmap(bitmap, bleedX, bleedY, targetBitmapWidth, targetBitmapHeight) - val bytes = bitmap.getRawBytes(recycle = true) + val bytes = BitmapUtils.getRawBytes(bitmap, recycle = true) result.success(bytes) } catch (e: Exception) { result.error("fetch-read-exception", "failed to initialize region decoder for uri=$uri regionRect=$regionRect", e.message) diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/ThumbnailFetcher.kt b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/ThumbnailFetcher.kt index 2dec1628a..1bfb478de 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/ThumbnailFetcher.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/ThumbnailFetcher.kt @@ -15,8 +15,8 @@ import com.bumptech.glide.request.RequestOptions import com.bumptech.glide.signature.ObjectKey import deckers.thibault.aves.decoder.AvesAppGlideModule import deckers.thibault.aves.decoder.MultiPageImage +import deckers.thibault.aves.utils.BitmapUtils import deckers.thibault.aves.utils.BitmapUtils.applyExifOrientation -import deckers.thibault.aves.utils.BitmapUtils.getRawBytes import deckers.thibault.aves.utils.MimeTypes import deckers.thibault.aves.utils.MimeTypes.SVG import deckers.thibault.aves.utils.MimeTypes.isVideo @@ -77,7 +77,9 @@ class ThumbnailFetcher internal constructor( } } - val bytes = bitmap?.getRawBytes(recycle = false) + // do not recycle bitmaps fetched from `ContentResolver` or Glide as their lifecycle is unknown + val recycle = false + val bytes = BitmapUtils.getRawBytes(bitmap, recycle = recycle) if (bytes != null) { result.success(bytes) } else { diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/TiffRegionFetcher.kt b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/TiffRegionFetcher.kt index 82f29f483..d753e1799 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/TiffRegionFetcher.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/channel/calls/fetchers/TiffRegionFetcher.kt @@ -1,9 +1,10 @@ package deckers.thibault.aves.channel.calls.fetchers import android.content.Context +import android.graphics.Bitmap import android.graphics.Rect import android.net.Uri -import deckers.thibault.aves.utils.BitmapUtils.getRawBytes +import deckers.thibault.aves.utils.BitmapUtils import io.flutter.plugin.common.MethodChannel import org.beyka.tiffbitmapfactory.DecodeArea import org.beyka.tiffbitmapfactory.TiffBitmapFactory @@ -31,8 +32,8 @@ class TiffRegionFetcher internal constructor( inSampleSize = sampleSize inDecodeArea = DecodeArea(regionRect.left, regionRect.top, regionRect.width(), regionRect.height()) } - val bitmap = TiffBitmapFactory.decodeFileDescriptor(fd, options) - val bytes = bitmap?.getRawBytes(recycle = true) + val bitmap: Bitmap? = TiffBitmapFactory.decodeFileDescriptor(fd, options) + val bytes = BitmapUtils.getRawBytes(bitmap, recycle = true) if (bytes != null) { result.success(bytes) } else { diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/channel/streams/ImageByteStreamHandler.kt b/android/app/src/main/kotlin/deckers/thibault/aves/channel/streams/ImageByteStreamHandler.kt index 91c88f3fa..b833ae70d 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/channel/streams/ImageByteStreamHandler.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/channel/streams/ImageByteStreamHandler.kt @@ -8,9 +8,8 @@ import android.util.Log import androidx.core.net.toUri import com.bumptech.glide.Glide import deckers.thibault.aves.decoder.AvesAppGlideModule +import deckers.thibault.aves.utils.BitmapUtils import deckers.thibault.aves.utils.BitmapUtils.applyExifOrientation -import deckers.thibault.aves.utils.BitmapUtils.getEncodedBytes -import deckers.thibault.aves.utils.BitmapUtils.getRawBytes import deckers.thibault.aves.utils.LogUtils import deckers.thibault.aves.utils.MemoryUtils import deckers.thibault.aves.utils.MimeTypes @@ -25,6 +24,7 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.SupervisorJob import kotlinx.coroutines.launch import kotlinx.coroutines.withContext +import java.io.ByteArrayInputStream import java.io.InputStream class ImageByteStreamHandler(private val context: Context, private val arguments: Any?) : EventChannel.StreamHandler { @@ -153,15 +153,16 @@ class ImageByteStreamHandler(private val context: Context, private val arguments bitmap = applyExifOrientation(context, bitmap, rotationDegrees, isFlipped) } if (bitmap != null) { + // do not recycle bitmaps fetched from Glide as their lifecycle is unknown val recycle = false val bytes = if (decoded) { - bitmap.getRawBytes(recycle) + BitmapUtils.getRawBytes(bitmap, recycle = recycle) } else { - bitmap.getEncodedBytes(canHaveAlpha = MimeTypes.canHaveAlpha(mimeType), recycle = recycle) + BitmapUtils.getEncodedBytes(bitmap, canHaveAlpha = MimeTypes.canHaveAlpha(mimeType), recycle = recycle) } if (MemoryUtils.canAllocate(sizeBytes)) { - success(bytes) + streamBytes(ByteArrayInputStream(bytes)) } else { error("streamImage-image-decode-large", "decoded image too large at $sizeBytes bytes, for mimeType=$mimeType uri=$uri", null) } @@ -184,15 +185,16 @@ class ImageByteStreamHandler(private val context: Context, private val arguments try { val bitmap = withContext(Dispatchers.IO) { target.get() } if (bitmap != null) { + // do not recycle bitmaps fetched from Glide as their lifecycle is unknown val recycle = false val bytes = if (decoded) { - bitmap.getRawBytes(recycle) + BitmapUtils.getRawBytes(bitmap, recycle = recycle) } else { - bitmap.getEncodedBytes(canHaveAlpha = false, recycle = false) + BitmapUtils.getEncodedBytes(bitmap, canHaveAlpha = false, recycle = recycle) } if (MemoryUtils.canAllocate(sizeBytes)) { - success(bytes) + streamBytes(ByteArrayInputStream(bytes)) } else { error("streamImage-video-large", "decoded image too large at $sizeBytes bytes, for mimeType=$mimeType uri=$uri", null) } diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/decoder/TiffGlideModule.kt b/android/app/src/main/kotlin/deckers/thibault/aves/decoder/TiffGlideModule.kt index 5e7d4a81e..48e1a3358 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/decoder/TiffGlideModule.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/decoder/TiffGlideModule.kt @@ -83,7 +83,7 @@ internal class TiffFetcher(val model: TiffImage, val width: Int, val height: Int inSampleSize = sampleSize } try { - val bitmap = TiffBitmapFactory.decodeFileDescriptor(fd, options) + val bitmap: Bitmap? = TiffBitmapFactory.decodeFileDescriptor(fd, options) if (bitmap == null) { callback.onLoadFailed(Exception("Decoding full TIFF yielded null bitmap")) } else if (customSize) { diff --git a/android/app/src/main/kotlin/deckers/thibault/aves/utils/BitmapUtils.kt b/android/app/src/main/kotlin/deckers/thibault/aves/utils/BitmapUtils.kt index 2375511bb..212398a9b 100644 --- a/android/app/src/main/kotlin/deckers/thibault/aves/utils/BitmapUtils.kt +++ b/android/app/src/main/kotlin/deckers/thibault/aves/utils/BitmapUtils.kt @@ -19,9 +19,6 @@ object BitmapUtils { private val LOG_TAG = LogUtils.createTag() private const val INITIAL_BUFFER_SIZE = 2 shl 17 // 256kB - // arbitrary size to detect buffer that may yield an OOM - private const val BUFFER_SIZE_DANGER_THRESHOLD = 3 * (1 shl 20) // MB - private val freeBaos = ArrayList() private val mutex = Mutex() @@ -61,7 +58,15 @@ object BitmapUtils { return pixelCount * getBytePerPixel(config) } - fun Bitmap.getRawBytes(recycle: Boolean): ByteArray? { + fun getRawBytes(bitmap: Bitmap?, recycle: Boolean): ByteArray? { + bitmap ?: return null + + val byteCount = bitmap.byteCount + val width = bitmap.width + val height = bitmap.height + val config = bitmap.config + val colorSpace = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) bitmap.colorSpace else null + if (!MemoryUtils.canAllocate(byteCount)) { throw Exception("bitmap buffer is $byteCount bytes, which cannot be allocated to a new byte array") } @@ -69,9 +74,12 @@ object BitmapUtils { try { // `ByteBuffer` initial order is always `BIG_ENDIAN` var bytes = ByteBuffer.allocate(byteCount + RAW_BYTES_TRAILER_LENGTH).apply { - copyPixelsToBuffer(this) + bitmap.copyPixelsToBuffer(this) }.array() + // do not access bitmap after recycling + if (recycle) bitmap.recycle() + // convert pixel format and color space, if necessary if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { colorSpace?.let { srcColorSpace -> @@ -91,9 +99,6 @@ object BitmapUtils { } } - // should not be called before accessing color space or other properties - if (recycle) this.recycle() - // append bitmap size for use by the caller to interpret the raw bytes val trailerOffset = bytes.size - RAW_BYTES_TRAILER_LENGTH bytes = ByteBuffer.wrap(bytes).apply { @@ -109,7 +114,9 @@ object BitmapUtils { return null } - suspend fun Bitmap.getEncodedBytes(canHaveAlpha: Boolean = false, quality: Int = 100, recycle: Boolean): ByteArray? { + suspend fun getEncodedBytes(bitmap: Bitmap?, canHaveAlpha: Boolean = false, quality: Int = 100, recycle: Boolean): ByteArray? { + bitmap ?: return null + val stream: ByteArrayOutputStream mutex.withLock { // this method is called a lot, so we try and reuse output streams @@ -123,15 +130,15 @@ object BitmapUtils { try { // `Bitmap.CompressFormat.PNG` is slower than `JPEG`, but it allows transparency // the BMP format allows an alpha channel, but Android decoding seems to ignore it - if (canHaveAlpha && hasAlpha()) { - this.compress(Bitmap.CompressFormat.PNG, quality, stream) + if (canHaveAlpha && bitmap.hasAlpha()) { + bitmap.compress(Bitmap.CompressFormat.PNG, quality, stream) } else { - this.compress(Bitmap.CompressFormat.JPEG, quality, stream) + bitmap.compress(Bitmap.CompressFormat.JPEG, quality, stream) } - if (recycle) this.recycle() + if (recycle) bitmap.recycle() val bufferSize = stream.size() - if (bufferSize > BUFFER_SIZE_DANGER_THRESHOLD && !MemoryUtils.canAllocate(bufferSize)) { + if (!MemoryUtils.canAllocate(bufferSize)) { throw Exception("bitmap compressed to $bufferSize bytes, which cannot be allocated to a new byte array") } diff --git a/lib/image_providers/uri_image_provider.dart b/lib/image_providers/uri_image_provider.dart index 7369958bf..20da0de75 100644 --- a/lib/image_providers/uri_image_provider.dart +++ b/lib/image_providers/uri_image_provider.dart @@ -49,22 +49,17 @@ class UriImage extends ImageProvider with EquatableMixin { ); } - // as of Flutter v3.16.4, with additional custom handling for SVG in Dart, - // while handling still PNG and JPEG on Android for color space and config conversion + // prefer Flutter for animation, as well as niche formats and SVG + // prefer Android for the rest, to rely on device codecs and handle config conversion bool _canDecodeWithFlutter(String mimeType, bool isAnimated) { switch(mimeType) { - case MimeTypes.gif: - case MimeTypes.webp: case MimeTypes.bmp: case MimeTypes.wbmp: case MimeTypes.ico: case MimeTypes.svg: return true; - case MimeTypes.jpeg: - case MimeTypes.png: - return isAnimated; default: - return false; + return isAnimated; } } diff --git a/lib/services/media/media_fetch_service.dart b/lib/services/media/media_fetch_service.dart index fc0d8cabc..3e84f8d16 100644 --- a/lib/services/media/media_fetch_service.dart +++ b/lib/services/media/media_fetch_service.dart @@ -110,15 +110,15 @@ class PlatformMediaFetchService implements MediaFetchService { @override Future getEncodedImage(ImageRequest request) { - return getBytes(request, decoded: false); + return _getBytes(request, decoded: false); } @override Future getDecodedImage(ImageRequest request) async { - return getBytes(request, decoded: true).then(InteropDecoding.bytesToCodec); + return _getBytes(request, decoded: true).then(InteropDecoding.bytesToCodec); } - Future getBytes(ImageRequest request, {required bool decoded}) async { + Future _getBytes(ImageRequest request, {required bool decoded}) async { final _onBytesReceived = request.onBytesReceived; try { final opCompleter = Completer(); @@ -157,6 +157,7 @@ class PlatformMediaFetchService implements MediaFetchService { // `await` here, so that `completeError` will be caught below return await opCompleter.future; } on PlatformException catch (e, stack) { + debugPrint('$runtimeType _getBytes failed with error=$e'); if (_isUnknownVisual(request.mimeType)) { await reportService.recordError(e, stack); }