From c9ca283aa9628774e45d19fed5985990d4e6d6a5 Mon Sep 17 00:00:00 2001 From: itsjoshpark Date: Wed, 26 Aug 2026 11:50:52 -0400 Subject: [PATCH] fix: re-encode MP3 and multichannel Opus audio when converting The copyable audio table lists codecs AVFoundation is known to decode from an MP4, but two entries do not hold up against the real thing. ffmpeg can only mux MP3 under the `mp4a` tag, and AVFoundation drops that track on open: the converted file plays video and is silent. Opus decodes at stereo and below, but above two channels it carries a channel mapping family AVFoundation will not load at all, so the whole MP4 fails to open rather than merely losing its audio. Re-encode both to AAC instead. Stereo Opus still copies untouched, so the common case keeps its original audio. The README also still named FLAC and Opus as codecs that get re-encoded, which stopped being true when they were made copyable. --- .sparkle/notes/next.md | 1 + .../Conversion/RemuxPlannerTests.swift | 31 +++++++++++++++++-- Front Row UI Tests/ConversionUITests.swift | 4 +-- .../Conversion/Models/RemuxPlanner.swift | 24 ++++++++++---- README.md | 2 +- 5 files changed, 51 insertions(+), 11 deletions(-) diff --git a/.sparkle/notes/next.md b/.sparkle/notes/next.md index 158f804..0009f8b 100644 --- a/.sparkle/notes/next.md +++ b/.sparkle/notes/next.md @@ -1,2 +1,3 @@ - Fixed: The Audio Track and Subtitle menus said “None” for files whose tracks they could not see, including the Play Spatial Audio Sample file - Fixed: Tracks with no language tag were listed as “Unknown language” rather than by their name or number +- Fixed: Handling files with multichannel Opus or MP3 audio tracks diff --git a/Front Row Tests/Conversion/RemuxPlannerTests.swift b/Front Row Tests/Conversion/RemuxPlannerTests.swift index 60cd147..fde689f 100644 --- a/Front Row Tests/Conversion/RemuxPlannerTests.swift +++ b/Front Row Tests/Conversion/RemuxPlannerTests.swift @@ -232,8 +232,8 @@ struct RemuxPlannerTests { #expect(plan == .unsupported(.noPlayableStreams)) } - /// Both are common in Matroska and decode from an MP4 intact, so re-encoding them to AAC threw - /// audio away for nothing. + /// Both decode from an MP4 intact - FLAC at any channel count, Opus at the stereo the helper + /// defaults to - so re-encoding them to AAC threw audio away for nothing. @Test(arguments: ["flac", "opus"]) func losslessAndModernAudioIsCopiedRatherThanReEncoded(codec: String) { let plan = RemuxPlanner.plan(for: [video(0, "h264"), audio(1, codec)]) @@ -242,6 +242,33 @@ struct RemuxPlannerTests { if case .remux = plan {} else { Issue.record("Expected a straight remux, got \(plan)") } } + /// ffmpeg can only mux MP3 under the `mp4a` tag, and AVFoundation drops that track on open, so + /// copying it produced a file that played silently. + @Test + func mp3IsReEncodedRatherThanCopied() { + let plan = RemuxPlanner.plan(for: [video(0, "h264"), audio(1, "mp3")]) + + #expect(plan.recipe?.transcodesAudio == true) + } + + /// Above two channels Opus carries a channel mapping family AVFoundation will not load, and + /// the whole MP4 fails to open rather than merely losing its audio. + @Test(arguments: [6, 8]) + func multichannelOpusIsReEncoded(channels: Int) { + let plan = RemuxPlanner.plan(for: [video(0, "h264"), audio(1, "opus", channels: channels)]) + + #expect(plan.recipe?.transcodesAudio == true) + } + + /// Whether an Opus track can be copied hinges on its channel count, so a missing one is + /// re-encoded rather than assumed to be stereo. + @Test + func opusWithNoChannelCountIsReEncoded() { + let plan = RemuxPlanner.plan(for: [video(0, "h264"), audio(1, "opus", channels: nil)]) + + #expect(plan.recipe?.transcodesAudio == true) + } + /// PCM decodes as well, but copying it keeps a stream far larger than the AAC it would become. @Test func pcmIsStillReEncoded() { diff --git a/Front Row UI Tests/ConversionUITests.swift b/Front Row UI Tests/ConversionUITests.swift index 31a43a8..5814692 100644 --- a/Front Row UI Tests/ConversionUITests.swift +++ b/Front Row UI Tests/ConversionUITests.swift @@ -46,8 +46,8 @@ final class ConversionUITests: FrontRowUITestCase { /// /// Written by ffmpeg rather than committed: nothing else on the machine can write Matroska, and /// a test that needs ffmpeg installed to reach the conversion at all has no use for a fixture - /// that exists to avoid needing it. FLAC because it has to be re-encoded, which is the plan the - /// offer alert describes in the most detail. + /// that exists to avoid needing it. FLAC copies into an MP4 untouched, so the offer alert + /// describes a straight remux. private func makeMatroska(named name: String) throws -> URL { guard let ffmpeg = locateTool("ffmpeg") else { throw XCTSkip("ffmpeg is not installed, so the app has no conversion to offer") diff --git a/Front Row/Conversion/Models/RemuxPlanner.swift b/Front Row/Conversion/Models/RemuxPlanner.swift index a67900f..eb25836 100644 --- a/Front Row/Conversion/Models/RemuxPlanner.swift +++ b/Front Row/Conversion/Models/RemuxPlanner.swift @@ -34,12 +34,12 @@ enum RemuxPlanner { /// Audio codecs AVFoundation decodes from an MP4. Everything else is re-encoded to AAC. /// - /// FLAC and Opus are here because they are common in Matroska and decode from an MP4 intact - - /// copying them keeps the original audio where re-encoding it to AAC would throw some away for - /// no gain. PCM is deliberately not: it decodes too, but it is far larger than the AAC it - /// would become. + /// FLAC and Opus are here because they decode from an MP4 intact, and re-encoding them to AAC + /// would throw audio away for no gain. PCM and MP3 are deliberately absent: PCM decodes but is + /// far larger than the AAC it would become, and MP3 only muxes under the `mp4a` tag, which + /// AVFoundation drops on open for a file that plays silently. private static let copyableAudioCodecs: Set = [ - "aac", "mp3", "alac", "ac3", "eac3", "flac", "opus", + "aac", "alac", "ac3", "eac3", "flac", "opus", ] /// Subtitle codecs that carry text, and so can become `mov_text`. @@ -98,7 +98,7 @@ enum RemuxPlanner { index: stream.index, codecName: stream.codecName, channels: stream.channels, - transcodes: !copyableAudioCodecs.contains(stream.codecName ?? "") + transcodes: !isCopyable(audio: stream) ) } @@ -132,6 +132,18 @@ enum RemuxPlanner { } } + /// Opus is capped at stereo. Above two channels it needs channel mapping family 1, which + /// AVFoundation will not load at all - the whole file fails to open, not just the track. + /// A stream with no channel count is re-encoded rather than guessed at. + private static func isCopyable(audio stream: ProbedStream) -> Bool { + guard let codecName = stream.codecName, copyableAudioCodecs.contains(codecName) else { + return false + } + + guard codecName == "opus" else { return true } + return stream.channels.map { $0 <= 2 } ?? false + } + /// A stream with no pixel format, or one this doesn't recognise, is refused rather than /// guessed at. The profile name is not read as a fallback: HEVC 4:2:2 reports itself as /// `Rext`, so the profile misses the very case it would be consulted for. diff --git a/README.md b/README.md index 13363da..6081123 100644 --- a/README.md +++ b/README.md @@ -50,7 +50,7 @@ brew install ffmpeg Open an MKV file and Front Row checks what's inside it, then offers to convert: - If the video and audio are both formats it can play, they're copied into an MP4 as they are — nothing is re-encoded, and it takes a few seconds -- If only the audio is unsupported (DTS, TrueHD, FLAC, Opus), the video is still copied and just the audio is re-encoded to AAC +- If only the audio is unsupported (DTS, TrueHD, MP3, multichannel Opus), the video is still copied and just the audio is re-encoded to AAC - Text subtitles come along as `mov_text`. Bitmap subtitles (Blu-ray, DVD) can't be stored in an MP4, and the dialog warns you before you start - If the video itself can't be decoded (VP9, for example), Front Row won't offer to convert the file because it will usually take much longer time