Make the two codec tables answer for each other, and stop describeAudio printing a NUL

The FFprobe codec vocabulary is written out in at least four places and none of them
had a test. Two had already drifted apart. `x264`, `hev1`, `x265` and `vp09` resolved
in `CodecNames.videoFromName` and returned null from
`AndroidDeviceCodecs.mimeForCodecName`, so the app identified the codec for the source
card and for routing and then ran the device capability check blind on the same string;
`mpeg4` ran the other way and rendered as a raw name. Nothing could notice, and the
reason is structural: a `when` cannot be enumerated, so no test can ask one table what
the other one knows.

Both are maps now, for that reason alone, and `CodecVocabularyTest` walks the two key
sets. A name added to -- or removed from -- one side alone fails the build. The one
legitimate asymmetry is listed rather than implied: `mpeg4` is decodable input with no
`VideoCodec` to name it, so `CodecNames` is right not to carry it. That list is itself
checked, because otherwise it is an escape hatch -- any future divergence could be waved
through by adding the name to it, and adding `x265` to it now fails.

THIS CHANGES BEHAVIOUR for `x264`, `hev1`, `x265` and `vp09`. A null from
`mimeForCodecName` means "unknown to us: assume the platform can handle it and let a
failed export trigger the FFmpeg fallback", which is the right policy for a name nobody
recognises and the wrong one for a name recognised one file over. A device without the
matching decoder now sends those four to FFmpeg up front instead of spending a doomed
hardware attempt to discover it. No input loses hardware it could have used: each alias
resolves to the MIME its canonical spelling already resolved to, so a device that has
the decoder still answers true. `ConversionRouterTest` still passes and that is not
evidence either way -- every `canDecode` in it is a hand-written stub that never reaches
this table.

#74 is the same family one level down. `describeVideo` answered "Unrecognised" for
`InputProbe.UNPARSEABLE` and `describeAudio` had no such arm, so an unparseable audio
codec would have fallen through to `?: name` -- and the sentinel opens with a NUL, so
the source-info card would have rendered a `Text` beginning with U+0000. The two now
share one body, which is what stops the next arm being added to one side only.

Two corrections to that ticket, taken from the file rather than from the ticket, since
it warns about exactly this:

  - It quotes `audioFromName` as opening with `null, InputProbe.UNPARSEABLE -> null`.
    It did not; it opened with `null -> null` and the sentinel reached `else`. Naming
    the sentinel in the shared lookup therefore changes no answer and is documentation,
    not the fix.
  - It says `describeVideo`'s arm has no test of its own. It did -- `descriptions stay
    readable for unknown and missing codecs` asserts it -- so deleting the shared arm
    now reddens three tests across both sides, not one.

Mutations run, each on the full 386-test suite:

  add "avc3" to CodecNames only    -> CodecVocabularyTest red on two counts,
                                      CodecNamesTest green: 8 tests, 0 failures, which
                                      is the ticket's point about per-table arm tests
  delete the UNPARSEABLE arm       -> CodecNamesTest red on three, one of them quoting
                                      the NUL back
  add "x265" to DECODE_ONLY_NAMES  -> CodecVocabularyTest red on the escape hatch
  delete "vp09" from the MIME map  -> CodecVocabularyTest red on three, which is the
                                      state this commit is fixing

Audio is not cross-checked, and that is a gap rather than a decision: the device
capability check is video-only, so this module has no second audio table to compare
`AUDIO_ALIASES` against. `Media3Engine.audioMimeTypeFor` is the other half and belongs
to #85. `MediaProbe.shortName` (#84) is the fourth table and is untouched here for the
same reason.

Closes #87.
Closes #74.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
This commit is contained in:
2026-08-24 22:44:55 -05:00
co-authored by Claude Opus 5
parent ad28293b72
commit 7f951baf8f
4 changed files with 351 additions and 40 deletions
@@ -0,0 +1,143 @@
package org.libremediaconverter.codec
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNotNull
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
import org.libremediaconverter.model.CodecNames
import org.libremediaconverter.model.VideoCodec
/**
* Bites on #87: two tables read one codec vocabulary and had stopped agreeing.
*
* `CodecNames.VIDEO_ALIASES` answers "which enum is this FFprobe name", for the source-info card
* and for routing. `AndroidDeviceCodecs.NAME_TO_MIME` answers "which MIME do I ask this device
* about", for the capability check. On `ad28293` five names lived in one and not the other: `x264`,
* `hev1`, `x265` and `vp09` were identified for display and then fell through the device check as
* unknown, so the app attempted a hardware path it had enough information to skip; `mpeg4` ran the
* other way and rendered as a raw name on the card.
*
* Per-table arm tests would have passed on both tables and encoded the disagreement, which is why
* these walk the key sets instead. A name added to — or removed from — one side alone fails here.
*/
class CodecVocabularyTest {
private val aliases = CodecNames.VIDEO_ALIASES
private val mimes = AndroidDeviceCodecs.NAME_TO_MIME
private val decodeOnly = AndroidDeviceCodecs.DECODE_ONLY_NAMES
@Test
fun `no video codec name resolves for display without also resolving for the device check`() {
assertEquals(
"resolve in CodecNames but return null from mimeForCodecName, so the device check runs blind",
emptySet<String>(),
aliases.keys - mimes.keys,
)
}
@Test
fun `no video codec name resolves for the device check without being a name the app can label`() {
assertEquals(
"resolve in AndroidDeviceCodecs but not in CodecNames, and are not listed as decode-only",
emptySet<String>(),
mimes.keys - aliases.keys - decodeOnly,
)
}
/**
* Membership is not enough: `"x265" to MIMETYPE_VIDEO_AVC` would satisfy both key sets and
* still ask the device about the wrong codec.
*/
@Test
fun `the two tables agree on what each name means, not merely that they know it`() {
aliases.forEach { (name, codec) ->
val expected = AndroidDeviceCodecs.mimeFor(codec)
assertNotNull("$name maps to $codec, which has no MIME to ask about", expected)
assertEquals("$name is $codec in CodecNames", expected, mimes[name])
}
}
/**
* The exception list is the escape hatch: any future divergence could be waved through by
* adding the name to it. Guard both directions so it cannot be.
*/
@Test
fun `the decode-only names are genuinely decode-only`() {
decodeOnly.forEach { name ->
assertNotNull("$name is listed as decode-only but the device check cannot resolve it", mimes[name])
assertNull(
"$name is listed as decode-only, but CodecNames does resolve it — that is a divergence " +
"being waved through rather than a documented exception",
CodecNames.videoFromName(name),
)
}
}
/**
* The five names #87 measured, pinned by name so the specific regression cannot come back
* quietly even if someone rewrites the tables above.
*/
@Test
fun `the names that used to resolve on one side only resolve on both`() {
mapOf(
"x264" to VideoCodec.H264,
"hev1" to VideoCodec.H265,
"x265" to VideoCodec.H265,
"vp09" to VideoCodec.VP9,
).forEach { (name, codec) ->
assertEquals("$name is a name FFmpeg emits", codec, CodecNames.videoFromName(name))
assertEquals(
"$name has to reach the device check too, or the app identifies it and then asks blind",
AndroidDeviceCodecs.mimeFor(codec),
AndroidDeviceCodecs.mimeForCodecName(name),
)
}
// The one that runs the other way: decodable input with no enum to name it.
assertNull("mpeg4 is not an output the app can target", CodecNames.videoFromName("mpeg4"))
assertNotNull("mpeg4 is still decodable input", AndroidDeviceCodecs.mimeForCodecName("mpeg4"))
}
/**
* Without this the agreement test above could pass on two nulls.
*
* `MediaFormat.MIMETYPE_VIDEO_AVC` is a Java compile-time constant, so it is inlined and the
* unit-test classpath's stubbed `android.jar` never has to supply it. If that ever stops being
* true, every MIME comparison here would be `null == null` and green — the vacuous-mutation
* failure this repo has counted before. Assert one literal so the stub fails loudly instead.
*/
@Test
fun `the MIME constants are real strings rather than stubs`() {
assertEquals("video/avc", AndroidDeviceCodecs.mimeForCodecName("h264"))
assertEquals("video/hevc", AndroidDeviceCodecs.mimeForCodecName("hevc"))
assertEquals("video/avc", AndroidDeviceCodecs.mimeFor(VideoCodec.H264))
}
@Test
fun `codec names are matched case-insensitively on both sides`() {
assertEquals(VideoCodec.H265, CodecNames.videoFromName("HEV1"))
assertEquals("video/hevc", AndroidDeviceCodecs.mimeForCodecName("HEV1"))
}
@Test
fun `a name neither table knows still resolves to nothing`() {
assertNull(CodecNames.videoFromName("cinepak"))
assertNull(AndroidDeviceCodecs.mimeForCodecName("cinepak"))
}
/**
* The behaviour #87 actually changes, at the seam that uses it.
*
* `canDecode` treats an unresolved name as "assume the platform copes". Before the alias
* landed, a device with no HEVC decoder answered true for `x265` and Media3 was handed a job it
* could not do; now the router sends it to FFmpeg without spending the attempt.
*/
@Test
fun `a device without the decoder now says so for the aliases it used to wave through`() {
val hevcOnly = AndroidDeviceCodecs.forTesting(encoders = emptySet(), decoders = setOf("video/hevc"))
assertTrue("x265 is HEVC by another name", hevcOnly.canDecode("x265"))
assertFalse("this device has no AVC decoder, and x264 is AVC", hevcOnly.canDecode("x264"))
assertTrue("a name nobody knows keeps the permissive answer", hevcOnly.canDecode("cinepak"))
}
}
@@ -1,6 +1,7 @@
package org.libremediaconverter.model
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Test
@@ -10,6 +11,16 @@ import org.junit.Test
* Three vocabularies meet: `MediaExtractor` MIME types, FFprobe `codec_name` strings, and the
* enums. Stream copy depends on the round trip, so a missing alias here shows up as "we could not
* identify the source codec" and silently costs the user a re-encode.
*
* Also bites on #74: `describeVideo` and `describeAudio` are one function apiece over one
* vocabulary and had stopped matching. Only the video side special-cased
* [InputProbe.UNPARSEABLE]; the audio side fell through to the raw name, and that sentinel opens
* with a NUL, so the source-info card would have rendered a control character. The arms are shared
* now, and the tests below assert both sides so the symmetric bug cannot reappear on the other one.
*
* The tables these read are cross-checked against the device capability check by
* `CodecVocabularyTest` (#87). Deliberately not repeated here: this file is what each name means,
* that one is whether the app's two copies of the vocabulary still agree.
*/
class CodecNamesTest {
@@ -48,4 +59,52 @@ class CodecNamesTest {
// An unrecognised but real codec name is more useful shown than hidden.
assertEquals("cinepak", CodecNames.describeVideo("cinepak"))
}
/** The audio row of the same card, which had none of the above. */
@Test
fun `audio descriptions degrade exactly the way video ones do`() {
assertEquals("AAC", CodecNames.describeAudio("mp4a"))
assertEquals("Unknown", CodecNames.describeAudio(null))
assertEquals("Unrecognised", CodecNames.describeAudio(InputProbe.UNPARSEABLE))
assertEquals("qdm2", CodecNames.describeAudio("qdm2"))
}
/**
* #74's actual failure mode, stated as the thing the user would have seen.
*
* `InputProbe.UNPARSEABLE` is `"\u0000unparseable"`. Falling through to `?: name` does not
* mislabel the track, it puts U+0000 into a `Text`.
*/
@Test
fun `no description can put a control character on the card`() {
listOf(CodecNames.describeAudio(InputProbe.UNPARSEABLE), CodecNames.describeVideo(InputProbe.UNPARSEABLE))
.forEach { assertFalse("$it leaks the sentinel", it.contains('\u0000')) }
}
/**
* Every alias, pinned one at a time.
*
* The tables became maps so `CodecVocabularyTest` could enumerate them; this is what catches a
* key mistyped or a value pointing at the wrong enum while that rewrite happened.
*/
@Test
fun `every name in the tables resolves to the codec it spells`() {
CodecNames.VIDEO_ALIASES.forEach { (name, codec) ->
assertEquals(name, codec, CodecNames.videoFromName(name))
}
CodecNames.AUDIO_ALIASES.forEach { (name, codec) ->
assertEquals(name, codec, CodecNames.audioFromName(name))
}
assertEquals(VideoCodec.H264, CodecNames.videoFromName("x264"))
assertEquals(VideoCodec.VP9, CodecNames.videoFromName("vp09"))
assertEquals(AudioCodec.MP3, CodecNames.audioFromName("mpga"))
assertEquals(AudioCodec.OPUS, CodecNames.audioFromName("opus"))
}
/** The audio lookup reads the sentinel the same way the video one does. */
@Test
fun `the unparseable sentinel resolves to nothing on the audio side too`() {
assertNull(CodecNames.audioFromName(InputProbe.UNPARSEABLE))
assertNull(CodecNames.audioFromName(null))
}
}