Compare commits

...
Author SHA1 Message Date
JMR-devandClaude Opus 5 2125763ebf Read FFprobe's answer without spawning FFprobe (#195)
readMediaInformation was 114 missed instructions and 24 missed branches -- the second
largest block on the wave-4 report -- and exactly one line of it needed a device:

    FFprobeKit.getMediaInformation(path).getMediaInformation()

Everything after it reads an ordinary object, so it moves into ffprobeInfoFrom and the edge
keeps the call and the null check. Verified JVM-safe rather than assumed: javap over the
committed AAR's runtime jar shows MediaInformation(JSONObject, List<StreamInformation>,
List<Chapter>) and StreamInformation(JSONObject) as plain public constructors whose <clinit>
does not load the native library, so a test builds its own without libffmpegkit present.

The decision worth reaching is containerFrom's SECOND argument. FFprobe reports
"matroska,webm" for both MKV and WebM because they share a demuxer, so the video codec is
the only thing separating them. containerFrom has thirty-three covered branches and not one
can notice that argument being dropped -- the mistake is at the call, not in the callee, so
every existing containerFrom test stays green while every VP9 WebM quietly becomes an MKV.

Two things the tests found rather than confirmed.

The format properties are NESTED under "format": getFormat() resolves through
getStringFormatProperty, not off the top-level object. The first fixture put the keys at the
top level and four cases failed with a null container. The helper says so now.

And one mutation SURVIVED on the first pass -- reading dimensions with
streams.firstNotNullOfOrNull { it.getWidth() } instead of video?.getWidth(). The fixture put
the dimensions on the chosen video stream, which is also the first stream carrying any, so
the two readings agreed and the test could not tell them apart. Separating them needs a
chosen video stream with NO dimensions and a later one that has them, which is a real shape:
FFprobe omits width/height for a stream it could not measure. That case is now its own test
and the mutation reddens it.

Mutations, each run and restored:

  drop the video codec argument to containerFrom        1 red
  take the LAST video stream instead of the first       1 red
  read dimensions from any stream, not the chosen one   0 red -> 1 red after the new case
  let an unparseable duration throw instead of zero     1 red

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-05 19:36:32 -05:00
JMR-devandClaude Opus 5 6d700f0014 Cut a seam through the codec enumeration, and say what a failed one actually does (#194)
probe() was 20 never-executed lines, the biggest single block on the report. It has been
looked at twice and left out twice, and both closes were right about what they closed:
#86 ruled it device-bound, and #133 re-checked that with ShadowMediaCodecList in hand and
still declined, because MediaCodecInfoBuilder has no setIsAlias and no setCanonicalName --
"so the alias skip and the canonical-name dedup, the two things the class's KDoc calls out
as easy to get wrong, are not reachable through it."

That objection is about the shadow. It does not apply to a function taking its own entry
type, which #133 did not evaluate. capabilitiesFrom(enumerate: () -> Sequence<CodecEntry>)
holds every rule; the edge keeps only the mapping from MediaCodecList onto CodecEntry.

The parameter is a Sequence rather than a List on purpose. runCatching has always wrapped
the *iteration*, so a MediaCodecInfo whose properties throw partway leaves the codecs
already read in place. A List parameter would move that throw outside the loop and turn a
partial answer into an empty one -- a behaviour change smuggled in as a refactor. There is
now a test for the partial case, and swapping the Sequence for an eager toList() reddens it.

The behaviour change this DOES make is one line of log, and it is the reason the seam was
worth cutting at all. The fallback said "Codec enumeration failed; assuming permissive" and
returned empty sets -- but "video/avc" in emptySet() is false, so canEncode and canDecode
answer no to everything and every job routes to FFmpeg. That is the restrictive answer, and
it is the right one: FFmpeg does whatever Media3 does, only slower. The code stays; the
message and the class KDoc now describe it.

Seven tests. One of them was wrong first and the mutation is what said so: the alias case
originally listed the alias *after* the codec it aliases and passed with the skip deleted,
because canonicalName is shared and the dedup catches the second entry either way. The two
rules overlap, so a fixture that does not separate them tests neither. Order separates
them -- an alias arriving first claims the canonical name in `seen` and gets its own types
credited, and the real codec is then dropped by the dedup. That is now the test, and it
also says what the rule is worth: with a Set accumulator, an alias declaring the same types
as its codec changes nothing, so the skip earns its place only when the two disagree.

Mutations, each run and restored, each reddening the test that owns it:

  drop !isSoftwareOnly from the encoder predicate     1 red
  remove the alias skip                               1 red  (0 before the fixture was fixed)
  remove the canonical-name dedup                     1 red
  remove the video/ prefix filter                     1 red
  apply the hardware predicate to decoders too        1 red
  make the failure fallback permissive                2 red
  eager toList() instead of the lazy Sequence         1 red

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-05 19:36:31 -05:00
JMR-devandClaude Opus 5 da8d53851b Render the container row for a video nothing could name (#199)
ConverterScreen.kt:668's null arm -- DetailRow("Container", probe.container?.label ?:
"Unknown") in the VIDEO branch -- had never rendered. Every video case in FileCardTest uses
VIDEO_PROBE, which carries container = MP4.

The argument for adding it is the asymmetry, not the coverage. FileCard renders that exact
expression twice, once in AUDIO_ONLY (:660) and once in VIDEO (:668), and "an audio-only
file nothing else could describe degrades one row at a time" drives only the first. Same
expression, same fallback, one kind covered and one not -- which is the same argument
CLAUDE.md records for including ContainerCapabilities:94.

Nor is null an edge case here. InputProbe.container's own KDoc says MediaExtractor cannot
report a container at all, so it comes from FFprobe alone: any run where FFprobe did not
answer produces exactly this shape -- real codec, real dimensions, real duration, no
container. An empty value in its place would read as a rendering bug rather than as a
probe that got half its sources.

The other three rows are asserted alongside, which is what keeps this from being a copy of
the audio-only case. There, everything is unknown at once; here one field is missing from
a probe that is otherwise complete, and the rest have to be unaffected by it.

Mutation: `?: "Unknown"` -> `?: ""` at :668 only, run and restored. The AUDIO_ONLY twin at
:660 is a separate expression, and mutating that one would redden the existing test instead
-- which would prove nothing about this one.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-05 19:36:30 -05:00
JMR-devandClaude Opus 5 cc215195ee Build a command for the audio the user turned off (#198)
audioArgs' Drop arm -- `AudioPlan.Drop -> listOf("-an")` -- was ci == 0. The suite's only
-an assertion lives in "gif generates a palette to avoid banding and drops audio", and that
one comes from the image path at FFmpegCommandBuilder.kt:79/:90, which emits -an directly
and never reaches audioArgs. Two sites, one string, one tested.

It is a live path rather than defensive code. AdvancedPicker renders all of
AudioCodec.entries including NONE, ContainerCapabilities.validate permits audio-off whenever
the input has video, and MKV routes the job to FFmpeg -- so "convert this and drop the
soundtrack" is something a user can do today and nothing had built the command for.

Both halves are asserted, and the second is not padding: -an alone still passes if the arm
falls through to the else and emits an AAC encoder beside the flag, which is a file that is
silent because the flag won while carrying an encoder nobody asked for.

Three mutations, all run and restored. The third is the one that justifies the second
assertion, since the first two break -an as a side effect and so cannot show it:

  Drop -> emptyList()                     red
  Drop -> the else arm's aac encoder      red (loses -an as well)
  Drop -> listOf("-an", "-c:a", "aac")    red -- -an intact, caught by assertFalse

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-05 19:36:28 -05:00
6 changed files with 550 additions and 13 deletions
@@ -21,6 +21,11 @@ import org.libremediaconverter.model.VideoCodec
* words, "cannot be tested for correctness". It is a hint, not a guarantee, which is
* why the router treats a failed hardware export as a signal to fall back rather
* than trusting this up front.
* - **An enumeration that fails answers no to everything**, which sends every job to
* FFmpeg. Empty sets are not a permissive default: `canEncode` looks a MIME type up in
* [hardwareEncodeMimes] and finds nothing there. That is the intended answer — FFmpeg
* can do whatever Media3 can, only slower — but it is the opposite of what this class
* said until #194, so it is written down rather than left to be re-derived.
*/
class AndroidDeviceCodecs private constructor(
private val hardwareEncodeMimes: Set<String>,
@@ -46,22 +51,62 @@ class AndroidDeviceCodecs private constructor(
fun get(): AndroidDeviceCodecs = cached ?: synchronized(this) { cached ?: probe().also { cached = it } }
private fun probe(): AndroidDeviceCodecs {
/**
* One entry of the platform's codec list, reduced to what the rules below read.
*
* The five booleans and the type list are the whole of what [capabilitiesFrom] needs, and
* none of them can be set on a `MediaCodecInfo` from a test: Robolectric ships
* `MediaCodecInfoBuilder`, but it has no `setIsAlias` and no `setCanonicalName`, which is
* exactly the objection #133 raised against reaching this code through
* `ShadowMediaCodecList`. That objection is about the shadow. It does not apply to a
* function that takes its own entry type, which is why this exists.
*/
internal data class CodecEntry(
val canonicalName: String,
val isAlias: Boolean,
val isEncoder: Boolean,
val isHardwareAccelerated: Boolean,
val isSoftwareOnly: Boolean,
val supportedTypes: List<String>,
)
/**
* The enumeration rules, over entries a caller chooses.
*
* [probe] is the only production caller and supplies the real codec list; a test supplies
* its own, which is the point — the two rules this class's KDoc calls out as easy to get
* wrong, the alias skip and the canonical-name dedup, are unreachable any other way.
*
* **`enumerate` returns a `Sequence`, deliberately.** The `runCatching` has to wrap the
* *iteration* rather than a list built before it, because a `MediaCodecInfo` whose
* properties throw does so partway through — and when that happens the codecs already read
* are kept. Taking a `List` here would move that throw outside the loop and silently turn a
* partial answer into an empty one. That behaviour predates this seam; a `List` parameter
* would have changed it as a side effect of a refactor.
*
* **An enumeration that fails answers restrictively, and that is deliberate.** The sets
* come back empty, and `"video/avc" in emptySet()` is `false`, so [canEncode] and
* [canDecode] both answer no and every job routes to FFmpeg. FFmpeg can do everything
* Media3 can, only slower, so refusing the hardware path is the safe reading of "we could
* not find out what this device supports". This used to log "assuming permissive", which
* described the opposite of what the code does.
*/
internal fun capabilitiesFrom(enumerate: () -> Sequence<CodecEntry>): AndroidDeviceCodecs {
val encoders = mutableSetOf<String>()
val decoders = mutableSetOf<String>()
val seen = mutableSetOf<String>()
runCatching {
MediaCodecList(MediaCodecList.REGULAR_CODECS).codecInfos.forEach { info ->
enumerate().forEach { entry ->
// Aliases point at the same underlying codec; counting both would
// double-count capabilities.
if (info.isAlias) return@forEach
if (!seen.add(info.canonicalName)) return@forEach
if (entry.isAlias) return@forEach
if (!seen.add(entry.canonicalName)) return@forEach
info.supportedTypes.forEach { mime ->
entry.supportedTypes.forEach { mime ->
if (!mime.startsWith("video/")) return@forEach
if (info.isEncoder) {
if (info.isHardwareAccelerated && !info.isSoftwareOnly) {
if (entry.isEncoder) {
if (entry.isHardwareAccelerated && !entry.isSoftwareOnly) {
encoders += mime
}
} else {
@@ -69,12 +114,32 @@ class AndroidDeviceCodecs private constructor(
}
}
}
}.onFailure { Log.w(TAG, "Codec enumeration failed; assuming permissive.", it) }
}.onFailure { Log.w(TAG, "Codec enumeration failed; routing everything to FFmpeg.", it) }
Log.i(TAG, "Hardware video encoders: $encoders")
return AndroidDeviceCodecs(encoders, decoders)
}
/**
* The thin edge: the real codec list, mapped onto [CodecEntry] one at a time.
*
* Lazily, so a property that throws does it inside [capabilitiesFrom]'s `runCatching` and
* on the entry that caused it — see that function's note on why the parameter is a
* `Sequence`.
*/
private fun probe(): AndroidDeviceCodecs = capabilitiesFrom {
MediaCodecList(MediaCodecList.REGULAR_CODECS).codecInfos.asSequence().map { info ->
CodecEntry(
canonicalName = info.canonicalName,
isAlias = info.isAlias,
isEncoder = info.isEncoder,
isHardwareAccelerated = info.isHardwareAccelerated,
isSoftwareOnly = info.isSoftwareOnly,
supportedTypes = info.supportedTypes.toList(),
)
}
}
/**
* `internal` rather than `private` so the cross-check test can ask what a [VideoCodec]
* means here and compare it with what [NAME_TO_MIME] says the same codec's names mean.
@@ -221,12 +221,39 @@ object MediaProbe {
null
}
private fun readMediaInformation(path: String): FFprobeInfo? {
// ffmpeg-kit-next is compiled from Kotlin with private backing fields, so these have to go
// through the Java getters rather than property syntax.
val info: MediaInformation = FFprobeKit.getMediaInformation(path).getMediaInformation()
?: return null
/**
* The thin edge: spawn FFprobe, hand what it said to [ffprobeInfoFrom].
*
* Everything device-bound is on this line and the null check under it. What FFprobe *said* is a
* `MediaInformation`, which is an ordinary object over a `JSONObject` — so the reading of it is
* a decision a test can choose the inputs for, and it lives below rather than here.
*/
private fun readMediaInformation(path: String): FFprobeInfo? =
FFprobeKit.getMediaInformation(path).getMediaInformation()?.let(::ffprobeInfoFrom)
/**
* What FFprobe's answer means, as a function of the answer alone.
*
* `internal` for the same reason [Extracted] and [FFprobeInfo] are: a test cannot name it
* otherwise, and the JVM test source set is a friend of `main`.
*
* **JVM-safe, verified rather than assumed.** `javap` over the committed AAR's runtime jar:
* `MediaInformation(JSONObject, List<StreamInformation>, List<Chapter>)` and
* `StreamInformation(JSONObject)` are plain public constructors, and neither class's `<clinit>`
* touches the native library — so a test builds its own without `libffmpegkit` being present.
* That is the whole reason this split is worth making: `readMediaInformation` was 114 missed
* instructions and 24 missed branches, of which exactly one line needed a device.
*
* The subtle part is the **second argument to [containerFrom]**. `matroska,webm` is reported
* for both MKV and WebM — they share a demuxer — so the video codec is the only thing that
* separates them, and dropping it silently turns every VP9 WebM into an MKV. `containerFrom`
* has thirty-three covered branches of its own and none of them can notice that, because the
* mistake is at the call rather than in the callee.
*
* ffmpeg-kit-next is compiled from Kotlin with private backing fields, so these go through the
* Java getters rather than property syntax.
*/
internal fun ffprobeInfoFrom(info: MediaInformation): FFprobeInfo {
val streams = info.getStreams().orEmpty()
val video = streams.firstOrNull { it.getType() == "video" }
val audio = streams.firstOrNull { it.getType() == "audio" }
@@ -0,0 +1,201 @@
package org.libremediaconverter.codec
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.VideoCodec
import org.robolectric.RobolectricTestRunner
/**
* The rules `AndroidDeviceCodecs.probe()` applies to the platform's codec list.
*
* ## Why this is not a third run of the #86/#133 spike
*
* #86 closed `probe()` as device-bound. #133 re-opened the question with
* `ShadowMediaCodecList` in hand and closed it again, for a reason that was right about what it
* was answering: `MediaCodecInfoBuilder` "has no `setIsAlias` and no `setCanonicalName`, so the
* alias skip and the canonical-name dedup — the two things the class's KDoc calls out as easy to
* get wrong — are not reachable through it."
*
* **That objection is about the shadow.** It does not apply to a function that takes its own entry
* type, which is what `capabilitiesFrom` now does. The half #133 named as unreachable is the half
* this file spends most of its cases on.
*
* ## What made the seam worth cutting, which is not coverage
*
* The `runCatching` fallback logged *"assuming permissive"* and returned empty sets — and empty
* sets are **restrictive**: `"video/avc" in emptySet()` is `false`, so `canEncode` and `canDecode`
* both answer no and every job routes to FFmpeg. The code was right and the message described the
* opposite of it. That is pinned below, so whichever reading a future change takes, it has to say
* so out loud.
*
* Robolectric only because `capabilitiesFrom` logs what it found; the rules themselves are pure.
*/
@RunWith(RobolectricTestRunner::class)
class CodecEnumerationTest {
/**
* The alias skip, in the one arrangement where it is observable — and finding that arrangement
* is the whole of this test.
*
* A first attempt listed the alias *after* the codec it aliases and passed with the skip
* deleted, because `canonicalName` is shared and the dedup below catches the second entry
* either way. The two rules overlap, so a fixture that does not separate them tests neither.
*
* What separates them is **order**. `MediaCodecInfo.getCanonicalName()` on an alias returns the
* underlying codec's name, so an alias arriving first claims that name in `seen` and has its
* own `supportedTypes` credited — and then the real codec is dropped by the dedup. Without the
* alias skip the device is described by whichever entry the platform happened to list first.
*
* That also says what the rule is worth. With a `Set` accumulator, an alias declaring the same
* types as its codec changes nothing whichever order they arrive in; the skip earns its place
* only when the two disagree, which is exactly when believing the wrong one matters.
*/
@Test
fun `an alias listed before the codec it aliases does not describe the device`() {
val codecs = capabilities(
entry("c2.qti.avc.encoder", encoder = true, types = listOf(HEVC), alias = true),
entry("c2.qti.avc.encoder", encoder = true, types = listOf(AVC)),
)
assertTrue("the real codec's types are the device's", codecs.canEncode(VideoCodec.H264))
assertFalse(
"an alias must not be credited with types the codec it aliases never claimed",
codecs.canEncode(VideoCodec.H265),
)
}
@Test
fun `two entries sharing a canonical name are read once`() {
val codecs = capabilities(
entry("c2.qti.avc.encoder", encoder = true, types = listOf(AVC)),
entry("c2.qti.avc.encoder", encoder = true, types = listOf(HEVC)),
)
assertEquals(setOf(AVC), codecs.hardwareEncoders())
}
/**
* Both halves of the hardware predicate, one arm at a time.
*
* A vendor may declare a codec hardware-accelerated *and* software-only; the class KDoc is
* explicit that the first flag "cannot be tested for correctness", so the second is what stops
* a mislabelled software encoder being treated as the fast path.
*/
@Test
fun `an encoder counts as hardware only when it is accelerated and not software-only`() {
assertEquals(
setOf(AVC),
capabilities(entry("hw", encoder = true, accelerated = true, types = listOf(AVC))).hardwareEncoders(),
)
assertEquals(
emptySet<String>(),
capabilities(entry("sw", encoder = true, accelerated = false, types = listOf(AVC))).hardwareEncoders(),
)
assertEquals(
"a codec claiming both must not be trusted as hardware",
emptySet<String>(),
capabilities(
entry("both", encoder = true, accelerated = true, softwareOnly = true, types = listOf(AVC)),
).hardwareEncoders(),
)
}
/**
* Decoders are collected regardless of the hardware flags, and that asymmetry is the design.
*
* `canDecode` asks whether the platform can read the input at all — a software decoder answers
* that as well as a hardware one. `canEncode` asks whether the *fast path* exists, which is a
* different question and why only encoders are filtered.
*/
@Test
fun `a software decoder still counts as something the platform can read`() {
val codecs = capabilities(
entry(
"c2.android.avc.decoder",
encoder = false,
accelerated = false,
softwareOnly = true,
types = listOf(AVC),
),
)
assertTrue(codecs.canDecode("h264"))
}
@Test
fun `audio types are ignored on both sides`() {
val codecs = capabilities(
entry("aac.encoder", encoder = true, accelerated = true, types = listOf("audio/mp4a-latm")),
entry("aac.decoder", encoder = false, types = listOf("audio/mp4a-latm")),
)
assertEquals(emptySet<String>(), codecs.hardwareEncoders())
// Not "the platform cannot decode AAC" -- `canDecode` is asked about *video* codec names,
// and an unknown name is answered permissively. The point is that nothing audio reached
// either set.
assertTrue("an unknown name stays permissive", codecs.canDecode("something-nobody-named"))
}
/**
* The failure fallback, pinned as the restrictive answer it actually is.
*
* #194 decided this rather than assuming it: the code stays, the message changes. If a later
* change wants the permissive reading its old log line described, this test is what makes that
* a decision instead of a drift.
*/
@Test
fun `an enumeration that fails sends every job to FFmpeg`() {
val codecs = AndroidDeviceCodecs.capabilitiesFrom { error("MediaCodecList exploded") }
assertFalse("a failed enumeration must not claim a hardware encoder", codecs.canEncode(VideoCodec.H264))
assertFalse(codecs.canDecode("h264"))
assertEquals(emptySet<String>(), codecs.hardwareEncoders())
}
/**
* A list that throws partway keeps what it already read.
*
* This predates the seam — `runCatching` has always wrapped the iteration rather than a list
* built before it — and it is asserted here because the seam is where it could quietly have
* been lost. Taking a `List` instead of a `Sequence` would move the throw outside the loop and
* turn this partial answer into an empty one, with no test to notice.
*/
@Test
fun `codecs read before a failing entry are kept`() {
val codecs = AndroidDeviceCodecs.capabilitiesFrom {
sequence {
yield(entry("good", encoder = true, accelerated = true, types = listOf(AVC)))
error("the sixth codec's properties threw")
}
}
assertEquals(setOf(AVC), codecs.hardwareEncoders())
}
private fun capabilities(vararg entries: AndroidDeviceCodecs.Companion.CodecEntry) =
AndroidDeviceCodecs.capabilitiesFrom { entries.asSequence() }
private fun entry(
canonicalName: String,
encoder: Boolean,
accelerated: Boolean = true,
softwareOnly: Boolean = false,
alias: Boolean = false,
types: List<String>,
) = AndroidDeviceCodecs.Companion.CodecEntry(
canonicalName = canonicalName,
isAlias = alias,
isEncoder = encoder,
isHardwareAccelerated = accelerated,
isSoftwareOnly = softwareOnly,
supportedTypes = types,
)
private companion object {
const val AVC = "video/avc"
const val HEVC = "video/hevc"
}
}
@@ -0,0 +1,192 @@
package org.libremediaconverter.convert
import com.arthenica.ffmpegkit.MediaInformation
import com.arthenica.ffmpegkit.StreamInformation
import org.json.JSONObject
import org.junit.Assert.assertEquals
import org.junit.Assert.assertFalse
import org.junit.Assert.assertNull
import org.junit.Assert.assertTrue
import org.junit.Test
import org.junit.runner.RunWith
import org.libremediaconverter.model.Container
import org.robolectric.RobolectricTestRunner
/**
* What FFprobe's answer means, read as a function of the answer alone.
*
* `readMediaInformation` was 114 missed instructions and 24 missed branches — the second-biggest
* block on the wave-4 report — of which **exactly one line needed a device**:
*
* ```kotlin
* FFprobeKit.getMediaInformation(path).getMediaInformation()
* ```
*
* Everything after it reads an ordinary object. `javap` over the committed AAR's runtime jar:
* `MediaInformation(JSONObject, List<StreamInformation>, List<Chapter>)` and
* `StreamInformation(JSONObject)` are plain public constructors, and neither class's `<clinit>`
* loads the native library — so the fixtures below are built without `libffmpegkit` present.
*
* ## The one that matters
*
* `containerFrom(formatName, video?.getCodec())`. FFprobe reports `matroska,webm` for **both** MKV
* and WebM, because they share a demuxer, so the video codec is the only thing separating them.
* `containerFrom` has thirty-three covered branches of its own and not one of them can notice the
* argument being dropped — the mistake would be at the call, not in the callee, and every existing
* `containerFrom` test would stay green while every VP9 WebM quietly became an MKV.
*
* Robolectric only for `org.json`, which is a stub in a plain JVM test.
*/
@RunWith(RobolectricTestRunner::class)
class FFprobeMappingTest {
@Test
fun `the video codec decides between matroska and webm`() {
assertEquals(
Container.WEBM,
MediaProbe.ffprobeInfoFrom(info("matroska,webm", stream("video", "vp9"))).container,
)
assertEquals(
Container.MKV,
MediaProbe.ffprobeInfoFrom(info("matroska,webm", stream("video", "h264"))).container,
)
}
/**
* The same format name with no video stream at all, which is what makes the case above about
* the *argument* rather than about the format string.
*/
@Test
fun `a matroska container with no video track cannot be told from webm and is not guessed`() {
val read = MediaProbe.ffprobeInfoFrom(info("matroska,webm", stream("audio", "opus")))
assertEquals(Container.MKV, read.container)
assertNull(read.videoCodec)
}
@Test
fun `the first stream of each type wins`() {
val read = MediaProbe.ffprobeInfoFrom(
info(
"mov,mp4,m4a,3gp,3g2,mj2",
stream("video", "h264", width = 1920, height = 1080),
stream("video", "hevc", width = 640, height = 480),
stream("audio", "aac"),
stream("audio", "mp3"),
),
)
assertEquals("h264", read.videoCodec)
assertEquals("aac", read.audioCodec)
assertEquals(1920, read.width)
assertEquals(1080, read.height)
}
/**
* Dimensions come from the stream the codec came from, not from whichever stream has some.
*
* The fixture is deliberately awkward: the chosen video stream carries **no** dimensions and a
* later one does. That is a real shape — FFprobe omits `width`/`height` for a stream it could
* not measure — and it is the only arrangement that separates the two readings.
*
* A first version of this file asserted the dimensions inside the case above, where the chosen
* stream was also the first one carrying any. Replacing `video?.getWidth()` with
* `streams.firstNotNullOfOrNull { it.getWidth() }` gave the same answer there and **the
* mutation survived**. It reddens here.
*/
@Test
fun `a video stream with no dimensions reports none rather than borrowing another stream's`() {
val read = MediaProbe.ffprobeInfoFrom(
info(
"mov,mp4,m4a,3gp,3g2,mj2",
stream("video", "h264"),
stream("video", "hevc", width = 640, height = 480),
),
)
assertEquals("h264", read.videoCodec)
assertEquals(0, read.width)
assertEquals(0, read.height)
}
/**
* Stream order is the file's, not a promise. An audio-first container must read the same as a
* video-first one.
*/
@Test
fun `an audio track listed first does not become the video track`() {
val read = MediaProbe.ffprobeInfoFrom(
info("mov,mp4,m4a,3gp,3g2,mj2", stream("audio", "aac"), stream("video", "h264")),
)
assertEquals("h264", read.videoCodec)
assertEquals("aac", read.audioCodec)
}
@Test
fun `a duration in seconds becomes milliseconds`() {
assertEquals(12_345L, MediaProbe.ffprobeInfoFrom(info("mp4", duration = "12.345")).durationMs)
}
/**
* Both ways a duration can be absent, and neither may throw.
*
* FFprobe reports `"N/A"` for a stream it could not measure, and omits the key entirely for
* some containers. `toDoubleOrNull` is what keeps the second from being an exception on the
* file-pick path, where there is no user-visible failure to report it as.
*/
@Test
fun `a duration that is not a number is no duration rather than a crash`() {
assertEquals(0L, MediaProbe.ffprobeInfoFrom(info("mp4", duration = "N/A")).durationMs)
assertEquals(0L, MediaProbe.ffprobeInfoFrom(info("mp4", duration = null)).durationMs)
}
@Test
fun `a file with no streams reports nothing rather than defaults that look measured`() {
val read = MediaProbe.ffprobeInfoFrom(info("mp4"))
assertNull(read.videoCodec)
assertNull(read.audioCodec)
assertEquals(0, read.width)
assertEquals(0, read.height)
}
@Test
fun `an image format is reported as one`() {
assertTrue(MediaProbe.ffprobeInfoFrom(info("png_pipe", stream("video", "png"))).isImage)
assertFalse(MediaProbe.ffprobeInfoFrom(info("mp4", stream("video", "h264"))).isImage)
}
private fun stream(type: String, codec: String, width: Int? = null, height: Int? = null) = StreamInformation(
JSONObject().apply {
put(StreamInformation.KEY_TYPE, type)
put(StreamInformation.KEY_CODEC, codec)
width?.let { put(StreamInformation.KEY_WIDTH, it) }
height?.let { put(StreamInformation.KEY_HEIGHT, it) }
},
)
/**
* The format properties are **nested** under `"format"`, which is how FFprobe reports them and
* what `MediaInformation` reads: `getFormat()` resolves through `getStringFormatProperty`, not
* off the top-level object. A first version of this helper put the keys at the top level and
* every format-dependent case failed with a null container, which is worth recording here so
* the next fixture does not have to rediscover it.
*
* Streams are the other half and are *not* nested — they come from the constructor argument.
*/
private fun info(formatName: String, vararg streams: StreamInformation, duration: String? = "1.0") =
MediaInformation(
JSONObject().apply {
put(
MediaInformation.KEY_FORMAT_PROPERTIES,
JSONObject().apply {
put(MediaInformation.KEY_FORMAT, formatName)
duration?.let { put(MediaInformation.KEY_DURATION, it) }
},
)
},
streams.toList(),
emptyList(),
)
}
@@ -205,6 +205,34 @@ class FileCardTest {
assertNoRow("Length")
}
/**
* A video the app knows a great deal about and cannot name the container of.
*
* Not an edge case. `InputProbe.container`'s own KDoc says `MediaExtractor` cannot report a
* container at all -- it comes from FFprobe -- so any run where FFprobe did not answer produces
* exactly this: real codec, real dimensions, real duration, `container = null`.
*
* **The twin was already tested and this one was not**, which is the argument for adding it.
* `FileCard` renders `probe.container?.label ?: "Unknown"` twice, once in the `AUDIO_ONLY`
* branch (`ConverterScreen.kt:660`) and once in the `VIDEO` branch (`:668`), and
* `an audio-only file nothing else could describe degrades one row at a time` drives only the
* first. Same expression, same fallback, one kind covered. That asymmetry is the same one
* `CLAUDE.md` records for including `ContainerCapabilities:94`.
*
* The other rows are asserted alongside so this is not a copy of the audio-only case: there,
* everything is unknown at once; here, one field is missing from a probe that is otherwise
* complete, and the rest must be unaffected by it.
*/
@Test
fun `a video file whose container nothing identified says so and keeps its other rows`() {
setFileCard(input(probe = VIDEO_PROBE.copy(container = null)))
assertRow("Container", "Unknown")
assertRow("Video", "${VideoCodec.H264.label} · 1920×1080")
assertRow("Audio", AudioCodec.AAC.label)
assertRow("Length", "1:30")
}
/**
* The row is one node, not a label node beside a value node. A test matching on `"Container"`
* alone would pass against either shape.
@@ -190,6 +190,30 @@ class FFmpegCommandBuilderTest {
assertPair(cmd(OutputFormat.M4A_AAC), "-b:a", "192k")
}
/**
* Turning audio off, which the Advanced picker offers and nothing had ever built a command for.
*
* `audioArgs`' `Drop` arm was `ci == 0`. The suite's only `-an` assertion is in
* `gif generates a palette to avoid banding and drops audio`, and that one comes from the image
* path (`FFmpegCommandBuilder.kt:79`/`:90`), which emits `-an` directly and never reaches
* `audioArgs`. Two sites, one string, one tested.
*
* It is a live path rather than defensive code: `AdvancedPicker` renders all of
* `AudioCodec.entries` including `NONE`, `ContainerCapabilities.validate` permits audio-off
* whenever the input has video, and MKV routes the job to FFmpeg.
*
* Both halves are asserted. `-an` alone would still pass if the arm fell through to the `else`
* and emitted an AAC encoder beside it -- a file that is silent because the flag won, carrying
* an encoder nobody asked for.
*/
@Test
fun `turning audio off drops the track instead of encoding one`() {
val args = cmd(OutputSpec(Container.MKV, VideoCodec.H264, AudioCodec.NONE))
assertTrue("audio turned off must emit -an, got $args", args.contains("-an"))
assertFalse("a dropped track must not also carry an encoder, got $args", args.contains("-c:a"))
}
@Test
fun `audio only formats never carry a video encoder`() {
listOf(OutputFormat.MP3, OutputFormat.FLAC, OutputFormat.WAV, OutputFormat.OPUS)