From c8b059104974b00cbc02f4ec138dd46d7aca3324 Mon Sep 17 00:00:00 2001 From: Autumn Skerritt Date: Wed, 29 Jul 2026 15:38:32 +0000 Subject: [PATCH 1/4] fix(player): accept ten-bit SDR scene inputs --- .../tachiyomi/ui/player/scene/SceneMediaProbe.kt | 13 +------------ .../ui/player/scene/SceneMediaProbeTest.kt | 13 +++++++++++-- 2 files changed, 12 insertions(+), 14 deletions(-) diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/SceneMediaProbe.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/SceneMediaProbe.kt index 1466b296f1..b111c0cdcc 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/SceneMediaProbe.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/SceneMediaProbe.kt @@ -24,19 +24,9 @@ internal object SceneMediaProbe { if (pixelFormat in setOf("none", "unknown")) { return false } - val rawBits = values.firstOrNull { it.first == "bits_per_raw_sample" } - ?.second - ?.toIntOrNull() val transfer = values.firstOrNull { it.first == "color_transfer" }?.second.orEmpty() val primaries = values.firstOrNull { it.first == "color_primaries" }?.second.orEmpty() - val profile = values.firstOrNull { it.first == "profile" }?.second.orEmpty() - if ( - rawBits?.let { it > 8 } == true || - TEN_BIT_PIXEL_FORMAT.containsMatchIn(pixelFormat) || - transfer in HDR_TRANSFERS || - primaries == "bt2020" || - profile.contains("main 10") - ) { + if (transfer in HDR_TRANSFERS || primaries == "bt2020") { return false } return true @@ -48,6 +38,5 @@ internal object SceneMediaProbe { } private val HDR_TRANSFERS = setOf("smpte2084", "arib-std-b67") - private val TEN_BIT_PIXEL_FORMAT = Regex("(p0(?:10|12|16)|p(?:9|10|12|14|16)(?:le|be)?)(?:$|[^0-9])") private val PROTECTION_MARKERS = setOf("cenc", "cbcs", "crypto", "encrypted", "encryption", "drm") } diff --git a/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/SceneMediaProbeTest.kt b/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/SceneMediaProbeTest.kt index 4f7eea75b5..4364aabc9f 100644 --- a/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/SceneMediaProbeTest.kt +++ b/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/SceneMediaProbeTest.kt @@ -21,12 +21,21 @@ class SceneMediaProbeTest { } @Test - fun `ten bit and HDR video are rejected`() { + fun `ten bit SDR video is safe`() { listOf( "pix_fmt=yuv420p10le\ncolor_transfer=bt709", + "pix_fmt=yuv420p\nbits_per_raw_sample=10", + "pix_fmt=yuv420p10le\nprofile=Main 10", + ).forEach { output -> + assertTrue(SceneMediaProbe.inspect(output)) + } + } + + @Test + fun `HDR video is rejected`() { + listOf( "pix_fmt=yuv420p\ncolor_transfer=smpte2084", "pix_fmt=yuv420p\ncolor_primaries=bt2020", - "pix_fmt=yuv420p\nbits_per_raw_sample=10", ).forEach { output -> assertFalse(SceneMediaProbe.inspect(output)) } From 3543c32d2fdc38781f1d1b47bbd09a363ef2787d Mon Sep 17 00:00:00 2001 From: Autumn Skerritt Date: Wed, 29 Jul 2026 15:38:32 +0000 Subject: [PATCH 2/4] fix(player): normalize MediaCodec AV1 packets --- .../scene/MediaCodecAv1StreamNormalizer.kt | 103 ++++++++++++++++++ .../MediaCodecAv1StreamNormalizerTest.kt | 57 ++++++++++ 2 files changed, 160 insertions(+) create mode 100644 app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/MediaCodecAv1StreamNormalizer.kt create mode 100644 app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/MediaCodecAv1StreamNormalizerTest.kt diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/MediaCodecAv1StreamNormalizer.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/MediaCodecAv1StreamNormalizer.kt new file mode 100644 index 0000000000..c7fd9f227e --- /dev/null +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/MediaCodecAv1StreamNormalizer.kt @@ -0,0 +1,103 @@ +package eu.kanade.tachiyomi.ui.player.scene + +import java.io.ByteArrayOutputStream + +/** + * Removes Android's AV1CodecConfigurationRecord and restores temporal-unit boundaries that raw + * packet output loses. FFmpeg's MediaCodec wrapper incorrectly prepends the record to frame data. + */ +internal object MediaCodecAv1StreamNormalizer { + fun normalize(input: ByteArray): ByteArray? { + if (input.size < AV1C_HEADER_SIZE + 1) return null + val start = if (isAv1CodecConfigurationRecord(input)) AV1C_HEADER_SIZE else 0 + val obus = parseObus(input, start) ?: return null + if (obus.none { it.type == OBU_SEQUENCE_HEADER } || + obus.count { it.type == OBU_FRAME || it.type == OBU_FRAME_HEADER } < 2 + ) { + return null + } + + val output = ByteArrayOutputStream(input.size + obus.size * TEMPORAL_DELIMITER.size) + var frameStarted = false + if (obus.first().type != OBU_TEMPORAL_DELIMITER) { + output.write(TEMPORAL_DELIMITER) + } + obus.forEach { obu -> + when (obu.type) { + OBU_TEMPORAL_DELIMITER -> { + if (!output.endsWithTemporalDelimiter()) { + output.write(TEMPORAL_DELIMITER) + } + frameStarted = false + } + OBU_FRAME, + OBU_FRAME_HEADER, + -> { + if (frameStarted) output.write(TEMPORAL_DELIMITER) + output.write(input, obu.offset, obu.length) + frameStarted = true + } + else -> output.write(input, obu.offset, obu.length) + } + } + return output.toByteArray() + } + + private fun isAv1CodecConfigurationRecord(input: ByteArray): Boolean { + val first = input[0].toInt() and 0xff + return first and 0x80 != 0 && first and 0x7f == 1 + } + + private fun parseObus(input: ByteArray, start: Int): List? { + val result = mutableListOf() + var offset = start + while (offset < input.size) { + val header = input[offset].toInt() and 0xff + if (header and 0x80 != 0 || header and 0x01 != 0 || header and 0x02 == 0) return null + val extensionBytes = if (header and 0x04 != 0) 1 else 0 + val sizeOffset = offset + 1 + extensionBytes + if (sizeOffset >= input.size) return null + val size = readLeb128(input, sizeOffset) ?: return null + val payloadOffset = sizeOffset + size.bytes + val end = payloadOffset.toLong() + size.value + if (end > input.size || end > Int.MAX_VALUE) return null + result += Obu( + type = header shr 3 and 0x0f, + offset = offset, + length = end.toInt() - offset, + ) + offset = end.toInt() + } + return result.takeIf { it.isNotEmpty() } + } + + private fun readLeb128(input: ByteArray, offset: Int): Leb128? { + var value = 0L + for (index in 0 until MAX_LEB128_BYTES) { + val position = offset + index + if (position >= input.size) return null + val byte = input[position].toInt() and 0xff + value = value or ((byte and 0x7f).toLong() shl (index * 7)) + if (byte and 0x80 == 0) return Leb128(value, index + 1) + } + return null + } + + private fun ByteArrayOutputStream.endsWithTemporalDelimiter(): Boolean { + val bytes = toByteArray() + return bytes.size >= TEMPORAL_DELIMITER.size && + bytes[bytes.lastIndex - 1] == TEMPORAL_DELIMITER[0] && + bytes[bytes.lastIndex] == TEMPORAL_DELIMITER[1] + } + + private data class Obu(val type: Int, val offset: Int, val length: Int) + private data class Leb128(val value: Long, val bytes: Int) + + private val TEMPORAL_DELIMITER = byteArrayOf(0x12, 0x00) + private const val AV1C_HEADER_SIZE = 4 + private const val MAX_LEB128_BYTES = 8 + private const val OBU_SEQUENCE_HEADER = 1 + private const val OBU_TEMPORAL_DELIMITER = 2 + private const val OBU_FRAME_HEADER = 3 + private const val OBU_FRAME = 6 +} diff --git a/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/MediaCodecAv1StreamNormalizerTest.kt b/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/MediaCodecAv1StreamNormalizerTest.kt new file mode 100644 index 0000000000..f16e9caa19 --- /dev/null +++ b/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/MediaCodecAv1StreamNormalizerTest.kt @@ -0,0 +1,57 @@ +package eu.kanade.tachiyomi.ui.player.scene + +import org.junit.jupiter.api.Assertions.assertArrayEquals +import org.junit.jupiter.api.Assertions.assertNull +import org.junit.jupiter.api.Test + +class MediaCodecAv1StreamNormalizerTest { + @Test + fun `strips av1C and restores temporal boundaries`() { + assertArrayEquals( + byteArrayOf( + 0x12, + 0x00, + 0x0a, + 0x01, + 0x00, + 0x32, + 0x01, + 0x11, + 0x12, + 0x00, + 0x32, + 0x01, + 0x22, + ), + MediaCodecAv1StreamNormalizer.normalize(mediaCodecAv1PacketStream()), + ) + } + + @Test + fun `rejects malformed and single-frame streams`() { + assertNull(MediaCodecAv1StreamNormalizer.normalize(byteArrayOf(0x81.toByte(), 0x00))) + assertNull( + MediaCodecAv1StreamNormalizer.normalize( + mediaCodecAv1PacketStream().dropLast(3).toByteArray(), + ), + ) + } +} + +internal fun mediaCodecAv1PacketStream(): ByteArray { + return byteArrayOf( + 0x81.toByte(), + 0x00, + 0x00, + 0x00, + 0x0a, + 0x01, + 0x00, + 0x32, + 0x01, + 0x11, + 0x32, + 0x01, + 0x22, + ) +} From 5a6e4b8d2be35fa0f935158b291e0c9e1490f543 Mon Sep 17 00:00:00 2001 From: Autumn Skerritt Date: Wed, 29 Jul 2026 15:38:32 +0000 Subject: [PATCH 3/4] fix(player): remux normalized AV1 into animated AVIF --- .../scene/AndroidSceneCaptureService.kt | 59 +++++++++++++------ .../ui/player/scene/SceneVideoInput.kt | 30 ++++++++-- .../scene/AndroidSceneCaptureServiceTest.kt | 51 +++++++++++++--- .../ui/player/scene/SceneVideoInputTest.kt | 47 ++++++++++++--- 4 files changed, 149 insertions(+), 38 deletions(-) diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureService.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureService.kt index 899b811ff7..985e43b05c 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureService.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureService.kt @@ -8,6 +8,7 @@ import chimahon.anki.AnkiMediaNaming import chimahon.anki.AnkiScreenshotPreparation import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.withContext import java.io.File import java.util.UUID @@ -47,25 +48,45 @@ internal class AndroidSceneCaptureService private constructor( val lease = inputAcquirer.acquire(input) ?: return@withContext AnkiScreenshotPreparation.Failed(stillFallback = null) sceneDirectory.mkdirs() - val output = File(sceneDirectory, "${UUID.randomUUID()}.avif") - val inputCleanup = SceneNativeCleanup(lease::close) - val outputCleanup = SceneNativeCleanup(output::delete) + val outputBaseName = UUID.randomUUID().toString() + val intermediate = File(sceneDirectory, "$outputBaseName.obu") + val output = File(sceneDirectory, "$outputBaseName.avif") var transferred = false try { - val result = commandExecutor.executeFfmpeg( - SceneFfmpegArguments.animatedAvif( - input = input, - acquiredInputValue = lease.ffmpegValue, - range = range, - outputFile = output.absolutePath, - encoderName = encoderName, - tlsCaFile = lease.tlsCaFile, - ), - ) { - inputCleanup.nativeFinished() - outputCleanup.nativeFinished() + val encodeResult = withContext(NonCancellable) { + commandExecutor.executeFfmpeg( + SceneFfmpegArguments.av1MediaCodecPackets( + input = input, + acquiredInputValue = lease.ffmpegValue, + range = range, + outputFile = intermediate.absolutePath, + encoderName = encoderName, + tlsCaFile = lease.tlsCaFile, + ), + ) } - when (result) { + when (encodeResult) { + SceneCommandResult.Failed -> { + return@withContext AnkiScreenshotPreparation.Failed(stillFallback = null) + } + is SceneCommandResult.Success -> Unit + } + val normalized = intermediate + .takeIf { it.isFile && it.length() in 1..MAX_INTERMEDIATE_BYTES } + ?.readBytes() + ?.let(MediaCodecAv1StreamNormalizer::normalize) + ?: return@withContext AnkiScreenshotPreparation.Failed(stillFallback = null) + intermediate.writeBytes(normalized) + + val remuxResult = withContext(NonCancellable) { + commandExecutor.executeFfmpeg( + SceneFfmpegArguments.animatedAvifFromObu( + inputFile = intermediate.absolutePath, + outputFile = output.absolutePath, + ), + ) + } + when (remuxResult) { SceneCommandResult.Failed -> { return@withContext AnkiScreenshotPreparation.Failed(stillFallback = null) } @@ -90,8 +111,9 @@ internal class AndroidSceneCaptureService private constructor( } catch (_: Exception) { AnkiScreenshotPreparation.Failed(stillFallback = null) } finally { - inputCleanup.release() - if (!transferred) outputCleanup.release() + lease.close() + intermediate.delete() + if (!transferred) output.delete() } } } @@ -113,6 +135,7 @@ internal class AndroidSceneCaptureService private constructor( internal companion object { private const val SCENE_CACHE_DIRECTORY = "chimahon_scene_capture" private const val MAX_OUTPUT_DIMENSION = 640 + private const val MAX_INTERMEDIATE_BYTES = 12L * 1024L * 1024L fun forTests( sceneDirectory: File, diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/SceneVideoInput.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/SceneVideoInput.kt index b6f19d112a..aafa648f8d 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/SceneVideoInput.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/SceneVideoInput.kt @@ -163,7 +163,7 @@ internal object SceneVideoInputResolver { } internal object SceneFfmpegArguments { - fun animatedAvif( + fun av1MediaCodecPackets( input: SceneVideoInputSpec, acquiredInputValue: String, range: SceneTimeRange, @@ -201,15 +201,37 @@ internal object SceneFfmpegArguments { add("1") add("-pix_fmt") add("yuv420p") - add("-loop") - add("0") add("-f") - add("avif") + add("data") add("-y") add(outputFile) }.toTypedArray() } + fun animatedAvifFromObu( + inputFile: String, + outputFile: String, + ): Array { + return arrayOf( + "-f", + "obu", + "-framerate", + FRAME_RATE.toInt().toString(), + "-i", + inputFile, + "-map", + "0:v:0", + "-c:v", + "copy", + "-loop", + "0", + "-f", + "avif", + "-y", + outputFile, + ) + } + fun videoProbe( input: SceneVideoInputSpec, acquiredInputValue: String, diff --git a/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureServiceTest.kt b/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureServiceTest.kt index 8e0ec461ec..2e0ff3ff1e 100644 --- a/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureServiceTest.kt +++ b/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureServiceTest.kt @@ -18,7 +18,7 @@ class AndroidSceneCaptureServiceTest { lateinit var tempDirectory: File @Test - fun `successful capture uses exact bounded AVIF command`() = runTest { + fun `successful capture normalizes AV1 packets then remuxes them to animated AVIF`() = runTest { val executor = RecordingExecutor(writeOutput = true) val service = service( executor = executor, @@ -30,10 +30,23 @@ class AndroidSceneCaptureServiceTest { val animated = result as AnkiScreenshotPreparation.Animated assertEquals("avif", animated.animation.extension) assertTrue(animated.animation.preferredBaseName.startsWith("chimahon_scene_")) + assertEquals(2, executor.ffmpegArguments.size) assertArrayEquals( - expectedAvifArguments(animated.animation.file.absolutePath), - executor.ffmpegArguments, + expectedAv1Arguments(animated.animation.file.absolutePath.replaceAfterLast('.', "obu")), + executor.ffmpegArguments[0], ) + assertArrayEquals( + expectedAvifRemuxArguments( + animated.animation.file.absolutePath.replaceAfterLast('.', "obu"), + animated.animation.file.absolutePath, + ), + executor.ffmpegArguments[1], + ) + val intermediate = File( + animated.animation.file.parentFile, + "${animated.animation.file.nameWithoutExtension}.obu", + ) + assertFalse(intermediate.exists()) animated.animation.file.delete() } @@ -110,7 +123,7 @@ class AndroidSceneCaptureServiceTest { ) } - private fun expectedAvifArguments(output: String): Array { + private fun expectedAv1Arguments(output: String): Array { return arrayOf( "-codec_whitelist", SceneFfmpegArguments.ALLOWED_INPUT_DECODERS, @@ -151,6 +164,25 @@ class AndroidSceneCaptureServiceTest { "1", "-pix_fmt", "yuv420p", + "-f", + "data", + "-y", + output, + ) + } + + private fun expectedAvifRemuxArguments(input: String, output: String): Array { + return arrayOf( + "-f", + "obu", + "-framerate", + "8", + "-i", + input, + "-map", + "0:v:0", + "-c:v", + "copy", "-loop", "0", "-f", @@ -165,7 +197,7 @@ class AndroidSceneCaptureServiceTest { ) : SceneCommandExecutor { var probeCalls = 0 var ffmpegCalls = 0 - var ffmpegArguments: Array = emptyArray() + val ffmpegArguments = mutableListOf>() override suspend fun executeFfmpeg( arguments: Array, @@ -173,9 +205,14 @@ class AndroidSceneCaptureServiceTest { ): SceneCommandResult { return try { ffmpegCalls++ - ffmpegArguments = arguments + ffmpegArguments += arguments if (writeOutput) { - File(arguments.last()).writeBytes(byteArrayOf(1, 2, 3)) + val output = File(arguments.last()) + val bytes = when (output.extension) { + "obu" -> mediaCodecAv1PacketStream() + else -> byteArrayOf(1, 2, 3) + } + output.writeBytes(bytes) } SceneCommandResult.Success() } finally { diff --git a/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/SceneVideoInputTest.kt b/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/SceneVideoInputTest.kt index 243daccfde..b181594929 100644 --- a/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/SceneVideoInputTest.kt +++ b/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/SceneVideoInputTest.kt @@ -62,13 +62,13 @@ class SceneVideoInputTest { } @Test - fun `AVIF command has the single bounded native recipe`() { + fun `AV1 encode writes raw MediaCodec packets`() { val input = supportedInput() - val arguments = SceneFfmpegArguments.animatedAvif( + val arguments = SceneFfmpegArguments.av1MediaCodecPackets( input = input, acquiredInputValue = "https://media.example/video.mp4", range = SceneTimeRange(1.25, 11.25), - outputFile = "/cache/output.avif", + outputFile = "/cache/output.obu", encoderName = TEST_AV1_ENCODER_NAME, tlsCaFile = "/files/cacert.pem", ).toList() @@ -88,7 +88,7 @@ class SceneVideoInputTest { ), ) assertTrue(arguments.containsAll(listOf("-ndk_codec", "1", "-pix_fmt", "yuv420p"))) - assertTrue(arguments.containsAll(listOf("-frames:v", "80", "-loop", "0", "-f", "avif"))) + assertTrue(arguments.containsAll(listOf("-frames:v", "80", "-f", "data"))) assertTrue( arguments.containsAll( listOf( @@ -106,7 +106,36 @@ class SceneVideoInputTest { assertEquals(1, arguments.count { it == "-c:v" }) assertEquals(SceneFfmpegArguments.FRAME_FILTER, arguments[arguments.indexOf("-vf") + 1]) assertTrue(SceneFfmpegArguments.FRAME_FILTER.contains("force_divisible_by=16")) - assertFalse(arguments.any { it.contains("webp", ignoreCase = true) }) + assertFalse(arguments.contains("avif")) + assertFalse(arguments.contains("-loop")) + } + + @Test + fun `AVIF remux copies the normalized OBU stream`() { + assertEquals( + listOf( + "-f", + "obu", + "-framerate", + "8", + "-i", + "/cache/input.obu", + "-map", + "0:v:0", + "-c:v", + "copy", + "-loop", + "0", + "-f", + "avif", + "-y", + "/cache/output.avif", + ), + SceneFfmpegArguments.animatedAvifFromObu( + inputFile = "/cache/input.obu", + outputFile = "/cache/output.avif", + ).toList(), + ) } @Test @@ -115,11 +144,11 @@ class SceneVideoInputTest { val range = SceneTimeRange(1.25, 2.25) val caFile = "/files/cacert.pem" val commands = listOf( - SceneFfmpegArguments.animatedAvif( + SceneFfmpegArguments.av1MediaCodecPackets( input = input, acquiredInputValue = input.value, range = range, - outputFile = "/cache/scene.avif", + outputFile = "/cache/scene.obu", encoderName = TEST_AV1_ENCODER_NAME, tlsCaFile = caFile, ), @@ -147,11 +176,11 @@ class SceneVideoInputTest { .sentenceAudio(input, input.value, range, "/cache/audio.m4a", caFile) .toList() val video = SceneFfmpegArguments - .animatedAvif( + .av1MediaCodecPackets( input = input, acquiredInputValue = input.value, range = range, - outputFile = "/cache/scene.avif", + outputFile = "/cache/scene.obu", encoderName = TEST_AV1_ENCODER_NAME, tlsCaFile = caFile, ) From bcd95c2a93d36076e1dded3d08bd8487861992b5 Mon Sep 17 00:00:00 2001 From: Autumn Skerritt Date: Wed, 29 Jul 2026 15:38:32 +0000 Subject: [PATCH 4/4] fix(player): preserve cancellation during AVIF remux --- .../scene/AndroidSceneCaptureService.kt | 54 +++++++++------ .../scene/FfmpegKitSceneCommandExecutor.kt | 48 ++++++++++--- .../scene/AndroidSceneCaptureServiceTest.kt | 68 ++++++++++++++++--- 3 files changed, 129 insertions(+), 41 deletions(-) diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureService.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureService.kt index 985e43b05c..aab3693d0c 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureService.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureService.kt @@ -8,7 +8,6 @@ import chimahon.anki.AnkiMediaNaming import chimahon.anki.AnkiScreenshotPreparation import kotlinx.coroutines.CancellationException import kotlinx.coroutines.Dispatchers -import kotlinx.coroutines.NonCancellable import kotlinx.coroutines.withContext import java.io.File import java.util.UUID @@ -51,20 +50,25 @@ internal class AndroidSceneCaptureService private constructor( val outputBaseName = UUID.randomUUID().toString() val intermediate = File(sceneDirectory, "$outputBaseName.obu") val output = File(sceneDirectory, "$outputBaseName.avif") + val inputCleanup = SceneNativeCleanup(lease::close) + val intermediateCleanup = SceneNativeCleanup(intermediate::delete) + var outputCleanup: SceneNativeCleanup? = null var transferred = false try { - val encodeResult = withContext(NonCancellable) { - commandExecutor.executeFfmpeg( - SceneFfmpegArguments.av1MediaCodecPackets( - input = input, - acquiredInputValue = lease.ffmpegValue, - range = range, - outputFile = intermediate.absolutePath, - encoderName = encoderName, - tlsCaFile = lease.tlsCaFile, - ), - ) + val encodeResult = commandExecutor.executeFfmpeg( + SceneFfmpegArguments.av1MediaCodecPackets( + input = input, + acquiredInputValue = lease.ffmpegValue, + range = range, + outputFile = intermediate.absolutePath, + encoderName = encoderName, + tlsCaFile = lease.tlsCaFile, + ), + ) { + inputCleanup.nativeFinished() + intermediateCleanup.nativeFinished() } + inputCleanup.release() when (encodeResult) { SceneCommandResult.Failed -> { return@withContext AnkiScreenshotPreparation.Failed(stillFallback = null) @@ -78,13 +82,17 @@ internal class AndroidSceneCaptureService private constructor( ?: return@withContext AnkiScreenshotPreparation.Failed(stillFallback = null) intermediate.writeBytes(normalized) - val remuxResult = withContext(NonCancellable) { - commandExecutor.executeFfmpeg( - SceneFfmpegArguments.animatedAvifFromObu( - inputFile = intermediate.absolutePath, - outputFile = output.absolutePath, - ), - ) + val currentOutputCleanup = SceneNativeCleanup(output::delete) + outputCleanup = currentOutputCleanup + val finishIntermediateRemuxUse = intermediateCleanup.retainNativeUse() + val remuxResult = commandExecutor.executeFfmpeg( + SceneFfmpegArguments.animatedAvifFromObu( + inputFile = intermediate.absolutePath, + outputFile = output.absolutePath, + ), + ) { + finishIntermediateRemuxUse() + currentOutputCleanup.nativeFinished() } when (remuxResult) { SceneCommandResult.Failed -> { @@ -111,9 +119,11 @@ internal class AndroidSceneCaptureService private constructor( } catch (_: Exception) { AnkiScreenshotPreparation.Failed(stillFallback = null) } finally { - lease.close() - intermediate.delete() - if (!transferred) output.delete() + inputCleanup.release() + intermediateCleanup.release() + if (!transferred) { + outputCleanup?.release() ?: output.delete() + } } } } diff --git a/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/FfmpegKitSceneCommandExecutor.kt b/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/FfmpegKitSceneCommandExecutor.kt index 3bc5f60bcd..da83f510d2 100644 --- a/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/FfmpegKitSceneCommandExecutor.kt +++ b/app/src/main/java/eu/kanade/tachiyomi/ui/player/scene/FfmpegKitSceneCommandExecutor.kt @@ -37,24 +37,52 @@ internal interface SceneCommandExecutor { internal class SceneNativeCleanup( private val cleanup: () -> Unit, ) { - private val nativeFinished = AtomicBoolean(false) - private val released = AtomicBoolean(false) - private val cleaned = AtomicBoolean(false) + private val lock = Any() + private val initialNativeFinished = AtomicBoolean(false) + private var activeNativeUses = 1 + private var released = false + private var cleaned = false fun nativeFinished() { - nativeFinished.set(true) - cleanIfReady() + finishNativeUse(initialNativeFinished) + } + + fun retainNativeUse(): () -> Unit { + synchronized(lock) { + check(!released) { "Cannot retain a released native resource" } + activeNativeUses++ + } + val finished = AtomicBoolean(false) + return { + finishNativeUse(finished) + } } fun release() { - released.set(true) - cleanIfReady() + val shouldClean = synchronized(lock) { + released = true + markCleanIfReady() + } + if (shouldClean) runCatching(cleanup) + } + + private fun finishNativeUse(finished: AtomicBoolean) { + if (finished.compareAndSet(false, true)) { + val shouldClean = synchronized(lock) { + check(activeNativeUses > 0) + activeNativeUses-- + markCleanIfReady() + } + if (shouldClean) runCatching(cleanup) + } } - private fun cleanIfReady() { - if (nativeFinished.get() && released.get() && cleaned.compareAndSet(false, true)) { - runCatching(cleanup) + private fun markCleanIfReady(): Boolean { + if (activeNativeUses == 0 && released && !cleaned) { + cleaned = true + return true } + return false } } diff --git a/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureServiceTest.kt b/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureServiceTest.kt index 2e0ff3ff1e..d5e8bb05aa 100644 --- a/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureServiceTest.kt +++ b/app/src/test/kotlin/eu/kanade/tachiyomi/ui/player/scene/AndroidSceneCaptureServiceTest.kt @@ -4,7 +4,14 @@ import android.graphics.Bitmap import chimahon.anki.AnkiScreenshotPreparation import io.mockk.every import io.mockk.mockk +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.cancelAndJoin +import kotlinx.coroutines.launch +import kotlinx.coroutines.suspendCancellableCoroutine import kotlinx.coroutines.test.runTest +import kotlinx.coroutines.withContext +import kotlinx.coroutines.withTimeout import org.junit.jupiter.api.Assertions.assertArrayEquals import org.junit.jupiter.api.Assertions.assertEquals import org.junit.jupiter.api.Assertions.assertFalse @@ -81,6 +88,32 @@ class AndroidSceneCaptureServiceTest { assertTrue(sceneDirectory.listFiles().isNullOrEmpty()) } + @Test + fun `cancellation reaches native remux and defers file cleanup until native return`() = runTest { + val executor = RecordingExecutor(writeOutput = true, suspendRemux = true) + val service = service(executor = executor) + val preparation = launch { service.prepare(request()) } + withContext(Dispatchers.Default) { + withTimeout(5_000) { executor.remuxStarted.await() } + } + val remuxArguments = executor.ffmpegArguments.last() + val intermediate = File(remuxArguments[remuxArguments.indexOf("-i") + 1]) + val output = File(remuxArguments.last()) + + preparation.cancelAndJoin() + withContext(Dispatchers.Default) { + withTimeout(5_000) { executor.cancellationObserved.await() } + } + + assertTrue(intermediate.isFile) + assertTrue(output.isFile) + + executor.finishNative() + + assertFalse(intermediate.exists()) + assertFalse(output.exists()) + } + private fun service( executor: RecordingExecutor, validate: (File) -> AnimatedAvifInfo? = { @@ -194,26 +227,39 @@ class AndroidSceneCaptureServiceTest { private class RecordingExecutor( private val writeOutput: Boolean, + private val suspendRemux: Boolean = false, ) : SceneCommandExecutor { var probeCalls = 0 var ffmpegCalls = 0 val ffmpegArguments = mutableListOf>() + val remuxStarted = CompletableDeferred() + val cancellationObserved = CompletableDeferred() + private lateinit var onRemuxFinished: () -> Unit override suspend fun executeFfmpeg( arguments: Array, onNativeFinished: () -> Unit, ): SceneCommandResult { - return try { - ffmpegCalls++ - ffmpegArguments += arguments - if (writeOutput) { - val output = File(arguments.last()) - val bytes = when (output.extension) { - "obu" -> mediaCodecAv1PacketStream() - else -> byteArrayOf(1, 2, 3) + ffmpegCalls++ + ffmpegArguments += arguments + val output = File(arguments.last()) + if (writeOutput) { + val bytes = when (output.extension) { + "obu" -> mediaCodecAv1PacketStream() + else -> byteArrayOf(1, 2, 3) + } + output.writeBytes(bytes) + } + if (suspendRemux && output.extension == "avif") { + onRemuxFinished = onNativeFinished + remuxStarted.complete(Unit) + return suspendCancellableCoroutine { continuation -> + continuation.invokeOnCancellation { + cancellationObserved.complete(Unit) } - output.writeBytes(bytes) } + } + return try { SceneCommandResult.Success() } finally { onNativeFinished() @@ -233,6 +279,10 @@ class AndroidSceneCaptureServiceTest { onNativeFinished() } } + + fun finishNative() { + onRemuxFinished() + } } private companion object {