From 50dd69ce32ad5e539dcacfd71fc01c3b7d39b696 Mon Sep 17 00:00:00 2001 From: Akash Bhat Date: Mon, 12 Oct 2026 13:02:48 +0530 Subject: [PATCH] fix(apv): skip oracle tests when ffmpeg predates APV support The APV demuxer is new in ffmpeg 8.0, so ubuntu-24.04 runners failed instead of skipping. testkit gains ffmpeg_with_format_or_skip: when ffmpeg -formats does not list apv it SKIPs naming the detected version, failing only under FILMCRAFT_REQUIRE_ORACLES=1. The apv oracle uses it; docs/testing.md notes ffmpeg >= 8.0. Fixes #1249. --- crates/apv/tests/oracle.rs | 4 +- crates/testkit/src/lib.rs | 4 +- crates/testkit/src/oracle.rs | 87 ++++++++++++++++++++++++++++++++++++ docs/testing.md | 2 + 4 files changed, 95 insertions(+), 2 deletions(-) diff --git a/crates/apv/tests/oracle.rs b/crates/apv/tests/oracle.rs index f23c32b5..8b9b87a1 100644 --- a/crates/apv/tests/oracle.rs +++ b/crates/apv/tests/oracle.rs @@ -8,7 +8,9 @@ use filmcraft_apv::{ }; fn ffmpeg() -> Option { - filmcraft_testkit::ffmpeg_or_skip("apv oracle") + // The APV demuxer is new in ffmpeg 8.0; older toolchains are found but + // cannot read the fixtures, so they skip like a missing tool (#1249). + filmcraft_testkit::ffmpeg_with_format_or_skip("apv oracle", "apv", "8.0") } fn fixture_dir() -> PathBuf { diff --git a/crates/testkit/src/lib.rs b/crates/testkit/src/lib.rs index 867df7b3..9846d907 100644 --- a/crates/testkit/src/lib.rs +++ b/crates/testkit/src/lib.rs @@ -16,7 +16,9 @@ pub mod oracle; pub mod wav; pub use fixtures::{fixtures_dir, temp_path, workspace_root}; -pub use oracle::{ffmpeg, ffmpeg_or_skip, ffprobe, ffprobe_or_skip, oracles_required, skip}; +pub use oracle::{ + ffmpeg, ffmpeg_or_skip, ffmpeg_supports_format, ffmpeg_version, ffmpeg_with_format_or_skip, ffprobe, ffprobe_or_skip, oracles_required, skip, +}; /// `let ff = require_ffmpeg!();` — the ffmpeg path, or return from the test after printing /// `SKIPPED: … ffmpeg not found` (a hard failure with `FILMCRAFT_REQUIRE_ORACLES=1`). diff --git a/crates/testkit/src/oracle.rs b/crates/testkit/src/oracle.rs index 8dbdbd3f..e0b13863 100644 --- a/crates/testkit/src/oracle.rs +++ b/crates/testkit/src/oracle.rs @@ -121,6 +121,59 @@ pub fn ffprobe_or_skip(context: &str) -> Option { tool_or_skip("ffprobe", ffprobe(), context) } +/// Whether `ffmpeg -formats` output lists `format_name` (e.g. `apv`, new in +/// ffmpeg 8.0). Pure function of the command output; matches a whole +/// whitespace-separated token so `APV` in a description cannot match. +fn formats_output_lists(output: &str, format_name: &str) -> bool { + output.lines().any(|line| line.split_whitespace().any(|token| token == format_name)) +} + +/// Whether `ff` can read `format_name`. An ffmpeg that cannot run (or prints +/// nothing usable) counts as unsupported; the caller skips rather than fails. +pub fn ffmpeg_supports_format(ff: &Path, format_name: &str) -> bool { + let Ok(out) = std::process::Command::new(ff).args(["-hide_banner", "-formats"]).output() else { + return false; + }; + if !out.status.success() { + return false; + } + formats_output_lists(&String::from_utf8_lossy(&out.stdout), format_name) +} + +/// The `ffmpeg -version` version string in banner output (`6.1.1-3ubuntu5`, +/// `8.0.1`, …), if the first line is an ffmpeg banner. Pure function of the +/// command output. +fn ffmpeg_version_in(output: &str) -> Option { + let first = output.lines().next()?; + let rest = first.strip_prefix("ffmpeg version")?.trim_start(); + let version: String = rest.chars().take_while(|c| !c.is_whitespace()).collect(); + if version.is_empty() { None } else { Some(version) } +} + +/// The `ffmpeg -version` version string (`6.1.1-3ubuntu5`, `8.0.1`, …), if `ff` +/// reports one. `None` when the tool cannot run or its banner is unexpected. +pub fn ffmpeg_version(ff: &Path) -> Option { + let out = std::process::Command::new(ff).arg("-version").output().ok()?; + if !out.status.success() { + return None; + } + ffmpeg_version_in(&String::from_utf8_lossy(&out.stdout)) +} + +/// ffmpeg that can read `format_name`, or `None` after reporting a skip (see +/// [`skip`]). An ffmpeg predating the format (APV needs 8.0+) skips with its +/// detected version named, exactly like a missing tool — and fails under +/// `FILMCRAFT_REQUIRE_ORACLES=1`. +pub fn ffmpeg_with_format_or_skip(context: &str, format_name: &str, min_version: &str) -> Option { + let ff = ffmpeg_or_skip(context)?; + if ffmpeg_supports_format(&ff, format_name) { + return Some(ff); + } + let version = ffmpeg_version(&ff).map_or_else(|| "unknown version".to_string(), |v| format!("version {v}")); + skip(context, &format!("ffmpeg {version} cannot read {format_name} (needs {min_version}+)")); + None +} + #[cfg(test)] mod tests { use super::*; @@ -167,4 +220,38 @@ mod tests { assert_eq!(got, Some(d.join("ffmpeg"))); assert_eq!(find_tool_in("ffmpeg-does-not-exist", None, None, &[ds.as_str()]).unwrap(), None); } + + const FORMATS_WITH_APV: &str = + "File formats:\n D mov QuickTime / MOV\n DE apv APV raw bitstream\n D mp4 MP4 (MPEG-4 Part 14)\n"; + const FORMATS_WITHOUT_APV: &str = + "File formats:\n D mov QuickTime / MOV\n D mp4 MP4 (MPEG-4 Part 14)\n D oapv Obsolete APV alias\n"; + + #[test] + fn formats_listing_detects_a_supported_format() { + assert!(formats_output_lists(FORMATS_WITH_APV, "apv")); + // No apv entry: neither a longer name containing it (oapv) nor an + // uppercase description mention (APV) counts as support. + assert!(!formats_output_lists(FORMATS_WITHOUT_APV, "apv")); + assert!(!formats_output_lists("", "apv")); + } + + #[test] + fn version_string_parses_ffmpeg_banner() { + assert_eq!(ffmpeg_version_in("ffmpeg version 7:6.1.1-3ubuntu5 Copyright (c) 2000-2023"), Some("7:6.1.1-3ubuntu5".to_string())); + assert_eq!(ffmpeg_version_in("ffmpeg version 8.0.1-full_build-www.gyan.dev xxx"), Some("8.0.1-full_build-www.gyan.dev".to_string())); + assert_eq!(ffmpeg_version_in(""), None); + assert_eq!(ffmpeg_version_in("ffprobe version 6.1"), None); + assert_eq!(ffmpeg_version_in("ffmpeg version "), None); + } + + #[test] + fn live_ffmpeg_reports_its_own_apv_support() { + let Some(ff) = ffmpeg() else { return }; + let supported = ffmpeg_supports_format(&ff, "apv"); + assert_eq!( + supported, + formats_output_lists(&String::from_utf8_lossy(&std::process::Command::new(&ff).args(["-hide_banner", "-formats"]).output().unwrap().stdout), "apv") + ); + assert!(ffmpeg_version(&ff).is_some_and(|v| !v.is_empty())); + } } diff --git a/docs/testing.md b/docs/testing.md index 98ab6b6e..67d312fa 100644 --- a/docs/testing.md +++ b/docs/testing.md @@ -52,6 +52,8 @@ are never linked or shipped ([AGENTS.md](../AGENTS.md) §2). - If a tool is missing the test prints `SKIPPED (): ffmpeg not found …` and passes. Set **`FILMCRAFT_REQUIRE_ORACLES=1`** (CI) to make every such skip a failure. In new tests use `let ff = filmcraft_testkit::require_ffmpeg!();` (also `require_ffprobe!`, `require_oracles!`). +- Some oracles need a recent ffmpeg: the APV tests (`crates/apv/tests/oracle.rs`) skip when + `ffmpeg -formats` does not list `apv` (ffmpeg ≥ 8.0), naming the detected version. - Fixtures are generated on first use into `/target/fixtures//` (`filmcraft_testkit::fixtures_dir`; independent of `CARGO_TARGET_DIR`, so agents with private target dirs share them; `FILMCRAFT_FIXTURES_DIR` overrides the root) and reused afterwards.