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

Rebuild the FFmpeg AAR with --enable-libvorbis, and implement Ogg Vorbis on it: the shipped binary has no libvorbis #254

New Issue
Closed
opened 2026-09-06 19:20:42 +00:00 by JMR-dev · 1 comment
JMR-dev commented 2026-09-06 19:20:42 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Found by the first-ever instrumented coverage measurement (2026-09-06). One of the 32 lines no
suite reaches:

app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilder.kt:188
    AudioCodec.VORBIS -> listOf("-c:a", "libvorbis", "-q:a", "5")

Every other arm of that when is covered — MP3, FLAC, PCM, OPUS and the else AAC default.

Why it is unreached, and why that is a question rather than an answer

No OutputFormat produces AudioCodec.VORBIS. The user-selectable list is MP4_H264/H265,
WEBM_VP9, MKV_H264/H265, REMUX_MP4/MKV, MP3, M4A_AAC, OPUS, FLAC, WAV, GIF and FRAMES_PNG; the
only Ogg entry is OPUS("Opus", OutputSpec(Container.OGG, VideoCodec.NONE, AudioCodec.OPUS)).

So on the direct path the arm is dead. But ContainerCapabilities disagrees, and that is
what makes this worth a ticket rather than a note:

Container.WEBM to setOf(AudioCodec.OPUS, AudioCodec.VORBIS)
Container.OGG  to setOf(AudioCodec.OPUS, AudioCodec.VORBIS, AudioCodec.FLAC)

repair picks a codec the target container can hold. If it can ever select VORBIS — for an
input whose audio is already Vorbis, say, where a copy is refused and a re-encode is planned —
then this arm is live, user-reachable and completely untested, and the command it builds has
never been run through FFmpeg once.

ConversionRouter:62 also lists VORBIS in MEDIA3_MUXABLE_AUDIO for MP4.

The work

  1. Determine whether any input can make ContainerCapabilities.repair choose VORBIS. Both sets
    list OPUS first, so a first-match implementation never reaches it — read the code, do not
    assume the ordering is what decides.
  2. If reachable: an FFmpegEngineTest case that encodes Vorbis and asserts the container
    magic, exactly as #228 did for FLAC (fLaC) and Opus (OggS). Asserting a non-empty file is
    what #228 was filed to fix; do not repeat it.
  3. If unreachable: delete the arm, or fold it into the else, and record which — this is
    then F4-shaped, like the COPY/NONE -> error(...) arms twenty lines above that are
    deliberately exempt.

Acceptance criterion

If a test is written: point AudioCodec.VORBIS at libopus and it must go red. If the arm is
removed instead: the answer to step 1 goes in docs/coverage-read-findings.md, because the next
read will otherwise re-derive it.

Found by the first-ever instrumented coverage measurement (2026-09-06). One of the **32 lines no suite reaches**: ``` app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilder.kt:188 AudioCodec.VORBIS -> listOf("-c:a", "libvorbis", "-q:a", "5") ``` Every other arm of that `when` is covered — MP3, FLAC, PCM, OPUS and the `else` AAC default. ## Why it is unreached, and why that is a question rather than an answer **No `OutputFormat` produces `AudioCodec.VORBIS.`** The user-selectable list is MP4_H264/H265, WEBM_VP9, MKV_H264/H265, REMUX_MP4/MKV, MP3, M4A_AAC, OPUS, FLAC, WAV, GIF and FRAMES_PNG; the only Ogg entry is `OPUS("Opus", OutputSpec(Container.OGG, VideoCodec.NONE, AudioCodec.OPUS))`. So on the direct path the arm is dead. **But `ContainerCapabilities` disagrees**, and that is what makes this worth a ticket rather than a note: ``` Container.WEBM to setOf(AudioCodec.OPUS, AudioCodec.VORBIS) Container.OGG to setOf(AudioCodec.OPUS, AudioCodec.VORBIS, AudioCodec.FLAC) ``` `repair` picks a codec the target container can hold. If it can ever select `VORBIS` — for an input whose audio is already Vorbis, say, where a copy is refused and a re-encode is planned — then this arm is **live, user-reachable and completely untested**, and the command it builds has never been run through FFmpeg once. `ConversionRouter:62` also lists `VORBIS` in `MEDIA3_MUXABLE_AUDIO` for MP4. ## The work 1. Determine whether any input can make `ContainerCapabilities.repair` choose `VORBIS`. Both sets list `OPUS` first, so a first-match implementation never reaches it — read the code, do not assume the ordering is what decides. 2. **If reachable:** an `FFmpegEngineTest` case that encodes Vorbis and asserts the container magic, exactly as #228 did for FLAC (`fLaC`) and Opus (`OggS`). Asserting a non-empty file is what #228 was filed to fix; do not repeat it. 3. **If unreachable:** delete the arm, or fold it into the `else`, and record which — this is then F4-shaped, like the `COPY`/`NONE -> error(...)` arms twenty lines above that are deliberately exempt. ## Acceptance criterion If a test is written: point `AudioCodec.VORBIS` at `libopus` and it must go red. If the arm is removed instead: the answer to step 1 goes in `docs/coverage-read-findings.md`, because the next read will otherwise re-derive it.
JMR-dev commented 2026-09-06 22:33:56 +00:00 (Migrated from github.com)
Copy Link
Copy Source

Rescoped 2026-09-06: this is not an uncovered arm, it is a latent defect

The premise of this ticket was wrong in the app's favour and against the user's. FFmpegCommandBuilder:188 did not merely never execute — it could not have succeeded. It emitted -c:a libvorbis, and that encoder is not in the binary this app ships. The first user to pick Ogg Vorbis would have got Unknown encoder 'libvorbis'.

Measured, three independent ways

bin/README.md configure line:
  --enable-libdav1d --enable-libjxl --enable-libmp3lame --enable-libopus
  --enable-libsvtav1 --enable-libvpx --enable-libx264 --enable-libx265
                                                        (no --enable-libvorbis)

strings jni/x86_64/libavcodec.so, external encoders named:
  libdav1d libjxl libmp3lame libopus libsvtav1 libvpx libx264 libx265

  grep -cx 'libvorbis'   -> 0
  grep -c  'vorbisenc.c' -> 1     (FFmpeg's own native encoder IS present)

tools/ffmpeg/build-ffmpeg.sh agrees: neither COMMON_LIBS nor EXTRA_LIBS names it. The shipped AAR is ffmpeg-kit-next-8.1.1.aar.

This is #238's shape again — two individually-correct facts, a builder arm and a configure line, that no test ever put together. A coverage number cannot find it; only building the command and running it can.

Why the native vorbis encoder is not an acceptable substitute

FFmpeg does ship an in-tree encoder, and a first pass at this ticket used it. It works, and it is not good enough to put in a picker beside MP3, FLAC and Opus:

libvorbis native vorbis
experimental gate none requires -strict experimental
channels mono, stereo, surround stereo only
quality knob, measured on one 3 s clip, q0..q10 10,931 -> 64,166 bytes 7,549 -> 14,645 bytes

The AV_CODEC_CAP_EXPERIMENTAL gate is the codebase saying do not ship this by accident. The stereo limit forces -ac 2, so a mono source is silently upmixed and a surround one downmixed — a user-visible compromise this app makes nowhere else. And the quality slider barely moves: roughly 2x its floor against libvorbis's 6x, so the user cannot ask it for a better file.

Corrected. An earlier version of this comment said those flag behaviours were measured against 8.1.2 "while the AAR is 8.1.1". That was wrong, and the mistake was reading the FFmpeg version off the AAR filename — which carries the ffmpeg-kit wrapper version, not FFmpeg's. bin/README.md distinguishes the two rows deliberately: Upstream … v8.1.1 and FFmpeg | 8.1.2, and the shipped libavutil.so carries FFmpeg version n8.1.2. The versions matched all along.

The real caveat is narrower and still worth stating: the measurements were taken on the host's build of 8.1.2 — same FFmpeg version, different build, different configure flags — and never by running the shipped binary. What corroborates them inside it is that both refusal strings are present in jni/x86_64/libavcodec.so:

The %s '%s' is experimental but experimental codecs are not enabled, add '-strict %d' ...
Current FFmpeg Vorbis encoder only supports 2 channels.

Stronger than "a near neighbour"; still short of a device run. None of it touches the libvorbis-absent finding, which came from the .so itself.

New scope

  1. Rebuild the FFmpeg AAR with --enable-libvorbis, per bin/README.md. Note what that costs: a cross-compile, a new ~35 MB blob permanently in git history, a new checksum, and bin/README.md's provenance and configure line updated in the same commit.
  2. Implement Ogg Vorbis properly on top of it — -c:a libvorbis -q:a 5, no -strict experimental, no forced -ac 2, and mono preserved.
  3. Keep the rest of the first pass, which stands on its own merits and is unaffected by which encoder is used:
    • a user-selectable OutputFormat entry, with VORBIS added to ENCODABLE_AUDIO
    • Container.OGG's 4th positional field is audioExtension, not a codec hint — it was hardcoded "opus", so every Ogg output was named .opus and a Vorbis file would have shipped as foo.opus. RFC 7845 s9 asks for .opus only on an Ogg carrying Opus alone. Fix is a per-codec extension map.
    • the e2e test asserting the track MIME, not just OggS magic — Vorbis and Opus are both OggS, so a magic-only assertion is vacuous against the mutation below.

Acceptance

Point AudioCodec.VORBIS at libopus and the e2e test must go red. Verified by MIME (MIMETYPE_AUDIO_VORBIS vs the mutant's audio/opus), because the two are indistinguishable by container magic.

And the encode must actually run on a device against the rebuilt AAR — that run is the whole point of this ticket, since the defect it found was invisible to everything short of executing the command.

Queue

Behind the work in flight (#252 and the shellcheck/actionlint gate work). The AAR rebuild is serialised deliberately: it is a long cross-compile and it changes a committed binary every other branch links against.

Two warnings for the rebuild, from the first pass

Do not guess the configure flag. tools/ffmpeg/build-ffmpeg.sh warns that ffmpeg-kit's library names come from its own get_library_name() and are not FFmpeg's: it passes --enable-lame for libmp3lame and --enable-opus for libopus. So the flag here may well be --enable-vorbis, not --enable-libvorbis. Read that function before starting a 40-minute cross-compile. libvorbis pulls libogg as a dependency; licensing is unaffected (BSD, and this build is already GPL-3.0).

A wrong flag name fails silently. ffmpeg-kit does not error on an unrecognised --enable-*, so a rebuild that quietly omitted libvorbis looks exactly like one that worked. Re-run the strings check and the e2e test before updating bin/README.md's configure line and SHA-256 — not after. Those two are the only things that would catch it, which is the same lesson this ticket already carries.

## Rescoped 2026-09-06: this is not an uncovered arm, it is a latent defect The premise of this ticket was wrong in the app's favour and against the user's. `FFmpegCommandBuilder:188` did not merely *never execute* — **it could not have succeeded.** It emitted `-c:a libvorbis`, and that encoder is not in the binary this app ships. The first user to pick Ogg Vorbis would have got `Unknown encoder 'libvorbis'`. ### Measured, three independent ways ``` bin/README.md configure line: --enable-libdav1d --enable-libjxl --enable-libmp3lame --enable-libopus --enable-libsvtav1 --enable-libvpx --enable-libx264 --enable-libx265 (no --enable-libvorbis) strings jni/x86_64/libavcodec.so, external encoders named: libdav1d libjxl libmp3lame libopus libsvtav1 libvpx libx264 libx265 grep -cx 'libvorbis' -> 0 grep -c 'vorbisenc.c' -> 1 (FFmpeg's own native encoder IS present) ``` `tools/ffmpeg/build-ffmpeg.sh` agrees: neither `COMMON_LIBS` nor `EXTRA_LIBS` names it. The shipped AAR is `ffmpeg-kit-next-8.1.1.aar`. **This is #238's shape again** — two individually-correct facts, a builder arm and a configure line, that no test ever put together. A coverage number cannot find it; only building the command and running it can. ### Why the native `vorbis` encoder is not an acceptable substitute FFmpeg does ship an in-tree encoder, and a first pass at this ticket used it. It works, and it is not good enough to put in a picker beside MP3, FLAC and Opus: | | `libvorbis` | native `vorbis` | |---|---|---| | experimental gate | none | **requires `-strict experimental`** | | channels | mono, stereo, surround | **stereo only** | | quality knob, measured on one 3 s clip, q0..q10 | 10,931 -> 64,166 bytes | 7,549 -> 14,645 bytes | The `AV_CODEC_CAP_EXPERIMENTAL` gate is the codebase saying *do not ship this by accident*. The stereo limit forces `-ac 2`, so a mono source is silently upmixed and a surround one downmixed — a user-visible compromise this app makes nowhere else. And the quality slider barely moves: roughly 2x its floor against libvorbis's 6x, so the user cannot ask it for a better file. > **Corrected.** An earlier version of this comment said those flag behaviours were measured against 8.1.2 *"while the AAR is 8.1.1"*. That was wrong, and the mistake was reading the FFmpeg version off the AAR **filename** — which carries the *ffmpeg-kit wrapper* version, not FFmpeg's. `bin/README.md` distinguishes the two rows deliberately: `Upstream … v8.1.1` and `FFmpeg | 8.1.2`, and the shipped `libavutil.so` carries `FFmpeg version n8.1.2`. The versions matched all along. > > The real caveat is narrower and still worth stating: the measurements were taken on the **host's build** of 8.1.2 — same FFmpeg version, different build, different configure flags — and never by running the shipped binary. What corroborates them inside it is that both refusal strings are present in `jni/x86_64/libavcodec.so`: > > ``` > The %s '%s' is experimental but experimental codecs are not enabled, add '-strict %d' ... > Current FFmpeg Vorbis encoder only supports 2 channels. > ``` > > Stronger than "a near neighbour"; still short of a device run. None of it touches the `libvorbis`-absent finding, which came from the `.so` itself. ### New scope 1. **Rebuild the FFmpeg AAR with `--enable-libvorbis`**, per `bin/README.md`. Note what that costs: a cross-compile, a new ~35 MB blob permanently in git history, a new checksum, and `bin/README.md`'s provenance and configure line updated in the same commit. 2. **Implement Ogg Vorbis properly on top of it** — `-c:a libvorbis -q:a 5`, no `-strict experimental`, no forced `-ac 2`, and mono preserved. 3. Keep the rest of the first pass, which stands on its own merits and is unaffected by which encoder is used: - a user-selectable `OutputFormat` entry, with `VORBIS` added to `ENCODABLE_AUDIO` - **`Container.OGG`'s 4th positional field is `audioExtension`, not a codec hint** — it was hardcoded `"opus"`, so *every* Ogg output was named `.opus` and a Vorbis file would have shipped as `foo.opus`. RFC 7845 s9 asks for `.opus` only on an Ogg carrying Opus alone. Fix is a per-codec extension map. - the e2e test asserting the **track MIME**, not just `OggS` magic — Vorbis and Opus are both `OggS`, so a magic-only assertion is vacuous against the mutation below. ### Acceptance Point `AudioCodec.VORBIS` at `libopus` and the e2e test must go red. Verified by MIME (`MIMETYPE_AUDIO_VORBIS` vs the mutant's `audio/opus`), because the two are indistinguishable by container magic. And the encode must actually run on a device against the rebuilt AAR — that run is the whole point of this ticket, since the defect it found was invisible to everything short of executing the command. ### Queue Behind the work in flight (#252 and the shellcheck/actionlint gate work). The AAR rebuild is serialised deliberately: it is a long cross-compile and it changes a committed binary every other branch links against. ### Two warnings for the rebuild, from the first pass **Do not guess the configure flag.** `tools/ffmpeg/build-ffmpeg.sh` warns that ffmpeg-kit's library names come from its own `get_library_name()` and are *not* FFmpeg's: it passes `--enable-lame` for libmp3lame and `--enable-opus` for libopus. So the flag here may well be `--enable-vorbis`, not `--enable-libvorbis`. Read that function before starting a 40-minute cross-compile. libvorbis pulls libogg as a dependency; licensing is unaffected (BSD, and this build is already GPL-3.0). **A wrong flag name fails silently.** ffmpeg-kit does not error on an unrecognised `--enable-*`, so a rebuild that quietly omitted libvorbis looks exactly like one that worked. Re-run the `strings` check *and* the e2e test **before** updating `bin/README.md`'s configure line and SHA-256 — not after. Those two are the only things that would catch it, which is the same lesson this ticket already carries.
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 bug
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#254