AndroidDeviceCodecs' codec-name aliases and its assume-supported fallthrough have no test #86

New Issue

Closed

opened 2026-08-25 03:21:56 +00:00 by JMR-dev · 4 comments

JMR-dev commented 2026-08-25 03:21:56 +00:00 (Migrated from github.com)

Copy Link

Copy Source

Filed from a coverage read on ad28293. AndroidDeviceCodecs.Companion reports 0/37 on the JVM.

The half that must stay device-bound

probe() (18/18) queries the real MediaCodecList for what this hardware can encode and decode. That is the class's whole purpose and it cannot be answered on the JVM. ConversionWorkerTest and RealMediaBenchmark exercise it on a device; JaCoCo measures testDebugUnitTest only, so its 0% is a boundary, not a gap. ConversionRouterTest already tests the decisions against fabricated DeviceCodecs profiles, which is the right seam and is why forTesting exists.

The half that is a gap

private fun mimeFor(codec: VideoCodec): String?          // 8 lines, 0 covered
private fun mimeForCodecName(name: String): String?      // 9 lines, 0 covered

Pure lookups, no device. mimeForCodecName handles aliases — avc/avc1, hvc1, av01 — which is exactly the kind of table that goes wrong quietly, and its else -> null arm carries a documented policy:

Unknown to us: assume the platform can handle it and let a failed export trigger the FFmpeg fallback, rather than pre-emptively refusing hardware.

That policy has a cost — a wasted hardware attempt — so which names fall through it is a decision worth pinning, not an accident worth inheriting.

mimeFor's COPY, NONE -> null arm likewise encodes a reasoned choice ("a copied or absent track places no demand on the hardware"), and nothing checks that canEncode still answers true for it.

Done means

Both lookups tested arm by arm including the alias spellings and both null arms, and one test that pins the COPY/NONE → canEncode == true consequence rather than just the mapping.

Mutation: drop "avc1" from the H.264 arm and the alias test goes red; make mimeFor return a MIME for COPY and the canEncode test goes red.

Read this first

These tables have already drifted from CodecNames — see the linked ticket. Test the two together or the tests will encode the disagreement instead of catching it.

_Filed from a coverage read on `ad28293`. `AndroidDeviceCodecs.Companion` reports **0/37** on the JVM._ ### The half that must stay device-bound `probe()` (18/18) queries the real `MediaCodecList` for what this hardware can encode and decode. That is the class's whole purpose and it cannot be answered on the JVM. `ConversionWorkerTest` and `RealMediaBenchmark` exercise it on a device; JaCoCo measures `testDebugUnitTest` only, so its 0% is a boundary, not a gap. `ConversionRouterTest` already tests the *decisions* against fabricated `DeviceCodecs` profiles, which is the right seam and is why `forTesting` exists. ### The half that is a gap ```kotlin private fun mimeFor(codec: VideoCodec): String? // 8 lines, 0 covered private fun mimeForCodecName(name: String): String? // 9 lines, 0 covered ``` Pure lookups, no device. `mimeForCodecName` handles **aliases** — `avc`/`avc1`, `hvc1`, `av01` — which is exactly the kind of table that goes wrong quietly, and its `else -> null` arm carries a documented policy: > Unknown to us: assume the platform can handle it and let a failed export trigger the FFmpeg fallback, rather than pre-emptively refusing hardware. That policy has a cost — a wasted hardware attempt — so which names fall through it is a decision worth pinning, not an accident worth inheriting. `mimeFor`'s `COPY, NONE -> null` arm likewise encodes a reasoned choice ("a copied or absent track places no demand on the hardware"), and nothing checks that `canEncode` still answers `true` for it. ### Done means Both lookups tested arm by arm including the alias spellings and both `null` arms, and one test that pins the `COPY`/`NONE` → `canEncode == true` consequence rather than just the mapping. **Mutation:** drop `"avc1"` from the H.264 arm and the alias test goes red; make `mimeFor` return a MIME for `COPY` and the `canEncode` test goes red. ### Read this first **These tables have already drifted from `CodecNames`** — see the linked ticket. Test the two together or the tests will encode the disagreement instead of catching it.

JMR-dev commented 2026-08-25 03:22:25 +00:00 (Migrated from github.com)

Copy Link

Copy Source

Read #87 before writing these tests. The tables this ticket covers have already drifted from CodecNames — five codec names resolve in one and not the other, in both directions.

Testing this table arm-by-arm in isolation would encode the disagreement rather than catch it. #87 carries the comparison and the mutation that actually bites (add an alias to one table only, and the cross-check goes red).

Read **#87** before writing these tests. The tables this ticket covers have already drifted from `CodecNames` — five codec names resolve in one and not the other, in both directions. Testing this table arm-by-arm in isolation would **encode the disagreement** rather than catch it. #87 carries the comparison and the mutation that actually bites (add an alias to one table only, and the cross-check goes red).

JMR-dev commented 2026-08-25 03:49:36 +00:00 (Migrated from github.com)

Copy Link

Copy Source

PR #90 (#87 + #74) covers most of this ticket's "done means", because it had to: this ticket's own
"read this first" is what #87 measured. Recording what is now pinned so nobody writes it twice —
and, more to the point, so nobody writes the per-arm version this ticket originally asked for,
which would encode the disagreement rather than catch it.

Already pinned, in app/src/test/java/org/libremediaconverter/codec/CodecVocabularyTest.kt:

• The alias spellings, including avc/avc1/hvc1/av01 — but cross-checked against
  CodecNames.VIDEO_ALIASES rather than asserted arm by arm. Both tables are maps now (a when
  cannot be enumerated), so the test walks both key sets in both directions and also asserts they
  agree on each name's meaning. Mutation: adding "avc3" to one side alone reddens it while
  CodecNamesTest stays green at 8 tests, 0 failures.
• The else -> null policy, at the seam that uses it: a device without the decoder now says so for the aliases it used to wave through builds an AndroidDeviceCodecs.forTesting(...) with
  a known decoder set and asserts canDecode("cinepak") is still true — the documented "assume
  the platform can handle it" answer — while canDecode("x264") on a device with no AVC decoder
  is now false. That last one is #87's behaviour change; four names that used to fall through
  the policy no longer do.
• The MIME constants are real, asserted against a literal "video/avc", so the comparisons
  cannot pass as null == null if MediaFormat's constants ever stop being inlined into the
  unit-test classpath.

mimeFor, mimeForCodecName, NAME_TO_MIME and DECODE_ONLY_NAMES are internal now rather
than private, per #57's precedent (MainActivity.kt:36-38).

Still open, and it is the half #90 did not touch: mimeFor's COPY, NONE -> null arm and the
consequence it exists for — canEncode must answer true for both, because a copied or absent
track places no demand on the hardware. Nothing asserts that today. The mutation from this ticket
still applies unchanged: make mimeFor return a MIME for COPY and the canEncode test must go
red. That is a few lines against AndroidDeviceCodecs.forTesting(encoders = emptySet(), ...).

PR #90 (#87 + #74) covers most of this ticket's "done means", because it had to: this ticket's own "read this first" is what #87 measured. Recording what is now pinned so nobody writes it twice — and, more to the point, so nobody writes the per-arm version this ticket originally asked for, which would encode the disagreement rather than catch it. Already pinned, in `app/src/test/java/org/libremediaconverter/codec/CodecVocabularyTest.kt`: - **The alias spellings**, including `avc`/`avc1`/`hvc1`/`av01` — but cross-checked against `CodecNames.VIDEO_ALIASES` rather than asserted arm by arm. Both tables are maps now (a `when` cannot be enumerated), so the test walks both key sets in both directions and also asserts they agree on each name's *meaning*. Mutation: adding `"avc3"` to one side alone reddens it while `CodecNamesTest` stays green at 8 tests, 0 failures. - **The `else -> null` policy**, at the seam that uses it: `a device without the decoder now says so for the aliases it used to wave through` builds an `AndroidDeviceCodecs.forTesting(...)` with a known decoder set and asserts `canDecode("cinepak")` is still `true` — the documented "assume the platform can handle it" answer — while `canDecode("x264")` on a device with no AVC decoder is now `false`. That last one is #87's behaviour change; four names that used to fall through the policy no longer do. - **The MIME constants are real**, asserted against a literal `"video/avc"`, so the comparisons cannot pass as `null == null` if `MediaFormat`'s constants ever stop being inlined into the unit-test classpath. `mimeFor`, `mimeForCodecName`, `NAME_TO_MIME` and `DECODE_ONLY_NAMES` are `internal` now rather than `private`, per #57's precedent (`MainActivity.kt:36-38`). **Still open, and it is the half #90 did not touch:** `mimeFor`'s `COPY, NONE -> null` arm and the consequence it exists for — `canEncode` must answer `true` for both, because a copied or absent track places no demand on the hardware. Nothing asserts that today. The mutation from this ticket still applies unchanged: make `mimeFor` return a MIME for `COPY` and the `canEncode` test must go red. That is a few lines against `AndroidDeviceCodecs.forTesting(encoders = emptySet(), ...)`.

JMR-dev commented 2026-08-25 04:08:36 +00:00 (Migrated from github.com)

Copy Link

Copy Source

Two things from the sibling work that change this ticket's shape. Read both before starting.

1. Most of the original scope is already pinned by #87 (PR #90). That PR turned mimeForCodecName and mimeFor from when expressions into enumerable maps, widened them to internal, and added CodecVocabularyTest covering the aliases, the else -> null policy at the canDecode seam, and a literal-MIME guard against vacuity. A when cannot be enumerated, which is why no cross-check was possible before.

What is still open, and deliberately not taken there: mimeFor's COPY, NONE -> null arm and the canEncode == true consequence that follows from it. That is the remaining bite here.

2. There is a fifth enum-to-MIME pairing on the same axis, and nothing checks that it agrees. From #85 (PR #91): AndroidDeviceCodecs.mimeFor(VideoCodec) and Media3Engine.videoMimeTypeFor(VideoCodec) are both VideoCodec -> MIME. They agree by value today. Nothing asserts it, and #85 deliberately left that cross-check here rather than reaching into a file another agent owned.

Both are now internal, so the assertion is available to write.

Audio has no partner yet. AndroidDeviceCodecs is video-only, so Media3Engine.audioMimeTypeFor has nothing to cross-check against. That is a gap rather than a decision — worth naming in whatever lands, instead of leaving the asymmetry unexplained.

Suggested acceptance, on top of the original: make the two VideoCodec -> MIME tables disagree on one codec and confirm the cross-check goes red. Per-table arm tests will not catch that — the same argument #87 made and then demonstrated.

Two things from the sibling work that change this ticket's shape. Read both before starting. **1. Most of the original scope is already pinned by #87 (PR #90).** That PR turned `mimeForCodecName` and `mimeFor` from `when` expressions into enumerable maps, widened them to `internal`, and added `CodecVocabularyTest` covering the aliases, the `else -> null` policy at the `canDecode` seam, and a literal-MIME guard against vacuity. A `when` cannot be enumerated, which is why no cross-check was possible before. **What is still open, and deliberately not taken there:** `mimeFor`'s `COPY, NONE -> null` arm and the `canEncode == true` consequence that follows from it. That is the remaining bite here. **2. There is a fifth enum-to-MIME pairing on the same axis, and nothing checks that it agrees.** From #85 (PR #91): `AndroidDeviceCodecs.mimeFor(VideoCodec)` and `Media3Engine.videoMimeTypeFor(VideoCodec)` are both `VideoCodec -> MIME`. They agree by value today. Nothing asserts it, and #85 deliberately left that cross-check here rather than reaching into a file another agent owned. Both are now `internal`, so the assertion is available to write. **Audio has no partner yet.** `AndroidDeviceCodecs` is video-only, so `Media3Engine.audioMimeTypeFor` has nothing to cross-check against. That is a gap rather than a decision — worth naming in whatever lands, instead of leaving the asymmetry unexplained. Suggested acceptance, on top of the original: make the two `VideoCodec -> MIME` tables disagree on one codec and confirm the cross-check goes red. Per-table arm tests will not catch that — the same argument #87 made and then demonstrated.

JMR-dev commented 2026-08-25 05:31:46 +00:00 (Migrated from github.com)

Copy Link

Copy Source

Correcting a premise I put in this ticket, before it propagates further.

My comment above said the two VideoCodec -> MIME tables "agree by value today". They do not. Verified against origin/main:

codec	AndroidDeviceCodecs.mimeFor	Media3Engine.videoMimeTypeFor
H264, H265	real MIME	real MIME
VP8, VP9, AV1	real MIME	null
COPY, NONE	null	null

They agree on four of seven.

I relayed that claim from #85's report rather than reading the two functions, which is the same mistake this session has produced several times over — and the reason it matters here is that acting on it would have been a regression. The obvious way to "make the tables agree" is to flatten mimeFor to null for VP8/VP9/AV1; canEncode(VP9) would then answer true on hardware with no VP9 encoder.

The divergence is legitimate and has a reason on each side: setVideoMimeType rejects those three so the router never asks Media3, while canEncode still has to answer truthfully about the device's own encoder. PR #98 records that by name in the test rather than forcing agreement, which is the right call and the opposite of what my comment implied.

For anyone reading the ticket history: the cross-check is still the deliverable and #87's argument still holds — PR #98's mutation (c) demonstrates it, by changing a table and its per-table expectation in lockstep and showing that only the cross-check goes red. What was wrong was the premise about the starting state, not the plan.

**Correcting a premise I put in this ticket, before it propagates further.** My comment above said the two `VideoCodec -> MIME` tables *"agree by value today"*. **They do not.** Verified against `origin/main`: | codec | `AndroidDeviceCodecs.mimeFor` | `Media3Engine.videoMimeTypeFor` | |---|---|---| | H264, H265 | real MIME | real MIME | | **VP8, VP9, AV1** | **real MIME** | **null** | | COPY, NONE | null | null | They agree on four of seven. I relayed that claim from #85's report rather than reading the two functions, which is the same mistake this session has produced several times over — and the reason it matters here is that **acting on it would have been a regression**. The obvious way to "make the tables agree" is to flatten `mimeFor` to null for VP8/VP9/AV1; `canEncode(VP9)` would then answer `true` on hardware with no VP9 encoder. The divergence is legitimate and has a reason on each side: `setVideoMimeType` rejects those three so the router never asks Media3, while `canEncode` still has to answer truthfully about the device's own encoder. PR #98 records that by name in the test rather than forcing agreement, which is the right call and the opposite of what my comment implied. **For anyone reading the ticket history:** the cross-check is still the deliverable and #87's argument still holds — PR #98's mutation (c) demonstrates it, by changing a table *and* its per-table expectation in lockstep and showing that only the cross-check goes red. What was wrong was the premise about the starting state, not the plan.

Sign in to join this conversation.

No Branch/Tag Specified

Branches Tags

main

fix/102-picker-back-press-overshoot

fix/268-saf-picker-determinism

feat/ogg-vorbis-libvorbis

feat/expedited-conversion-work

fix/gate-cache-in-worktrees

chore/gate-runs-shellcheck

test/publish-delete-arm-real-provider

docs/e8-instrumented-coverage

docs/api37-carrier-count-drift

docs/e7-second-constraint

test/publish-to-a-real-saf-destination

fix/api37-report-match-line

fix/api37-task-snapshot-crash

test/join-failure-message-on-device

docs/e2e-read-findings-e7

test/cancelling-a-running-export

test/reattach-to-a-running-job

test/content-uri-reaches-ffmpeg

fix/launcher-wiring-waits-for-the-pick

fix/cancel-tests-need-a-slower-encode

test/cancelling-a-running-join

test/cancelling-a-running-session

test/notification-cancel-action

test/ffmpeg-progress-is-observed

test/fallback-asserts-the-path

test/flac-and-opus-assert-their-format

docs/e2e-read-findings

docs/wave4-coverage-numbers

fix/injectable-startup-sweep-scope

test/session-outcome-seam

test/launcher-callback-identity

test/theme-follows-system-dark

test/audio-drop-arm

fix/rotation-waits-for-recreation

fix/convert-guards-on-ready

test/retry-save-mime

test/hardware-progress-reaches-workmanager

test/ffprobe-mapping-seam

test/device-codec-enumeration-seam

test/unknown-container-row

test/null-message-fallbacks

test/cancel-reaches-workmanager

docs/coverage-wave3-recovery

test/concat-engine-seam

docs/coverage-wave3

test/mediaprobe-merge-seam

test/adaptive-shell-wiring

test/aac-audio-args

test/notification-progress-text

test/media3-muxer-guard

test/hardware-fallback-and-cancellation

test/unprobeable-join-clip

test/one-branch-outcomes

test/foreground-type-regimes

fix/bound-wedge-diagnostics

docs/coverage-wave2

test/screen-wiring

test/viewmodel-setters

test/join-state-mapping

test/conversion-state-mapping

test/dedupe-user-messages

fix/restore-stack-merges

test/refused-jobs

test/concatworker-failure-arms

test/container-capabilities-audio

test/readspec-enum-fallbacks

test/outputpublisher-seams

test/mediaprobe-track-seam

test/outputpublisher-partial-branches

test/fake-provider-scaffolding

docs/coverage-read-findings

chore/gitignore-kotlin

test/bound-the-hangs

docs/coverage-remeasure

ci/baseline-counter-precision

fix/invalid-suggestion-chip

ci/wedged-leg-report

fix/reattachment-overwrites-pick

test/theme-live-branches

fix/failed-save-retry

fix/empty-composition-crash

ci/advisory-failure-report

docs/seven-run-counts

test/release-permission-guard

ci/build-workflow-permissions

docs/api37-point-release

docs/benchmark-populate-path

fix/dead-assertion-probe-test

ci/actionlint

test/device-codecs-encode-consequence

fix/sdkmanager-pipefail

docs/readme-restart-claim

fix/saf-picker-root-discovery

fix/probe-dispatcher-seam

test/media3engine-mime-tables

test/mediaprobe-pure-helpers

fix/codec-vocabulary-drift

docs/robolectric-rationale-correction

docs/api37-advisory-counts

test/r38-8-saf-e2e

test/r38-7-join-states

test/r38-6-conversion-states

test/r38-5-state-seam

fix/jacoco-robolectric-coverage

docs/instrumented-tests-correction

test/r38-2-filecard

test/r38-4-advanced-picker

test/r38-3-pickers

tools/file-issue-script

tools/api-37-emulator

fix/review-app-gaps

docs/review-corrections

No results found.

Labels

Clear labels

above-cut

Worked autonomously overnight: local, JVM-verifiable, no product decision

accessibility

Barrier affecting people with disabilities

backlog

Held for manual review: product/UX call, CI/workflow, hardware, or unverifiable here

bug

Something isn't working

confirmed

Reviewer demonstrated the defect

documentation

Improvements or additions to documentation

duplicate

This issue or pull request already exists

enhancement

New feature or request

good first issue

Good for newcomers

help wanted

Extra attention is needed

invalid

This doesn't seem right

plausible

Reviewer could not fully demonstrate it; treat as unproven

question

Further information is requested

sev:high

High severity

sev:low

Low severity

sev:medium

Medium severity

wontfix

This will not be worked on

No labels confirmed enhancement sev:low

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

JMR-dev

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#86