Test the three MediaProbe helpers nothing else would catch

MediaProbe's MIME table, its image-demuxer rule and its Int reader are pure
functions with no test at all, and each fails silently rather than loudly.
shortName falls through to substringAfter('/') and reports a plausible-looking
string that CodecNames may or may not still recognise, so a dropped arm turns a
stream-copyable file into a re-encode. isImageFormat is checked before anything
else in classify, so a wrong answer overrides both probes. intOr's runCatching
is the only thing standing between a Float frame rate and losing every other
track property the loop had read.

Widen the three to internal, as #57 did, and say in each KDoc why the shape is
what it is -- the _pipe suffix is not a substring test because yuv4mpegpipe is
raw video, and getInteger casts rather than coerces.

Every format name asserted came from ffprobe rather than from memory: a picked
.png reports png_pipe, a .jpg reports jpeg_pipe, a .y4m reports yuv4mpegpipe.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>

app/src/test/java/org/libremediaconverter/convert/MediaProbeImageFormatTest.kt

@@ -0,0 +1,81 @@
+package org.libremediaconverter.convert
+
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
+import org.junit.Test
+
+/**
+ * The image-demuxer rule, which looks arbitrary until it is read as a suffix.
+ *
+ * `MediaProbe.classify` asks [MediaProbe.isImageFormat] before anything else, so this one boolean
+ * overrides everything both probes found: true and the source-info card says "Image" and a size,
+ * false and it says container, codec and length. Neither mistake fails loudly.
+ *
+ * The rule has two halves and they are not the same shape. `image2` is a whole format name —
+ * FFprobe reports it for a numbered image sequence — while the piped demuxers are named one per
+ * image codec, so `_pipe` has to be matched as a *suffix*: `png_pipe`, `jpeg_pipe`, `webp_pipe`
+ * and some thirty more. Widening that suffix to a substring is the tempting simplification and it
+ * is wrong, because `yuv4mpegpipe` is raw video.
+ *
+ * Every format name asserted here was read back from `ffprobe -show_entries format=format_name`
+ * rather than guessed: a picked `.png` reports `png_pipe`, a picked `.jpg` reports `jpeg_pipe`, a
+ * `.y4m` reports `yuv4mpegpipe`, and `image2` needs the demuxer named explicitly.
+ *
+ * One known gap, deliberately not asserted either way: `image2pipe` is a real FFprobe format name
+ * for an image read from a stream, and this rule answers false for it — it is not `image2` and
+ * does not end in `_pipe`. Whether that is worth fixing is a question about picked-file behaviour
+ * on a device, not something to settle by pinning today's answer here.
+ */
+class MediaProbeImageFormatTest {
+
+    @Test
+    fun `a numbered image sequence is an image`() {
+        assertTrue(MediaProbe.isImageFormat("image2"))
+    }
+
+    /** What a picked PNG or JPEG actually reports, and the reason the suffix rule exists. */
+    @Test
+    fun `the per-codec piped demuxers are images`() {
+        assertTrue(MediaProbe.isImageFormat("png_pipe"))
+        assertTrue(MediaProbe.isImageFormat("jpeg_pipe"))
+        assertTrue(MediaProbe.isImageFormat("webp_pipe"))
+    }
+
+    /**
+     * The half that a substring match would break.
+     *
+     * `yuv4mpegpipe` contains `pipe` and is not an image: it is raw uncompressed video, and
+     * describing it as an image would hide its codec, its size and its length from the card while
+     * leaving the file perfectly convertible.
+     */
+    @Test
+    fun `a format that merely contains pipe is not an image`() {
+        assertFalse(MediaProbe.isImageFormat("yuv4mpegpipe"))
+    }
+
+    /** The ordinary media containers, which is what the false answer is mostly for. */
+    @Test
+    fun `a real container is not an image`() {
+        assertFalse(MediaProbe.isImageFormat("mov,mp4,m4a,3gp,3g2,mj2"))
+        assertFalse(MediaProbe.isImageFormat("matroska,webm"))
+        assertFalse(MediaProbe.isImageFormat("mp3"))
+    }
+
+    /**
+     * FFprobe names every format sharing the demuxer, so the entry that matters can be anywhere in
+     * the list — and the padding and case are normalised the same way [MediaProbe.containerFrom]
+     * normalises them.
+     */
+    @Test
+    fun `an image entry is found anywhere in the list, whatever its spacing or case`() {
+        assertTrue(MediaProbe.isImageFormat("PNG_PIPE"))
+        assertTrue(MediaProbe.isImageFormat(" image2 "))
+        assertTrue(MediaProbe.isImageFormat("something_else, tiff_pipe"))
+    }
+
+    /** Nothing to go on is not an image; the card falls back to describing an unknown container. */
+    @Test
+    fun `an empty format name is not an image`() {
+        assertFalse(MediaProbe.isImageFormat(""))
+    }
+}

app/src/test/java/org/libremediaconverter/convert/MediaProbeMimeNamesTest.kt

@@ -0,0 +1,108 @@
+package org.libremediaconverter.convert
+
+import android.media.MediaFormat
+import org.junit.Assert.assertEquals
+import org.junit.Test
+
+/**
+ * The MIME -> short codec name table, which nothing downstream would notice going wrong.
+ *
+ * `MediaExtractor` answers in platform MIME spellings; the router, the copy planner and the
+ * source-info card all speak FFmpeg's short names. [MediaProbe.shortName] is the one place those
+ * two vocabularies meet, and most of its arms are translations rather than trimming — `video/avc`
+ * is `h264`, `audio/mp4a-latm` is `aac`, `video/x-vnd.on2.vp9` is `vp9`.
+ *
+ * So a dropped or mistyped arm does not throw. It falls through to `substringAfter('/')` and
+ * reports a different, entirely plausible-looking string. `CodecNames` carries alias lists that
+ * happen to rescue some of those (`avc`, `av01`, `raw`) and not others (`mp4a-latm`,
+ * `x-vnd.on2.vp9`), which is exactly why leaning on the rescue is not a plan: an unrecognised
+ * codec is how a stream-copyable file quietly becomes a re-encode, and how the card ends up naming
+ * a codec no user has heard of. This table is the only place those arms are pinned.
+ *
+ * A plain JVM test rather than Robolectric: `MediaFormat.MIMETYPE_*` are Java compile-time String
+ * constants, so this test and `MediaProbe` alike carry the literals in their own bytecode and the
+ * framework class is never loaded.
+ *
+ * Every case names its MIME in the failure message, because the MIME is the thing that has to be
+ * looked up when one of these goes red.
+ */
+class MediaProbeMimeNamesTest {
+
+    @Test
+    fun `an AVC track is reported as h264, which is what everything downstream calls it`() {
+        assertShortName("h264", MediaFormat.MIMETYPE_VIDEO_AVC)
+    }
+
+    /** On2's vendor MIME looks nothing like the codec name FFmpeg and the router use. */
+    @Test
+    fun `the VP8 and VP9 vendor MIMEs are reported without their vendor prefix`() {
+        assertShortName("vp8", MediaFormat.MIMETYPE_VIDEO_VP8)
+        assertShortName("vp9", MediaFormat.MIMETYPE_VIDEO_VP9)
+    }
+
+    @Test
+    fun `AV1 and MPEG-4 are reported by codec name rather than by MIME spelling`() {
+        assertShortName("av1", MediaFormat.MIMETYPE_VIDEO_AV1)
+        assertShortName("mpeg4", MediaFormat.MIMETYPE_VIDEO_MPEG4)
+    }
+
+    @Test
+    fun `an AAC track is reported as aac, not as the mp4a-latm its MIME says`() {
+        assertShortName("aac", MediaFormat.MIMETYPE_AUDIO_AAC)
+    }
+
+    @Test
+    fun `uncompressed audio is reported as pcm, which is not what its MIME says either`() {
+        assertShortName("pcm", MediaFormat.MIMETYPE_AUDIO_RAW)
+    }
+
+    /**
+     * Four arms produce exactly what the fallback would produce anyway.
+     *
+     * `video/hevc` -> `hevc`, `audio/opus` -> `opus`, `audio/flac` -> `flac`,
+     * `audio/vorbis` -> `vorbis`: for these the `when` arm and `substringAfter('/')` agree, so
+     * deleting the arm changes no observable behaviour and no test can catch it. That is a
+     * property of the code rather than a gap here, and it is reported as such rather than dressed
+     * up as coverage. The assertions still earn their place — they pin the promise the router is
+     * given (`hevc`, whatever the MIME happens to spell) against a later edit that changes the
+     * mapping rather than deleting it.
+     */
+    @Test
+    fun `the arms whose MIME subtype already is the short name still map to it`() {
+        assertShortName("hevc", MediaFormat.MIMETYPE_VIDEO_HEVC)
+        assertShortName("opus", MediaFormat.MIMETYPE_AUDIO_OPUS)
+        assertShortName("flac", MediaFormat.MIMETYPE_AUDIO_FLAC)
+        assertShortName("vorbis", MediaFormat.MIMETYPE_AUDIO_VORBIS)
+    }
+
+    /**
+     * The fallback, which is what makes an unlisted codec describable at all.
+     *
+     * These are real `MediaFormat` MIMEs with no arm of their own. Dropping the subtype is the
+     * right guess far more often than reporting the whole MIME would be — FFprobe calls the first
+     * of these `ac3` too.
+     */
+    @Test
+    fun `a MIME with no arm of its own falls back to its subtype`() {
+        assertShortName("ac3", MediaFormat.MIMETYPE_AUDIO_AC3)
+        assertShortName("mpeg2", MediaFormat.MIMETYPE_VIDEO_MPEG2)
+        assertShortName("dolby-vision", MediaFormat.MIMETYPE_VIDEO_DOLBY_VISION)
+    }
+
+    /**
+     * The surprising half of `substringAfter`'s contract, pinned deliberately.
+     *
+     * With no `/` in the string it returns the whole input rather than the empty string. Today's
+     * callers gate on a `video/` or `audio/` prefix so they cannot reach this, but "report what
+     * you were given" rather than "report nothing" is what would keep a malformed MIME visible on
+     * the card instead of blank.
+     */
+    @Test
+    fun `a MIME with no subtype separator is reported unchanged`() {
+        assertShortName("weird", "weird")
+        assertShortName("", "")
+    }
+
+    private fun assertShortName(expected: String, mime: String) =
+        assertEquals("shortName(\"$mime\")", expected, MediaProbe.shortName(mime))
+}

app/src/test/java/org/libremediaconverter/convert/MediaProbeTrackFieldsTest.kt

@@ -0,0 +1,79 @@
+package org.libremediaconverter.convert
+
+import android.media.MediaFormat
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertTrue
+import org.junit.Test
+import org.junit.runner.RunWith
+import org.robolectric.RobolectricTestRunner
+
+/**
+ * Reading Int track properties out of a `MediaFormat`, which is a heterogeneous map.
+ *
+ * [MediaProbe.intOr] guards two different failures with one expression, and only one of them is
+ * obvious. A key the format does not carry is the easy half. The other is a key it *does* carry
+ * with a value of another type: `getInteger` casts rather than coerces, so a frame rate stored as
+ * a Float answers with a `ClassCastException`. `probeForConcat` reads `KEY_FRAME_RATE`, which the
+ * platform accepts either way, and its `catch` sits outside the track loop — so without the
+ * `runCatching` one oddly-typed field would discard the codec and dimensions already read from
+ * that file and the join would re-encode for no reason.
+ *
+ * Robolectric rather than a plain JVM test, unlike the two sibling `MediaProbe` helper tests: this
+ * one needs a real `MediaFormat` instance, not just its compile-time String constants.
+ */
+@RunWith(RobolectricTestRunner::class)
+class MediaProbeTrackFieldsTest {
+
+    @Test
+    fun `a property the format carries as an Int is read`() {
+        val format = videoFormat()
+
+        assertEquals(1920, with(MediaProbe) { format.intOr(MediaFormat.KEY_WIDTH) })
+        assertEquals(1080, with(MediaProbe) { format.intOr(MediaFormat.KEY_HEIGHT) })
+    }
+
+    /**
+     * A track that simply does not say. `MediaExtractor` omits `KEY_FRAME_RATE` for plenty of real
+     * files, and 0 is what `ConcatPlanner` reads as "cannot prove a match".
+     */
+    @Test
+    fun `a key the format does not carry gives the fallback`() {
+        val format = videoFormat()
+
+        assertEquals(0, with(MediaProbe) { format.intOr(MediaFormat.KEY_FRAME_RATE) })
+        assertEquals(-1, with(MediaProbe) { format.intOr(MediaFormat.KEY_FRAME_RATE, -1) })
+    }
+
+    /**
+     * The premise of the `runCatching`, pinned against the platform rather than assumed.
+     *
+     * If `getInteger` coerced a Float instead of throwing, the guard below would be testing
+     * nothing at all — so the throw is asserted directly first.
+     */
+    @Test
+    fun `getInteger refuses a Float rather than coercing it`() {
+        val format = videoFormat()
+        format.setFloat(MediaFormat.KEY_FRAME_RATE, NON_INTEGRAL_FRAME_RATE)
+
+        val thrown = runCatching { format.getInteger(MediaFormat.KEY_FRAME_RATE) }.exceptionOrNull()
+
+        assertTrue("expected getInteger to refuse a Float, got $thrown", thrown is ClassCastException)
+    }
+
+    /** And that refusal is answered with the fallback, not passed on to the caller. */
+    @Test
+    fun `a frame rate the format carries as a Float gives the fallback rather than throwing`() {
+        val format = videoFormat()
+        format.setFloat(MediaFormat.KEY_FRAME_RATE, NON_INTEGRAL_FRAME_RATE)
+
+        assertEquals(0, with(MediaProbe) { format.intOr(MediaFormat.KEY_FRAME_RATE) })
+        assertEquals(-1, with(MediaProbe) { format.intOr(MediaFormat.KEY_FRAME_RATE, -1) })
+    }
+
+    private fun videoFormat(): MediaFormat = MediaFormat.createVideoFormat(MediaFormat.MIMETYPE_VIDEO_AVC, 1920, 1080)
+
+    private companion object {
+        /** NTSC's 30000/1001, the frame rate that cannot be stored as an Int in the first place. */
+        const val NON_INTEGRAL_FRAME_RATE = 29.97f
+    }
+}