Commit Graph
12 Commits
Author SHA1 Message Date
Jason Ross 73482520aa Merge branch 'main' into feat/ogg-vorbis-libvorbis 2026-09-07 12:16:43 -05:00
JMR-devandClaude Opus 5 a4ca93b00e Say what the artefact check measured, not what it implies
Two claims in E9 went further than the evidence, in an entry whose whole subject
is a figure that was quoted past its own.

"All 16 sat on that one line" was consistent with what was measured, not
established by it: `MediaProbe:321` accounts for a 16-branch difference and the
denominators now agree at 1338, but the other lines were never enumerated in both
reports, so a line that lost coverage elsewhere would have been invisible to the
check that was run. Says that now.

And `matroskaOrWebm` is not "the only string-literal `when` in that file" --
`shortName` at :429 is a second one. The parenthetical was never checked; it is
gone rather than repaired, since the mechanism was not what the measurement
established anyway.

The previous commit message carries the stronger wording. It is left alone rather
than rewritten, because that would need a force push.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-07 12:03:08 -05:00
JMR-devandClaude Opus 5 aaa1b64f87 Read the union's branch tier, and decide 12 of its 22 sites (E9)
E8 classified the 32 lines neither suite executes and stopped there. It never
asked which *arms* neither suite takes on lines both suites run, and that tier
is where what is left actually lives.

Rebuilt the union rather than reusing E8's artifact: a fresh jacocoTestReport
merged with E8's own API 34 .ec against one set of current class files. Union is
99.0% line, 90.1% branch. Controls confirm the device half applied -- 32 lines in
FFmpegEngine, 24 in Media3Engine, 15 in ConcatEngine, 9 in MainActivity that the
JVM suite never reaches.

Three things came out of it that are worth more than the count:

- **E8's denominator artefact is retired.** It warned the union's branch
  denominator ran 16 ahead "entirely inside MediaProbe" and told readers not to
  quote a MediaProbe branch figure raw. Rebuilt, both denominators are 1338 and
  MediaProbe:321 reads mb=0 cb=4. All 16 sat on that one line, so it was a
  property of how the report was built and never of the code.

- **CLAUDE.md's "18" is not stale.** It reproduces exactly under `mi == 0 && mb > 0`
  (19 branches on 18 lines) and not under `ci > 0`, which admits partially-executed
  signature lines and gives 139. Different metrics, not drift -- stated in E9 so
  the next read does not "correct" a figure that is right.

- **Tier 1 is 22, down from 32.** #252 closed the ten getForegroundInfo lines and
  added none. Two classes carry a one-line blind spot because #252 changed their
  bytecode and JaCoCo rightly rejected E8's .ec for them; the bound is measured,
  not assumed, and the line is not a gap.

Of the 22 branch sites, 12 are decided here -- compiler codegen, an F4/F6/F10
exemption already on record, or a thread race -- and recorded in E9 rather than as
new F-entries, since coverage-read-findings.md is being rewritten by #261. The
other 10 are filed as #262-#266, each naming the mutation that must go red or
saying that the read is the ticket.

FFmpegCommandBuilder:183's missed arm is VORBIS, which #261 is closing as it
lands, so the set is 21 the day it merges. E9 says to re-derive rather than edit
that sentence.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-07 11:56:44 -05:00
JMR-dev e6ac84cd24 Merge remote-tracking branch 'origin/main' into feat/ogg-vorbis-libvorbis 2026-09-07 11:47:17 -05:00
JMR-devandClaude Opus 5 d45abe7409 Rebuild the FFmpeg AAR with libvorbis, and make Ogg Vorbis reachable (#254)
`FFmpegCommandBuilder` has emitted `-c:a libvorbis` since the day it was
written, and libvorbis was not in the AAR this app ships: the configure line
omitted `--enable-libvorbis`, and `strings` on both ABIs' `libavcodec.so`
named every other external encoder and not that one. The arm was unreachable
from both ends, so nobody ever hit it -- but the first user to pick Ogg
Vorbis would have got `Unknown encoder 'libvorbis'`. That is #238's shape
again: two individually-correct facts, a builder arm and a configure line,
that no test put together, and that no coverage number can see.

So the binary is rebuilt rather than the arm rewritten. FFmpeg's in-tree
`vorbis` encoder was already in there and was tried first; it is
experimental, stereo-only, and its quality knob spans 2x its floor against
libvorbis's 6x. Shipping it would have meant `-strict experimental`, a
forced `-ac 2` that silently upmixes every mono source, and a slider with
nowhere to go. What ships instead is the arm as originally written,
`-c:a libvorbis -q:a 5`, with `OGG_VORBIS` added to the presets, `VORBIS`
added to `ENCODABLE_AUDIO`, and Ogg's per-codec extension fixed so a Vorbis
file is not named `.opus`.

The flag is `--enable-libvorbis`, read out of ffmpeg-kit's
`get_library_name()` rather than guessed: the `--enable-lame` /
`--enable-opus` rule predicts `--enable-vorbis`, and that is not it. An
unrecognised `--enable-*` is ignored silently, so the artifact was checked
before `bin/README.md` was touched -- `libvorbis` present in both ABIs, the
configure line otherwise identical, FFmpeg still n8.1.2, 10 shared libraries
per ABI, every LOAD still `0x4000`.

Both mutations were run on API 34 rather than predicted. Pointing the arm at
`libopus` reddens the e2e test with `expected:<[audio/vorbis]> but
was:<[audio/opus]>` while its `OggS` assertion still passes, which is why
the track MIME is asserted and the container magic is not enough. Adding
`-ac 2` back reddens it with `expected:<[1]> but was:<[2]>`: this class's
own fixture is mono, so mono staying mono is an assertion rather than a
claim.

The unit test's load-bearing assertion inverts with this change and is
rewritten to say so. It used to assert that `libvorbis` was *absent*; it now
asserts the encoder name plus the two flags that must not be there. Nothing
on the JVM can tell a real encoder name from a fictional one -- which is
exactly how this survived four coverage waves -- so the e2e test is the only
thing that proves the positive.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 18:31:36 -05:00
JMR-devandClaude Opus 5 6a8cc01862 Expedite user-initiated work, and give both getForegroundInfo overrides a caller (#252)
`ConversionWorker.request` and `ConcatWorker.request` now carry
`setExpedited(RUN_AS_NON_EXPEDITED_WORK_REQUEST)`. Conversions and joins are
started by a tap; the jobs that have to go back through JobScheduler because no
process is left to start them should not queue behind a background chore. The
class KDoc that said expedited was "deliberately not used" is replaced with what
was actually read out of work-runtime 2.11.2: retries are never expedited
(`SystemJobInfoConverter:135`), and a job the system stops mid-run is resolved as
`ResetWorkerStatus` and re-enqueued rather than answered by `FailureOutcome`.

#252's own premise does not survive measurement, and that is the second half of
this change. `getForegroundInfo()` is WorkManager's expedited-work hook, but
`WorkForeground.kt:38` opens the library's only caller with
`if (!spec.expedited || Build.VERSION.SDK_INT >= 31) return`, and minSdk is 33 --
so `setExpedited` alone leaves both overrides exactly as cold as the first
instrumented coverage read found them. Measured on API 34 rather than argued:
with the flag set and `doWork` still building its own notification, both methods
report `missed 1 / covered 0` and all ten lines `ci=0`, and the whole
instrumented suite is green anyway at 71/71.

What makes them live is that each worker held two definitions of one
notification. `doWork` now posts the override's instead of an identical copy, so
`ConcatWorker`'s countless "Joining files" -- which nothing executed and which
was therefore free to drift from the "Joining N files" that ran -- is gone.
After: both `getForegroundInfo` report `LINE 0 missed / 5 covered`.

Five mutations were run and all five went red: dropping `setExpedited` from
either request, hard-coding the conversion title, moving its `percent` off zero,
and dropping the join's input count.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 17:06:00 -05:00
JMR-devandClaude Opus 5 4de169c99b Record the first instrumented coverage measurement, and classify the 32 it found
E8. The instrumented suite had never been measured: enableAndroidTestCoverage was
unset, so a connected run emitted no .ec at all, and jacocoTestReport reads only
testDebugUnitTest. Measured on API 34 by setting the flag temporarily.

  JVM     2236/2374 line 94.2%   1171/1338 branch 87.5%
  E2E     1711/2374 line 72.1%    669/1354 branch 49.4%
  UNION   2342/2374 line 98.7%   1212/1354 branch 89.5%

The JVM row reproduced the committed figure exactly, which is the control that
says both exec sets match the current class files.

The device suite closes 106 lines the JVM suite misses, and the first four are
the 81 wave 4 wrote off as native or device edges -- FFmpegEngine 32,
Media3Engine 24, ConcatEngine 15, MainActivity 10. The union leaves one. That
confirms the read's own hypothesis rather than overturning it; nobody had
measured past the boundary it named.

All 32 lines reached by neither suite were read, and none is an e2e test gap:
nine are compiler-generated, ten are getForegroundInfo() for expedited work this
app never enqueues (#252), three are F5, three are uncalled members (#253), one
is the Vorbis encode arm (#254), and four are F4-shaped error guards.

MediaProbe:210-212 gained a measurement rather than an assumption. F7 ruled
probeWithExtractor's catch unreachable because Robolectric's MediaExtractor never
throws; probeWithFFprobe calls native ffmpeg-kit, so that reasoning does not
transfer. But probe() calls both, and RemuxTest drives it with garbage bytes on a
device -- so the ffprobe path has had malformed input on real hardware and did
not throw. Same conclusion as F7, different mechanism, now on record.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 14:22:24 -05:00
JMR-devandClaude Opus 5 e81403c5f3 Measure the fourth claim rather than asserting it, and fix three slips
Review of the previous commit found three things of exactly the kind it corrects.

"Both of the advisory runs that exist" asserted exhaustiveness that had not been
checked -- two jobs were read, and the #248 branch had three status_check runs.
All four advisory runs at baseline 6 are now read: 34041156680, 34041593697,
34042397320 and 34043502322 each report expected: 6, received: 4, failed: 4,
and the only SAF test reporting in any of them is the rotation one. The claim
was right; the wording claimed more than the evidence.

CONVERSION_TIMEOUT_MS's KDoc said the bound is "two orders of magnitude" clear
of the real cost. 120 s against a measured 11.8 s is one.

status_check.yml dated the save test's marker to 2026-09-05. fa10d94 is dated
2026-09-06.

Also reworded that comment's summary: "two abort the framework and two never
report" double-counts the picker test, so a reader summing gets seven.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 11:20:05 -05:00
JMR-devandClaude Opus 5 54167c052f Re-derive the API 37 carrier counts, and correct what #226 left behind
The 2026-09-06 re-check of the instrumented suite. Every drifted line it found
came from #226, the last PR of the e2e read's own wave.

The suite is 70 tests in 14 classes, 6 carrying @FailsOnEmulatorApi37, gating
leg 64. The committed baseline says 6 and the advisory job agrees. Four places
still said five carriers of 69:

  - CLAUDE.md, three sites
  - FailsOnEmulatorApi37.kt's KDoc
  - two comments in status_check.yml

The gating figure is what hid it. 69 - 5 and 70 - 6 are both 64, so the one
number a reader checks against a run had not moved -- which is exactly why
CLAUDE.md says to derive these rather than remember them.

Two KDoc claims in SafPickerRoundTripTest described a draft rather than the
code. The save test says MP3 was chosen so the setup could not depend on device
codecs; the code converts at the default MP4_H265/FAST, which routes on
canEncode(H265). The negation of the stated reason was true. That is E1 and E3's
failure mode committed by the wave that found it, so it is written down as such
rather than quietly corrected.

Neither picker test has ever reported on the advisory leg. The marker's KDoc
said the picker test fails there behind the rotation test; with six carriers the
rotation test truncates the run first, and both advisory runs since #226 --
34042397320 and 34043502322 -- report expected: 6, received: 4, the four being
the three Media3 tests plus the rotation. The save test is therefore marked by
inheritance, not measurement, and both KDocs now say so.

FixtureDocumentsProvider.deletedDocumentIds() has no callers: #226 proved D4's
premise and drove only the success path, so deletePartialOutput against a real
DocumentsProvider is still asserted nowhere. Filed as #250 with the forcing
condition and the mutation; the accessor is kept with a KDoc naming that ticket
rather than removed and re-added.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 11:16:15 -05:00
JMR-devandClaude Opus 5 3b0c262030 Record E7's second constraint, and that the premise held (#226)
Doing #226 turned up a second obstacle underneath E7's, with the same
cause. The obvious way to avoid driving the app was a host Activity in
androidTest owning its own CreateDocument launcher; it cannot be started at
all, because instrumentation runs in the target app's process and the
component is in the instrumentation one. That is the same fact as E7's
second bullet arriving from the other side, and it leaves the app's own
Save button as the only launcher available to drive.

And the answer #226 was filed for: on API 34, stock DocumentsUI hands back
a document URI reporting a size of exactly zero, so destinationIsKnownEmpty
can return true and D4's fix is live rather than inert.

A "no defect found", and not one that could have been reached by reading --
which is the argument for having done it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 10:49:09 -05:00
JMR-devandClaude Opus 5 c5b2dc0f55 Record what working the e2e tickets found (E7, and #238)
Two results from #223-#230 that belong with the read rather than only in
their own tickets.

E7 re-scoped its own ticket. #226 split into a cheap headless half and an
expensive picker-driven one, on the premise that a real DocumentsProvider
can be reached without DocumentsUI. It cannot: an unprotected one is
refused at install, instrumentation runs in the app's uid so the test APK's
own identity is no help, and adopting shell identity is denied too -- each
denial naming ACTION_OPEN_DOCUMENT as the only way in. Measured three ways.
So #226 is one item at the picker's cost, not two.

The useful half of that distinction is that the input bridge needs no
documents provider at all. getSafParameterForRead opens a descriptor
through the resolver, so any readable content:// URI exercises it, which is
what kept #225 headless.

And that is how the read's one production defect surfaced. #238: joining
files picked through the system picker failed outright on the stream-copy
path, because the concat demuxer whitelists protocols separately from
-safe 0 and ffkitsaf was not on the list. Only STREAM_COPY feeds the
demuxer a list file, and every existing join test passed Uri.fromFile, so
the one broken combination was the only one a user could reach.

Worth stating plainly next to the coverage entry: it was not a missed line
and not an unasserted value, but two covered things no test put together --
the gap shape a coverage number is worst at, and the reason the read
happened.

E4 is marked fixed; #243 made that KDoc name the constant rather than
restate it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-06 02:50:37 -05:00
JMR-devandClaude Opus 5 c757565d64 Read the instrumented suite, and find the test that proves nothing
Four coverage waves have been steered by JaCoCo, which measures
testDebugUnitTest only and cannot see app/src/androidTest at all. So
nothing had ever asked what the 60 device tests pin, only that they were
green. This is that read: a triage, not a test push, in the shape of
docs/coverage-read-findings.md.

Six findings (E1-E6) are things a test would not fix, and five of those
six are prose rather than code — the suite itself is in good condition.
Eight tickets carry the rest (#223-#230), each naming the mutation that
has to go red rather than a coverage delta.

The one that matters is #223. HardwareFallbackTest is the only automated
check of the hardware->software fallback against a real codec failure,
and it has never attempted the hardware path. Measured on run
34004304566: the API 33, 34, 35 and 37 legs each log

  Routing sample_h264_444.mp4 -> ... via FFMPEG (NO_HARDWARE_ENCODER)

because emulators expose no hardware encoder, so the router sends the job
straight to FFmpeg and runMedia3OrFallBack's catch is never entered. Its
two assertions — succeeded, output non-empty — are true anyway. It
finishes in 448 ms, which is not long enough to fail a hardware export
and then software-encode a three-second clip. Deleting that catch reddens
nothing anywhere.

Two things generalise. A test can assert and still not reach, which
neither a coverage number nor a "does it assert something" review can
see; the filter that works is whether the test's premise holds on the
machine running it. And the codebase already knew — ForcedFailureTest
pins DeviceCodecs.PERMISSIVE against this exact hazard and says why, as
does ConversionWorkerTest. Their assertions are about the path, so
without the pin they fail loudly; HardwareFallbackTest's are about the
output, so it passes quietly. That asymmetry is why nobody noticed.

E5 records the structural reason this document is separate: F7 in the
coverage findings calls probeWithExtractor's catch uncovered when
RemuxTest drives it on a device every leg. A JaCoCo-derived document
cannot see androidTest, so it will keep re-deriving that.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
2026-09-05 21:54:56 -05:00