diff --git a/app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt b/app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt index fb25f71..8ade749 100644 --- a/app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt +++ b/app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt @@ -138,31 +138,6 @@ class Media3Engine(private val context: Context) : HardwareTranscoder { .build() } - /** - * Media3 encodes only H.264 and H.265 of the codecs this app offers. - * - * VP8/VP9/AV1 targets never reach here — the router sends them to FFmpeg because - * `Transformer.setVideoMimeType` rejects them — so anything unexpected returns null and lets - * Transformer pick, rather than silently substituting H.265 the way the old mapping did. - */ - private fun videoMimeTypeFor(codec: VideoCodec): String? = when (codec) { - VideoCodec.H264 -> MimeTypes.VIDEO_H264 - VideoCodec.H265 -> MimeTypes.VIDEO_H265 - // Never reached: only an Encode plan consults this, and COPY/NONE are not Encode. - VideoCodec.COPY, VideoCodec.NONE -> null - VideoCodec.VP8, VideoCodec.VP9, VideoCodec.AV1 -> null - } - - private fun audioMimeTypeFor(codec: AudioCodec): String? = when (codec) { - AudioCodec.AAC -> MimeTypes.AUDIO_AAC - AudioCodec.OPUS -> MimeTypes.AUDIO_OPUS - AudioCodec.VORBIS -> MimeTypes.AUDIO_VORBIS - AudioCodec.PCM -> MimeTypes.AUDIO_RAW - AudioCodec.COPY, AudioCodec.NONE -> null - // MP3 and FLAC have no Android encoder; the router routes them to FFmpeg. - AudioCodec.MP3, AudioCodec.FLAC -> null - } - /** * Polls export progress on the Transformer's own thread. * @@ -192,7 +167,57 @@ class Media3Engine(private val context: Context) : HardwareTranscoder { thread.quitSafely() } - private companion object { + /** + * The progress interval, and the two enum-to-MIME tables. + * + * The tables are pure functions of a codec enum, so they sit here rather than on the instance: + * a JVM test can then exercise every arm without constructing an engine, which would start a + * real [HandlerThread] to answer a lookup. `internal` rather than `private` for the reason + * `MainActivity`'s `Destination` records — the JVM test source set is a friend of `main`, so + * these stay invisible to anything outside the module. + */ + internal companion object { const val PROGRESS_INTERVAL_MS = 250L + + /** + * Media3 encodes only H.264 and H.265 of the codecs this app offers. + * + * VP8/VP9/AV1 targets never reach here — the router sends them to FFmpeg because + * `Transformer.setVideoMimeType` rejects them — so anything unexpected returns null and + * lets Transformer pick, rather than silently substituting H.265 as the old mapping did. + */ + internal fun videoMimeTypeFor(codec: VideoCodec): String? = when (codec) { + VideoCodec.H264 -> MimeTypes.VIDEO_H264 + VideoCodec.H265 -> MimeTypes.VIDEO_H265 + // Never reached, and no longer only asserted: `Media3EngineMimeTypesTest` drives + // `CopyPlanner` over every spec it can be handed and shows that no Encode plan carries + // either, which is what turns "COPY/NONE are not Encode" into a checked claim. + VideoCodec.COPY, VideoCodec.NONE -> null + VideoCodec.VP8, VideoCodec.VP9, VideoCodec.AV1 -> null + } + + /** + * Media3 encodes AAC, Opus and PCM. Three arms below are dead, not two. + * + * The comment this replaces named MP3 and FLAC as the exceptions, which reads as though + * every other arm were live. **Vorbis is not.** A single router rule diverts every audio + * codec outside {AAC, Opus, PCM} to FFmpeg, and Vorbis is outside it, so + * `VORBIS -> AUDIO_VORBIS` names a MIME type Transformer is never actually asked for. + * + * The arm stays because the mapping is correct — deleting a right answer out of + * unreachable code buys nothing — but it is an entry waiting on a routing change rather + * than a live one. `Media3EngineMimeTypesTest` routes all six encodable codecs and asserts + * which three arrive, so if that set moves, the disagreement fails rather than surprises. + */ + internal fun audioMimeTypeFor(codec: AudioCodec): String? = when (codec) { + AudioCodec.AAC -> MimeTypes.AUDIO_AAC + AudioCodec.OPUS -> MimeTypes.AUDIO_OPUS + AudioCodec.VORBIS -> MimeTypes.AUDIO_VORBIS + AudioCodec.PCM -> MimeTypes.AUDIO_RAW + AudioCodec.COPY, AudioCodec.NONE -> null + // MP3 and FLAC have no Android encoder at any API level, so the router sends them to + // FFmpeg before an encoder is ever asked for. + AudioCodec.MP3, AudioCodec.FLAC -> null + } } } diff --git a/app/src/test/java/org/libremediaconverter/convert/Media3EngineMimeTypesTest.kt b/app/src/test/java/org/libremediaconverter/convert/Media3EngineMimeTypesTest.kt new file mode 100644 index 0000000..7e4c2f4 --- /dev/null +++ b/app/src/test/java/org/libremediaconverter/convert/Media3EngineMimeTypesTest.kt @@ -0,0 +1,287 @@ +package org.libremediaconverter.convert + +import androidx.media3.common.MimeTypes +import androidx.media3.common.util.UnstableApi +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotNull +import org.junit.Assert.assertTrue +import org.junit.Test +import org.libremediaconverter.model.AudioCodec +import org.libremediaconverter.model.AudioPlan +import org.libremediaconverter.model.Container +import org.libremediaconverter.model.ConversionPlan +import org.libremediaconverter.model.ConversionRequest +import org.libremediaconverter.model.ConversionRouter +import org.libremediaconverter.model.CopyPlanner +import org.libremediaconverter.model.DeviceCodecs +import org.libremediaconverter.model.Engine +import org.libremediaconverter.model.InputProbe +import org.libremediaconverter.model.OutputSpec +import org.libremediaconverter.model.VideoCodec +import org.libremediaconverter.model.VideoPlan + +/** + * Guards [Media3Engine]'s two enum-to-MIME tables and the claims written above them. + * + * The defect: neither table was exercised at all, so nothing stood between a wrong entry and the + * user's file. Point `H265` at `VIDEO_H264` and every hardware HEVC export writes H.264 into a + * file the user asked to be H.265 — Transformer does exactly as told, the export succeeds, and + * the only symptom is a codec nobody chose. + * + * Worse, one arm carried an assertion instead of a value: + * + * ``` + * // Never reached: only an Encode plan consults this, and COPY/NONE are not Encode. + * ``` + * + * That is a claim about *callers* parked in a branch of a callee. It happens to be true, and + * nothing whatsoever checked it, so it would have gone on reading as true after it stopped being. + * + * Three kinds of test, because arm-by-arm equality alone would only pin today's answers: + * + * 1. Every arm of both tables, nulls included. + * 2. The "never reached" claim, proved over every plan [CopyPlanner] can produce. + * 3. The tables against [ConversionRouter]'s actual decisions rather than against its codec sets — + * the comments claim behaviour ("the router routes them to FFmpeg"), and a set can be right + * while the rule that reads it is wrong. + * + * A JVM test rather than an instrumented one: both tables take an enum and return a constant. + */ +@UnstableApi +class Media3EngineMimeTypesTest { + + @Test + fun `every video codec maps to the MIME type Transformer will be given`() { + assertEquals( + "EXPECTED_VIDEO_MIME must name every VideoCodec, so a new one cannot arrive untested", + VideoCodec.entries.toSet(), + EXPECTED_VIDEO_MIME.keys, + ) + VideoCodec.entries.forEach { codec -> + assertEquals( + "videoMimeTypeFor(${codec.label})", + EXPECTED_VIDEO_MIME.getValue(codec), + Media3Engine.videoMimeTypeFor(codec), + ) + } + } + + @Test + fun `every audio codec maps to the MIME type Transformer will be given`() { + assertEquals( + "EXPECTED_AUDIO_MIME must name every AudioCodec, so a new one cannot arrive untested", + AudioCodec.entries.toSet(), + EXPECTED_AUDIO_MIME.keys, + ) + AudioCodec.entries.forEach { codec -> + assertEquals( + "audioMimeTypeFor(${codec.label})", + EXPECTED_AUDIO_MIME.getValue(codec), + Media3Engine.audioMimeTypeFor(codec), + ) + } + } + + /** + * The "never reached" claim, proved rather than repeated. + * + * [Media3Engine] asks these tables only for `plan.video as? VideoPlan.Encode`, and every plan + * it sees comes from [CopyPlanner]. So the claim reduces to a property of the planner: over + * every spec it can be handed, an `Encode` never carries `COPY` or `NONE`. That holds because + * both codecs are answered before the `Encode` branch, and the fallback draws from + * `ContainerCapabilities.encodableVideo`, which contains neither — but this asserts it instead + * of trusting the reading. + * + * The counters are not decoration. `(plan.video as? VideoPlan.Encode)?.let { ... }` asserts + * nothing at all for a `Drop` or `Copy` plan, so a sweep that stopped producing `Encode` plans + * would stay green while checking nothing. + */ + @Test + fun `no plan CopyPlanner can produce carries COPY or NONE inside an Encode`() { + var videoEncodes = 0 + var audioEncodes = 0 + everyPlan().forEach { (spec, probe, plan) -> + (plan.video as? VideoPlan.Encode)?.let { + videoEncodes++ + assertTrue( + "CopyPlanner produced VideoPlan.Encode(${it.codec}) for $spec against $probe", + it.codec != VideoCodec.COPY && it.codec != VideoCodec.NONE, + ) + } + (plan.audio as? AudioPlan.Encode)?.let { + audioEncodes++ + assertTrue( + "CopyPlanner produced AudioPlan.Encode(${it.codec}) for $spec against $probe", + it.codec != AudioCodec.COPY && it.codec != AudioCodec.NONE, + ) + } + } + assertTrue("the sweep produced no video Encode plan, so it asserted nothing", videoEncodes > 0) + assertTrue("the sweep produced no audio Encode plan, so it asserted nothing", audioEncodes > 0) + } + + /** + * The video table's other claim: VP8, VP9 and AV1 targets "never reach here". + * + * Asked of the router rather than of its private codec set, so the rule is what is under test. + */ + @Test + fun `the router sends exactly H264 and H265 video encodes to Media3`() { + val onMedia3 = REAL_VIDEO_CODECS.filter { engineForVideoEncode(it) == Engine.MEDIA3 } + assertEquals(listOf(VideoCodec.H264, VideoCodec.H265), onMedia3) + } + + /** + * The audio table's sibling claim, and where it turned out to be incomplete. + * + * The comment named MP3 and FLAC. One rule — `audioEncode !in MEDIA3_AUDIO` — diverts Vorbis + * by exactly the same logic, so three of the six encodable codecs never reach the table, not + * two. Asserted as the whole set rather than as two memberships, which is what makes the + * omission visible. + */ + @Test + fun `the router keeps MP3 FLAC and Vorbis audio encodes off Media3`() { + val onMedia3 = REAL_AUDIO_CODECS.filter { engineForAudioEncode(it) == Engine.MEDIA3 } + assertEquals(listOf(AudioCodec.AAC, AudioCodec.OPUS, AudioCodec.PCM), onMedia3) + } + + /** + * The binding that makes the two halves above one test rather than two coincidences. + * + * A codec the router starts sending to Media3 must have a MIME type here, or Transformer is + * left to pick its own and the user gets a codec they did not choose. + */ + @Test + fun `every codec the router sends to Media3 has a MIME type`() { + REAL_VIDEO_CODECS.filter { engineForVideoEncode(it) == Engine.MEDIA3 }.forEach { codec -> + assertNotNull( + "${codec.label} is routed to Media3 but videoMimeTypeFor returns null", + Media3Engine.videoMimeTypeFor(codec), + ) + } + REAL_AUDIO_CODECS.filter { engineForAudioEncode(it) == Engine.MEDIA3 }.forEach { codec -> + assertNotNull( + "${codec.label} is routed to Media3 but audioMimeTypeFor returns null", + Media3Engine.audioMimeTypeFor(codec), + ) + } + } + + /** + * The reverse direction, which holds for video and not for audio. + * + * Every video codec the router withholds has a null entry, so that table is exactly the set of + * codecs Media3 is asked to encode. Audio has one entry more than the router will ever use: + * `VORBIS -> AUDIO_VORBIS` is correct and unreachable. Pinned deliberately — if a routing + * change makes Vorbis live, this is the test that says the arm above stopped being dead. + */ + @Test + fun `Vorbis is the one MIME type the router never asks for`() { + REAL_VIDEO_CODECS.filter { engineForVideoEncode(it) == Engine.FFMPEG }.forEach { codec -> + assertEquals( + "${codec.label} never reaches Media3, so it must not name a MIME type", + null, + Media3Engine.videoMimeTypeFor(codec), + ) + } + val namedButUnrouted = REAL_AUDIO_CODECS + .filter { Media3Engine.audioMimeTypeFor(it) != null } + .filter { engineForAudioEncode(it) == Engine.FFMPEG } + assertEquals(listOf(AudioCodec.VORBIS), namedButUnrouted) + assertEquals(MimeTypes.AUDIO_VORBIS, Media3Engine.audioMimeTypeFor(AudioCodec.VORBIS)) + } + + /** + * Routes a video-only re-encode to [codec] and reports the engine chosen. + * + * `mpeg2video` is the load-bearing detail: [CopyPlanner] upgrades a request to a stream copy + * when the source codec matches, and a `Copy` plan would answer a different question. A name + * `CodecNames` cannot resolve forces an `Encode` for every codec, which the assertion pins so + * that a planner change cannot quietly turn this sweep into a sweep of `Copy` plans. + */ + private fun engineForVideoEncode(codec: VideoCodec): Engine { + val request = ConversionRequest( + spec = OutputSpec(Container.MP4, codec, AudioCodec.NONE), + probe = InputProbe(videoCodec = "mpeg2video", container = Container.MKV), + ) + assertEquals( + "this request no longer plans a video Encode, so its engine says nothing about $codec", + VideoPlan.Encode(codec), + CopyPlanner.plan(request.spec, request.probe).video, + ) + return ConversionRouter.route(request, DeviceCodecs.PERMISSIVE).engine + } + + /** The audio counterpart. `ac3` is unresolvable for the same reason `mpeg2video` is. */ + private fun engineForAudioEncode(codec: AudioCodec): Engine { + val request = ConversionRequest( + spec = OutputSpec(Container.MP4, VideoCodec.NONE, codec), + probe = InputProbe(audioCodec = "ac3", hasVideo = false, container = Container.MKV), + ) + assertEquals( + "this request no longer plans an audio Encode, so its engine says nothing about $codec", + AudioPlan.Encode(codec), + CopyPlanner.plan(request.spec, request.probe).audio, + ) + return ConversionRouter.route(request, DeviceCodecs.PERMISSIVE).engine + } + + private fun everyPlan(): List> = + ALL_SPECS.flatMap { spec -> PROBES.map { Triple(spec, it, CopyPlanner.plan(spec, it)) } } + + private companion object { + + /** Every arm of `videoMimeTypeFor`, including the ones the tests above prove unreachable. */ + val EXPECTED_VIDEO_MIME: Map = mapOf( + VideoCodec.H264 to MimeTypes.VIDEO_H264, + VideoCodec.H265 to MimeTypes.VIDEO_H265, + VideoCodec.VP8 to null, + VideoCodec.VP9 to null, + VideoCodec.AV1 to null, + // Unreachable, and asserted anyway: the proof lives in another test, and a reader + // deleting these would leave the arms themselves unexercised. + VideoCodec.COPY to null, + VideoCodec.NONE to null, + ) + + val EXPECTED_AUDIO_MIME: Map = mapOf( + AudioCodec.AAC to MimeTypes.AUDIO_AAC, + AudioCodec.OPUS to MimeTypes.AUDIO_OPUS, + AudioCodec.VORBIS to MimeTypes.AUDIO_VORBIS, + AudioCodec.PCM to MimeTypes.AUDIO_RAW, + AudioCodec.MP3 to null, + AudioCodec.FLAC to null, + AudioCodec.COPY to null, + AudioCodec.NONE to null, + ) + + /** Codecs a user can actually ask to be produced: `COPY` and `NONE` are instructions. */ + val REAL_VIDEO_CODECS = VideoCodec.entries - VideoCodec.COPY - VideoCodec.NONE + val REAL_AUDIO_CODECS = AudioCodec.entries - AudioCodec.COPY - AudioCodec.NONE + + /** Every output a spec can name — 15 containers by 7 video codecs by 8 audio codecs. */ + val ALL_SPECS: List = Container.entries.flatMap { container -> + VideoCodec.entries.flatMap { video -> + AudioCodec.entries.map { audio -> OutputSpec(container, video, audio) } + } + } + + /** Inputs chosen to reach each of [CopyPlanner]'s branches. */ + val PROBES = listOf( + // Nothing known about the source at all. + InputProbe(), + // Identified, and the container changes: the copy upgrade applies. + InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.MKV), + // Identified, container unchanged: the copy upgrade deliberately does not apply. + InputProbe(videoCodec = "h264", audioCodec = "aac", container = Container.MP4), + // Copyable but not encodable by either engine — the fallback's reason for existing. + InputProbe(videoCodec = "av1", audioCodec = "flac", container = Container.MKV), + // Real codecs this app cannot name, so a copy is never proven safe. + InputProbe(videoCodec = "mpeg2video", audioCodec = "ac3", container = Container.AVI), + // The platform extractor could not open it. + InputProbe(videoCodec = InputProbe.UNPARSEABLE), + // Audio only. + InputProbe(videoCodec = null, audioCodec = "opus", hasVideo = false, container = Container.OGG), + ) + } +}