diff --git a/CLAUDE.md b/CLAUDE.md index 7605396..b20e9ab 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -76,11 +76,11 @@ days. Read it as the current answer, and see the git history if you need the old `angle_indirect` and `swangle_indirect` all boot, while `auto`, `off`, `guest` and `swiftshader_indirect` do not. `docs/local-emulator.md` has the evidence and the per-API renderer table. -- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. **Three** of the 59 instrumented +- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. **Three** of the 60 instrumented tests cannot pass on that image, for two unrelated reasons: two Media3 hardware transcodes fail inside the emulator's own `c2.goldfish.h264.decoder`, and one SAF test takes the framework down when it rotates the display. All three carry `@FailsOnEmulatorApi37` and run in a separate - `continue-on-error` job; the gating leg runs the other 56. + `continue-on-error` job; the gating leg runs the other 57. That job is still called `E2E API 37 Media3 hardware transcode (advisory)`, which no longer describes everything in it. The name is kept deliberately — it is not a required context and diff --git a/app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt b/app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt index 08c5da7..3ae4173 100644 --- a/app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt +++ b/app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt @@ -11,15 +11,26 @@ import kotlinx.coroutines.runBlocking import kotlinx.coroutines.withTimeout import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertNull import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Test import org.junit.runner.RunWith import org.libremediaconverter.FailsOnEmulatorApi37 +import org.libremediaconverter.model.AudioCodec +import org.libremediaconverter.model.AudioPlan +import org.libremediaconverter.model.Container import org.libremediaconverter.model.ConversionRequest +import org.libremediaconverter.model.CopyPlanner +import org.libremediaconverter.model.InputKind +import org.libremediaconverter.model.InputProbe import org.libremediaconverter.model.OutputFormat +import org.libremediaconverter.model.OutputSpec +import org.libremediaconverter.model.VideoCodec +import org.libremediaconverter.model.VideoPlan import java.io.File +import java.util.concurrent.CancellationException import java.util.concurrent.Executors import java.util.concurrent.TimeUnit @@ -170,6 +181,63 @@ class Media3EngineTest { ) } + /** + * The builders that used to throw where nothing could catch them. + * + * `EditedMediaItem.Builder` rejects a composition with both tracks removed — + * checkState("Audio and video cannot both be removed") — and the engine builds it on its own + * HandlerThread. That build sat *between* two narrow `runCatching` blocks, one around + * `buildTransformer` and one around `start`, so the exception reached the thread's uncaught + * handler and took the process with it while the continuation was never resumed. + * + * `ContainerCapabilities.validate` now refuses the spec that gets here from the picker; this + * is the other half — the engine surviving a request that arrives without being validated. + * Deliberately not `@FailsOnEmulatorApi37`: nothing here decodes or encodes, so no emulator + * codec is involved. The builder refuses the input before any media is touched. + */ + @Test + fun aPlanThatRemovesBothTracksFailsInsteadOfKillingTheProcess() { + val request = ConversionRequest( + spec = OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE), + probe = InputProbe( + videoCodec = null, + audioCodec = "mp3", + hasVideo = false, + container = Container.MP3, + kind = InputKind.AUDIO_ONLY, + ), + ) + // Asserted rather than assumed: ConversionRequest's default probe says hasVideo = true, + // and with it this same spec plans to (Encode, Drop) and nothing throws at all — which + // would make the whole test vacuous without a word of warning. + val plan = CopyPlanner.plan(request.spec, request.probe) + assertEquals(VideoPlan.Drop, plan.video) + assertEquals(AudioPlan.Drop, plan.audio) + + val failure = runCatching { + runBlocking { + withTimeout(BUILDER_TIMEOUT_MS) { + engine.transcode(Uri.fromFile(input), output, request) {} + } + } + }.exceptionOrNull() + + // Two assertions, and the second is not pedantry. withTimeout raises + // TimeoutCancellationException, and `java.util.concurrent.CancellationException` *extends* + // IllegalStateException — so testing only the type below would call an unresumed + // continuation a pass. A hang is the other half of this defect and every bit as bad as the + // crash: the worker would sit holding a foreground service forever. + assertFalse( + "the continuation was never resumed — the failure escaped instead of being reported: " + + "$failure", + failure is CancellationException, + ) + assertTrue( + "the builder's refusal must surface as a failed job, not a dead process; got $failure", + failure is IllegalStateException, + ) + } + private fun durationMsOf(file: File): Long { val extractor = MediaExtractor() return try { @@ -211,5 +279,12 @@ class Media3EngineTest { private companion object { const val TIMEOUT_SECONDS = 120L + + /** + * Short on purpose. Nothing is decoded or encoded on this path — the builder refuses the + * input outright — so anything approaching this is a hang, which is what the test is + * looking for. + */ + const val BUILDER_TIMEOUT_MS = 30_000L } } diff --git a/app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt b/app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt index 8ade749..fefcb57 100644 --- a/app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt +++ b/app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt @@ -68,42 +68,64 @@ class Media3Engine(private val context: Context) : HardwareTranscoder { ): Unit = suspendCancellableCoroutine { cont -> val plan = CopyPlanner.plan(request.spec, request.probe) handler.post { - val transformer = runCatching { buildTransformer(plan, cont) } - .getOrElse { - cont.resumeWithException(it) - return@post - } - - // Dropping the tracks the target does not have is what stops an audio-only export - // from carrying a re-encoded video track. Without setRemoveVideo, asking for M4A - // produced an HEVC stream in a file named .m4a. - val item = EditedMediaItem.Builder(MediaItem.fromUri(input)) - .setRemoveVideo(plan.video == VideoPlan.Drop) - .setRemoveAudio(plan.audio == AudioPlan.Drop) - .build() - - // A Composition is the only way to ask for transmuxing; the plain - // start(EditedMediaItem, path) overload always re-encodes. This is the remux path. - val composition = Composition.Builder(EditedMediaItemSequence.Builder(item).build()) - .setTransmuxVideo(plan.video == VideoPlan.Copy) - .setTransmuxAudio(plan.audio == AudioPlan.Copy) - .build() - - cont.invokeOnCancellation { - // cancel() has the same single-thread requirement as start(). - handler.post { runCatching { transformer.cancel() } } - } - - runCatching { transformer.start(composition, output.absolutePath) } - .onFailure { - cont.resumeWithException(it) - return@post - } - - pollProgress(transformer, cont, onProgress) + // One guard around the whole body, deliberately. + // + // This used to be two narrow ones — around `buildTransformer` and around + // `transformer.start` — with the two Media3 builders sitting unguarded between them. + // On this thread that is not a small gap: nothing here has a caller to throw back to, + // so an escaping exception reaches the HandlerThread's uncaught handler and takes the + // process down, while [cont] is never resumed either way. `EditedMediaItem.Builder` + // does exactly that for a plan that drops both tracks + // ("Audio and video cannot both be removed"), which a queued job can still carry. + // Widening the guard costs nothing on success and turns every such refusal into a + // failed job with a reason. + runCatching { startExport(input, output, plan, cont, onProgress) } + .onFailure { if (cont.isActive) cont.resumeWithException(it) } } } + /** + * Builds the export and hands it to Transformer. Runs on the HandlerThread; may throw. + * + * Everything Transformer's single-thread contract covers lives here, so that the caller has + * exactly one place to catch. Returning normally means the export is running and [cont] belongs + * to the listener; throwing means it never started and the caller owns resuming. + */ + private fun startExport( + input: Uri, + output: File, + plan: ConversionPlan, + cont: CancellableContinuation, + onProgress: (Int) -> Unit, + ) { + val transformer = buildTransformer(plan, cont) + + // Dropping the tracks the target does not have is what stops an audio-only export + // from carrying a re-encoded video track. Without setRemoveVideo, asking for M4A + // produced an HEVC stream in a file named .m4a. + val item = EditedMediaItem.Builder(MediaItem.fromUri(input)) + .setRemoveVideo(plan.video == VideoPlan.Drop) + .setRemoveAudio(plan.audio == AudioPlan.Drop) + .build() + + // A Composition is the only way to ask for transmuxing; the plain + // start(EditedMediaItem, path) overload always re-encodes. This is the remux path. + val composition = Composition.Builder(EditedMediaItemSequence.Builder(item).build()) + .setTransmuxVideo(plan.video == VideoPlan.Copy) + .setTransmuxAudio(plan.audio == AudioPlan.Copy) + .build() + + // Registered before start(), so a cancellation racing the export always finds a + // transformer to cancel. + cont.invokeOnCancellation { + // cancel() has the same single-thread requirement as start(). + handler.post { runCatching { transformer.cancel() } } + } + + transformer.start(composition, output.absolutePath) + pollProgress(transformer, cont, onProgress) + } + /** * @throws IllegalArgumentException if [plan] names a container Media3 cannot mux. That is a * routing bug rather than a runtime condition — [org.libremediaconverter.model.ConversionRouter] diff --git a/app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt b/app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt index 538b33f..294ee7d 100644 --- a/app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt +++ b/app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt @@ -129,9 +129,25 @@ object ContainerCapabilities { } } - if (spec.videoCodec == VideoCodec.NONE && spec.audioCodec == AudioCodec.NONE) { + // Two faces of one rule: the output would carry no tracks at all. + // + // The first is visible in the spec alone — NONE on both axes. The second only emerges once + // the spec meets the probe, because [CopyPlanner] drops a video track the *input* does not + // have no matter which codec was named for it, so "H.265 + no audio" on an MP3 plans to + // (Drop, Drop) exactly as "None + None" does. Asking the spec alone answered the first and + // missed the second, and the miss was not cosmetic: `EditedMediaItem.Builder` refuses that + // composition with IllegalStateException("Audio and video cannot both be removed"), on + // Transformer's own thread, where the user would have seen a dead app rather than a reason. + if (spec.audioCodec == AudioCodec.NONE && (spec.videoCodec == VideoCodec.NONE || !probe.hasVideo)) { return Validation.Invalid( - "This would produce an empty file — keep at least one track.", + if (spec.videoCodec == VideoCodec.NONE) { + "This would produce an empty file — keep at least one track." + } else { + // Names both halves. "No video track" alone reads as though the video setting + // were the only thing wrong, and the user would fix that and still be stuck. + "This file has no video track, so turning the audio off too would produce an " + + "empty file." + }, suggestions( // Ask for both tracks back, then let repair settle what this container and // this input can actually give. @@ -284,8 +300,12 @@ object ContainerCapabilities { private fun repairVideo(spec: OutputSpec, probe: InputProbe): VideoCodec { val container = spec.container if (spec.videoCodec == VideoCodec.NONE || !container.canHoldVideo) return VideoCodec.NONE + // There is no video track to make one out of, so naming a codec would be a suggestion + // [CopyPlanner] drops on the floor. It also read as a non-sequitur: before this line, the + // repair offered for an MP3 was "H.264", the first codec MP4 happens to encode. + if (!probe.hasVideo) return VideoCodec.NONE - val source = CodecNames.videoFromName(probe.videoCodec).takeIf { probe.hasVideo } + val source = CodecNames.videoFromName(probe.videoCodec) val copyable = source != null && accepts(container, source, CodecMode.COPY) return when { diff --git a/app/src/test/java/org/libremediaconverter/convert/Media3EngineEmptyCompositionTest.kt b/app/src/test/java/org/libremediaconverter/convert/Media3EngineEmptyCompositionTest.kt new file mode 100644 index 0000000..5fa41f7 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/convert/Media3EngineEmptyCompositionTest.kt @@ -0,0 +1,105 @@ +package org.libremediaconverter.convert + +import android.net.Uri +import androidx.media3.common.util.UnstableApi +import kotlinx.coroutines.runBlocking +import kotlinx.coroutines.withTimeout +import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse +import org.junit.Assert.assertTrue +import org.junit.Test +import org.junit.runner.RunWith +import org.libremediaconverter.model.AudioCodec +import org.libremediaconverter.model.AudioPlan +import org.libremediaconverter.model.Container +import org.libremediaconverter.model.ConversionRequest +import org.libremediaconverter.model.CopyPlanner +import org.libremediaconverter.model.InputKind +import org.libremediaconverter.model.InputProbe +import org.libremediaconverter.model.OutputSpec +import org.libremediaconverter.model.VideoCodec +import org.libremediaconverter.model.VideoPlan +import org.robolectric.RobolectricTestRunner +import org.robolectric.RuntimeEnvironment +import java.io.File +import java.util.concurrent.CancellationException + +/** + * What happens when Media3 refuses the export before it starts. + * + * `EditedMediaItem.Builder` rejects a composition with both tracks removed — + * checkState("Audio and video cannot both be removed") — and [Media3Engine] builds it on its own + * HandlerThread. That build used to sit *between* two narrow `runCatching` blocks, one around + * `buildTransformer` and one around `start`, so the exception escaped `handler.post`'s body: it + * reached the thread's uncaught handler, which on Android takes the process down, and the + * continuation was left unresumed either way. + * + * Robolectric runs the real [android.os.HandlerThread] and the real Media3 builders, so the whole + * sequence happens here — the engine really posts, really builds, and really throws. What it cannot + * reproduce is the *consequence* of an escaped throw: a JVM background thread dying is not process + * death. So the assertion is on the half that is observable everywhere and is the half that + * matters to the user — the suspension is resolved, with the reason, rather than left hanging. + * `Media3EngineTest.aPlanThatRemovesBothTracksFailsInsteadOfKillingTheProcess` is the same case on + * a device. + */ +@UnstableApi +@RunWith(RobolectricTestRunner::class) +class Media3EngineEmptyCompositionTest { + + @Test + fun `a plan that removes both tracks fails the job instead of escaping the handler thread`() { + val context = RuntimeEnvironment.getApplication() + val engine = Media3Engine(context) + val request = ConversionRequest( + spec = OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE), + probe = InputProbe( + videoCodec = null, + audioCodec = "mp3", + hasVideo = false, + container = Container.MP3, + kind = InputKind.AUDIO_ONLY, + ), + ) + + // Asserted rather than assumed: ConversionRequest's default probe says hasVideo = true, and + // with it this same spec plans to (Encode, Drop), nothing throws, and the test would pass + // over a code path it never entered. + val plan = CopyPlanner.plan(request.spec, request.probe) + assertEquals(VideoPlan.Drop, plan.video) + assertEquals(AudioPlan.Drop, plan.audio) + + val failure = try { + runCatching { + runBlocking { + withTimeout(TIMEOUT_MS) { + engine.transcode(Uri.parse("file:///dev/null"), File(context.cacheDir, "empty.mp4"), request) {} + } + } + }.exceptionOrNull() + } finally { + engine.close() + } + + // Both halves are load-bearing, and the second is not pedantry: withTimeout raises + // TimeoutCancellationException, and `java.util.concurrent.CancellationException` *extends* + // IllegalStateException — so testing only the first would call an unresumed continuation a + // pass. This assertion was written that way, and the mutation is what found it. + assertFalse( + "the continuation was never resumed — the failure escaped instead of being reported: $failure", + failure is CancellationException, + ) + assertTrue( + "the builder's refusal must surface as a failed job; got $failure", + failure is IllegalStateException, + ) + } + + private companion object { + /** + * Short on purpose. Nothing is decoded, encoded or muxed on this path — the builder refuses + * the input outright — so anything approaching this is a hang, which is the failure mode + * this test is looking for. + */ + const val TIMEOUT_MS = 10_000L + } +} diff --git a/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt b/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt index ff88663..9315ba0 100644 --- a/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt +++ b/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt @@ -20,6 +20,21 @@ class ContainerCapabilitiesTest { container = Container.MP4, ) + /** + * An MP3, and the reason several rules below need a second probe. + * + * `hasVideo = false` is the load-bearing field. Every rule that reads only the spec answers the + * same for this input as for a video file, which is exactly how a spec naming a video codec was + * called valid for a file with no video track to put in it. + */ + private val mp3Source = InputProbe( + videoCodec = null, + audioCodec = "mp3", + hasVideo = false, + kind = InputKind.AUDIO_ONLY, + container = Container.MP3, + ) + // --- copy and encode are different questions ---------------------------- /** @@ -85,17 +100,27 @@ class ContainerCapabilitiesTest { /** A suggestion that is itself invalid is worse than no suggestion. */ @Test fun `every suggestion is itself valid`() { - val broken = OutputSpec(Container.WEBM, VideoCodec.H264, AudioCodec.AAC) - val result = ContainerCapabilities.validate(broken, h264Source) + val cases = listOf( + OutputSpec(Container.WEBM, VideoCodec.H264, AudioCodec.AAC) to h264Source, + // The audio-only input. Every rejection it can reach used to hand back `None + None` + // — a spec validation refuses in the next breath — because these branches built their + // suggestion by hand instead of going through the repair-and-filter path. + OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE) to mp3Source, + OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.NONE) to mp3Source, + OutputSpec(Container.MP4, VideoCodec.NONE, AudioCodec.NONE) to mp3Source, + OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.AAC) to mp3Source, + ) - val invalid = result as? Validation.Invalid - ?: throw AssertionError("expected H.264 in WebM to be rejected") - assertTrue("no alternatives offered", invalid.suggestions.isNotEmpty()) - invalid.suggestions.forEach { suggestion -> - assertTrue( - "suggested $suggestion is itself invalid", - ContainerCapabilities.validate(suggestion, h264Source).isValid, - ) + cases.forEach { (spec, probe) -> + val invalid = ContainerCapabilities.validate(spec, probe) as? Validation.Invalid + ?: throw AssertionError("expected $spec to be rejected") + assertTrue("no alternatives offered for $spec", invalid.suggestions.isNotEmpty()) + invalid.suggestions.forEach { suggestion -> + assertTrue( + "suggested $suggestion for $spec is itself invalid", + ContainerCapabilities.validate(suggestion, probe).isValid, + ) + } } } @@ -127,6 +152,117 @@ class ContainerCapabilitiesTest { assertTrue((result as Validation.Invalid).suggestions.isNotEmpty()) } + /** + * The same rule, seen only against the probe. + * + * A video codec named for a file with no video track is dropped, not encoded — so + * MP4/H.265/None on an MP3 empties the output exactly as None/None does. Reading the spec + * alone answered "valid" because the spec names a video codec, and the job went to Media3, + * where `EditedMediaItem.Builder` refuses a composition with both tracks removed by throwing + * on Transformer's own HandlerThread. + */ + @Test + fun `a video codec named for a file with no video track and no audio is refused`() { + ContainerCapabilities.encodableVideo(Container.MP4).forEach { codec -> + val spec = OutputSpec(Container.MP4, codec, AudioCodec.NONE) + val result = ContainerCapabilities.validate(spec, mp3Source) + + assertFalse( + "MP4/${codec.label}/None on an audio-only input plans to (Drop, Drop) and would " + + "produce an empty file; it must be refused. Got $result", + result.isValid, + ) + } + } + + /** + * The refusal is only worth having if it leads somewhere. + * + * The COPY form of this was already refused, but its one hand-built suggestion was + * `None + None` — which validation refuses in the next breath, so the Advanced picker offered + * a one-tap fix that fixed nothing. Every face of the rule now goes through the shared + * suggestion path, so the offer keeps the one track the input actually has. + */ + @Test + fun `refusing an empty output still offers a way to keep the audio`() { + listOf(VideoCodec.H265, VideoCodec.H264, VideoCodec.COPY, VideoCodec.NONE).forEach { codec -> + val spec = OutputSpec(Container.MP4, codec, AudioCodec.NONE) + val invalid = ContainerCapabilities.validate(spec, mp3Source) as? Validation.Invalid + ?: throw AssertionError("expected MP4/${codec.label}/None to be rejected") + + assertTrue( + "a refusal with no way out is a dead end in the Advanced picker", + invalid.suggestions.isNotEmpty(), + ) + assertTrue( + "every suggestion must keep a track, got ${invalid.suggestions}", + invalid.suggestions.all { it.audioCodec != AudioCodec.NONE }, + ) + } + } + + /** + * A repair must not name a track the input does not have. + * + * `repairVideo` used to fall through to "the first codec this container can encode" whenever + * nothing else fitted, and for an MP3 that produced the non-sequitur `MP4 · H.264 · Copy`. + * It validated, so nothing caught it — but [CopyPlanner] drops that video track anyway, which + * makes the codec in the offer a fiction. + */ + @Test + fun `a repair for a file with no video track never names a video codec`() { + listOf( + OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE), + OutputSpec(Container.MP4, VideoCodec.NONE, AudioCodec.NONE), + OutputSpec(Container.MP4, VideoCodec.COPY, AudioCodec.NONE), + ).forEach { spec -> + val invalid = ContainerCapabilities.validate(spec, mp3Source) as Validation.Invalid + invalid.suggestions.forEach { + assertEquals( + "offering ${it.videoCodec.label} for a file with no video track is a fiction; " + + "CopyPlanner drops it. Suggested $it for $spec", + VideoCodec.NONE, + it.videoCodec, + ) + } + } + } + + /** + * The rule stated as the property it is, over the whole matrix. + * + * A plan of (Drop, Drop) is precisely the composition `EditedMediaItem.Builder` refuses to + * build, so no non-image spec that reaches it may be called valid. Sweeping every container × + * codec × codec against both probes is what stops the next container or codec from + * reintroducing the gap on an axis nobody thought to write a case for. + * + * Image outputs are exempt and deliberately so: GIF and PNG frames carry no codecs at all, and + * `None + None` is the only spec they accept — but they never reach Media3, because the router + * sends every image output to FFmpeg. + */ + @Test + fun `no valid non-image spec plans to remove both tracks`() { + val specs = Container.entries + .filterNot { it == Container.GIF || it == Container.IMAGE_SEQUENCE } + .flatMap { container -> VideoCodec.entries.map { container to it } } + .flatMap { (container, video) -> AudioCodec.entries.map { OutputSpec(container, video, it) } } + val cases = specs.flatMap { spec -> listOf(h264Source, mp3Source).map { spec to it } } + + val empties = cases.filter { (spec, probe) -> + val plan = CopyPlanner.plan(spec, probe) + plan.video == VideoPlan.Drop && plan.audio == AudioPlan.Drop + } + + assertTrue("the sweep found nothing to check — the filter has gone wrong", empties.isNotEmpty()) + empties.forEach { (spec, probe) -> + assertFalse( + "$spec on $probe plans to (Drop, Drop) — an empty file, and the composition " + + "Media3 cannot build — so it must not validate", + ContainerCapabilities.validate(spec, probe).isValid, + ) + } + } + @Test fun `copying is offered as the fix when the codec is right but unencodable`() { val av1Source = InputProbe(videoCodec = "av1", audioCodec = "aac", container = Container.MKV) diff --git a/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt b/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt index f762f14..b5a915c 100644 --- a/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt +++ b/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt @@ -350,6 +350,34 @@ class ConversionRouterTest { } } + /** + * Why `Media3Engine` still needs a guard of its own. + * + * `ContainerCapabilities.validate` now refuses "a video codec with the audio off" for an input + * with no video track, so neither the picker nor `ConversionWorker` will start one. Routing is + * a separate question and still answers MEDIA3 — nothing about a dropped track makes the job + * un-hardware-able — so a request that skips validation, from a direct + * `ConversionWorker.request(...)` or a job queued before the settings changed, arrives at the + * engine with a plan Media3 cannot build. That has to fail the job, not the process. + */ + @Test + fun `a plan that drops both tracks still routes to media3`() { + val audioOnly = InputProbe( + videoCodec = null, + audioCodec = "mp3", + hasVideo = false, + container = Container.MP3, + kind = InputKind.AUDIO_ONLY, + ) + val spec = OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE) + + val plan = CopyPlanner.plan(spec, audioOnly) + assertEquals(VideoPlan.Drop, plan.video) + assertEquals(AudioPlan.Drop, plan.audio) + + assertEquals(Engine.MEDIA3, route(spec, probe = audioOnly).engine) + } + @Test fun `audio-only formats are flagged as such`() { assertEquals(true, OutputFormat.MP3.isAudioOnly) diff --git a/app/src/test/java/org/libremediaconverter/model/CopyPlannerTest.kt b/app/src/test/java/org/libremediaconverter/model/CopyPlannerTest.kt index 58fb1cd..d2d4da5 100644 --- a/app/src/test/java/org/libremediaconverter/model/CopyPlannerTest.kt +++ b/app/src/test/java/org/libremediaconverter/model/CopyPlannerTest.kt @@ -146,6 +146,33 @@ class CopyPlannerTest { assertTrue("copying the only track is still a remux", plan.isPureRemux) } + /** + * The one plan `Media3Engine` cannot be handed. + * + * `EditedMediaItem.Builder` refuses a composition with both tracks removed — + * checkState("Audio and video cannot both be removed") — and this is how an ordinary-looking + * spec reaches it: a video codec named for a file that has no video, with the audio switched + * off. Neither half is unusual on its own, which is why validation could read the spec, see a + * video codec, and call it fine. + */ + @Test + fun `an audio-only source with the audio dropped removes both tracks`() { + val audioOnly = InputProbe( + videoCodec = null, + audioCodec = "mp3", + hasVideo = false, + container = Container.MP3, + kind = InputKind.AUDIO_ONLY, + ) + val plan = CopyPlanner.plan( + OutputSpec(Container.MP4, VideoCodec.H265, AudioCodec.NONE), + audioOnly, + ) + assertEquals(VideoPlan.Drop, plan.video) + assertEquals(AudioPlan.Drop, plan.audio) + assertTrue("an empty plan is not a remux", !plan.isPureRemux) + } + @Test fun `copying one track and encoding the other is not a pure remux`() { val plan = CopyPlanner.plan(