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