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

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()) {
+                splitTallImageIfNeeded(page, tmpDir)
-                    splitTallImage(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.
      */

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.
  */

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
     }
 }

app/src/main/res/values/strings.xml

@@ -815,6 +815,7 @@
     <string name="download_notifier_download_paused">Download paused</string>
     <string name="download_notifier_download_finish">Download completed</string>
     <string name="download_notifier_split_page_not_found">Page %d not found while splitting</string>
+    <string name="download_notifier_split_page_path_not_found">Couldn\'t find file path of page %d</string>
 
     <!-- Notification channels -->
     <string name="channel_common">Common</string>