JMR-dev/LibreMediaConverter
1
0
Fork 0
Code Issues 16 Pull Requests Actions Packages Projects Releases Wiki Activity

S1 — MediaProbe's track-walking loop is reachable on the JVM after all, which revises #84's boundary #141

New Issue
Closed
opened 2026-08-27 03:07:04 +00:00 by JMR-dev · 1 comment
JMR-dev commented 2026-08-27 03:07:04 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Child 1 of 3 decomposing #133. Independent of S2 (#142) and S3 (#143). This one revises a closed ticket's boundary, so read that part before writing any code.

Why this exists

app/src/main/java/org/libremediaconverter/convert/MediaProbe.kt:114-132   probeWithExtractor
app/src/main/java/org/libremediaconverter/convert/MediaProbe.kt:279-293   probeForConcat

#84 closed by classifying these as device-bound and explicitly not a gap:

probeWithExtractor (13/25) drives MediaExtractor … These are exercised by RemuxTest,
ConcatEngineTest and RealMediaBenchmark in androidTest — which JaCoCo does not measure at
all. Do not read their 0% as untested, and do not try to fix it by mocking FFprobe.

That is right about FFprobe, right about the measurement boundary, and wrong that these are only
orchestration. Both halves of that need answering before this is worth doing.

Reachability — verified, not assumed

Robolectric 4.16.1 ships ShadowMediaExtractor, already on the test classpath. Read out of
shadows-framework-4.16.1.jar:

protected void setDataSource(android.content.Context, android.net.Uri, java.util.Map<String,String>)
public static void addTrack(org.robolectric.shadows.util.DataSource, android.media.MediaFormat, byte[])
protected int getTrackCount()
protected android.media.MediaFormat getTrackFormat(int)

MediaProbe.kt:106 and :271 both call setDataSource(context, uri, null) — the shadowed overload
exactly. The entry point is reachable on the JVM today.

Why it is worth reaching — the branch matrix no fixture chooses

what where the case that provokes it
first-track-wins, per type :120, :126, :281, :286 a file with two video tracks — video == null is the whole guard
maxOf duration across tracks :117 a file whose audio track is longer than its video track
the containsKey(KEY_DURATION) guard :116 a track that omits duration — MediaProbeTrackFieldsTest's KDoc notes this is common
track ordering :119-127 audio declared before video, which must change nothing

RemuxTest reaches this code only through whatever the committed fixtures happen to contain, so
none of those four is chosen by any test — they are incidental to files picked for other reasons.
ShadowMediaExtractor.addTrack constructs each directly.

The decision this child has to make first

Two ways to do it, and they are not equivalent:

  1. Test through the shadow. Cheapest. Keeps the loop where it is.
  2. Cut the track-walking loop into a pure function over a List<MediaFormat>, and test that. The
    work/FailureOutcome.kt pure-seam pattern, which CLAUDE.md names as the thing to reach for
    before concluding something cannot be unit tested. It leaves androidTest covering the genuinely
    thin edge — setDataSource and release — which is what device coverage is for.

(2) is more in keeping with the repo, but (1) is a legitimate answer if the extraction turns out
to drag MediaExtractor state through the seam. Decide explicitly and write down which and why.

Done means

The four cases above, each named, plus a correction to #84's boundary paragraph — a comment on
that ticket, or a line in MediaProbe's KDoc — so the next coverage read does not re-derive all of
this. Whichever way the decision goes, say so there too.

Mutation: change video == null to true at :120. The two-video-track test must go red.

_Child 1 of 3 decomposing #133. Independent of S2 (#142) and S3 (#143). **This one revises a closed ticket's boundary, so read that part before writing any code.**_ ### Why this exists ``` app/src/main/java/org/libremediaconverter/convert/MediaProbe.kt:114-132 probeWithExtractor app/src/main/java/org/libremediaconverter/convert/MediaProbe.kt:279-293 probeForConcat ``` **#84 closed by classifying these as device-bound and explicitly not a gap:** > `probeWithExtractor` (13/25) drives `MediaExtractor` … These are exercised by `RemuxTest`, > `ConcatEngineTest` and `RealMediaBenchmark` in `androidTest` — which JaCoCo does not measure at > all. **Do not read their 0% as untested**, and do not try to fix it by mocking FFprobe. That is right about FFprobe, right about the measurement boundary, and **wrong that these are only orchestration**. Both halves of that need answering before this is worth doing. ### Reachability — verified, not assumed Robolectric 4.16.1 ships `ShadowMediaExtractor`, already on the test classpath. Read out of `shadows-framework-4.16.1.jar`: ``` protected void setDataSource(android.content.Context, android.net.Uri, java.util.Map<String,String>) public static void addTrack(org.robolectric.shadows.util.DataSource, android.media.MediaFormat, byte[]) protected int getTrackCount() protected android.media.MediaFormat getTrackFormat(int) ``` `MediaProbe.kt:106` and `:271` both call `setDataSource(context, uri, null)` — the shadowed overload exactly. The entry point is reachable on the JVM today. ### Why it is worth reaching — the branch matrix no fixture chooses | what | where | the case that provokes it | |---|---|---| | first-track-wins, per type | `:120`, `:126`, `:281`, `:286` | a file with **two** video tracks — `video == null` is the whole guard | | `maxOf` duration across tracks | `:117` | a file whose audio track is longer than its video track | | the `containsKey(KEY_DURATION)` guard | `:116` | a track that omits duration — `MediaProbeTrackFieldsTest`'s KDoc notes this is common | | track ordering | `:119-127` | audio declared **before** video, which must change nothing | `RemuxTest` reaches this code only through whatever the committed fixtures happen to contain, so **none of those four is chosen by any test** — they are incidental to files picked for other reasons. `ShadowMediaExtractor.addTrack` constructs each directly. ### The decision this child has to make first Two ways to do it, and they are not equivalent: 1. **Test through the shadow.** Cheapest. Keeps the loop where it is. 2. **Cut the track-walking loop into a pure function** over a `List<MediaFormat>`, and test that. The `work/FailureOutcome.kt` pure-seam pattern, which `CLAUDE.md` names as the thing to reach for before concluding something cannot be unit tested. It leaves `androidTest` covering the genuinely thin edge — `setDataSource` and `release` — which is what device coverage is *for*. **(2) is more in keeping with the repo**, but (1) is a legitimate answer if the extraction turns out to drag `MediaExtractor` state through the seam. Decide explicitly and write down which and why. ### Done means The four cases above, each named, plus **a correction to #84's boundary paragraph** — a comment on that ticket, or a line in `MediaProbe`'s KDoc — so the next coverage read does not re-derive all of this. Whichever way the decision goes, say so there too. **Mutation:** change `video == null` to `true` at `:120`. The two-video-track test must go red.
JMR-dev commented 2026-09-02 01:40:39 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Already done, and verified on main today — closing.

This stayed open through a bookkeeping failure, not an unfinished one. PR #150 carried Closes #141, but GitHub only fires a closing keyword when the PR merges into the default branch. #150 merged into its stack base instead (the async-retarget race written up on #160 and now in CLAUDE.md), so the keyword never ran. The content reached main later via #160, which did not carry the keywords.

Verified against main at d354f64 just now, by re-running this ticket's own named mutation rather than by checking the files exist:

The ticket named video == null -> true at :120. That line moved when the seam was cut, so the equivalent guard was mutated in extractedFrom:

mime.startsWith("video/") && video == null ->   (guard dropped, last track wins)
  red: the first video track wins when a file carries two
mime.startsWith("audio/") && audio == null ->   (same, audio side)
  red: the first audio track wins when a file carries two

The ticket also required a correction to #84%s boundary paragraph. Posted there on 2026-08-27 ("Revisiting the boundary this ticket drew — with new evidence, not a re-reading of the old"), and MediaProbeTrackWalkTest%s KDoc carries it too.

Gate green on main: 546 tests in 76 classes, 0 failures.

**Already done, and verified on `main` today — closing.** This stayed open through a bookkeeping failure, not an unfinished one. PR #150 carried `Closes #141`, but **GitHub only fires a closing keyword when the PR merges into the default branch.** #150 merged into its stack base instead (the async-retarget race written up on #160 and now in `CLAUDE.md`), so the keyword never ran. The content reached `main` later via #160, which did not carry the keywords. Verified against `main` at `d354f64` just now, by re-running **this ticket's own named mutation** rather than by checking the files exist: The ticket named `video == null -> true` at `:120`. That line moved when the seam was cut, so the equivalent guard was mutated in `extractedFrom`: ``` mime.startsWith("video/") && video == null -> (guard dropped, last track wins) red: the first video track wins when a file carries two mime.startsWith("audio/") && audio == null -> (same, audio side) red: the first audio track wins when a file carries two ``` The ticket also required a correction to #84%s boundary paragraph. Posted there on 2026-08-27 ("Revisiting the boundary this ticket drew — with new evidence, not a re-reading of the old"), and `MediaProbeTrackWalkTest`%s KDoc carries it too. Gate green on `main`: 546 tests in 76 classes, 0 failures.
Sign in to join this conversation.
No Branch/Tag Specified
Branches Tags
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 enhancement sev:low
Milestone
No items
No Milestone
Projects
Clear projects
No projects
Assignees
Clear assignees
JMR-dev
No Assignees
1 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: JMR-dev/LibreMediaConverter#141