diff --git a/README.md b/README.md index f8c1cbb..fbd72fd 100644 --- a/README.md +++ b/README.md @@ -28,9 +28,10 @@ overlays, and audio to AAC. Fully hardware accelerated end to end — MediaCodec a GL surface and MediaCodec re-encodes, so frames never round-trip through the CPU. Roughly 7–8× realtime on 720p. -It writes MP4, WebM, Ogg, WAV and raw AAC — the five containers `media3-muxer` provides a -muxer for. It *reads* far more than it writes, Matroska included, which is what makes -MKV → MP4 a hardware remux. +It writes **MP4 and nothing else**. `media3-muxer` ships WebM, Ogg, WAV and AAC muxers too, +but none can be driven by Transformer — they throw from `addMetadataEntry`, which the muxer +wrapper calls for every metadata entry a real recording carries. It *reads* far more than it +writes, Matroska included, which is what makes MKV → MP4 a hardware remux. ### FFmpeg — the long tail diff --git a/app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt b/app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt index 32d5431..993c78e 100644 --- a/app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt +++ b/app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt @@ -109,48 +109,6 @@ class Media3EngineTest { } } - /** - * WAV output, which reaches Media3 for the same reason M4A does. - * - * `Container.WAV` is in the router's Media3 set, but until the engine passed a muxer factory - * the only muxer Transformer ever used was the MP4 one — so asking for WAV produced an MP4. - * Asserting the RIFF header proves the container, not merely that a file appeared; this - * follows what `FFmpegEngineTest` already does for its formats. - */ - @Test - fun wavExportWritesARiffHeader(): Unit = runBlocking { - val wav = File(context.cacheDir, "out_audio.wav") - wav.delete() - try { - engine.transcode(Uri.fromFile(input), wav, ConversionRequest(OutputFormat.WAV.spec)) - - assertTrue("export produced no file", wav.exists() && wav.length() > 0) - assertEquals("RIFF", String(wav.readBytes().copyOfRange(0, 4), Charsets.US_ASCII)) - } finally { - wav.delete() - } - } - - /** - * Ogg/Opus output, the third container the router claims for Media3. - * - * Asserted by magic bytes rather than `MediaExtractor`: platform extractor support for raw - * Ogg is inconsistent across the API levels in the CI matrix, and "OggS" is unambiguous. - */ - @Test - fun opusExportWritesAnOggHeader(): Unit = runBlocking { - val ogg = File(context.cacheDir, "out_audio.opus") - ogg.delete() - try { - engine.transcode(Uri.fromFile(input), ogg, ConversionRequest(OutputFormat.OPUS.spec)) - - assertTrue("export produced no file", ogg.exists() && ogg.length() > 0) - assertEquals("OggS", String(ogg.readBytes().copyOfRange(0, 4), Charsets.US_ASCII)) - } finally { - ogg.delete() - } - } - /** * Regression guard for the Transformer threading trap. * diff --git a/app/src/main/java/org/libremediaconverter/convert/Media3Muxers.kt b/app/src/main/java/org/libremediaconverter/convert/Media3Muxers.kt index 9fc4786..aa686a0 100644 --- a/app/src/main/java/org/libremediaconverter/convert/Media3Muxers.kt +++ b/app/src/main/java/org/libremediaconverter/convert/Media3Muxers.kt @@ -1,56 +1,59 @@ package org.libremediaconverter.convert -import androidx.media3.common.C -import androidx.media3.common.MimeTypes import androidx.media3.common.util.UnstableApi -import androidx.media3.muxer.AacMuxer import androidx.media3.muxer.Muxer -import androidx.media3.muxer.MuxerException -import androidx.media3.muxer.OggMuxer -import androidx.media3.muxer.SeekableMuxerOutput -import androidx.media3.muxer.WavMuxer -import androidx.media3.muxer.WebmMuxer import androidx.media3.transformer.DefaultMuxer -import com.google.common.collect.ImmutableList import org.libremediaconverter.model.Container -import java.io.FileOutputStream /** - * Muxer factories for the containers Media3 can write. + * Which containers Media3 can actually write, and why it is only one. * - * `media3-muxer` ships `Mp4Muxer`, `WebmMuxer`, `OggMuxer`, `WavMuxer` and `AacMuxer`, but - * `media3-transformer` only wraps the MP4 ones as a [Muxer.Factory]. Everything else needs the - * three lines of glue below, which is why the app previously wrote MP4 no matter what container - * was asked for: [androidx.media3.transformer.Transformer.Builder] defaults to - * `DefaultMuxer.Factory`, and nothing ever overrode it. + * ## The four other muxers cannot be driven by Transformer * - * ## The MIME lists are load-bearing + * `media3-muxer` 1.11.0 ships `WebmMuxer`, `OggMuxer`, `WavMuxer` and `AacMuxer` alongside the MP4 + * ones, which reads like four more containers on the hardware path. It is not: **all four throw + * `UnsupportedOperationException` from `addMetadataEntry`**, and + * `MuxerWrapper.addTrackFormat` calls it for every metadata entry on the track format. Any real + * recording carries some — a creation timestamp is enough — so the export dies partway through: * - * `Transformer` calls [Muxer.Factory.getSupportedSampleMimeTypes] to decide whether a track can be - * copied through or has to be re-encoded, and `Transformer.Builder.build()` validates the - * requested MIME types against it. A list that over-claims produces a file the muxer cannot - * actually write; one that under-claims forces a needless re-encode. The values here were read out - * of each muxer's own `isMimeTypeSupported` check rather than assumed. + * ``` + * Caused by: java.lang.UnsupportedOperationException + * at androidx.media3.muxer.OggMuxer.addMetadataEntry(OggMuxer.java:123) + * at androidx.media3.transformer.MuxerWrapper.addTrackFormat(MuxerWrapper.java:488) + * ``` + * + * They are standalone muxers, not Transformer-compatible ones. WAV fails a second way even before + * that: `DefaultEncoderFactory` has no PCM encoder, so Transformer reports "No MIME type is + * supported by both encoder and muxer" rather than passing raw samples through. + * + * Both were observed on a CI API 35 emulator, not inferred — the tests that found them were written + * on the assumption these containers worked. + * + * So Media3 writes MP4 and nothing else. WebM, Ogg, WAV and raw AAC belong to FFmpeg, which already + * produces all of them and has instrumented coverage asserting the produced files. */ @UnstableApi object Media3Muxers { /** - * The factory for [container], or null when Media3 cannot mux it at all. + * The factory for [container], or null when Media3 cannot write it. * - * A null here must agree with [org.libremediaconverter.model.ConversionRouter]'s container set - * — if the router sends Media3 a job this cannot mux, the conversion fails and falls back to - * FFmpeg, which is a slow way to discover a routing bug. `Media3MuxersTest` asserts they agree. + * A null here must agree with [org.libremediaconverter.model.ConversionRouter]'s container set; + * `Media3MuxersTest` asserts they do. They drifted once already, and expensively: the router + * claimed five containers while the engine silently wrote MP4 for all of them. */ fun factoryFor(container: Container): Muxer.Factory? = when (container) { - // Transformer's own default. Named explicitly so the MP4 path reads the same as the rest. + // Transformer's own default. Named explicitly so the engine states its container rather + // than inheriting one, which is how the MP4-for-everything bug went unnoticed. Container.MP4 -> DefaultMuxer.Factory() - Container.WEBM -> WebmFactory - Container.OGG -> OggFactory - Container.WAV -> WavFactory - Container.AAC_ADTS -> AacFactory - // Everything else has no Media3 muxer. Matroska and the legacy containers are FFmpeg's, - // and `Mp4Muxer` exposes no QuickTime file format, so MOV is too. + + // The four muxers described above, plus the containers Media3 never had one for. MOV is + // among them despite being MP4's own family: `Mp4Muxer` exposes only FILE_FORMAT_DEFAULT + // and FILE_FORMAT_MP4_WITH_AUXILIARY_TRACKS_EXTENSION — no QuickTime. + Container.WEBM, + Container.OGG, + Container.WAV, + Container.AAC_ADTS, Container.MKV, Container.MOV, Container.MPEG_TS, @@ -63,73 +66,4 @@ object Media3Muxers { Container.IMAGE_SEQUENCE, -> null } - - private object WebmFactory : Muxer.Factory { - override fun create(path: String): Muxer = wrapFailure(path) { - WebmMuxer.Builder(SeekableMuxerOutput.of(path)).build() - } - - override fun getSupportedSampleMimeTypes(trackType: Int): ImmutableList = - when (trackType) { - C.TRACK_TYPE_VIDEO -> ImmutableList.of(MimeTypes.VIDEO_VP8, MimeTypes.VIDEO_VP9) - C.TRACK_TYPE_AUDIO -> ImmutableList.of(MimeTypes.AUDIO_OPUS, MimeTypes.AUDIO_VORBIS) - else -> ImmutableList.of() - } - } - - private object OggFactory : Muxer.Factory { - override fun create(path: String): Muxer = wrapFailure(path) { - OggMuxer.Builder(FileOutputStream(path).channel).build() - } - - override fun getSupportedSampleMimeTypes(trackType: Int): ImmutableList = - if (trackType == C.TRACK_TYPE_AUDIO) { - ImmutableList.of(MimeTypes.AUDIO_OPUS, MimeTypes.AUDIO_VORBIS) - } else { - ImmutableList.of() - } - } - - private object WavFactory : Muxer.Factory { - override fun create(path: String): Muxer = wrapFailure(path) { - WavMuxer(SeekableMuxerOutput.of(path)) - } - - override fun getSupportedSampleMimeTypes(trackType: Int): ImmutableList = - if (trackType == C.TRACK_TYPE_AUDIO) { - ImmutableList.of(MimeTypes.AUDIO_RAW) - } else { - ImmutableList.of() - } - } - - private object AacFactory : Muxer.Factory { - override fun create(path: String): Muxer = wrapFailure(path) { - AacMuxer(FileOutputStream(path)) - } - - override fun getSupportedSampleMimeTypes(trackType: Int): ImmutableList = - if (trackType == C.TRACK_TYPE_AUDIO) { - ImmutableList.of(MimeTypes.AUDIO_AAC) - } else { - ImmutableList.of() - } - } - - /** - * Turns an I/O failure into a [MuxerException]. - * - * Opening the output can throw `FileNotFoundException`, which is not what `Muxer.Factory` - * declares. Transformer's error handling only recognises `MuxerException`, so letting the raw - * IOException escape turns a bad output path into an unhandled crash instead of a reported - * export failure — the case `Media3EngineTest.anUnwritableOutputPathFailsInsteadOfHanging` - * exists to pin down. - */ - private inline fun wrapFailure(path: String, open: () -> Muxer): Muxer = - try { - open() - } catch (e: Exception) { - if (e is MuxerException) throw e - throw MuxerException("Could not open $path for muxing", e) - } } diff --git a/app/src/main/java/org/libremediaconverter/model/ConversionRouter.kt b/app/src/main/java/org/libremediaconverter/model/ConversionRouter.kt index 06afebd..31af371 100644 --- a/app/src/main/java/org/libremediaconverter/model/ConversionRouter.kt +++ b/app/src/main/java/org/libremediaconverter/model/ConversionRouter.kt @@ -25,17 +25,16 @@ object ConversionRouter { /** * Containers Media3 can mux. Anything else has to go to FFmpeg. * + * MP4 alone. This set used to name WebM, Ogg, WAV and AAC-ADTS as well, on the strength of + * `media3-muxer` shipping a muxer for each — but none of those four can be driven by + * Transformer at all, for the reasons `Media3Muxers` records. Nothing caught it because the + * engine ignored the container entirely and wrote MP4 regardless, so the set being wrong and + * the engine being wrong cancelled out. + * * Not private: `Media3Muxers` has to supply a `Muxer.Factory` for every entry, and a test - * asserts the two agree. They drifted once already — this set was correct while the engine - * silently wrote MP4 for all five. + * asserts the two agree. */ - internal val MEDIA3_CONTAINERS = setOf( - Container.MP4, - Container.WEBM, - Container.OGG, - Container.WAV, - Container.AAC_ADTS, - ) + internal val MEDIA3_CONTAINERS = setOf(Container.MP4) /** * What Media3's muxers can *carry*, as distinct from what Media3 can encode. @@ -45,25 +44,21 @@ object ConversionRouter { * — legal, and something FFmpeg does without complaint — has to leave the hardware path. Before * remuxing existed nothing could reach that combination, so nothing had to know. * - * Transcribed from each `Muxer.Factory.getSupportedSampleMimeTypes` in `Media3Muxers`; + * One entry, because MP4 is the only container Transformer can write — see `Media3Muxers` for + * why the four other muxers in `media3-muxer` cannot be driven by it. Every other container + * has already been sent to FFmpeg by the time these are consulted. + * + * Transcribed from `Muxer.Factory.getSupportedSampleMimeTypes` in `Media3Muxers`; * `Media3MuxersTest` asserts the transcription still matches. */ internal val MEDIA3_MUXABLE_VIDEO: Map> = mapOf( Container.MP4 to setOf(VideoCodec.H264, VideoCodec.H265, VideoCodec.VP9, VideoCodec.AV1), - Container.WEBM to setOf(VideoCodec.VP8, VideoCodec.VP9), - Container.OGG to emptySet(), - Container.WAV to emptySet(), - Container.AAC_ADTS to emptySet(), ) internal val MEDIA3_MUXABLE_AUDIO: Map> = mapOf( Container.MP4 to setOf( AudioCodec.AAC, AudioCodec.OPUS, AudioCodec.VORBIS, AudioCodec.PCM, ), - Container.WEBM to setOf(AudioCodec.OPUS, AudioCodec.VORBIS), - Container.OGG to setOf(AudioCodec.OPUS, AudioCodec.VORBIS), - Container.WAV to setOf(AudioCodec.PCM), - Container.AAC_ADTS to setOf(AudioCodec.AAC), ) /** Video codecs Media3 can encode (`Transformer.setVideoMimeType`). */ @@ -109,15 +104,10 @@ object ConversionRouter { // The container is supported but its muxer is codec-restricted. What matters is what ends // up in the file, so a copied track is judged by its source codec rather than the request. + // Only MP4 reaches here now — WebM is rejected by the container check above — so the + // WebM-specific wording no longer applies. if (!media3CanMux(plan, request.probe)) { - return Decision( - Engine.FFMPEG, - if (plan.container == Container.WEBM) { - Reason.WEBM_CODEC_UNSUPPORTED - } else { - Reason.CONTAINER_CODEC_UNSUPPORTED - }, - ) + return Decision(Engine.FFMPEG, Reason.CONTAINER_CODEC_UNSUPPORTED) } // A file the platform extractor could not open cannot be read at all, copied or not. @@ -195,7 +185,6 @@ object ConversionRouter { HARDWARE_CAPABLE("Hardware accelerated"), REMUX_NO_REENCODE("Remuxed — streams copied, nothing re-encoded"), CONTAINER_UNSUPPORTED("This container needs FFmpeg"), - WEBM_CODEC_UNSUPPORTED("WebM only supports VP8/VP9 with Opus or Vorbis"), CONTAINER_CODEC_UNSUPPORTED("That codec needs FFmpeg for this container"), NO_PLATFORM_ENCODER("Android has no encoder for this format"), NO_PLATFORM_DECODER("This device cannot decode the input in hardware"), diff --git a/app/src/test/java/org/libremediaconverter/convert/Media3MuxersTest.kt b/app/src/test/java/org/libremediaconverter/convert/Media3MuxersTest.kt index 73c97a3..04198f7 100644 --- a/app/src/test/java/org/libremediaconverter/convert/Media3MuxersTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/Media3MuxersTest.kt @@ -19,9 +19,10 @@ import org.libremediaconverter.model.VideoCodec /** * Guards the agreement between the router's container set and the muxers that back it. * - * These two drifted once already, and the failure was invisible: [ConversionRouter] correctly - * claimed WebM, Ogg, WAV and AAC for Media3 while [Media3Engine] had no way to write any of them - * and produced MP4 regardless. Nothing failed — the file was simply the wrong container. + * These two drifted once already, and the failure was invisible in both directions: the router + * claimed WebM, Ogg, WAV and AAC-ADTS for Media3 — none of which Transformer can actually write — + * while [Media3Engine] ignored the container and produced MP4 for all of them. Two bugs that + * cancelled out, so nothing failed and the file was simply the wrong container. * * A JVM test rather than an instrumented one: constructing a factory touches no Android APIs, only * `create()` does, so the mapping is checkable without a device. @@ -51,54 +52,18 @@ class Media3MuxersTest { } } - @Test - fun `WebM advertises only the codecs its muxer accepts`() { - val factory = requireNotNull(Media3Muxers.factoryFor(Container.WEBM)) - - assertEquals( - listOf(MimeTypes.VIDEO_VP8, MimeTypes.VIDEO_VP9), - factory.getSupportedSampleMimeTypes(C.TRACK_TYPE_VIDEO), - ) - assertEquals( - listOf(MimeTypes.AUDIO_OPUS, MimeTypes.AUDIO_VORBIS), - factory.getSupportedSampleMimeTypes(C.TRACK_TYPE_AUDIO), - ) - } - /** - * The audio-only containers must not claim a video track. + * MP4 is the whole of it. * - * Transformer reads these lists to decide what it may hand the muxer. Claiming video for a - * container that cannot hold it turns a routing mistake into a corrupt file rather than a - * clean failure. + * Pinned as a value rather than left implicit, because `media3-muxer` shipping a `WebmMuxer`, + * `OggMuxer`, `WavMuxer` and `AacMuxer` makes it look like there should be five. Those four + * throw `UnsupportedOperationException` from `addMetadataEntry`, which `MuxerWrapper` calls + * unconditionally — see [Media3Muxers]. If a future Media3 release fixes that, this test is + * where the decision to widen the set gets made deliberately. */ @Test - fun `audio-only containers advertise no video codecs`() { - listOf(Container.OGG, Container.WAV, Container.AAC_ADTS).forEach { container -> - val factory = requireNotNull(Media3Muxers.factoryFor(container)) - assertTrue( - "$container claims video support", - factory.getSupportedSampleMimeTypes(C.TRACK_TYPE_VIDEO).isEmpty(), - ) - assertTrue( - "$container claims no audio support", - factory.getSupportedSampleMimeTypes(C.TRACK_TYPE_AUDIO).isNotEmpty(), - ) - } - } - - @Test - fun `WAV carries PCM and AAC-ADTS carries AAC`() { - assertEquals( - listOf(MimeTypes.AUDIO_RAW), - requireNotNull(Media3Muxers.factoryFor(Container.WAV)) - .getSupportedSampleMimeTypes(C.TRACK_TYPE_AUDIO), - ) - assertEquals( - listOf(MimeTypes.AUDIO_AAC), - requireNotNull(Media3Muxers.factoryFor(Container.AAC_ADTS)) - .getSupportedSampleMimeTypes(C.TRACK_TYPE_AUDIO), - ) + fun `Media3 can write MP4 and nothing else`() { + assertEquals(setOf(Container.MP4), ConversionRouter.MEDIA3_CONTAINERS) } /** diff --git a/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt b/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt index 3d1a81f..8dfbb95 100644 --- a/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt +++ b/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt @@ -50,13 +50,16 @@ class ConversionRouterTest { } @Test - fun `webm vp9 routes to ffmpeg because media3 cannot encode vp9`() { - // Transformer.setVideoMimeType accepts only H.263/H.264/H.265/MP4V. The WebM - // muxer exists, but there is no VP9 *encoder* behind it, so producing WebM - // video is FFmpeg's job even though the container is nominally supported. + fun `webm vp9 routes to ffmpeg`() { + // Two independent reasons, either of which is sufficient: Transformer.setVideoMimeType + // accepts only H.263/H.264/H.265/MP4V, so there is no VP9 encoder — and Media3 cannot mux + // WebM at all, which it was previously credited with. The container check runs first and + // is the more fundamental of the two: even given a VP9 encoder, the file could not be + // written. This asserted NO_PLATFORM_ENCODER while the container was wrongly believed to + // be supported. val d = route(OutputFormat.WEBM_VP9) assertEquals(Engine.FFMPEG, d.engine) - assertEquals(Reason.NO_PLATFORM_ENCODER, d.reason) + assertEquals(Reason.CONTAINER_UNSUPPORTED, d.reason) } @Test @@ -304,6 +307,31 @@ class ConversionRouterTest { } } + // --- containers Media3 cannot write ------------------------------------- + + /** + * WAV, Opus and raw AAC used to be claimed for Media3 and are not any more. + * + * `media3-muxer` ships a muxer for each, which is why they were listed — but none can be driven + * by Transformer, so the export dies at the muxer. They were never actually produced on the + * hardware path: the engine ignored the container and wrote MP4 into a file named `.wav`. + * FFmpeg produces all three, and `FFmpegEngineTest` asserts the produced files. + */ + @Test + fun `audio containers Media3 cannot mux route to FFmpeg`() { + listOf(OutputFormat.WAV, OutputFormat.OPUS).forEach { format -> + val d = route(format) + assertEquals("$format should need FFmpeg", Engine.FFMPEG, d.engine) + assertEquals(Reason.CONTAINER_UNSUPPORTED, d.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`() { + assertEquals(Engine.MEDIA3, route(OutputFormat.M4A_AAC).engine) + } + // --- exhaustiveness ---------------------------------------------------- @Test