diff --git a/CLAUDE.md b/CLAUDE.md index 5709851..d577cf8 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -76,7 +76,7 @@ days. Read it as the current answer, and see the git history if you need the old `angle_indirect` and `swangle_indirect` all boot, while `auto`, `off`, `guest` and `swiftshader_indirect` do not. `docs/local-emulator.md` has the evidence and the per-API renderer table. -- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. **Seven** of the 71 instrumented +- **CI runs API 37, and it gates.** The matrix is 33/34/35/36/37. **Seven** of the 72 instrumented tests cannot be *run* on that image, for three measured reasons and two inherited: three Media3 tests fail inside the emulator's own `c2.goldfish.h264.decoder`, one SAF test takes the framework down when it rotates the display, and its sibling — the SAF picker round trip — aborts @@ -88,9 +88,11 @@ days. Read it as the current answer, and see the git history if you need the old Media3 tests plus the rotation — runs 34041156680, 34041593697, 34042397320, 34043502322 and 34045105857. **No picker test has ever reported on the advisory leg**, which is a correction to what the marker's own KDoc used to say. All seven carry `@FailsOnEmulatorApi37` and run in a - separate `continue-on-error` job; the gating leg runs the other 64 — **the same 64 for the third - time running**, which is exactly how this paragraph goes stale unnoticed: 69−5, 70−6 and 71−7 - are all 64. + separate `continue-on-error` job; the gating leg runs the other **65**. That figure had been 64 + three times running — 69−5, 70−6 and 71−7 are all 64 — which is exactly how this paragraph went + stale unnoticed, because the one number a reader checks against a run had not moved while the + suite grew twice underneath it. #254 is the first change since to move it, by adding a test and + no marker. **These two numbers move with the suite and are derived, not remembered.** `grep -cE '^\s*@Test' ` over `app/src/androidTest` is the first; the second is that minus the marker @@ -431,6 +433,14 @@ install for code that can never run — and on API 37 the full APK does not fit variable**, and `--no-verify` needs the repo owner's say-so each time rather than being reached for when the gate is inconvenient. + **What that keying cannot see is `bin/`.** The classifier matches `app/src/main/*` and + `app/src/{test,androidTest}/*` and nothing else, so a commit that replaces only the committed + FFmpeg AAR — a *different native binary* under every instrumented test — invalidates no cache and + sweeps nothing, while the JVM gate that does run cannot execute FFmpeg at all. #254 is where that + was noticed, and it did not hit it: the AAR and the `app/src` change that needs it are one commit, + so the sweep ran. An AAR rebuilt on its own would not be, and should be committed alongside + something under `app/src` or swept by hand. + Why it is worth tens of minutes a commit: the alternative was measured on 2026-09-06, when one PR spent several gating legs learning one leg at a time what a sweep answers in one pass — and the failing leg **moved** between runs (API 35 red then green, API 34 green then red). One leg at a diff --git a/README.md b/README.md index 28d9b0a..e6ce44b 100644 --- a/README.md +++ b/README.md @@ -48,6 +48,8 @@ Everything Media3 structurally cannot do: - Containers outside MP4/WebM/Ogg/WAV/AAC — MKV, MOV, AVI, FLV, MPEG-TS, WMV/ASF - **MP3 output** — Android has no MP3 encoder at any version; this is a platform gap +- **Ogg Vorbis output** — the same gap: Android has no Vorbis encoder either. Encoded with + `libvorbis`, which the bundled build carries since #254 - GIF and image sequences - Input codecs with no platform decoder on the device - CRF and 2-pass rate control, for the quality tier diff --git a/app/src/androidTest/java/org/libremediaconverter/ffmpeg/FFmpegEngineTest.kt b/app/src/androidTest/java/org/libremediaconverter/ffmpeg/FFmpegEngineTest.kt index 02a72a0..89a9635 100644 --- a/app/src/androidTest/java/org/libremediaconverter/ffmpeg/FFmpegEngineTest.kt +++ b/app/src/androidTest/java/org/libremediaconverter/ffmpeg/FFmpegEngineTest.kt @@ -86,6 +86,25 @@ class FFmpegEngineTest { } } + /** + * Channels per track, or 0 for a track that does not declare any. + * + * Read out of the container rather than assumed from the request, because the thing worth + * catching is an encoder that quietly changed the channel count on the way through — which is + * exactly what a stereo-only encoder does to this class's mono fixture. + */ + private fun channelCounts(file: File): List { + val extractor = MediaExtractor() + return try { + extractor.setDataSource(file.absolutePath) + (0 until extractor.trackCount).map { + extractor.getTrackFormat(it).getInteger(MediaFormat.KEY_CHANNEL_COUNT, 0) + } + } finally { + extractor.release() + } + } + // --- the formats that justify bundling FFmpeg at all ------------------- @Test @@ -149,6 +168,60 @@ class FFmpegEngineTest { assertEquals("OggS", magic) } + /** + * The first execution, ever, of the Vorbis encode arm — and the reason it needed one. + * + * `FFmpegCommandBuilder` carried `-c:a libvorbis` from the day it was written and nothing + * could ask for it: no preset produced `AudioCodec.VORBIS` and `ContainerCapabilities` left it + * out of the encodable set, so the arm was unreachable from both ends (#254). It was also + * **wrong**: `--enable-libvorbis` was in neither `bin/README.md`'s configure line nor + * `tools/ffmpeg/build-ffmpeg.sh`, and `libvorbis` was not among the encoder names in the + * shipped `libavcodec.so`. The first user to pick Ogg Vorbis would have got "Unknown encoder + * 'libvorbis'". #254 rebuilt the AAR with `--enable-libvorbis`; **this test is the only thing + * in the repo that can tell whether that rebuild actually included it**, because a wrong + * ffmpeg-kit `--enable-*` name is ignored silently and the JVM cannot tell a real encoder name + * from a fictional one. + * + * ## Why the container magic is not enough here + * + * `encodesOpus` above stops at `OggS`, and for that test it is sufficient. Here it would be + * **vacuous**: Vorbis and Opus are both Ogg streams, so this ticket's acceptance mutation — + * pointing the arm at `libopus` — produces a file with byte-identical first four bytes. + * Measured, not assumed: `-c:a libopus -b:a 128k -f ogg` on this class's own fixture writes + * `OggS` too. So the assertion has to reach the track, and `MediaExtractor` reporting + * `audio/vorbis` against `audio/opus` is what separates them. + * + * Asserted as the whole track list rather than as "contains Vorbis", which also pins that the + * `-vn` from the audio-only path really dropped the video: a stray video track would fail here + * rather than pass an `any { ... }` check. + * + * ## The channel count is the second claim, and it is not decoration + * + * `sample_h264.mp4` is **mono** — one AAC channel — and that is what makes this assertion + * bite. FFmpeg's in-tree `vorbis` encoder is stereo-only, so building on it forces `-ac 2` and + * silently upmixes every mono source, a compromise this app makes in no other arm. That + * compromise is the reason #254 rebuilt the binary rather than shipping the in-tree encoder, + * so re-adding `-ac 2` has to redden something: it reddens this. + */ + @Test + fun encodesOggVorbisThroughAnEncoderTheBundledBinaryActuallyHas() { + val out = convert(OutputFormat.OGG_VORBIS) + assertTrue("no Ogg produced", out.exists() && out.length() > 0) + + val magic = out.inputStream().use { String(it.readNBytes(4), Charsets.US_ASCII) } + assertEquals("OggS", magic) + assertEquals( + "expected a lone Vorbis track -- an Opus one would carry the same OggS magic", + listOf(MediaFormat.MIMETYPE_AUDIO_VORBIS), + trackMimes(out), + ) + assertEquals( + "the fixture is mono and libvorbis takes any channel count, so nothing may upmix it", + listOf(1), + channelCounts(out), + ) + } + /** * The percentage itself, which every other test in this class computes and none of them reads. * diff --git a/app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt b/app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt index fefcb57..77cb31c 100644 --- a/app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt +++ b/app/src/main/java/org/libremediaconverter/convert/Media3Engine.kt @@ -230,6 +230,12 @@ class Media3Engine(private val context: Context) : HardwareTranscoder { * unreachable code buys nothing — but it is an entry waiting on a routing change rather * than a live one. `Media3EngineMimeTypesTest` routes all six encodable codecs and asserts * which three arrive, so if that set moves, the disagreement fails rather than surprises. + * + * **Unreachable here is not the same as unreachable.** Since #254 a Vorbis encode is a + * thing a user can ask for — `OutputFormat.OGG_VORBIS` — and it is served by + * `FFmpegCommandBuilder`, which is the whole point of the router rule above sending it + * there. What stays dead is this arm specifically, because `MEDIA3_AUDIO` still excludes + * Vorbis: Android has no Vorbis encoder at any API level, exactly as with MP3. */ internal fun audioMimeTypeFor(codec: AudioCodec): String? = when (codec) { AudioCodec.AAC -> MimeTypes.AUDIO_AAC diff --git a/app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilder.kt b/app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilder.kt index a06206b..aed549c 100644 --- a/app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilder.kt +++ b/app/src/main/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilder.kt @@ -53,6 +53,44 @@ object FFmpegCommandBuilder { /** Containers in the ISO base-media family, where HEVC needs the hvc1 brand. */ private val MP4_FAMILY = setOf(Container.MP4, Container.MOV) + /** + * Ogg Vorbis, through libvorbis. + * + * ## This named an encoder the binary did not have, for as long as it existed + * + * These are the exact flags the arm carried before #254, and the arm had never run: `VORBIS` + * was absent from `ContainerCapabilities.ENCODABLE_AUDIO` and no `OutputFormat` offered it. + * It could not have run either. `--enable-libvorbis` was not in the AAR's configure line, and + * `strings` on the shipped `libavcodec.so` named `libx264`, `libx265`, `libvpx`, `libmp3lame`, + * `libopus`, `libdav1d`, `libsvtav1` and `libjxl` — no `libvorbis`. The first user to pick Ogg + * Vorbis would have got "Unknown encoder 'libvorbis'". #254 rebuilt the AAR with + * `--enable-libvorbis` (`bin/README.md` carries the new configure line and checksum) and made + * the arm reachable. The flags did not have to change; the binary under them did. + * + * **Nothing on the JVM can tell a real encoder name from a fictional one**, which is exactly + * how that survived four coverage waves. `FFmpegCommandBuilderTest` can only pin that this is + * what the builder emits. That `libvorbis` is really in there is proved by `FFmpegEngineTest`'s + * `encodesOggVorbisThroughAnEncoderTheBundledBinaryActuallyHas`, on a device, and by nothing + * else in this repo. + * + * Two flags are deliberately *absent*, and both would be forced by FFmpeg's in-tree `vorbis` + * encoder — the one the binary already had, and the one a first pass at #254 used: + * + * - **no `-strict experimental`**. The in-tree encoder carries `AV_CODEC_CAP_EXPERIMENTAL` + * and libavcodec refuses it without the flag. libvorbis is not experimental. + * - **no `-ac 2`**. The in-tree encoder is stereo-only — *"Current FFmpeg Vorbis encoder only + * supports 2 channels."* — so it would silently upmix a mono source and downmix a surround + * one, a compromise this app makes nowhere else. libvorbis takes any channel count, so mono + * stays mono — the e2e test's fixture is mono and it asserts the output still is. + * + * `-q:a 5` is libvorbis's classic ~160 kbps setting, and the scale behind it is the third + * reason for the rebuild. Over one 3 s clip libvorbis spans 10931..64166 bytes across q0..q10 + * where the in-tree encoder spans 7549..14645 — so libvorbis at this setting (16429 bytes) + * already writes more than the in-tree encoder can at q10, and the knob has somewhere to go + * if this app ever exposes it. + */ + private val VORBIS_ARGS = listOf("-c:a", "libvorbis", "-q:a", "5") + fun build(request: ConversionRequest, inputPath: String, outputPath: String): List { val plan = CopyPlanner.plan(request.spec, request.probe) return buildList { @@ -185,7 +223,7 @@ object FFmpegCommandBuilder { AudioCodec.FLAC -> listOf("-c:a", "flac") AudioCodec.PCM -> listOf("-c:a", "pcm_s16le") AudioCodec.OPUS -> listOf("-c:a", "libopus", "-b:a", "128k") - AudioCodec.VORBIS -> listOf("-c:a", "libvorbis", "-q:a", "5") + AudioCodec.VORBIS -> VORBIS_ARGS else -> listOf("-c:a", "aac", "-b:a", "192k") } } diff --git a/app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt b/app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt index aa19b89..05063a1 100644 --- a/app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt +++ b/app/src/main/java/org/libremediaconverter/model/ContainerCapabilities.kt @@ -7,11 +7,15 @@ package org.libremediaconverter.model * * "Can MP4 carry AV1?" and "can this app make AV1?" have different answers, and remux is exactly * where the difference shows. MP4 carries AV1 and ALAC happily; neither engine here encodes them. - * Matroska carries Vorbis; nothing in [org.libremediaconverter.ffmpeg.FFmpegCommandBuilder] emits a - * Vorbis encoder. A single `isValid` boolean would answer one of those questions and give the wrong + * Matroska carries VP8; nothing in [org.libremediaconverter.ffmpeg.FFmpegCommandBuilder] emits a + * VP8 encoder. A single `isValid` boolean would answer one of those questions and give the wrong * error for the other — telling a user "MP4 cannot hold AV1" when the truth is "your AV1 file can be * copied into MP4, just not re-encoded to it". * + * The example used to be Vorbis, and #254 is what stopped it being true — by rebuilding the + * bundled FFmpeg, because the Vorbis arm named `libvorbis` and the binary did not carry it. The + * gap is a video-only one now. + * * So the matrix is indexed by mode: [CodecMode.COPY] asks only what the muxer accepts, * [CodecMode.ENCODE] additionally asks what this app can encode. * @@ -81,10 +85,28 @@ object ContainerCapabilities { */ private val ENCODABLE_VIDEO = setOf(VideoCodec.H264, VideoCodec.H265, VideoCodec.VP9) - /** Vorbis is absent for the same reason: nothing here emits a Vorbis encoder. */ + /** + * Audio codecs this app can encode. Every codec any container here carries, as of #254. + * + * The comment this replaces said "Vorbis is absent for the same reason: nothing here emits a + * Vorbis encoder", and it was false as written — `FFmpegCommandBuilder.audioArgs` has had a + * Vorbis arm since the builder existed. Its absence from this set was what made that arm + * unreachable, and nothing recorded the decision either way. It also hid a second fault: the + * arm named `libvorbis`, which was not compiled into the bundled binary, so the format the app + * declined to offer was one it could not actually have produced. #254 rebuilt the AAR with + * `--enable-libvorbis` and added the codec here in the same change. + * + * That makes this set equal to the union of [CARRIES_AUDIO], which `ContainerCapabilitiesTest` + * now asserts rather than leaving to be noticed. The consequence is that [validateAudio]'s + * "this app cannot encode X audio" arm has no reachable input. It stays: the video half of the + * same rule is live (VP8 and AV1), and this is where an ALAC or an AC-3 entry would land the + * day the matrix carries one. It is F4-shaped — a second line of defence that cannot currently + * be provoked — and the set-equality assertion is what turns that from a hope into a check. + */ private val ENCODABLE_AUDIO = setOf( AudioCodec.AAC, AudioCodec.OPUS, + AudioCodec.VORBIS, AudioCodec.MP3, AudioCodec.FLAC, AudioCodec.PCM, diff --git a/app/src/main/java/org/libremediaconverter/model/OutputFormat.kt b/app/src/main/java/org/libremediaconverter/model/OutputFormat.kt index 8e5b23f..5863067 100644 --- a/app/src/main/java/org/libremediaconverter/model/OutputFormat.kt +++ b/app/src/main/java/org/libremediaconverter/model/OutputFormat.kt @@ -12,8 +12,19 @@ package org.libremediaconverter.model * and without it `.mka`, MP4 is `.mp4` or `.m4a`. That distinction is why they are functions rather * than properties. * + * The extension turned out to depend on a second thing, which is what [audioCodecExtensions] is + * for — see its parameter note. + * * @param ffmpegFormat the `-f` value. Named explicitly rather than left to extension inference, * which is unreliable for MPEG-TS and ASF. + * @param audioCodecExtensions per-codec overrides of [audioExtension]. Ogg is the only container + * that needs one, and it is the reason this parameter exists: one Ogg stream can hold Vorbis, + * Opus or FLAC, and RFC 7845 §9 asks for `.opus` on an Ogg that carries Opus alone while + * everything else in an Ogg is a plain `.ogg`. A single container-wide extension cannot say + * both — and it said `opus` for *every* Ogg until [OutputFormat.OGG_VORBIS] existed, which + * would have named a Vorbis file `.opus`. That is the same defect as the `FLAC` preset that + * once declared Matroska with a `.flac` extension, which is what moved these fields onto the + * container in the first place. */ enum class Container( val label: String, @@ -22,6 +33,7 @@ enum class Container( private val audioExtension: String, private val videoMime: String?, private val audioMime: String, + private val audioCodecExtensions: Map = emptyMap(), ) { MP4("MP4", "mp4", "mp4", "m4a", "video/mp4", "audio/mp4"), MOV("MOV", "mov", "mov", "m4a", "video/quicktime", "audio/mp4"), @@ -32,7 +44,7 @@ enum class Container( FLV("FLV", "flv", "flv", "flv", "video/x-flv", "video/x-flv"), ASF("WMV/ASF", "asf", "wmv", "wma", "video/x-ms-wmv", "audio/x-ms-wma"), - OGG("Ogg", "ogg", null, "opus", null, "audio/ogg"), + OGG("Ogg", "ogg", null, "ogg", null, "audio/ogg", mapOf(AudioCodec.OPUS to "opus")), WAV("WAV", "wav", null, "wav", null, "audio/wav"), AAC_ADTS("AAC", "adts", null, "aac", null, "audio/aac"), MP3("MP3", "mp3", null, "mp3", null, "audio/mpeg"), @@ -45,7 +57,17 @@ enum class Container( /** Whether this container can hold a video track at all. */ val canHoldVideo: Boolean get() = videoExtension != null - fun extensionFor(hasVideo: Boolean): String = if (hasVideo) videoExtension ?: audioExtension else audioExtension + /** + * The filename extension for an output in this container. + * + * [audioCodec] takes no default on purpose. A default would let a caller get `.ogg` for an + * Opus output by saying nothing, which is exactly the silent-wrong-answer shape the audio + * codec argument was added to close. + */ + fun extensionFor(hasVideo: Boolean, audioCodec: AudioCodec): String = when { + hasVideo -> videoExtension ?: audioExtension + else -> audioCodecExtensions[audioCodec] ?: audioExtension + } fun mimeTypeFor(hasVideo: Boolean): String = if (hasVideo) videoMime ?: audioMime else audioMime } @@ -102,7 +124,7 @@ data class OutputSpec(val container: Container, val videoCodec: VideoCodec, val audioCodec.isCopyOrAbsent() && (videoCodec == VideoCodec.COPY || audioCodec == AudioCodec.COPY) - val extension: String get() = container.extensionFor(hasVideo) + val extension: String get() = container.extensionFor(hasVideo, audioCodec) val mimeType: String get() = container.mimeTypeFor(hasVideo) private fun VideoCodec.isCopyOrAbsent() = this == VideoCodec.COPY || this == VideoCodec.NONE @@ -129,6 +151,19 @@ enum class OutputFormat(val label: String, val spec: OutputSpec) { MP3("MP3", OutputSpec(Container.MP3, VideoCodec.NONE, AudioCodec.MP3)), M4A_AAC("M4A (AAC)", OutputSpec(Container.MP4, VideoCodec.NONE, AudioCodec.AAC)), OPUS("Opus", OutputSpec(Container.OGG, VideoCodec.NONE, AudioCodec.OPUS)), + + /** + * The other codec Ogg carries, and the only preset added to make an existing arm reachable. + * + * `FFmpegCommandBuilder` has emitted a Vorbis encoder since the builder was written, and + * nothing could ask for it: no preset produced [AudioCodec.VORBIS] and `ContainerCapabilities` + * refused it on the Advanced picker, so the arm was dead in both directions (#254). It was also + * naming `libvorbis`, which the bundled FFmpeg did not carry until that same ticket rebuilt it, + * so making it reachable meant rebuilding the binary under it. Named for + * the container as well as the codec because [OPUS] shares that container and the two produce + * differently-named files — `.opus` against `.ogg`. + */ + OGG_VORBIS("Ogg Vorbis", OutputSpec(Container.OGG, VideoCodec.NONE, AudioCodec.VORBIS)), FLAC("FLAC", OutputSpec(Container.FLAC, VideoCodec.NONE, AudioCodec.FLAC)), WAV("WAV", OutputSpec(Container.WAV, VideoCodec.NONE, AudioCodec.PCM)), diff --git a/app/src/test/java/org/libremediaconverter/convert/Media3MuxersTest.kt b/app/src/test/java/org/libremediaconverter/convert/Media3MuxersTest.kt index 04198f7..04eab43 100644 --- a/app/src/test/java/org/libremediaconverter/convert/Media3MuxersTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/Media3MuxersTest.kt @@ -106,7 +106,10 @@ class Media3MuxersTest { * has changed its mind and somebody should say so on purpose. * * - Media3's MP4 muxer accepts Vorbis; [ContainerCapabilities] declines to offer it, because - * Vorbis-in-MP4 is poorly supported by players. + * Vorbis-in-MP4 is poorly supported by players. That refusal is about **this container**, + * not about the codec: since #254 the app encodes Vorbis for Ogg, Matroska and WebM, and + * the assertion below is what keeps MP4 out of that list on purpose rather than by + * omission — it is `CARRIES_AUDIO[MP4]`, so widening the encodable set cannot reach it. * - The matrix offers MP3 and FLAC in MP4, which is legal and which FFmpeg writes happily, but * Media3's MP4 muxer carries neither — so those jobs route to FFmpeg rather than failing. */ diff --git a/app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilderTest.kt b/app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilderTest.kt index 1274d11..b131578 100644 --- a/app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilderTest.kt +++ b/app/src/test/java/org/libremediaconverter/ffmpeg/FFmpegCommandBuilderTest.kt @@ -166,6 +166,47 @@ class FFmpegCommandBuilderTest { assertPair(cmd(OutputFormat.OPUS), "-c:a", "libopus") } + /** + * The arm that named an encoder the shipped binary did not contain. + * + * This read `-c:a libvorbis` from the day the builder was written and had never been run: no + * preset produced [AudioCodec.VORBIS] and `ContainerCapabilities` refused it. It could not + * have worked either — `--enable-libvorbis` was in neither `bin/README.md`'s configure line + * nor `tools/ffmpeg/build-ffmpeg.sh`, and the string `libvorbis` was not in the shipped + * `libavcodec.so` while `libopus`, `libmp3lame`, `libx264` and five others were. #254 rebuilt + * the AAR with it. + * + * **What this test cannot do is tell you that.** `-c:a libvorbis` and `-c:a libvorbisss` are + * the same string to a JVM assertion, which is precisely how the defect survived four coverage + * waves and a review that asked whether every test asserted something. The positive claim — + * that this encoder exists in the binary and produces a Vorbis track — is proved by + * `FFmpegEngineTest.encodesOggVorbisThroughAnEncoderTheBundledBinaryActuallyHas` on a device, + * and by nothing else in this repo. + * + * The two negatives are the assertions that carry real weight here, because each pins a + * decision rather than a name. `-strict experimental` and `-ac 2` are what FFmpeg's in-tree + * `vorbis` encoder forces, and taking the in-tree encoder would silently upmix mono; the arm's + * KDoc has the measurements. `-f ogg` is asserted because encoder and muxer together are what + * make the file — an encoder without its muxer is how a Vorbis stream ends up in a container + * that will not open. + */ + @Test + fun `ogg vorbis names libvorbis, with no experimental gate and no forced stereo`() { + val args = cmd(OutputFormat.OGG_VORBIS) + + assertPair(args, "-c:a", "libvorbis") + assertPair(args, "-q:a", "5") + assertPair(args, "-f", "ogg") + assertFalse( + "libvorbis is not experimental; -strict belongs to FFmpeg's in-tree encoder: $args", + args.contains("-strict"), + ) + assertFalse( + "libvorbis takes any channel count, so mono must not be upmixed: $args", + args.contains("-ac"), + ) + } + /** * The arm most conversions actually take, and the only one in `audioArgs` with no test. * @@ -216,7 +257,13 @@ class FFmpegCommandBuilderTest { @Test fun `audio only formats never carry a video encoder`() { - listOf(OutputFormat.MP3, OutputFormat.FLAC, OutputFormat.WAV, OutputFormat.OPUS) + listOf( + OutputFormat.MP3, + OutputFormat.FLAC, + OutputFormat.WAV, + OutputFormat.OPUS, + OutputFormat.OGG_VORBIS, + ) .forEach { format -> val args = cmd(format) assertFalse("$format should not set -c:v", args.contains("-c:v")) diff --git a/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt b/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt index a1308ea..84b8555 100644 --- a/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt +++ b/app/src/test/java/org/libremediaconverter/model/ContainerCapabilitiesTest.kt @@ -19,6 +19,16 @@ import org.junit.Test */ class ContainerCapabilitiesTest { + /** + * The codecs the matrix can actually be asked about. + * + * `COPY` and `NONE` are excluded because [ContainerCapabilities.accepts] refuses the first + * outright — `resolving COPY before asking the matrix is required` covers that — and answers + * the second `true` for every container without consulting any table. + */ + private val realAudioCodecs = AudioCodec.entries - AudioCodec.COPY - AudioCodec.NONE + private val realVideoCodecs = VideoCodec.entries - VideoCodec.COPY - VideoCodec.NONE + private val h264Source = InputProbe( videoCodec = "h264", audioCodec = "aac", @@ -79,10 +89,50 @@ class ContainerCapabilitiesTest { } @Test - fun `Matroska carries Vorbis on copy but nothing here encodes it`() { - assertTrue(ContainerCapabilities.accepts(Container.MKV, AudioCodec.VORBIS, CodecMode.COPY)) + fun `Matroska carries VP8 on copy but nothing here encodes it`() { + assertTrue(ContainerCapabilities.accepts(Container.MKV, VideoCodec.VP8, CodecMode.COPY)) assertFalse( - ContainerCapabilities.accepts(Container.MKV, AudioCodec.VORBIS, CodecMode.ENCODE), + ContainerCapabilities.accepts(Container.MKV, VideoCodec.VP8, CodecMode.ENCODE), + ) + } + + /** + * Where the copy/encode gap actually is, asserted as a set rather than as examples. + * + * This used to have an audio twin — Matroska carries Vorbis, and nothing was thought to encode + * it. That was never true of the code: `FFmpegCommandBuilder` has emitted a Vorbis encoder + * since it was written, and only `ENCODABLE_AUDIO`'s omission made the arm unreachable (#254). + * With Vorbis in the set, **the audio gap is empty** and the mode axis earns its place on the + * video side alone. + * + * Two consequences worth having pinned rather than rediscovered: + * + * - `validateAudio`'s "this app cannot encode X audio" arm now has no reachable input, which + * is why no test drives it. It stays in production as the landing spot for the first ALAC + * or AC-3 entry, and this test is what will fail the day one is carried without an encoder + * — where before, an omission like Vorbis's could sit unnoticed for the life of the file. + * - The video list is the real one, and asserting it as a set is what makes an accidental + * addition visible: an encoder added for VP8 without a matching `ENCODABLE_VIDEO` entry + * would leave this passing, but a *carried* codec quietly dropped from the encodable set + * would not. + */ + @Test + fun `the copy-only gap is video-only, and VP8 and AV1 are all of it`() { + fun gap(codecs: List, accepts: (Container, T, CodecMode) -> Boolean): Set = + Container.entries.flatMap { container -> + codecs + .filter { accepts(container, it, CodecMode.COPY) } + .filterNot { accepts(container, it, CodecMode.ENCODE) } + }.toSet() + + assertEquals( + "no container may carry an audio codec this app cannot also encode", + emptySet(), + gap(realAudioCodecs, ContainerCapabilities::accepts), + ) + assertEquals( + setOf(VideoCodec.VP8, VideoCodec.AV1), + gap(realVideoCodecs, ContainerCapabilities::accepts), ) } @@ -405,21 +455,30 @@ class ContainerCapabilitiesTest { assertEverySuggestionValid(invalid, mp3Source) } + /** + * The spec that used to be this class's example of an unencodable audio codec, now valid. + * + * It asserted `"This app cannot encode Vorbis audio. It can still be copied from a Vorbis + * source."` for exactly this spec, and the message was wrong about the app: the encoder + * existed, unreachable (#254). Asserting the positive is what stops the omission coming back — + * a revert of `ENCODABLE_AUDIO` fails here rather than merely restoring an old refusal that + * reads plausible. + * + * The audio arm it used to cover no longer has a reachable input; `the copy-only gap is + * video-only` above is where that is now recorded, and `copying is offered as the fix when the + * codec is right but unencodable` still covers the live video half of the same rule. + */ @Test - fun `an audio codec this app cannot encode is refused, and copying is offered instead`() { - // Matroska carries Vorbis; nothing here encodes it. The refusal has to say so *and* say - // what would work, which is the audio twin of `copying is offered as the fix when the codec - // is right but unencodable`. + fun `Vorbis into Matroska is a re-encode this app will do`() { val spec = OutputSpec(Container.MKV, VideoCodec.H264, AudioCodec.VORBIS) - val invalid = ContainerCapabilities.validate(spec, h264Source) as? Validation.Invalid - ?: throw AssertionError("encoding Vorbis must be refused") - - assertEquals( - "This app cannot encode Vorbis audio. It can still be copied from a Vorbis source.", - invalid.message, + assertTrue( + "Vorbis is encodable, so this spec must validate: ${ContainerCapabilities.validate(spec, h264Source)}", + ContainerCapabilities.validate(spec, h264Source).isValid, ) - assertEverySuggestionValid(invalid, h264Source) + // The plan has to reach the encoder, not merely be permitted: an AAC source into Matroska + // cannot be upgraded to a copy, so this is an Encode carrying the codec that was asked for. + assertEquals(AudioPlan.Encode(AudioCodec.VORBIS), CopyPlanner.plan(spec, h264Source).audio) } @Test diff --git a/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt b/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt index b5a915c..cd0dcce 100644 --- a/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt +++ b/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt @@ -330,6 +330,28 @@ class ConversionRouterTest { } } + /** + * Ogg Vorbis leaves the hardware path one rule earlier than its Ogg sibling, and the reason + * shown to the user is the difference. + * + * Two rules would each send it to FFmpeg — Media3 cannot encode Vorbis, and it cannot write + * Ogg at all — and the order decides which explanation appears. The audio-encoder check runs + * first deliberately: `NO_PLATFORM_ENCODER` ("Android has no encoder for this format") is true + * of Vorbis on every Android version and tells the user something about their choice, where + * `CONTAINER_UNSUPPORTED` would name an internal boundary they cannot act on. That ordering is + * documented in the router and this is what holds it — asserting only the engine would pass + * with the two rules swapped. + */ + @Test + fun `ogg vorbis routes to ffmpeg because Android has no Vorbis encoder`() { + val d = route(OutputFormat.OGG_VORBIS) + assertEquals(Engine.FFMPEG, d.engine) + assertEquals(Reason.NO_PLATFORM_ENCODER, d.reason) + // The sibling in the same container stops at the container rule instead, because Media3 + // *can* encode Opus. One container, two reasons, and only the codec differs. + assertEquals(Reason.CONTAINER_UNSUPPORTED, route(OutputFormat.OPUS).reason) + } + /** M4A is the audio format that does stay on hardware, because its container is MP4. */ @Test fun `m4a stays on hardware because MP4 is a container Media3 can write`() { diff --git a/app/src/test/java/org/libremediaconverter/model/OutputFormatTest.kt b/app/src/test/java/org/libremediaconverter/model/OutputFormatTest.kt index 25fb4a0..e82e708 100644 --- a/app/src/test/java/org/libremediaconverter/model/OutputFormatTest.kt +++ b/app/src/test/java/org/libremediaconverter/model/OutputFormatTest.kt @@ -75,9 +75,14 @@ class OutputFormatTest { fun `every container names an extension, a mime type and an ffmpeg muxer`() { Container.entries.forEach { container -> listOf(true, false).forEach { hasVideo -> - val ext = container.extensionFor(hasVideo) - assertTrue("$container has no extension", ext.isNotBlank()) - assertFalse("$container extension has a dot", ext.startsWith(".")) + // Every audio codec, because the extension now varies by one — see the Ogg pair + // below. A container that answered blank for a codec it carries would be a + // filename with no extension at all. + AudioCodec.entries.forEach { audioCodec -> + val ext = container.extensionFor(hasVideo, audioCodec) + assertTrue("$container/$audioCodec has no extension", ext.isNotBlank()) + assertFalse("$container/$audioCodec extension has a dot", ext.startsWith(".")) + } assertTrue( "$container has no mime type", container.mimeTypeFor(hasVideo).contains('/'), @@ -89,10 +94,34 @@ class OutputFormatTest { @Test fun `audio-only variants of a container get their own extension`() { - assertEquals("mp4", Container.MP4.extensionFor(hasVideo = true)) - assertEquals("m4a", Container.MP4.extensionFor(hasVideo = false)) - assertEquals("mkv", Container.MKV.extensionFor(hasVideo = true)) - assertEquals("mka", Container.MKV.extensionFor(hasVideo = false)) + assertEquals("mp4", Container.MP4.extensionFor(hasVideo = true, audioCodec = AudioCodec.AAC)) + assertEquals("m4a", Container.MP4.extensionFor(hasVideo = false, audioCodec = AudioCodec.AAC)) + assertEquals("mkv", Container.MKV.extensionFor(hasVideo = true, audioCodec = AudioCodec.AAC)) + assertEquals("mka", Container.MKV.extensionFor(hasVideo = false, audioCodec = AudioCodec.AAC)) + } + + /** + * The second thing the extension depends on, and the reason [Container.extensionFor] takes a + * codec at all. + * + * One Ogg stream holds Vorbis or Opus, and the two are named differently: RFC 7845 §9 asks for + * `.opus` on an Ogg carrying Opus alone, while a Vorbis one is a plain `.ogg`. The container + * declared `opus` for every Ogg until [OutputFormat.OGG_VORBIS] existed, which would have + * shipped a Vorbis file called `.opus` — the same shape as the `FLAC` preset that once + * declared Matroska with a `.flac` extension, which is the regression guarded above. + * + * Both halves are asserted. Pinning only the Vorbis one would pass just as well if the + * override map were deleted and every Ogg went back to a single extension, which is the + * mutation that has to fail. + */ + @Test + fun `Ogg names its file after the codec in it, not after the container`() { + assertEquals("opus", OutputFormat.OPUS.extension) + assertEquals("ogg", OutputFormat.OGG_VORBIS.extension) + // The MIME type does not split the same way: audio/ogg is correct for both, so the SAF + // create-document contract sees one type for the two formats. + assertEquals("audio/ogg", OutputFormat.OPUS.mimeType) + assertEquals("audio/ogg", OutputFormat.OGG_VORBIS.mimeType) } /** Regression guard: FLAC used to be declared as Matroska with a `.flac` extension. */ diff --git a/bin/README.md b/bin/README.md index 703e34c..a344dd5 100644 --- a/bin/README.md +++ b/bin/README.md @@ -22,19 +22,33 @@ It also removes roughly forty minutes from every cold CI run. | API level | 33, matching the app's minSdk | | ABIs | arm64-v8a, x86_64 | | Shared libraries | 20 (10 per ABI) | -| SHA-256 | `ae188c9aec3c89a1c87a169589253c85438d57cfdcc3ce8b40fb3e87de368ff2` | +| SHA-256 | `c8f4491d2c626566cbf18d5035513c1a5d8049e6696531342ea030c5427df507` | +| Rebuilt | 2026-09-06, to add libvorbis (#254). Previous archive: `ae188c9a…`, same tag and FFmpeg version, one library fewer | Configure line, read back out of the shipped `libavutil.so`: ``` ---enable-asm --enable-cross-compile --enable-gpl --enable-iconv ---enable-inline-asm --enable-jni --enable-libass --enable-libdav1d ---enable-libfontconfig --enable-libfreetype --enable-libfribidi ---enable-libharfbuzz --enable-libjxl --enable-libmp3lame --enable-libopus ---enable-libsvtav1 --enable-libvpx --enable-libx264 --enable-libx265 ---enable-lto --enable-mediacodec --enable-neon --enable-optimizations ---enable-pic --enable-pthreads --enable-shared --enable-small ---enable-swscale --enable-v4l2-m2m --enable-version3 --enable-zlib +--enable-asm --enable-cross-compile --enable-gpl --enable-iconv +--enable-inline-asm --enable-jni --enable-libass --enable-libdav1d +--enable-libfontconfig --enable-libfreetype --enable-libfribidi +--enable-libharfbuzz --enable-libjxl --enable-libmp3lame --enable-libopus +--enable-libsvtav1 --enable-libvorbis --enable-libvpx --enable-libx264 +--enable-libx265 --enable-lto --enable-mediacodec --enable-neon +--enable-optimizations --enable-pic --enable-pthreads --enable-shared +--enable-small --enable-swscale --enable-v4l2-m2m --enable-version3 +--enable-zlib +``` + +`--enable-libvorbis` is the one that arrived late, in #254, and the two ways to get it wrong are +worth having written down. ffmpeg-kit's `--enable-*` names are its own — `--enable-lame` for +libmp3lame, `--enable-opus` for libopus — so `--enable-vorbis` is the plausible guess and it is not +the flag; `get_library_name()` in the upstream `scripts/function.sh` calls library 9 `libvorbis`. +And an unrecognised `--enable-*` is **ignored silently**, so a build that dropped it looks exactly +like one that worked. What tells them apart is the binary: + +```sh +unzip -p bin/ffmpeg-kit-next-8.1.1.aar 'jni/x86_64/libavcodec.so' > /tmp/libavcodec.so +strings /tmp/libavcodec.so | grep -x libvorbis # and the same for arm64-v8a ``` Every `.so` reports `LOAD align 0x4000`, so the archive satisfies the 16 KB page-size diff --git a/bin/ffmpeg-kit-next-8.1.1.aar b/bin/ffmpeg-kit-next-8.1.1.aar index 36a6e4c..afad2fe 100644 Binary files a/bin/ffmpeg-kit-next-8.1.1.aar and b/bin/ffmpeg-kit-next-8.1.1.aar differ diff --git a/docs/coverage-read-findings.md b/docs/coverage-read-findings.md index eecf91a..47501cc 100644 --- a/docs/coverage-read-findings.md +++ b/docs/coverage-read-findings.md @@ -1,9 +1,11 @@ # Coverage-read findings -**Status:** ten findings, none fixed, none urgent. F1-F4 came from the 2026-08-26 read; F5 was added -on 2026-08-27 while decomposing #132; **F6-F10 were added on 2026-09-02 from the wave-4 read**. Every -entry here is a *code* observation — something a test would document rather than repair. The test -gaps found in the same reads are tickets, not entries here; see [Not covered here](#not-covered-here). +**Status:** ten findings; **F1 is closed — by #254 on 2026-09-06, which found it was a defect rather +than the dead arm it was filed as** — and the other nine stand, none urgent. F1-F4 came from the +2026-08-26 read; F5 was added on 2026-08-27 while decomposing #132; **F6-F10 were added on +2026-09-02 from the wave-4 read**. Every entry here is a *code* observation — something a test +would document rather than repair. The test gaps found in the same reads are tickets, not entries +here; see [Not covered here](#not-covered-here). **Scope:** what a JaCoCo read turned up that writing a test would not fix. This is a survey, not a work order. Acting on any entry is a separate decision and would be its own commit. **Last verified:** `main` at `54ca2dd`, 2026-09-02. Coverage measured that day with @@ -40,7 +42,9 @@ Same vocabulary as `defect-audit.md`, deliberately, so the two read alike: - **No action** — recorded because it looks like a finding and is not. Nothing below was observed on a device, and nothing below needs to be: every entry is a claim about -what the code says, checkable by reading it. +what the code says, checkable by reading it. **F1's resolution is the exception, and it had to be**: +what that entry turned on — whether the encoder it named exists in the shipped binary — is not +readable from the source at all. --- @@ -110,6 +114,56 @@ files agree and to say so in one place. 2. Correct the `ContainerCapabilities.kt:84` comment, which is false as written, and give the `FFmpegCommandBuilder` arm the treatment `Media3Engine.kt:221-233` already models. +### Resolved 2026-09-06 (#254) — and the arm was not merely unreached, it was unrunnable + +Vorbis is now in `ENCODABLE_AUDIO`, `OutputFormat.OGG_VORBIS` is a one-tap preset beside `OPUS`, +and `FFmpegEngineTest.encodesOggVorbisThroughAnEncoderTheBundledBinaryActuallyHas` asserts the +produced track's MIME and its channel count. The false comment is gone. + +**The finding this entry did not have is that `-c:a libvorbis` could never have worked.** Three +independent sources agree and none of them is the coverage report: + +| source | says | +|---|---| +| `bin/README.md`'s configure line, read back out of the shipped `libavutil.so` | `--enable-libopus`, `--enable-libmp3lame`, `--enable-libvpx`, `--enable-libx264/5`, `--enable-libdav1d`, `--enable-libsvtav1`, `--enable-libjxl` — **no `--enable-libvorbis`** | +| `tools/ffmpeg/build-ffmpeg.sh` | neither `COMMON_LIBS` nor `EXTRA_LIBS` names it | +| `strings` on `jni/x86_64/libavcodec.so` | the `lib*` encoder names present are `libdav1d libjxl libmp3lame libopus libsvtav1 libvpx libx264 libx265`. `libvorbis` is absent; `libavcodec/vorbisenc.c` is present | + +So the first user to pick Ogg Vorbis would have got `Unknown encoder 'libvorbis'`. The arm was +*wrong*, not just dead — and **nothing short of building the command and running it could have +found that**, which is why the e2e half of this ticket is the load-bearing half. It is #238's shape +again: two covered facts (a builder arm, a configure line) that no test put together. + +**The AAR was rebuilt rather than the arm rewritten, and the measurements are why.** A first pass +at this ticket implemented Vorbis on FFmpeg's in-tree `vorbisenc.c`, which the binary already had. +It works, and it is not good enough to sit in a picker beside MP3, FLAC and Opus: + +| | `libvorbis` | in-tree `vorbis` | +|---|---|---| +| experimental gate | none | **needs `-strict experimental`** | +| channels | mono, stereo, surround | **stereo only** | +| `-q:a 0..10`, one 3 s clip | 10931 -> 64166 bytes | 7549 -> 14645 bytes | + +`AV_CODEC_CAP_EXPERIMENTAL` is upstream FFmpeg saying *do not ship this by accident*. The +stereo limit forces `-ac 2`, so a mono source is silently upmixed — and **this repo's own fixture, +`sample_h264.mp4`, is mono**, so the compromise was not hypothetical. And a quality knob spanning +2x its floor against libvorbis's 6x has nowhere to go: libvorbis at `-q:a 5` writes 16429 bytes of +that clip, more than the in-tree encoder produces at q10. + +So #254 added `--enable-libvorbis` to `tools/ffmpeg/build-ffmpeg.sh` and rebuilt: a new ~35 MB blob +in git history permanently, a new configure line and SHA-256 in `bin/README.md`. What that bought +is the arm as originally written — `-c:a libvorbis -q:a 5`, no experimental gate, no forced +channel count — and mono that stays mono, which the e2e test asserts alongside the track MIME. + +Two things about the flag are worth keeping, because both are ways to get this wrong quietly. +ffmpeg-kit's `--enable-*` names come from its own `get_library_name()` and are not FFmpeg's — it is +`--enable-lame` for libmp3lame and `--enable-opus` for libopus — so `--enable-vorbis` is the +plausible guess and it is **wrong**; id 9 is literally `libvorbis`, so `--enable-libvorbis` is +right, and it pulls libogg in with it. And ffmpeg-kit does **not** error on an unrecognised +`--enable-*`, so a rebuild that quietly omitted the library looks exactly like one that worked. +`strings jni/*/libavcodec.so | grep -x libvorbis` and the e2e test are the only two things that +tell those apart. + --- ## F2 — `ConversionRequest.hardwareEncodeAvailable` is written, read by nothing, and its KDoc describes behaviour that was removed @@ -444,7 +498,7 @@ the cheaper order. | ID | Finding | Severity | Evidence | Action | |---|---|---|---|---| -| F1 | `FFmpegCommandBuilder` emits a Vorbis encoder `ContainerCapabilities` says does not exist | low | confirmed by inspection; unreachability traced through four call sites | **decide**: feature or dead arm — the comment is false either way | +| F1 | `FFmpegCommandBuilder` emits a Vorbis encoder `ContainerCapabilities` says does not exist | low → **the severity was wrong** | confirmed by inspection; unreachability traced through four call sites | **closed #254 as a feature** — and the encoder it named is not in the shipped binary, so the arm could never have run | | F2 | `hardwareEncodeAvailable` written, never read; KDoc describes removed behaviour | low | confirmed by inspection; `FFmpegCommandBuilderTest:132` corroborates | **decide**: delete or mark vestigial | | F3 | `ConversionRequest.videoCodec` / `.audioCodec` have no callers | low | confirmed by inspection | delete, or keep for symmetry — **not** a test gap | | F4 | Two private guards reachable only by direct call | n/a | confirmed by inspection | **no action** — named exemption, per #88 | diff --git a/docs/e2e-read-findings.md b/docs/e2e-read-findings.md index e480025..0451026 100644 --- a/docs/e2e-read-findings.md +++ b/docs/e2e-read-findings.md @@ -500,7 +500,7 @@ Every one was read. **None of them is an e2e test gap**, which is the result: | 3 | `CopyPlanner:28`, `OutputFormat:222-223` | public members with no callers. **#253**, with F5 | | 3 | `MediaProbe:210-212` | `probeWithFFprobe`'s `catch` — **F7's sibling, and now measured**. See below | | 2 | `FFmpegCommandBuilder:167-168` | `COPY`/`NONE -> error(...)` — F4-shaped, deliberately exempt | -| 1 | `FFmpegCommandBuilder:188` | the `VORBIS` encode arm. No `OutputFormat` produces it, but `ContainerCapabilities` lists it for WEBM and OGG. **#254** | +| 1 | `FFmpegCommandBuilder:188` | the `VORBIS` encode arm. No `OutputFormat` produced it, but `ContainerCapabilities` listed it for WEBM and OGG. **#254 — closed, and it was the row that turned out to be a defect**: the arm named `libvorbis`, which was not compiled into the shipped AAR at all, so it could never have run. Closing it meant rebuilding the AAR with `--enable-libvorbis`, not editing the arm. See F1 in `coverage-read-findings.md` | | 1 | `ConversionWorker:231` | `?: error("Could not open the input file.")`. `UnopenableUriTest` fails the job *downstream* of it, so the elvis is unprovoked — F4-shaped, same as the two above | **`MediaProbe:210-212` is the one that gained a measurement.** F7 ruled `probeWithExtractor`'s catch diff --git a/tools/ffmpeg/README.md b/tools/ffmpeg/README.md index 8f87392..3202b88 100644 --- a/tools/ffmpeg/README.md +++ b/tools/ffmpeg/README.md @@ -60,11 +60,22 @@ Only `arm64-v8a` and `x86_64` are built, matching the app's `abiFilters`. Droppi ## Library selection -Flag names come from `get_library_name()` in the upstream `scripts/function.sh`. Two +Flag names come from `get_library_name()` in the upstream `scripts/function.sh`. Three that are easy to get wrong: - It is **`--enable-lame`**, not `--enable-libmp3lame`. - It is **`--enable-libsvtav1`** for SVT-AV1. +- It *is* **`--enable-libvorbis`** — the rule above makes `--enable-vorbis` the natural + guess and it is wrong. Read the function rather than extrapolating from the first two; + library 9 is named `libvorbis` there. Enabling it also enables libogg, which ffmpeg-kit + pulls in as its dependency without being asked. + +**An unrecognised `--enable-*` is ignored silently.** ffmpeg-kit does not error on one, so a +build that quietly dropped a library looks exactly like one that worked, and forty minutes +later there is an AAR that is wrong in a way nothing in the log says. #254 is where that +was learned, from the other end: the builder carried `-c:a libvorbis` for months against a +binary with no libvorbis in it — unreachable, so no user ever hit it, and no build log ever +mentioned it. Check the artifact, not the log — `strings jni/*/libavcodec.so | grep -x `. MP3 deserves a note: **Android has no MP3 encoder at any API level**. That is a platform gap, not a Media3 limitation, so `--enable-lame` is the only way the app can output MP3. @@ -100,11 +111,15 @@ and `x86_64`. Confirmed against the artifact rather than assumed: `--enable-gpl --enable-version3 --enable-libx264 --enable-libx265 --enable-libsvtav1 --enable-libvpx --enable-libmp3lame --enable-libopus --enable-libdav1d --enable-libass --enable-libfontconfig --enable-libfreetype --enable-libfribidi --enable-libharfbuzz - --enable-mediacodec --enable-jni --enable-shared --enable-small --enable-lto` + --enable-mediacodec --enable-jni --enable-shared --enable-small --enable-lto`. + **Since 2026-09-06 it also carries `--enable-libvorbis`** (#254), which is the only + difference between that build and the one in `bin/` today — same tag, same FFmpeg + version, same 10 shared libraries per ABI, all still `LOAD align 0x4000`. - Present and verified: `libx264` (with an x264 core banner, so genuinely linked), `libx265`, `libsvtav1`, `libmp3lame`, `h264_mediacodec`, `hevc_mediacodec`, `libopus`, - `libdav1d`, the GIF encoder and muxer, libass internals (`ass_shaper_new`), and the - `subtitles`, `scale`, `palettegen`, `paletteuse` and `concat` filters. + `libdav1d`, `libvorbis` (from 2026-09-06), the GIF encoder and muxer, libass internals + (`ass_shaper_new`), and the `subtitles`, `scale`, `palettegen`, `paletteuse` and + `concat` filters. Note `--enable-version3`: combined with `--enable-gpl` this makes the binary **GPL-3.0**, which is what `LICENSES/README.md` states. diff --git a/tools/ffmpeg/build-ffmpeg.sh b/tools/ffmpeg/build-ffmpeg.sh index cf195a6..b272ab6 100755 --- a/tools/ffmpeg/build-ffmpeg.sh +++ b/tools/ffmpeg/build-ffmpeg.sh @@ -28,7 +28,10 @@ OUT=/work/out # Library selection # --------------------------------------------------------------------------- # Flag names come from get_library_name() in scripts/function.sh — note it is -# --enable-lame, NOT --enable-libmp3lame. +# --enable-lame, NOT --enable-libmp3lame. Read that function before adding one: the +# names are ffmpeg-kit's, not FFmpeg's, and they agree only sometimes. libvorbis is +# one that does agree (id 9 is literally "libvorbis"), so --enable-libvorbis is right +# and the --enable-vorbis this rule would predict is not. # # android-media-codec gives FFmpeg the h264_mediacodec / hevc_mediacodec wrappers. # Those are the fallback-within-the-fallback: hardware encode from the FFmpeg side @@ -41,6 +44,11 @@ COMMON_LIBS=( --enable-lame # MP3 encode. Android has NO MP3 encoder at any API level, # so this is the only way the app can output MP3 at all. --enable-opus + --enable-libvorbis # Ogg Vorbis encode. Android has no Vorbis ENCODER at any API + # level either, and FFmpeg's own in-tree vorbis encoder is + # experimental, stereo-only and barely responds to -q:a, so + # this is the only usable route. Pulls libogg in as its + # dependency (ffmpeg-kit sets LIBRARY_LIBOGG with it). --enable-dav1d # fast AV1 decode )