From 9f655e0d41a690752ac440d9bc8fb56188d4ef0c Mon Sep 17 00:00:00 2001 From: FourTOne5 <59261191+FourTOne5@users.noreply.github.com> Date: Wed, 11 May 2022 03:06:18 +0600 Subject: [PATCH] Fix download splitter potentially throwing OOM on huge images (#7099) * Fix download splitter potentially throwing OOM on huge images Also move the splitting to ImageUtil * Change variable name and logcat output --- .../tachiyomi/data/download/Downloader.kt | 66 +++----------- .../util/system/ContextExtensions.kt | 3 + .../kanade/tachiyomi/util/system/ImageUtil.kt | 86 ++++++++++++++++--- app/src/main/res/values/strings.xml | 1 + 4 files changed, 90 insertions(+), 66 deletions(-) diff --git a/app/src/main/java/eu/kanade/tachiyomi/data/download/Downloader.kt b/app/src/main/java/eu/kanade/tachiyomi/data/download/Downloader.kt index b4535cfdec..7e62a35a16 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/data/download/Downloader.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/data/download/Downloader.kt @@ -1,8 +1,6 @@ package eu.kanade.tachiyomi.data.download import android.content.Context -import android.graphics.Bitmap -import android.graphics.BitmapFactory import android.webkit.MimeTypeMap import com.hippo.unifile.UniFile import com.jakewharton.rxrelay.BehaviorRelay @@ -29,8 +27,6 @@ import eu.kanade.tachiyomi.util.lang.withUIContext import eu.kanade.tachiyomi.util.storage.DiskUtil import eu.kanade.tachiyomi.util.storage.saveTo import eu.kanade.tachiyomi.util.system.ImageUtil -import eu.kanade.tachiyomi.util.system.ImageUtil.isAnimatedAndSupported -import eu.kanade.tachiyomi.util.system.ImageUtil.isTallImage import eu.kanade.tachiyomi.util.system.logcat import kotlinx.coroutines.async import logcat.LogPriority @@ -42,12 +38,9 @@ import rx.subscriptions.CompositeSubscription import uy.kohesive.injekt.injectLazy import java.io.BufferedOutputStream import java.io.File -import java.io.FileOutputStream import java.util.zip.CRC32 import java.util.zip.ZipEntry import java.util.zip.ZipOutputStream -import kotlin.math.ceil -import kotlin.math.min /** * This class is the one in charge of downloading chapters. @@ -353,9 +346,7 @@ class Downloader( .onBackpressureLatest() // Do when page is downloaded. .doOnNext { page -> - if (preferences.splitTallImages().get()) { - splitTallImage(page, tmpDir) - } + splitTallImageIfNeeded(page, tmpDir) notifier.onProgressChange(download) } .toList() @@ -364,6 +355,7 @@ class Downloader( .doOnNext { ensureSuccessfulDownload(download, mangaDir, tmpDir, chapterDirname) } // If the page list threw, it will resume here .onErrorReturn { error -> + logcat(LogPriority.ERROR, error) download.status = Download.State.ERROR notifier.onError(error.message, download.chapter.name, download.manga.title) download @@ -487,6 +479,18 @@ class Downloader( return MimeTypeMap.getSingleton().getExtensionFromMimeType(mime) ?: "jpg" } + private fun splitTallImageIfNeeded(page: Page, tmpDir: UniFile) { + if (!preferences.splitTallImages().get()) return + + val filename = String.format("%03d", page.number) + val imageFile = tmpDir.listFiles()?.find { it.name!!.startsWith("$filename.") } + ?: throw Error(context.getString(R.string.download_notifier_split_page_not_found, page.number)) + val imageFilePath = imageFile.filePath + ?: throw Error(context.getString(R.string.download_notifier_split_page_path_not_found, page.number)) + + ImageUtil.splitTallImage(imageFile, imageFilePath) + } + /** * Checks if the download was successful. * @@ -557,48 +561,6 @@ class Downloader( tmpDir.delete() } - /** - * Splits tall images to improve performance of reader - */ - private fun splitTallImage(page: Page, tmpDir: UniFile) { - val filename = String.format("%03d", page.number) - val imageFile = tmpDir.listFiles()?.find { it.name!!.startsWith("$filename.") } - ?: throw Error(context.getString(R.string.download_notifier_split_page_not_found, page.number)) - - if (isAnimatedAndSupported(imageFile.openInputStream()) || !isTallImage(imageFile.openInputStream())) { - return - } - - val bitmap = BitmapFactory.decodeFile(imageFile.filePath) - val splitsCount = bitmap.height / context.resources.displayMetrics.heightPixels + 1 - val heightPerSplit = ceil(bitmap.height / splitsCount.toDouble()).toInt() - logcat { "Splitting height ${bitmap.height} by $splitsCount * $heightPerSplit" } - - try { - (0 until splitsCount).forEach { split -> - logcat { "Split #$split at y=${split * heightPerSplit}" } - val splitPath = imageFile.filePath!!.substringBeforeLast(".") + "__${"%03d".format(split + 1)}.jpg" - val splitHeight = split * heightPerSplit - FileOutputStream(splitPath).use { stream -> - Bitmap.createBitmap( - bitmap, - 0, - splitHeight, - bitmap.width, - min(heightPerSplit, bitmap.height - splitHeight), - ).compress(Bitmap.CompressFormat.JPEG, 100, stream) - } - } - imageFile.delete() - } catch (e: Exception) { - // Image splits were not successfully saved so delete them and keep the original image - (0 until splitsCount) - .map { imageFile.filePath!!.substringBeforeLast(".") + "__${"%03d".format(it + 1)}.jpg" } - .forEach { File(it).delete() } - throw e - } - } - /** * Completes a download. This method is called in the main thread. */ diff --git a/app/src/main/java/eu/kanade/tachiyomi/util/system/ContextExtensions.kt b/app/src/main/java/eu/kanade/tachiyomi/util/system/ContextExtensions.kt index 134d8470d8..94b69e9d06 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/util/system/ContextExtensions.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/util/system/ContextExtensions.kt @@ -162,6 +162,9 @@ fun Context.hasPermission(permission: String) = ContextCompat.checkSelfPermissio } } +val getDisplayHeightInPx: Int + get() = Resources.getSystem().displayMetrics.heightPixels + /** * Converts to dp. */ diff --git a/app/src/main/java/eu/kanade/tachiyomi/util/system/ImageUtil.kt b/app/src/main/java/eu/kanade/tachiyomi/util/system/ImageUtil.kt index a596600aaa..2a4978aeed 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/util/system/ImageUtil.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/util/system/ImageUtil.kt @@ -4,6 +4,7 @@ import android.content.Context import android.content.res.Configuration import android.graphics.Bitmap import android.graphics.BitmapFactory +import android.graphics.BitmapRegionDecoder import android.graphics.Color import android.graphics.Rect import android.graphics.drawable.ColorDrawable @@ -16,14 +17,18 @@ import androidx.core.graphics.blue import androidx.core.graphics.createBitmap import androidx.core.graphics.green import androidx.core.graphics.red +import com.hippo.unifile.UniFile import tachiyomi.decoder.Format import tachiyomi.decoder.ImageDecoder import java.io.BufferedInputStream import java.io.ByteArrayInputStream import java.io.ByteArrayOutputStream +import java.io.File +import java.io.FileOutputStream import java.io.InputStream import java.net.URLConnection import kotlin.math.abs +import kotlin.math.min object ImageUtil { @@ -67,8 +72,7 @@ object ImageUtil { Format.Webp -> type.isAnimated && Build.VERSION.SDK_INT >= Build.VERSION_CODES.P else -> false } - } catch (e: Exception) { - } + } catch (e: Exception) { /* Do Nothing */ } return false } @@ -106,20 +110,9 @@ object ImageUtil { */ fun isWideImage(imageStream: BufferedInputStream): Boolean { val options = extractImageOptions(imageStream) - imageStream.reset() return options.outWidth > options.outHeight } - /** - * Check whether the image is considered a tall image. - * - * @return true if the height:width ratio is greater than 3. - */ - fun isTallImage(imageStream: InputStream): Boolean { - val options = extractImageOptions(imageStream) - return (options.outHeight / options.outWidth) > 3 - } - /** * Extract the 'side' part from imageStream and return it as InputStream. */ @@ -183,6 +176,70 @@ object ImageUtil { RIGHT, LEFT } + /** + * Check whether the image is considered a tall image. + * + * @return true if the height:width ratio is greater than 3. + */ + fun isTallImage(imageStream: InputStream): Boolean { + val options = extractImageOptions(imageStream, false) + return (options.outHeight / options.outWidth) > 3 + } + + /** + * Splits tall images to improve performance of reader + */ + fun splitTallImage(imageFile: UniFile, imageFilePath: String) { + if (isAnimatedAndSupported(imageFile.openInputStream()) || !isTallImage(imageFile.openInputStream())) { + return + } + + val options = extractImageOptions(imageFile.openInputStream(), false).apply { inJustDecodeBounds = false } + // Values are stored as they get modified during split loop + val imageHeight = options.outHeight + val imageWidth = options.outWidth + + val splitHeight = getDisplayHeightInPx + // -1 so it doesn't try to split when imageHeight = getDisplayHeightInPx + val partCount = (imageHeight - 1) / getDisplayHeightInPx + 1 + + logcat { "Splitting ${imageHeight}px height image into $partCount part with estimated ${splitHeight}px per height" } + + val bitmapRegionDecoder = if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S) { + BitmapRegionDecoder.newInstance(imageFile.openInputStream()) + } else { + @Suppress("DEPRECATION") + BitmapRegionDecoder.newInstance(imageFile.openInputStream(), false) + } + + try { + (0 until partCount).forEach { splitIndex -> + val splitPath = imageFilePath.substringBeforeLast(".") + "__${"%03d".format(splitIndex + 1)}.jpg" + + val topOffset = splitIndex * splitHeight + val outputImageHeight = min(splitHeight, imageHeight - topOffset) + val bottomOffset = topOffset + outputImageHeight + logcat { "Split #$splitIndex with topOffset=$topOffset height=$outputImageHeight bottomOffset=$bottomOffset" } + + val region = Rect(0, topOffset, imageWidth, bottomOffset) + + FileOutputStream(splitPath).use { outputStream -> + val splitBitmap = bitmapRegionDecoder!!.decodeRegion(region, options) + splitBitmap.compress(Bitmap.CompressFormat.JPEG, 100, outputStream) + } + } + imageFile.delete() + } catch (e: Exception) { + // Image splits were not successfully saved so delete them and keep the original image + (0 until partCount) + .map { imageFile.filePath!!.substringBeforeLast(".") + "__${"%03d".format(it + 1)}.jpg" } + .forEach { File(it).delete() } + throw e + } finally { + bitmapRegionDecoder?.recycle() + } + } + /** * Algorithm for determining what background to accompany a comic/manga page */ @@ -401,12 +458,13 @@ object ImageUtil { /** * Used to check an image's dimensions without loading it in the memory. */ - private fun extractImageOptions(imageStream: InputStream): BitmapFactory.Options { + private fun extractImageOptions(imageStream: InputStream, resetAfterExtraction: Boolean = true): BitmapFactory.Options { imageStream.mark(imageStream.available() + 1) val imageBytes = imageStream.readBytes() val options = BitmapFactory.Options().apply { inJustDecodeBounds = true } BitmapFactory.decodeByteArray(imageBytes, 0, imageBytes.size, options) + if (resetAfterExtraction) imageStream.reset() return options } } diff --git a/app/src/main/res/values/strings.xml b/app/src/main/res/values/strings.xml index c48ae6bbd5..5a1d4b42db 100644 --- a/app/src/main/res/values/strings.xml +++ b/app/src/main/res/values/strings.xml @@ -815,6 +815,7 @@ Download paused Download completed Page %d not found while splitting + Couldn\'t find file path of page %d Common