From 6302b992f7f67d090dc38c387908fc585bf76758 Mon Sep 17 00:00:00 2001 From: Gunwoo Kim Date: Sat, 7 Nov 2026 14:13:00 +0900 Subject: [PATCH 1/3] fix(ffmpeg): accept identity audio encode options Signed-off-by: Gunwoo Kim --- .github/workflows/ci.yml | 7 + crates/coreav-cli/src/ffmpeg/args.rs | 31 +- crates/coreav-cli/src/ffmpeg/mod.rs | 60 ++- .../coreav-cli/tests/ffmpeg_audio_options.rs | 369 ++++++++++++++++++ docs/agents/ffmpeg-cli.md | 2 +- docs/design/transcode.md | 11 + test-support/oracle-dependent.json | 4 + 7 files changed, 453 insertions(+), 31 deletions(-) create mode 100644 crates/coreav-cli/tests/ffmpeg_audio_options.rs diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index f80377f61..8a4e3aaf5 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1366,6 +1366,13 @@ jobs: --target-dir target/fallback cargo test --locked --release -p coreav-cli --features ffmpeg-fallback,dav1d,libvpx,libopus,aac --test aac_worker \ --target-dir target/fallback + - name: Identity audio options run without whole-command fallback + env: + COREAV_REQUIRE_FFMPEG: '1' + COREAV_FFMPEG: ${{ runner.temp }}/coreav-oracle-bin/ffmpeg + run: | + cargo test --locked --release -p coreav-cli --features native-h264,aac,libopus \ + --test ffmpeg_audio_options --target-dir target/fallback - name: Copied video with AAC encoding preserves exact fallback env: COREAV_REQUIRE_FFMPEG: '1' diff --git a/crates/coreav-cli/src/ffmpeg/args.rs b/crates/coreav-cli/src/ffmpeg/args.rs index a4bb78925..76d4b77d4 100644 --- a/crates/coreav-cli/src/ffmpeg/args.rs +++ b/crates/coreav-cli/src/ffmpeg/args.rs @@ -1238,7 +1238,7 @@ pub fn parse(args: &[String]) -> Result { )); } if let Some(option) = audio_encoders.first() - && command.muxer != Muxer::Mp3 + && !matches!(command.muxer, Muxer::Mp3 | Muxer::Mp4 | Muxer::Ipod) && !command .codecs .iter() @@ -1279,13 +1279,19 @@ pub fn parse(args: &[String]) -> Result { "it is not an option of native audio output", )); } - } else if command.sample_rate.is_some() { + } else if command.sample_rate.is_some() + && !matches!(command.muxer, Muxer::Mp4 | Muxer::Ipod | Muxer::Mp3) + && command.muxer.matroska().is_none() + { return Err(not_native( "-ar", None, "it is not supported on this output", )); - } else if command.channels.is_some() { + } else if command.channels.is_some() + && !matches!(command.muxer, Muxer::Mp4 | Muxer::Ipod | Muxer::Mp3) + && command.muxer.matroska().is_none() + { return Err(not_native( "-ac", None, @@ -1973,14 +1979,15 @@ mod tests { ] ); assert_eq!(c.encoding.audio_bitrate.as_deref(), Some("128k")); - assert!(matches!( - parse(&args("-i in.mkv -c:v libx264 -b:a 128k out.mp4")), - Err(Refusal::NotNative(_)) - )); + let c = parse(&args("-i in.mkv -c:v libx264 -b:a 128k out.mp4")).unwrap(); + assert_eq!(c.encoding.audio_bitrate.as_deref(), Some("128k")); } #[test] fn aac_and_its_bit_rate() { + let c = parse(&args("-i in.mkv -c:a aac -ar 44100 -ac 1 out.mp4")).unwrap(); + assert_eq!(c.sample_rate, Some(44_100)); + assert_eq!(c.channels, Some(1)); for line in [ "-i in.mkv -c:v libx264 -c:a aac -b:a 96k out.mp4", "-i in.mkv -c:v libx264 -acodec aac -b:a 96k out.mp4", @@ -2009,8 +2016,6 @@ mod tests { ("-aac_coder twoloop", "-aac_coder", None), ("-q:a 2", "-q:a", None), ("-vbr 3", "-vbr", None), - ("-ar 44100", "-ar", None), - ("-ac 1", "-ac", None), ("-af volume=2", "-af", None), ("-strict experimental", "-strict", None), ("-c:a libfdk_aac", "-c:a", Some("libfdk_aac")), @@ -2055,11 +2060,9 @@ mod tests { for (option, value) in [("-ar", "48000"), ("-ac", "1")] { for codec in ["copy", "aac"] { let line = format!("-i in.mp4 -c:a {codec} -vn {option} {value} out.mp4"); - assert_eq!( - refused(&line), - Reason::Option(option.to_owned(), None), - "{line}" - ); + let command = parse(&args(&line)).unwrap(); + assert_eq!(command.sample_rate, (option == "-ar").then_some(48_000)); + assert_eq!(command.channels, (option == "-ac").then_some(1)); } } assert!(parse(&args("-i in.mp4 -c copy -movflags +faststart out.mp4")).is_ok()); diff --git a/crates/coreav-cli/src/ffmpeg/mod.rs b/crates/coreav-cli/src/ffmpeg/mod.rs index 1e3a74c45..d594ec42a 100644 --- a/crates/coreav-cli/src/ffmpeg/mod.rs +++ b/crates/coreav-cli/src/ffmpeg/mod.rs @@ -35,7 +35,7 @@ const ENCODE_LIMIT: std::time::Duration = std::time::Duration::from_secs(3600); use crate::error::Failure; use crate::fallback::NotNative; use crate::ffprobe::args::Kind2; -use crate::ffprobe::model::Value; +use crate::ffprobe::model::{Unit, Value}; use crate::ffprobe::{Report, Stream, StreamKind}; use crate::output::{self, WriteFailure}; use crate::split::Pattern; @@ -730,7 +730,7 @@ pub fn plan( "pcm_s16le outside WAV or raw s16le output", )); } - non_pcm_audio_options(command)?; + non_pcm_audio_options(command, &output, &decided)?; if decided.contains(&Some(Codec::Flac)) { return Err(Refusal::not_native("FLAC encoding outside FLAC output")); } @@ -1599,19 +1599,47 @@ fn select_sources( Ok(selected) } -fn non_pcm_audio_options(command: &Command) -> Result<(), Refusal> { - if command.muxer == Muxer::Mp4 { - for (option, given) in [ - ("-ar", command.sample_rate.is_some()), - ("-ac", command.channels.is_some()), - ] { - if given { - return Err(Refusal::NotNative(NotNative::option( - option, - None, - format!("'{option}' is not supported: it is not supported on this output"), - ))); - } +fn non_pcm_audio_options( + command: &Command, + output: &[(usize, &Stream)], + decided: &[Option], +) -> Result<(), Refusal> { + for (option, asked) in [ + ("-ar", command.sample_rate.map(i64::from)), + ("-ac", command.channels.map(i64::from)), + ] { + let Some(asked) = asked else { continue }; + let mut audio = output + .iter() + .zip(decided) + .filter(|((_, stream), _)| stream.kind == StreamKind::Audio) + .peekable(); + if audio.peek().is_none() + || !audio.all(|((_, stream), codec)| { + if !matches!(codec, Some(Codec::Audio(_))) { + return false; + } + if option == "-ac" { + return int(stream, "channels") == asked; + } + if matches!(codec, Some(Codec::Audio(args::AudioEncoder::Libopus))) + && asked != 48_000 + { + return false; + } + stream.entries.iter().any(|(key, value)| { + *key == "sample_rate" + && matches!(value, Value::Unit(rate, Unit::Hertz) if *rate == asked) + }) + }) + { + return Err(Refusal::NotNative(NotNative::option( + option, + None, + format!( + "'{option}' is not supported: native audio encoding requires unchanged sample rate and channels" + ), + ))); } } Ok(()) @@ -1623,7 +1651,7 @@ fn plan_inputs(command: &Command, reports: &[Report]) -> Result { "multiple-input output other than MP4 with default metadata", )); } - non_pcm_audio_options(command)?; + non_pcm_audio_options(command, &[], &[])?; if command.maps.is_empty() { return Err(Refusal::not_native( "automatic stream selection across several inputs", diff --git a/crates/coreav-cli/tests/ffmpeg_audio_options.rs b/crates/coreav-cli/tests/ffmpeg_audio_options.rs new file mode 100644 index 000000000..226da41cc --- /dev/null +++ b/crates/coreav-cli/tests/ffmpeg_audio_options.rs @@ -0,0 +1,369 @@ +//! Native admission for identity audio properties and default AAC bitrate. +#![cfg(all(feature = "native-h264", any(target_os = "linux", target_os = "macos")))] +#![allow(clippy::unwrap_used, clippy::indexing_slicing, clippy::panic)] + +#[path = "../../../test-support/oracle.rs"] +mod test_oracle; + +use std::path::{Path, PathBuf}; +use std::process::{Command, Output}; + +struct Fixture(PathBuf); + +impl Drop for Fixture { + fn drop(&mut self) { + std::fs::remove_dir_all(&self.0).unwrap(); + } +} + +fn success(result: Output) -> Vec { + assert!(result.status.success(), "{result:?}"); + result.stdout +} + +fn policy() -> impl Iterator { + include_str!("../../../scripts/oracle-audio-args.txt").split_whitespace() +} + +fn fixture(name: &str) -> Fixture { + let dir = Path::new(env!("CARGO_TARGET_TMPDIR")) + .join(format!("audio-options-{name}-{}", std::process::id())); + std::fs::create_dir(&dir).unwrap(); + success( + Command::new(test_oracle::require().ffmpeg()) + .args(["-nostdin", "-v", "error", "-f", "lavfi"]) + .args(policy()) + .arg("-i") + .arg("testsrc2=size=32x24:rate=24:duration=0.25") + .args(["-f", "lavfi"]) + .args(policy()) + .arg("-i") + .arg("sine=frequency=701:sample_rate=48000:duration=0.25") + .args(["-f", "lavfi"]) + .args(policy()) + .arg("-i") + .arg("sine=frequency=997:sample_rate=44100:duration=0.25") + .args([ + "-map", + "0:v", + "-map", + "1:a", + "-map", + "2:a", + "-c:v", + "libx264", + "-threads:v", + "1", + "-bf", + "0", + "-c:a", + "flac", + "-ac:a:1", + "2", + "-disposition:a", + "0", + "-strict", + "-2", + ]) + .arg(dir.join("source.mp4")) + .output() + .unwrap(), + ); + Fixture(dir) +} + +fn native(dir: &Path, output: &Path, options: &[&str], strict: bool) -> Output { + let mut command = Command::new(env!("CARGO_BIN_EXE_coreav")); + if strict { + command.arg("--strict-safe"); + } + command + .args(["ffmpeg", "-y"]) + .arg("-i") + .arg(dir.join("source.mp4")) + .args(options) + .arg(output) + .env("COREAV_FFMPEG", test_oracle::require().ffmpeg()) + .env("COREAV_FALLBACK", dir.join("no-whole-command-fallback")) + .env_remove("COREAV_FALLBACK_ACTIVE") + .env_remove("COREAV_STRICT_SAFE") + .output() + .unwrap() +} + +fn audio_properties(path: &Path) -> serde_json::Value { + let bytes = success( + Command::new(test_oracle::require().ffprobe()) + .args(["-v", "error"]) + .args(policy()) + .args(["-select_streams", "a"]) + .args([ + "-show_entries", + "stream=codec_name,sample_rate,channels", + "-of", + "json", + ]) + .arg(path) + .output() + .unwrap(), + ); + serde_json::from_slice(&bytes).unwrap() +} + +#[test] +fn identity_audio_options_and_default_aac_bitrate_run_natively() { + let dir = fixture("encode"); + let mut cases = vec![ + ( + "default-mp4", + "mp4", + "-vn -map 0:a:0 -b:a 96k", + "48000", + "1", + "aac", + ), + ( + "default-m4a", + "m4a", + "-vn -map 0:a:1 -b:a 96k", + "44100", + "2", + "aac", + ), + ( + "explicit", + "mp4", + "-vn -map 0:a:0 -c:a aac -b:a 96k", + "48000", + "1", + "aac", + ), + ( + "last-kind", + "mp4", + "-vn -map 0:a:1 -c copy -c:a aac", + "44100", + "2", + "aac", + ), + ( + "last-all", + "mp4", + "-vn -map 0:a:0 -c:a copy -c aac", + "48000", + "1", + "aac", + ), + ( + "output-index", + "mp4", + "-vn -map 0:a:1 -c copy -c:0 aac", + "44100", + "2", + "aac", + ), + ( + "kind-index", + "mp4", + "-vn -map 0:a:1 -c copy -c:a:0 aac", + "44100", + "2", + "aac", + ), + ( + "unmatched", + "mp4", + "-vn -map 0:a:0 -c:a:1 copy -c:v copy", + "48000", + "1", + "aac", + ), + ( + "overridden-pcm", + "mp4", + "-vn -map 0:a:0 -c:a pcm_s16le -c:a aac", + "48000", + "1", + "aac", + ), + ( + "video-default", + "mp4", + "-map 0:v -map 0:a:0 -threads 1 -b:a 96k", + "48000", + "1", + "aac", + ), + ( + "mp3", + "mp3", + "-vn -map 0:a:1 -c:a libmp3lame -b:a 96k", + "44100", + "2", + "mp3", + ), + ]; + if cfg!(feature = "libopus") { + cases.push(( + "opus", + "mp4", + "-vn -map 0:a:0 -c:a libopus", + "48000", + "1", + "opus", + )); + cases.push(( + "matroska", + "mkv", + "-map 0:v -map 0:a:0 -c:v libx264 -c:a libopus -threads 1", + "48000", + "1", + "opus", + )); + } + for (name, extension, options, rate, channels, codec) in cases { + let options: Vec<_> = options.split_whitespace().collect(); + let control = dir.0.join(format!("{name}-control.{extension}")); + let mut explicit = vec![ + "-c:a", + if codec == "mp3" { + "libmp3lame" + } else if codec == "opus" { + "libopus" + } else { + "aac" + }, + ]; + explicit.extend(&options); + success(native(&dir.0, &control, &explicit, false)); + let reference = dir.0.join(format!("{name}-reference.{extension}")); + success( + Command::new(test_oracle::require().ffmpeg()) + .args(["-nostdin", "-v", "error"]) + .args(policy()) + .arg("-i") + .arg(dir.0.join("source.mp4")) + .args(&options) + .args(["-ar", rate, "-ac", channels]) + .arg(&reference) + .output() + .unwrap(), + ); + let properties = audio_properties(&reference); + assert_eq!(properties["streams"][0]["codec_name"], codec, "{name}"); + assert_eq!(properties["streams"][0]["sample_rate"], rate, "{name}"); + assert_eq!( + properties["streams"][0]["channels"], + channels.parse::().unwrap(), + "{name}" + ); + for identity in [ + vec![], + vec!["-ar", rate], + vec!["-ac", channels], + vec!["-ar", rate, "-ac", channels], + ] { + let actual = dir + .0 + .join(format!("{name}-{}.{}", identity.len(), extension)); + let mut args = options.clone(); + args.extend(identity); + let result = native(&dir.0, &actual, &args, false); + assert!(result.status.success(), "{name} {args:?}: {result:?}"); + assert!(!String::from_utf8_lossy(&result.stderr).contains("Fallback")); + assert_eq!( + std::fs::read(&actual).unwrap(), + std::fs::read(&control).unwrap(), + "{name} {args:?}" + ); + assert_eq!(audio_properties(&actual), properties, "{name} {args:?}"); + } + } +} + +#[test] +fn nonidentity_and_copy_audio_options_preserve_destinations() { + let dir = fixture("refuse"); + let mut cases: Vec> = [ + "-vn -map 0:a:0 -ar 44100", + "-vn -map 0:a:0 -ac 2", + "-vn -map 0:a:1 -ar 48000", + "-vn -map 0:a:1 -ac 1", + "-f ipod -vn -map 0:a:0 -ar 44100", + "-f mp3 -vn -map 0:a:0 -c:a libmp3lame -ac 2", + "-vn -map 0:a:0 -c:a aac -c copy -ar 48000", + "-vn -map 0:a:0 -c:a aac -c:a:0 copy -ac 1", + "-vn -map 0:a:0 -c:a:1 aac -c copy -ar 48000", + "-vn -map 0:a:0 -c:a pcm_s16le -c:a copy -ar 48000", + "-vn -map 0:a:0 -c:v aac -c:a copy -ac 1", + "-map 0:v -an -ar 48000", + "-map 0:v -map 0:a:0 -c:a copy -ac 1", + "-vn -map 0:a:0 -c copy -b:a 96k", + ] + .into_iter() + .map(|options| options.split_whitespace().collect()) + .collect(); + if cfg!(feature = "libopus") { + cases.push(vec![ + "-vn", "-map", "0:a:1", "-c:a", "libopus", "-ar", "44100", + ]); + } + let input = dir.0.join("source.mp4"); + let mut copy = vec![ + "-i", + input.to_str().unwrap(), + "-map", + "0:v", + "-map", + "1:a:0", + "-c", + "copy", + ]; + success(native(&dir.0, &dir.0.join("multiple.mp4"), ©, false)); + copy.extend(["-ar", "48000"]); + cases.push(copy); + cases.push(vec![ + "-i", + input.to_str().unwrap(), + "-map", + "1:a:0", + "-c", + "copy", + "-ac", + "1", + ]); + for options in cases { + for strict in [false, true] { + for existing in [false, true] { + let output = dir.0.join("refused.mp4"); + if existing { + std::fs::write(&output, b"keep destination").unwrap(); + } + let result = native(&dir.0, &output, &options, strict); + assert_eq!(result.status.code(), Some(4), "{options:?}: {result:?}"); + let error = String::from_utf8_lossy(&result.stderr); + let option = if options.contains(&"-ar") { + "'-ar'" + } else { + "'-ac'" + }; + assert!( + error.contains(option) + || error.contains("encoder options") + || strict && error.contains("strict-safe"), + "{options:?}: {error}" + ); + if existing { + assert_eq!(std::fs::read(&output).unwrap(), b"keep destination"); + std::fs::remove_file(&output).unwrap(); + } else { + assert!(!output.exists(), "{options:?}"); + } + assert!(std::fs::read_dir(&dir.0).unwrap().all(|entry| { + let name = entry.unwrap().file_name(); + !name.to_string_lossy().contains(".coreav-") + })); + } + } + } +} diff --git a/docs/agents/ffmpeg-cli.md b/docs/agents/ffmpeg-cli.md index fa2ca2b7d..309a1a538 100644 --- a/docs/agents/ffmpeg-cli.md +++ b/docs/agents/ffmpeg-cli.md @@ -15,7 +15,7 @@ CLI and ffprobe behavior, the FFmpeg pin, and fallback names. | A single-temporal-layer HEVC summary never accepts temporal switching access or stepwise temporal switching access NAL units | The `slice_selects` closed type set excludes TSA/STSA types 2 through 5 at the only admitted TemporalId, zero. The independent all-64-type expectation, failing-before temporal-switching regression and whole-track rejection tests enforce H.265 7.4.2.2. A parsed NAL type alone cannot encode its relationship with the admitted temporal-layer count | | Native `hev1` summaries require a video handler, complete linked out-of-band parameter sets and one selected coded slice in every sample | The common track-admission predicate rejects non-video `hev1` before video-only dispatch; `only_video_handlers_can_qualify_hev1_summaries` fails on the former subtitle bypass and covers subtitle, audio, metadata and unknown handlers. `hevc_summary_config` consumes the VPS/SPS/PPS subset and profile/ordering links; the summary-only `hevc_summary_authority` inspects the entire single-video track with bounded reads. Parameter truncation, tail, selector, late-set, source-error and limit regressions plus generated complete-summary/JSON/fallback comparisons enforce admission. Runtime handler metadata and record identifiers cannot prove track type, complete syntax or the absence of later in-band sets | | Native PCM MP4 retains platform-independent payload coverage without requiring an unregistered oracle | `generated_wave_writes_complete_pcm_mp4_without_an_oracle` verifies every stored sample and the complete extent on every platform. Pinned comparisons compile only for the registered Linux/macOS oracle platforms and still require qualification there; a missing or unverified supported-platform oracle never selects a passing skip | -| A provisional PCM parser exception cannot enable rate or channel options on an effectively non-PCM MP4 job | `non_pcm_audio_options` owns the refusal after the existing authoritative codec selection and at the copy-only multiple-input boundary. Overridden and unmatched audio/index/video selector regressions cover both options, copied and encoded jobs, and both contributing-input positions; effective PCM remains admitted. A parsed option's presence cannot prove which codec decides the selected output stream | +| Rate and channel options on a non-PCM encode must match the selected encoded audio, never an overridden or unmatched codec option | `non_pcm_audio_options` consumes authoritative codec selection and source properties; copy-only multiple-input commands supply no encoded streams. `ffmpeg_audio_options` exercises generated media with mapping, ordered overrides, default/explicit AAC, bitrate and destination preservation; the PCM and multiple-input regressions retain their separate admission. Parsed option presence cannot prove the effective codec or selected source properties | | Audio-only PCM MP4 does not inherit the shared writer's one-second chunk boundary or change its existing defaults | The explicit single-audio chunk mode keeps the true timescale and rejects incompatible modes, tracks and empty chunks before output; the pre-change full-header characterization pins all default bytes and scheduling. `ffmpeg_pcm_mp4` compares complete lossless records, all PCM bytes, packet clocks and terminal extents beyond one second; AAC retains its separate numerical contract. Per-input packet clocks require runtime evidence rather than a codec label | | PCM MP4 publishes only the complete preflight-qualified payload, and late content, decoder and I/O failures cannot restart fallback | The existing raw-audio decoder and staging owner drives both passes through one length/fingerprint tally; source-change, truncation, missing-source, extent-overflow and destination regressions preserve the destination. The fingerprint detects accidental changes, not hostile collisions; `frame_count` bounds the nonempty 32-bit `mdat` before header preparation | | Generated concat output checks bind copied video bitrate declarations to the selected input, not a host-specific encoder result | The shared `concat_output::compare` reads the first input through the existing concat-list reader and independently parses its stored `btrt`; the complete-output test rejects a changed copied bitrate. Encoder output varies by host, so a fixed local value cannot prove metadata preservation | diff --git a/docs/design/transcode.md b/docs/design/transcode.md index 463f23ed4..56b0c3e40 100644 --- a/docs/design/transcode.md +++ b/docs/design/transcode.md @@ -1151,6 +1151,17 @@ encoder is libx264. Explicit copy selection still refuses encoder options, and audio-only encoding still refuses video encoder options. See [FFmpeg's stream handling documentation](https://ffmpeg.org/ffmpeg.html#Stream-handling). +Audio bitrate `-b:a` also accepts the default AAC +encoder for MP4 and M4A. Other encoder options retain their existing gates. + +On audio encode routes, output `-ar` and `-ac` may explicitly repeat the +selected source audio's sample rate and channel count. Validation uses the +effective codec after mapping and ordered codec overrides. Copy-only audio, +absent audio, rate conversion and channel mixing remain unsupported here. +Opus `-ar` is limited to a 48 kHz source because the existing Opus path +always feeds its encoder at 48 kHz. +The separate PCM output admission and multiple-input copy paths are unchanged. + An Opus source can use default AAC encoding. Its current verification uses the separately registered [composed Opus-to-AAC check](composed-opus-aac-v2.md): existing decoder conformance, exact AAC output from the PCM actually written diff --git a/test-support/oracle-dependent.json b/test-support/oracle-dependent.json index 370ffcd35..9e812f490 100644 --- a/test-support/oracle-dependent.json +++ b/test-support/oracle-dependent.json @@ -73,6 +73,10 @@ "crates/coreav-cli/tests/ffmpeg_aac_filter.rs": [ "aac_filter_is_an_exact_concat_identity_or_preserved_fallback" ], + "crates/coreav-cli/tests/ffmpeg_audio_options.rs": [ + "identity_audio_options_and_default_aac_bitrate_run_natively", + "nonidentity_and_copy_audio_options_preserve_destinations" + ], "crates/coreav-cli/tests/ffmpeg_concat.rs": [ "concat_audio_gaps_are_refused_with_exact_pinned_fallback", "lossless_audiovisual_concat_preserves_complete_native_output", From b5cf85fe55fe0f9f79c46f228a3651a773de22b3 Mon Sep 17 00:00:00 2001 From: Gunwoo Kim Date: Sat, 7 Nov 2026 16:04:14 +0900 Subject: [PATCH 2/3] fix(ffmpeg): narrow default audio bitrate admission Signed-off-by: Gunwoo Kim --- crates/coreav-cli/src/ffmpeg/mod.rs | 18 ++++++ .../coreav-cli/tests/ffmpeg_audio_options.rs | 64 +++++++++++++++++++ crates/coreav-cli/tests/transcode.rs | 9 ++- docs/agents/docs.md | 1 + docs/agents/ffmpeg-cli.md | 2 + docs/design/transcode.md | 12 ++-- test-support/oracle-dependent.json | 1 + 7 files changed, 102 insertions(+), 5 deletions(-) diff --git a/crates/coreav-cli/src/ffmpeg/mod.rs b/crates/coreav-cli/src/ffmpeg/mod.rs index d594ec42a..365c81bf2 100644 --- a/crates/coreav-cli/src/ffmpeg/mod.rs +++ b/crates/coreav-cli/src/ffmpeg/mod.rs @@ -1604,6 +1604,24 @@ fn non_pcm_audio_options( output: &[(usize, &Stream)], decided: &[Option], ) -> Result<(), Refusal> { + if command.encoding.audio_bitrate.is_some() + && matches!(command.muxer, Muxer::Mp4 | Muxer::Ipod) + && !command + .codecs + .iter() + .any(|(_, codec)| matches!(codec, Codec::Audio(_) | Codec::AviAudio(_))) + && !output.iter().zip(decided).any(|((_, stream), codec)| { + stream.kind == StreamKind::Audio + && *codec == Some(Codec::Audio(args::AudioEncoder::Aac)) + }) + { + return Err(Refusal::NotNative(NotNative::option( + "-b:a", + None, + "'-b:a' without an explicit audio encoder requires selected default AAC encoding" + .to_owned(), + ))); + } for (option, asked) in [ ("-ar", command.sample_rate.map(i64::from)), ("-ac", command.channels.map(i64::from)), diff --git a/crates/coreav-cli/tests/ffmpeg_audio_options.rs b/crates/coreav-cli/tests/ffmpeg_audio_options.rs index 226da41cc..75762b97c 100644 --- a/crates/coreav-cli/tests/ffmpeg_audio_options.rs +++ b/crates/coreav-cli/tests/ffmpeg_audio_options.rs @@ -281,6 +281,68 @@ fn identity_audio_options_and_default_aac_bitrate_run_natively() { } } +#[test] +fn default_bitrate_admission_preserves_legacy_native_commands() { + let dir = fixture("bitrate"); + let source = dir.0.join("aac-source.mp4"); + success( + Command::new(test_oracle::require().ffmpeg()) + .args(["-nostdin", "-v", "error"]) + .args(policy()) + .arg("-i") + .arg(dir.0.join("source.mp4")) + .args([ + "-map", "0:v", "-map", "0:a:0", "-c:v", "copy", "-c:a", "aac", + ]) + .arg(&source) + .output() + .unwrap(), + ); + std::fs::rename(source, dir.0.join("source.mp4")).unwrap(); + for options in [ + "-c:a aac -c:a copy", + "-c:a:9 aac -c:a copy", + "-an -c:a aac", + "-an -c:a aac -c:a copy", + ] { + let mut args = vec!["-c:v", "libx264", "-threads", "1"]; + args.extend(options.split_whitespace()); + let control = dir.0.join("control.mp4"); + success(native(&dir.0, &control, &args, false)); + args.extend(["-b:a", "96k"]); + let actual = dir.0.join("actual.mp4"); + success(native(&dir.0, &actual, &args, false)); + assert_eq!( + std::fs::read(control).unwrap(), + std::fs::read(actual).unwrap(), + "{options}" + ); + } + for options in ["-c:a copy", "-an", "-c:a:9 copy -an"] { + let mut args = vec!["-c:v", "libx264", "-b:a", "96k"]; + args.extend(options.split_whitespace()); + for existing in [false, true] { + let output = dir.0.join("refused.mp4"); + if existing { + std::fs::write(&output, b"keep destination").unwrap(); + } + let result = native(&dir.0, &output, &args, false); + assert_eq!(result.status.code(), Some(4), "{options}: {result:?}"); + assert!( + String::from_utf8_lossy(&result.stderr) + .contains("'-b:a' without an explicit audio encoder"), + "{result:?}" + ); + if existing { + assert_eq!(std::fs::read(&output).unwrap(), b"keep destination"); + std::fs::remove_file(output).unwrap(); + } else { + assert!(!output.exists()); + } + } + } +} + #[test] fn nonidentity_and_copy_audio_options_preserve_destinations() { let dir = fixture("refuse"); @@ -344,6 +406,8 @@ fn nonidentity_and_copy_audio_options_preserve_destinations() { let error = String::from_utf8_lossy(&result.stderr); let option = if options.contains(&"-ar") { "'-ar'" + } else if options.contains(&"-b:a") { + "'-b:a'" } else { "'-ac'" }; diff --git a/crates/coreav-cli/tests/transcode.rs b/crates/coreav-cli/tests/transcode.rs index 2f49d6244..a30b88926 100644 --- a/crates/coreav-cli/tests/transcode.rs +++ b/crates/coreav-cli/tests/transcode.rs @@ -4226,11 +4226,18 @@ fn ffmpeg_libopus_lines_encode_the_audio() { ), ( vec!["-i", input, "-c:v", "libx264", "-b:a", "64k", out], - "-b:a", + "strict-safe mode never runs the ffmpeg program", ), ] { let (code, stderr) = ffmpeg(&args, true); assert_eq!(code, 4, "{args:?}: {stderr}"); + if !coreav_worker::enforces_memory_limit() { + assert_eq!( + stderr, + "coreav ffmpeg: strict-safe: no enforceable memory limit for the sandbox on this platform, so it starts no sandboxed decoder or encoder, or run without --strict-safe and COREAV_STRICT_SAFE\n" + ); + continue; + } assert!(stderr.contains(says), "{args:?}: {stderr}"); } nothing_left(&dir); diff --git a/docs/agents/docs.md b/docs/agents/docs.md index dfab14e0d..8ea32c40f 100644 --- a/docs/agents/docs.md +++ b/docs/agents/docs.md @@ -4,6 +4,7 @@ Documentation-site prose, schema wording, and design-note updates. | Rule | Enforced by | |---|---| +| An audio-option admission change updates both its explanatory prose and the option table | Not enforced: review judgment. The generated CLI tests establish effective codec and identity behavior, but types cannot establish whether separate prose statements agree | | Final HEVC SIMD guidance requires separate scalar and automatic evidence rather than treating a newest-dispatch verifier as a two-mode check | The owning HEVC tier note defines both modes; the central CI procedure links it and requires independently checked run attempts. `test_manual_c3_failure_blocks_verification_but_scalar_skip_has_no_c3_credit` demonstrates the verifier's scalar limit. Operator verification of both runs remains review judgment | | A weekly full-run drift claim distinguishes re-executing jobs from re-fetching cached pinned inputs | Not enforced: review judgment. Existing acquisition conditions can skip remote downloads on cache hits, so a schedule alone cannot establish fresh remote-input availability | | The documented SVT verification command includes feature-gated process integration targets | `test_documented_checks_include_feature_gated_process_targets` derives required targets from Cargo metadata and checks the published command. Rust types cannot establish arguments in prose; the required AV1 CI job executes that same explicit test selection | diff --git a/docs/agents/ffmpeg-cli.md b/docs/agents/ffmpeg-cli.md index 309a1a538..7bb49d749 100644 --- a/docs/agents/ffmpeg-cli.md +++ b/docs/agents/ffmpeg-cli.md @@ -6,6 +6,8 @@ CLI and ffprobe behavior, the FFmpeg pin, and fallback names. |---|---| | Newly admitted default-video options match the pinned oracle's decoded frames and aspect, not merely the existing explicit encoder path | The MP4 parser keeps implicit scale and bitrate on fallback; `transcode_job` admits implicit output size only when source dimensions match. `ffmpeg_default_video_encoder_applies_options` compares complete frame hashes and geometry/aspect with the oracle and checks unchanged destinations plus byte-exact fallback for excluded paths. Runtime source dimensions and external output cannot be established by argument types | | Generated default-video option coverage treats x264 as optional only outside required verification, without requiring unrelated fixtures | `ffmpeg_default_video_encoder_applies_options` checks the shared `required()` policy when x264 is absent. Its explicit macOS CI step sets `COREAV_REQUIRE_FFMPEG=1` and requires the exact test-success witness. Filesystem availability is a runtime fact, so a type cannot decide whether the selected executable exists | +| New default AAC bitrate admission requires effective selected AAC encoding without removing pre-existing explicit-encoder command admission | `non_pcm_audio_options` checks the resolved audio codec only for the newly admitted implicit-encoder case. `default_bitrate_admission_preserves_legacy_native_commands` covers copied and absent audio, ordered and unmatched overrides, and destination preservation. A syntax type cannot establish selected source streams | +| A strict-safe default AAC bitrate refusal checks its resolved external-encoder policy rather than an obsolete parser diagnostic | `ffmpeg_libopus_lines_encode_the_audio` retains the same command and exit 4 assertion and requires the strict-safe AAC diagnostic; syntax types cannot establish which later policy refusal a command reaches | | Shell oracle consumers retain the complete pair and reject qualification or response failures before creating output, including under macOS's Bash 3.2 | The shared owner's two-line CLI and plain assignment own qualification; guarded array expansions and read loops avoid newer Bash builtins and nounset behavior. Shell regressions exercise malformed responses, retained paths, explicit overrides and audio arguments without `mapfile`; they default to `/bin/bash` and accept `COREAV_TEST_BASH` for legacy-interpreter checks. A path type cannot propagate shell exit status or establish an external interpreter's behavior | | Invocation evidence is exact-site, source-bound and dependency-bound, including import-candidate absence and same-file fold severity | The classifier's single binding owner normalizes checkout roots and binds reviewed inward links to their literal text and tracked regular targets. Its schema validator and freshness controls reject changed targets, retargeting, outward or indirect links, stale files, import shadows, missing relay dependencies, duplicate or dead entries and weaker fold origins. Observer-copy status prevents a development source from inheriting the candidate's registered-source exclusion. The closure, source-span and proposal regressions exercise these external-file facts; a schema type alone cannot establish current checkout bytes | | Invocation discovery normalizes its repository root and never opens missing index entries or nonregular gitlinks as source files | `inventory` owns root resolution and regular-file selection; the classifier retains missing and nonregular entries as unresolved closure rows. Symlink-root, deleted-file and gitlink regressions cover the public inventory boundary. A path type cannot establish the current filesystem kind | diff --git a/docs/design/transcode.md b/docs/design/transcode.md index 56b0c3e40..d00448564 100644 --- a/docs/design/transcode.md +++ b/docs/design/transcode.md @@ -1151,8 +1151,12 @@ encoder is libx264. Explicit copy selection still refuses encoder options, and audio-only encoding still refuses video encoder options. See [FFmpeg's stream handling documentation](https://ffmpeg.org/ffmpeg.html#Stream-handling). -Audio bitrate `-b:a` also accepts the default AAC -encoder for MP4 and M4A. Other encoder options retain their existing gates. +Audio bitrate `-b:a` also accepts the default AAC encoder for MP4 and M4A +only when selected audio is actually encoded as AAC. Commands with an explicit +audio encoder option retain their earlier admission, including overridden or +unmatched choices. This does not add bitrate metadata overrides for copied +audio; those declarations can differ from the pinned FFmpeg. Other encoder +options retain their existing gates. On audio encode routes, output `-ar` and `-ac` may explicitly repeat the selected source audio's sample rate and channel count. Validation uses the @@ -1182,8 +1186,8 @@ explicit AAC, and default libx264 can use only the x264 program. | `-c:a`, `-acodec` `copy`, or `-c copy` for the other streams | the selected audio streams copied (`AudioMode::Streams`) | | `-c:v:N copy`, or `-c copy`, for a selected MP4/MOV cover picture | the cover picture kept in the output's item list, unchanged; a cover picture the selection leaves out (the default selection does) is left out (`TranscodeOptions::left_out_covers`, [`compat-cli.md`](compat-cli.md), "Cover pictures"), and one the command would encode with libx264 is refused (`ffmpeg` fails on it) | | `-c:a`, `-acodec` `libopus` | `--audio opus` for the one selected audio stream ("Opus audio"), beside a video encoded with libx264; refused when other audio is selected beside it, for audio this build does not decode (FLAC, MP3, Opus, or AAC-LC with `aac`), for more than two channels (`ffmpeg` writes a surround Opus stream), and without `-c:v libx264`. In a build without the `libopus` feature every `-c:a libopus` line is not native: FFmpeg runs it, or strict-safe mode refuses it and nothing runs | -| `-c:a`, `-acodec` `aac` | `--audio aac` for the one selected audio stream ("AAC audio"), beside a video encoded with libx264, by FFmpeg's built-in `aac` encoder: the `ffmpeg` program `--audio aac` finds without `--ffmpeg` (`COREAV_FFMPEG`, else `PATH`), run as a process of its own. Every build has it. Refused when other audio is selected beside it, for audio this build does not decode (FLAC, MP3, Opus, or AAC-LC with `aac`), for more than two channels (`ffmpeg` writes a surround AAC stream), for a sample rate outside 8 to 64 kHz (nothing is resampled; `ffmpeg` encodes 88.2 and 96 kHz as they are), without `-c:v libx264`, when no `ffmpeg` program is found or it has no `aac` encoder (`--audio aac`'s refusals), and always in strict-safe mode. Its other options (`-profile:a`, `-aac_coder`, `-q:a`, `-ar`, `-ac`, `-af`, `-strict`) and other AAC encoders (`libfdk_aac`) are refused as unlisted | -| `-b:a RATE` | `--audio-bitrate`: FFmpeg's number syntax, in bits a second, 6k to 510k with `-c:a libopus` as libopus takes it, 16k to 512k with `-c:a aac`; without it, the encoder's default; only with `-c:a libopus` or `aac` | +| `-c:a`, `-acodec` `aac` | `--audio aac` for the one selected audio stream ("AAC audio"), beside a video encoded with libx264, by FFmpeg's built-in `aac` encoder: the `ffmpeg` program `--audio aac` finds without `--ffmpeg` (`COREAV_FFMPEG`, else `PATH`), run as a process of its own. Every build has it. Refused when other audio is selected beside it, for audio this build does not decode (FLAC, MP3, Opus, or AAC-LC with `aac`), for more than two channels (`ffmpeg` writes a surround AAC stream), for a sample rate outside 8 to 64 kHz (nothing is resampled; `ffmpeg` encodes 88.2 and 96 kHz as they are), without `-c:v libx264`, when no `ffmpeg` program is found or it has no `aac` encoder (`--audio aac`'s refusals), and always in strict-safe mode. Output `-ar` and `-ac` may repeat the selected source values as described above. Its other options (`-profile:a`, `-aac_coder`, `-q:a`, `-af`, `-strict`) and other AAC encoders (`libfdk_aac`) are refused as unlisted | +| `-b:a RATE` | `--audio-bitrate`: FFmpeg's number syntax, in bits a second, 6k to 510k with `-c:a libopus` as libopus takes it, 16k to 512k with explicit or default AAC; without it, the encoder's default; audio encoding only | | `-an` | no audio | | `-crf Q` | `--crf`, 0 to 51, fractions as given | | `-q:v Q` | ignored on the effective libx264 video encode route, including MP4's default codec, as in the pinned FFmpeg; only finite decimal numbers, optionally with an exponent, are admitted, not expressions or nonfinite values. Other encoders, copy and audio-only outputs still refuse it | diff --git a/test-support/oracle-dependent.json b/test-support/oracle-dependent.json index 9e812f490..523a2c3f1 100644 --- a/test-support/oracle-dependent.json +++ b/test-support/oracle-dependent.json @@ -75,6 +75,7 @@ ], "crates/coreav-cli/tests/ffmpeg_audio_options.rs": [ "identity_audio_options_and_default_aac_bitrate_run_natively", + "default_bitrate_admission_preserves_legacy_native_commands", "nonidentity_and_copy_audio_options_preserve_destinations" ], "crates/coreav-cli/tests/ffmpeg_concat.rs": [ From 04206f4d03ca88fa3254639c348b5233ba98a766 Mon Sep 17 00:00:00 2001 From: Gunwoo Kim Date: Sat, 7 Nov 2026 18:43:17 +0900 Subject: [PATCH 3/3] test(ffmpeg): require the full AAC refusal Signed-off-by: Gunwoo Kim --- crates/coreav-cli/tests/transcode.rs | 8 ++++++-- docs/agents/ffmpeg-cli.md | 2 +- 2 files changed, 7 insertions(+), 3 deletions(-) diff --git a/crates/coreav-cli/tests/transcode.rs b/crates/coreav-cli/tests/transcode.rs index a30b88926..33d2b00d8 100644 --- a/crates/coreav-cli/tests/transcode.rs +++ b/crates/coreav-cli/tests/transcode.rs @@ -4226,7 +4226,7 @@ fn ffmpeg_libopus_lines_encode_the_audio() { ), ( vec!["-i", input, "-c:v", "libx264", "-b:a", "64k", out], - "strict-safe mode never runs the ffmpeg program", + "coreav ffmpeg: strict-safe mode never runs the ffmpeg program, so it cannot encode AAC audio: use -c:a copy or libopus, or run without --strict-safe and COREAV_STRICT_SAFE\n", ), ] { let (code, stderr) = ffmpeg(&args, true); @@ -4238,7 +4238,11 @@ fn ffmpeg_libopus_lines_encode_the_audio() { ); continue; } - assert!(stderr.contains(says), "{args:?}: {stderr}"); + if says.starts_with("coreav ffmpeg: ") { + assert_eq!(stderr, says, "{args:?}: {stderr}"); + } else { + assert!(stderr.contains(says), "{args:?}: {stderr}"); + } } nothing_left(&dir); } diff --git a/docs/agents/ffmpeg-cli.md b/docs/agents/ffmpeg-cli.md index 7bb49d749..69b061ca4 100644 --- a/docs/agents/ffmpeg-cli.md +++ b/docs/agents/ffmpeg-cli.md @@ -7,7 +7,7 @@ CLI and ffprobe behavior, the FFmpeg pin, and fallback names. | Newly admitted default-video options match the pinned oracle's decoded frames and aspect, not merely the existing explicit encoder path | The MP4 parser keeps implicit scale and bitrate on fallback; `transcode_job` admits implicit output size only when source dimensions match. `ffmpeg_default_video_encoder_applies_options` compares complete frame hashes and geometry/aspect with the oracle and checks unchanged destinations plus byte-exact fallback for excluded paths. Runtime source dimensions and external output cannot be established by argument types | | Generated default-video option coverage treats x264 as optional only outside required verification, without requiring unrelated fixtures | `ffmpeg_default_video_encoder_applies_options` checks the shared `required()` policy when x264 is absent. Its explicit macOS CI step sets `COREAV_REQUIRE_FFMPEG=1` and requires the exact test-success witness. Filesystem availability is a runtime fact, so a type cannot decide whether the selected executable exists | | New default AAC bitrate admission requires effective selected AAC encoding without removing pre-existing explicit-encoder command admission | `non_pcm_audio_options` checks the resolved audio codec only for the newly admitted implicit-encoder case. `default_bitrate_admission_preserves_legacy_native_commands` covers copied and absent audio, ordered and unmatched overrides, and destination preservation. A syntax type cannot establish selected source streams | -| A strict-safe default AAC bitrate refusal checks its resolved external-encoder policy rather than an obsolete parser diagnostic | `ffmpeg_libopus_lines_encode_the_audio` retains the same command and exit 4 assertion and requires the strict-safe AAC diagnostic; syntax types cannot establish which later policy refusal a command reaches | +| A strict-safe default AAC bitrate refusal checks its resolved external-encoder policy rather than an obsolete parser diagnostic | `ffmpeg_libopus_lines_encode_the_audio` retains the same command and exit 4 assertion and requires the complete AAC stderr, so an earlier x264 refusal with the shared FFmpeg prefix cannot pass. Unsupported hosts retain the exact memory-limit refusal. Syntax types cannot establish which runtime policy refusal a command reaches | | Shell oracle consumers retain the complete pair and reject qualification or response failures before creating output, including under macOS's Bash 3.2 | The shared owner's two-line CLI and plain assignment own qualification; guarded array expansions and read loops avoid newer Bash builtins and nounset behavior. Shell regressions exercise malformed responses, retained paths, explicit overrides and audio arguments without `mapfile`; they default to `/bin/bash` and accept `COREAV_TEST_BASH` for legacy-interpreter checks. A path type cannot propagate shell exit status or establish an external interpreter's behavior | | Invocation evidence is exact-site, source-bound and dependency-bound, including import-candidate absence and same-file fold severity | The classifier's single binding owner normalizes checkout roots and binds reviewed inward links to their literal text and tracked regular targets. Its schema validator and freshness controls reject changed targets, retargeting, outward or indirect links, stale files, import shadows, missing relay dependencies, duplicate or dead entries and weaker fold origins. Observer-copy status prevents a development source from inheriting the candidate's registered-source exclusion. The closure, source-span and proposal regressions exercise these external-file facts; a schema type alone cannot establish current checkout bytes | | Invocation discovery normalizes its repository root and never opens missing index entries or nonregular gitlinks as source files | `inventory` owns root resolution and regular-file selection; the classifier retains missing and nonregular entries as unresolved closure rows. Symlink-root, deleted-file and gitlink regressions cover the public inventory boundary. A path type cannot establish the current filesystem kind |