diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index baebe8b98..b9ccd4244 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -1287,6 +1287,14 @@ jobs: --target-dir target/fallback cargo test --locked --release -p coreav-cli --features ffmpeg-fallback,dav1d,libvpx,libopus,aac --test mp3_worker \ --target-dir target/fallback + - name: Check the AAC worker encoder-only protocol + env: + COREAV_FFMPEG: ${{ runner.temp }}/coreav-oracle-bin/ffmpeg + run: | + cargo test --locked --release -p coreav-cli --features ffmpeg-fallback,dav1d,libvpx,libopus --test aac_worker \ + --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: Copied video with AAC encoding preserves exact fallback env: COREAV_REQUIRE_FFMPEG: '1' diff --git a/crates/coreav-cli/tests/aac_worker.rs b/crates/coreav-cli/tests/aac_worker.rs new file mode 100644 index 000000000..767789c87 --- /dev/null +++ b/crates/coreav-cli/tests/aac_worker.rs @@ -0,0 +1,32 @@ +//! Actual AAC worker starts and generated output checked against the pinned oracle. +#![cfg(all(unix, feature = "transcode"))] +#![allow(clippy::unwrap_used, clippy::panic)] + +#[path = "../../../test-support/oracle.rs"] +mod test_oracle; + +#[test] +fn aac_worker_starts_and_output_pass_the_frozen_checks() { + let oracle = test_oracle::require(); + let root = std::path::Path::new(env!("CARGO_MANIFEST_DIR")).join("../.."); + let work = std::path::Path::new(env!("CARGO_TARGET_TMPDIR")).join("aac-worker-protocol"); + if work.exists() { + std::fs::remove_dir_all(&work).unwrap(); + } + let result = std::process::Command::new("python3") + .arg(root.join("scripts/aac_worker_check.py")) + .args(["--coreav", env!("CARGO_BIN_EXE_coreav"), "--work"]) + .arg(&work) + .env("COREAV_FFMPEG", oracle.ffmpeg()) + .env("COREAV_FFPROBE", oracle.ffprobe()) + .output() + .unwrap(); + assert!( + result.status.success(), + "{}{}", + String::from_utf8_lossy(&result.stdout), + String::from_utf8_lossy(&result.stderr) + ); + print!("{}", String::from_utf8_lossy(&result.stdout)); + std::fs::remove_dir_all(work).unwrap(); +} diff --git a/crates/coreav-cli/tests/transcode.rs b/crates/coreav-cli/tests/transcode.rs index 8486e8547..39844636a 100644 --- a/crates/coreav-cli/tests/transcode.rs +++ b/crates/coreav-cli/tests/transcode.rs @@ -3408,13 +3408,21 @@ fn audio_encodes_preserve_the_decoders_presented_samples() { &dir, "capture-ffmpeg", &format!( - "case \" $* \" in *' -h '*) exec '{}' \"$@\";; esac\n/usr/bin/cmp - '{}' || exit 1\nexec '{}' \"$@\" < '{}'", - tools.ffmpeg.display(), + "exec /usr/bin/python3 -I -B -c 'import pathlib, subprocess, sys\ndata = sys.stdin.buffer.read()\nif data and data != pathlib.Path(sys.argv[1]).read_bytes():\n print(\"PCM mismatch\", file=sys.stderr)\n sys.exit(1)\nsys.exit(subprocess.run([sys.argv[2], *sys.argv[3:]], input=data).returncode)' '{}' '{}' \"$@\"", capture.display(), - tools.ffmpeg.display(), - capture.display() + tools.ffmpeg.display() ), ); + let mut wrong = expected.0.clone(); + wrong[0] ^= 1; + let rejected = bounded::output( + &mut Command::new(&wrapper), + Some(wrong), + std::time::Duration::from_secs(5), + ) + .unwrap(); + assert_eq!(rejected.status.code(), Some(1)); + assert_eq!(rejected.stderr, b"PCM mismatch\n"); let mut modes = vec![("aac", "aac")]; if cfg!(feature = "libopus") { modes.push(("opus", "libopus")); @@ -3850,7 +3858,7 @@ fn audio_the_aac_encode_does_not_take_is_refused_first() { let without = script( &dir, "ffmpeg-without-aac", - "echo \"Codec 'aac' is not recognized by FFmpeg.\"", + "printf \"Unknown encoder 'aac'\\n\" >&2; exit 1", ); let flac = made(&tools, &dir, "flac.mp4", &["-c:a", "flac"]); for (name, source, ffmpeg, says) in [ @@ -4126,7 +4134,7 @@ fn ffmpeg_aac_lines_encode_the_audio() { let without = script( &dir, "ffmpeg-without-aac", - "echo \"Codec 'aac' is not recognized by FFmpeg.\"", + "printf \"Unknown encoder 'aac'\\n\" >&2; exit 1", ); let line = |source: &Path, extra: &[&'static str]| { let mut args = vec!["-i".to_owned(), source.to_str().unwrap().to_owned()]; diff --git a/crates/coreav-transcode/src/aac.rs b/crates/coreav-transcode/src/aac.rs index 56f81dfe6..286219032 100644 --- a/crates/coreav-transcode/src/aac.rs +++ b/crates/coreav-transcode/src/aac.rs @@ -134,19 +134,6 @@ pub fn encode_args(rate: u32, channels: u8, bitrate: Option) -> Vec Vec { - ["-hide_banner", "-loglevel", "error", "-h", "encoder=aac"] - .map(OsString::from) - .to_vec() -} - -/// Whether the program's answer to [`probe_args`] describes the `aac` -/// encoder; a program without it says it does not know the codec. -pub fn has_encoder(answer: &[u8]) -> bool { - answer.starts_with(b"Encoder aac [") -} - /// The most output that `samples` samples per channel of `channels` /// make: as many frames as cover the largest delay and the samples, and /// one more, each of the largest size, with their Matroska. @@ -421,9 +408,6 @@ mod tests { -map 0:a:0 -c:a aac -f matroska pipe:1" ); assert!(args(Some(64_000)).contains(" -c:a aac -b:a 64000 -f matroska ")); - assert!(has_encoder(b"Encoder aac [AAC (Advanced Audio Coding)]:\n")); - assert!(!has_encoder(b"Codec 'aac' is not recognized by FFmpeg.\n")); - assert!(!has_encoder(b"")); } #[test] diff --git a/crates/coreav-transcode/src/audio.rs b/crates/coreav-transcode/src/audio.rs index 87a1d1e75..a7f31b157 100644 --- a/crates/coreav-transcode/src/audio.rs +++ b/crates/coreav-transcode/src/audio.rs @@ -56,7 +56,7 @@ const ENCODER_MEMORY: u64 = 1 << 30; /// libraries it maps. const AAC_ENCODER_MEMORY: u64 = 2 << 30; -/// Bytes of the `ffmpeg` program's answer about its `aac` encoder kept. +/// Bytes retained from an encoder availability encode. const PROBE_ANSWER: usize = 64 << 10; /// The default bit rate in bits a second, for `channels` (1 or 2): 64,000 @@ -749,23 +749,22 @@ fn encode_with( }) } -/// Asks the `ffmpeg` program about its requested encoder, as it runs as the -/// encoder (through `launcher`, killed at `deadline`): one without it is -/// refused before the source is decoded. +/// Checks the requested encoder with an empty raw-PCM encode through +/// `launcher`, killed at `deadline`, before decoding the source. fn probe_encoder( program: &Path, launcher: Option<&ExecLauncher>, deadline: Instant, codec: &str, ) -> Result<(), Error> { - struct Answer(Vec, bool); + struct Answer(Vec); impl Sink for Answer { fn write(&mut self, bytes: &[u8]) -> Result<(), coreav_decode::Error> { let room = PROBE_ANSWER.saturating_sub(self.0.len()); - if self.1 && bytes.len() > room { + if bytes.len() > room { return Err(coreav_decode::Error::new( ErrorKind::Limit, - "the MP3 encoder probe exceeded its output limit", + "the audio encoder probe exceeded its output limit", )); } self.0 @@ -776,15 +775,12 @@ fn probe_encoder( let args = if codec == "libmp3lame" { mp3::encode_args(48000, 1, mp3::Mode::Constant(None)) } else { - [ - "-hide_banner", - "-loglevel", - "error", - "-h", - &format!("encoder={codec}"), - ] - .map(std::ffi::OsString::from) - .to_vec() + aac::encode_args(48000, 1, None) + }; + let name = if codec == "aac" { + "the AAC encoder" + } else { + "the MP3 encoder" }; let command = command( program, @@ -794,44 +790,45 @@ fn probe_encoder( &EncoderLimits::new(AAC_ENCODER_MEMORY), deadline, ); - let Answer(answer, _) = Process::spawn( - if codec == "aac" { - "the AAC encoder" - } else { - "the MP3 encoder" - }, + let Answer(answer) = Process::spawn( + name, "sample blocks", command, program, deadline, - Answer(Vec::new(), codec == "libmp3lame"), + Answer(Vec::new()), ) .and_then(Process::finish) .map_err(|error| { - if codec == "libmp3lame" - && error.kind == ErrorKind::Backend + if error.kind == ErrorKind::Backend + && error + .message + .starts_with(&format!("{name} failed (exit status: ")) && error .message - .starts_with("the MP3 encoder failed (exit status: ") - && error.message.contains("Unknown encoder 'libmp3lame'") + .contains(&format!("Unknown encoder '{codec}'")) { - Error::Unsupported("the ffmpeg program has no libmp3lame encoder".into()) + Error::Unsupported(format!("the ffmpeg program has no {codec} encoder")) } else { Error::Encode(error) } })?; - if codec == "libmp3lame" && !answer.is_empty() { - if !answer.starts_with(b"ID3\x04\0\0") { - return Err(encoder_error("invalid MP3 encoder probe output".into())); - } - return Ok(()); - } - if !answer.starts_with(format!("Encoder {codec} [").as_bytes()) { + if answer.is_empty() { return Err(Error::Unsupported(format!( "the ffmpeg program {} has no {codec} encoder; install FFmpeg with that encoder", program.display() ))); } + let signature: &[u8] = if codec == "aac" { + b"\x1a\x45\xdf\xa3" + } else { + b"ID3\x04\0\0" + }; + if !answer.starts_with(signature) { + return Err(encoder_error(format!( + "invalid {codec} encoder probe output" + ))); + } Ok(()) } @@ -1595,12 +1592,12 @@ mod tests { for (name, body, says) in [ ( "with", - "echo 'Encoder aac [AAC (Advanced Audio Coding)]:'", + "cat >/dev/null; printf '\\032\\105\\337\\243'", None, ), ( "without", - "echo \"Codec 'aac' is not recognized by FFmpeg.\"", + "printf \"Unknown encoder 'aac'\\n\" >&2; exit 1", Some("has no aac encoder"), ), ("silent", "true", Some("has no aac encoder")), @@ -1617,6 +1614,53 @@ mod tests { } } + #[cfg(unix)] + #[test] + fn aac_availability_errors_keep_their_classification_and_output_bound() { + for (name, body, unsupported) in [ + ("aac-silent", "true", true), + ( + "aac-unavailable-status-eight", + "printf \"Unknown encoder 'aac'\\n\" >&2; exit 8", + true, + ), + ( + "aac-unknown-before-flood", + "printf \"Unknown encoder 'aac'\\n\" >&2; exec yes", + false, + ), + ( + "aac-unknown-before-signal", + "printf \"Unknown encoder 'aac'\\n\" >&2; kill -TERM $$", + false, + ), + ("aac-invalid", "printf bad", false), + ("aac-flood", "exec yes", false), + ("aac-failure", "echo 'other failure' >&2; exit 1", false), + ] { + let (dir, script) = ffmpeg_stand_in(name, body); + let result = probe_encoder( + &script, + None, + Instant::now() + Duration::from_secs(5), + "aac", + ); + assert!(result.is_err(), "{name}"); + assert_eq!( + matches!(result, Err(Error::Unsupported(_))), + unsupported, + "{name}" + ); + if name.contains("flood") { + assert!( + matches!(result, Err(Error::Encode(ref error)) if error.kind == ErrorKind::Limit), + "{name}: {result:?}" + ); + } + std::fs::remove_dir_all(dir).unwrap(); + } + } + #[cfg(unix)] #[test] fn mp3_availability_errors_keep_their_classification_and_output_bound() { diff --git a/docs/agents/transcode.md b/docs/agents/transcode.md index 3a6ff9711..8dbd8a0d9 100644 --- a/docs/agents/transcode.md +++ b/docs/agents/transcode.md @@ -16,6 +16,8 @@ Encode path, sandboxed workers, and audio-encode selection. | MP3 encoding validates the full PCM count including its EOF zero guard, then removes that explicit guard in the gapless tag | The shared audio feeder counts original and fed samples separately; `mp3::read_output` requires the frame-aligned libmp3lame flush count and documented default encoder delay. `ffmpeg_mp3_eof_guard_preserves_exact_boundary_lengths` checks seekable oracle decoding and ffprobe at residues 1, 46 and 47 in all three MPEG rate families; malformed, truncated and out-of-bound output is covered by `mp3` unit tests and the `mp3_output` fuzz target | | MP3 encoder readback audits the entire spool and rejects extra metadata, information frames and changed framing under the encode deadline | `mp3::read_output` owns the strict ID3 and complete-frame walk without resynchronization. `full_output_structure_auxiliary_data_and_timestamps_are_checked` covers appended bytes, metadata mutations, information frames, changed channels and bit rates. `TimedSource` checks cached parser reads too; `readback_checks_the_deadline_inside_parser_reads` requires timeout classification and at most one underlying read after a controlled deadline crossing | | An external MP3 encode starts only encoder-only MP3 commands, with no help probe or external container adaptation | The availability probe and MP3 feeder share the argument owner; `mp3_worker_starts_and_output_pass_the_frozen_checks` observes every sandboxed start with the existing FIFO logger, applies the unchanged frozen classifier and compares complete decoded output and timing with the pinned oracle in both source builds (`ci.yml`). Static argument types cannot prove actual external process starts | +| AAC availability starts only an empty raw-PCM encode and retains the worker's limits and terminal failure classes | `probe_encoder` shares the AAC feeder's argument owner and bounds both codec probes to 64 KiB. `aac_worker_starts_and_output_pass_the_frozen_checks` observes every actual start through the unchanged classifier and compares complete decoded output and frame timing on generated lossless sources in both source builds. The availability regressions preserve missing-encoder classification, signal failures and output-limit errors. Runtime subprocess starts and exits cannot be established by an argument vector's type | +| AAC PCM-verification stand-ins accept empty availability input without substituting source samples, and still reject changed nonempty input | `audio_encodes_preserve_the_decoders_presented_samples` passes each actual stdin byte to FFmpeg, compares nonempty input with the native decoder output and proves rejection with a one-byte mutation. Missing-encoder stand-ins use the real nonzero-exit diagnostic. A subprocess argument type cannot establish its stdin bytes or an external diagnostic | | A missing external MP3 encoder refuses on its diagnostic after a normal nonzero exit, without assuming exit status 1 or converting resource and signal failures into refusals | `probe_encoder` owns the classification; `mp3_availability_errors_keep_their_classification_and_output_bound` checks statuses 1 and 8 and preserves terminal flood and signal failures even when their stderr contains the missing-encoder diagnostic. The external program's exit convention and diagnostic require runtime checks rather than a static type | | A failed B9 baseline preserves explicit `-an` and otherwise retains the remux audio expectation; it supplies no quality evidence | `check_compat` derives explicit audio absence from its command tokens. The baseline-failure regressions in `scripts/transcode_check_selftest.py` check both expectations, rejection of unexpected audio and failed Q1/Q2 judgments. External command outcomes need runtime checks | | Audio encodes take the first source stream the build decodes, or the explicitly selected stream, applying source trimming once before output encoder delay | `coreav_audio::decode` owns stream selection and presentation trimming; `Codec::from_name` supplies the CLI's build-aware capability check. The AAC-only, Opus-only, Opus-before-FLAC and timing cases in `coreav-cli/tests/transcode.rs` run with and without `aac` (`ci.yml`) | diff --git a/docs/design/transcode.md b/docs/design/transcode.md index d3000df89..4410d8242 100644 --- a/docs/design/transcode.md +++ b/docs/design/transcode.md @@ -829,6 +829,14 @@ without it the encoder's own default applies, which the summary gives as **Pipeline.** As for Opus, the audio is encoded before the video: +AAC availability is checked before source decoding with an empty 48 kHz +mono raw-PCM encode through the same argument owner and sandbox as the +real encode. Both starts are encoder-only invocations, never encoder-help +queries. The probe retains at most 64 KiB and requires a successful exit +with an EBML header. A normal nonzero exit naming the missing AAC encoder +is unsupported; malformed output, resource failures and signals remain +terminal errors. The real encode retains its complete Matroska readback. + 1. `coreav-audio`'s `decode` reads the stream as it does for Opus (the first audio stream this build decodes, with container trimming applied) to 32-bit float at the stream's own rate: nothing is resampled. More than diff --git a/scripts/aac_worker_check.py b/scripts/aac_worker_check.py new file mode 100644 index 000000000..4e7975890 --- /dev/null +++ b/scripts/aac_worker_check.py @@ -0,0 +1,68 @@ +"""Check actual AAC worker starts with the unchanged compatibility classifier.""" +import argparse +import os +from pathlib import Path +import subprocess +import sys + +ROOT = Path(__file__).resolve().parents[1] +sys.path[:0] = [str(ROOT / 'tools/compat-corpus'), str(ROOT / 'scripts')] +import decoded_output +import encode_breadth +import test_oracle + + +def run(args, env=None): + result = subprocess.run(list(map(str, args)), env=env, capture_output=True, timeout=60) + if result.returncode: + raise AssertionError(f'command exited {result.returncode}: {result.stderr.decode(errors="replace")}') + return result + + +def main(): + parser = argparse.ArgumentParser(description=__doc__) + parser.add_argument('--coreav', type=Path, required=True) + parser.add_argument('--work', type=Path, required=True) + args = parser.parse_args() + oracle = test_oracle.require() + args.work.mkdir(parents=True, exist_ok=True) + log = encode_breadth.ProgramLog(args.work / 'programs', oracle.ffmpeg, oracle.ffmpeg) + env = {**os.environ, 'COREAV_FFMPEG': str(log.wrappers['ffmpeg']), + 'COREAV_FALLBACK': '/nonexistent/coreav-aac-worker-fallback', + 'COREAV_STRICT_SAFE': '0'} + checked = 0 + try: + for rate, channels in ((48000, 1), (44100, 2)): + source = args.work / f'source-{rate}-{channels}.mp4' + run([oracle.ffmpeg, '-nostdin', '-v', 'error', '-f', 'lavfi', '-i', + f'sine=frequency=440:sample_rate={rate}', '-af', 'atrim=end_sample=4992', + '-ac', channels, '-c:a', 'flac', source]) + for mode, options, extension in ( + ('implicit', [], 'mp4'), ('explicit', ['-c:a', 'aac'], 'mp4'), + ('audio-only', ['-vn', '-c:a', 'aac', '-b:a', '96k'], 'm4a')): + name = f'{rate}-{channels}-{mode}' + candidate, reference = [args.work / f'{name}-{side}.{extension}' + for side in ('candidate', 'reference')] + run([args.coreav, 'ffmpeg', '-i', source, *options, candidate], env) + starts = log.drain() + assert len(starts) == 2, (name, starts) + assert all(start['program'] == 'ffmpeg' and + encode_breadth.aac_encoder_start(start['args']) and + encode_breadth.encoder_start(start['program'], start['args']) + for start in starts), (name, starts) + run([oracle.ffmpeg, '-nostdin', '-v', 'error', '-i', source, *options, reference]) + scratch = args.work / f'{name}-comparison' + scratch.mkdir() + verdict = decoded_output.compare(candidate, reference, oracle.ffmpeg, + oracle.ffprobe, os.environ, scratch, + registered_codecs=('aac',), + exact_frame_timeline=True) + assert verdict == ('verified', 'exact decoded output'), (name, verdict) + checked += 1 + finally: + log.close() + print(f'{checked} AAC commands: both encoder starts accepted; exact decoded output and timing') + + +if __name__ == '__main__': + main() diff --git a/test-support/oracle-dependent.json b/test-support/oracle-dependent.json index ccdb5bdd7..1a45d189e 100644 --- a/test-support/oracle-dependent.json +++ b/test-support/oracle-dependent.json @@ -37,6 +37,9 @@ "stated_sizes_are_the_decoded_sizes", "vp8_key_frames_state_the_decoded_size" ], + "crates/coreav-cli/tests/aac_worker.rs": [ + "aac_worker_starts_and_output_pass_the_frozen_checks" + ], "crates/coreav-cli/tests/decode_audio.rs": [ "decodes_an_mp4_aac_track_as_ffmpeg_does" ],