diff --git a/app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt b/app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt index cf6d1f0..82577c7 100644 --- a/app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt +++ b/app/src/androidTest/java/org/libremediaconverter/convert/Media3EngineTest.kt @@ -108,48 +108,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, OutputFormat.WAV) - - 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, OutputFormat.OPUS) - - 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 5417186..d9a65ac 100644 --- a/app/src/main/java/org/libremediaconverter/convert/Media3Muxers.kt +++ b/app/src/main/java/org/libremediaconverter/convert/Media3Muxers.kt @@ -1,128 +1,60 @@ 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 - // Matroska, MP3 and the image outputs have no Media3 muxer. FFmpeg owns them. + + Container.WEBM, + Container.OGG, + Container.WAV, + Container.AAC_ADTS, Container.MKV, Container.MP3, Container.GIF, 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 49c3fd3..d23616b 100644 --- a/app/src/main/java/org/libremediaconverter/model/ConversionRouter.kt +++ b/app/src/main/java/org/libremediaconverter/model/ConversionRouter.kt @@ -16,17 +16,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) /** WebM is codec-restricted: Media3's WebmMuxer writes only these. */ private val WEBM_AUDIO = setOf(AudioCodec.OPUS, AudioCodec.VORBIS) diff --git a/app/src/test/java/org/libremediaconverter/convert/Media3MuxersTest.kt b/app/src/test/java/org/libremediaconverter/convert/Media3MuxersTest.kt index 4a4b314..fed5dfd 100644 --- a/app/src/test/java/org/libremediaconverter/convert/Media3MuxersTest.kt +++ b/app/src/test/java/org/libremediaconverter/convert/Media3MuxersTest.kt @@ -1,12 +1,9 @@ package org.libremediaconverter.convert -import androidx.media3.common.C -import androidx.media3.common.MimeTypes import androidx.media3.common.util.UnstableApi import org.junit.Assert.assertEquals import org.junit.Assert.assertNotNull import org.junit.Assert.assertNull -import org.junit.Assert.assertTrue import org.junit.Test import org.libremediaconverter.model.Container import org.libremediaconverter.model.ConversionRouter @@ -14,9 +11,10 @@ import org.libremediaconverter.model.ConversionRouter /** * 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. @@ -46,53 +44,17 @@ 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 36ab736..a00432b 100644 --- a/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt +++ b/app/src/test/java/org/libremediaconverter/model/ConversionRouterTest.kt @@ -39,13 +39,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 @@ -173,6 +176,31 @@ class ConversionRouterTest { assertEquals(Reason.USER_FORCED_SOFTWARE, d.reason) } + // --- 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