chexit/horizon-airdef
1
0
Fork 0
Code Issues Pull Requests Actions 1 Packages Projects Releases Wiki Activity

Free a failed film take's files and keep the finished one

Browse Source 
The writer removed its video, raw sound and log only when a take was
cancelled, so an encoder error or a non-zero ffmpeg exit left them in the
films folder. A failed take now removes exactly the files it wrote; the
log's last lines move into the error, since the log itself goes.

The encoder always writes <name>.video.mp4, and <name>.mp4 appears only
after a successful mux or rename. A failed, cancelled or no-audio take
therefore cannot overwrite or delete a film of an earlier export with the
same name. The writer's entry point takes the ffmpeg and its arguments so
a test can drive a failing encoder.

Co-Authored-By: Kimi Code <noreply@moonshot.ai>
This commit is contained in:
chexit
2026-11-04 18:06:25 +03:00
parent 45b43f00df
commit 73b9d30704
1 changed files with 240 additions and 38 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/horizon-client/src/game/film.rs
+240 -38
View File
@@ -166,7 +166,8 @@ fn encoder(ffmpeg: &Path, crf: u32) -> Option<Vec<String>> {
}
/// The writer thread: frames to `ffmpeg`'s pipe (or PNG files), sound to a raw file, then the
/// file is closed and the sound muxed in.
/// file is closed and the sound muxed in. `<name>.mp4` is only ever produced by a successful
/// finish, so a failed or cancelled take cannot damage an earlier film of the same name.
fn write(
rx: mpsc::Receiver<Msg>,
dir: PathBuf,
@@ -176,32 +177,38 @@ fn write(
crf: u32,
audio: bool,
outcome: Arc<Mutex<Option<Outcome>>>,
) {
let ffmpeg = find_ffmpeg();
let encode = ffmpeg.as_deref().and_then(|f| encoder(f, crf));
write_with(rx, dir, name, fps, size, crf, audio, outcome, ffmpeg, encode);
}
/// [`write`], with the `ffmpeg` and its encoder arguments given: a test passes a failing one.
fn write_with(
rx: mpsc::Receiver<Msg>,
dir: PathBuf,
name: String,
fps: u32,
size: [u32; 2],
crf: u32,
audio: bool,
outcome: Arc<Mutex<Option<Outcome>>>,
ffmpeg: Option<PathBuf>,
encode: Option<Vec<String>>,
) {
let result = (|| -> Result<(String, String), String> {
// This capture's own files: the encoder's picture, the raw sound, the log and the PNG
// frames. The finished `<name>.mp4` is not among them.
let video_path = dir.join(format!("{name}.video.mp4"));
let pcm_path = dir.join(format!("{name}.audio.raw"));
let mut pcm = if audio {
Some(std::io::BufWriter::new(
std::fs::File::create(&pcm_path).map_err(|e| format!("audio file: {e}"))?,
))
} else {
None
};
let ffmpeg = find_ffmpeg();
let encode = ffmpeg.as_deref().and_then(|f| encoder(f, crf));
let video_path = dir.join(if audio {
format!("{name}.video.mp4")
} else {
format!("{name}.mp4")
});
let log_path = dir.join(format!("{name}.ffmpeg.log"));
let frames_dir = dir.join(format!("{name}_frames"));
let out = dir.join(format!("{name}.mp4"));
let mut child: Option<Child> = None;
let mut log_path = None;
if let (Some(ffmpeg), Some(encode)) = (&ffmpeg, &encode) {
let log = dir.join(format!("{name}.ffmpeg.log"));
let stderr = std::fs::File::create(&log).map_err(|e| format!("log file: {e}"))?;
log_path = Some(log);
let stderr = std::fs::File::create(&log_path).map_err(|e| format!("log file: {e}"))?;
let bgra = true;
let c = Command::new(ffmpeg)
let spawned = Command::new(ffmpeg)
.args(["-y", "-hide_banner", "-loglevel", "error", "-f", "rawvideo"])
.args(["-pix_fmt", if bgra { "bgra" } else { "rgba" }])
.args(["-video_size", &format!("{}x{}", size[0], size[1])])
@@ -225,11 +232,37 @@ fn write(
.stdin(Stdio::piped())
.stdout(Stdio::null())
.stderr(stderr)
.spawn()
.map_err(|e| format!("could not run ffmpeg: {e}"))?;
child = Some(c);
.spawn();
match spawned {
Ok(c) => child = Some(c),
Err(e) => {
let _ = std::fs::remove_file(&log_path);
return Err(format!("could not run ffmpeg: {e}"));
}
}
}
let mut pcm = if audio {
match std::fs::File::create(&pcm_path) {
Ok(f) => Some(std::io::BufWriter::new(f)),
Err(e) => {
if let Some(mut c) = child.take() {
let _ = c.kill();
let _ = c.wait();
}
let _ = std::fs::remove_file(&video_path);
let _ = std::fs::remove_file(&log_path);
return Err(format!("audio file: {e}"));
}
}
} else {
std::fs::create_dir_all(&frames_dir).map_err(|e| format!("frames folder: {e}"))?;
None
};
if child.is_none()
&& let Err(e) = std::fs::create_dir_all(&frames_dir)
{
let _ = std::fs::remove_file(&pcm_path);
let _ = std::fs::remove_dir_all(&frames_dir);
return Err(format!("frames folder: {e}"));
}
let mut stdin = child.as_mut().and_then(|c| c.stdin.take());
let mut n = 0u32;
@@ -251,7 +284,7 @@ fn write(
}
}
if let Err(e) = pipe.write_all(&data) {
failed = Some(format!("ffmpeg stopped (see {name}.ffmpeg.log): {e}"));
failed = Some(format!("ffmpeg stopped: {e}"));
}
} else if let Err(e) =
write_png(&frames_dir.join(format!("frame_{n:06}.png")), &f)
@@ -289,25 +322,48 @@ fn write(
let _ = std::fs::remove_file(p);
}
};
// A cancelled or failed take keeps nothing of its own: the encoder's picture, the raw
// sound and the log go. A finished film of an earlier export with the same name is not
// among these paths and stays (a PNG sequence from an earlier one stays too: an encoder
// take never writes into it).
let discard = || {
tidy(&[&video_path, &pcm_path, &log_path]);
};
let discard_frames = || {
tidy(&[&pcm_path]);
let _ = std::fs::remove_dir_all(&frames_dir);
};
if let Some(mut c) = child {
let status = c.wait().map_err(|e| format!("ffmpeg: {e}"))?;
let status = match c.wait() {
Ok(status) => status,
Err(e) => {
let _ = c.kill();
discard();
return Err(format!("ffmpeg: {e}"));
}
};
if cancelled || failed.is_some() || !status.success() {
let why = failed.unwrap_or_else(|| {
let mut why = failed.unwrap_or_else(|| {
if cancelled {
"cancelled".into()
} else {
format!("ffmpeg failed (see {name}.ffmpeg.log)")
"ffmpeg failed".into()
}
});
if cancelled {
tidy(&[&video_path, &pcm_path]);
if let Some(l) = &log_path {
tidy(&[l]);
if !cancelled {
// The log goes with the take; what it said is the only trace left, so the
// error carries it.
let log = std::fs::read(&log_path)
.map(|b| String::from_utf8_lossy(&b).into_owned())
.unwrap_or_default();
let log = log.split_whitespace().collect::<Vec<_>>().join(" ");
if !log.is_empty() {
why.push_str(&format!(": {}", log.chars().take(400).collect::<String>()));
}
}
discard();
return Err(why);
}
let out = dir.join(format!("{name}.mp4"));
let mut note = String::new();
if audio {
let mux = Command::new(ffmpeg.as_ref().expect("ffmpeg"))
@@ -336,10 +392,16 @@ fn write(
);
}
}
} else {
// The finished picture replaces a film of an earlier export with this name;
// Windows cannot rename over an existing file.
let _ = std::fs::remove_file(&out);
if let Err(e) = std::fs::rename(&video_path, &out) {
discard();
return Err(format!("the film could not be finished: {e}"));
}
}
if let Some(l) = &log_path {
tidy(&[l]);
}
tidy(&[&log_path]);
let encoder_note = if encode
.as_ref()
.is_some_and(|e| e.iter().any(|a| a == "h264_videotoolbox"))
@@ -357,11 +419,11 @@ fn write(
Ok((out.display().to_string(), note))
} else {
if cancelled {
let _ = std::fs::remove_dir_all(&frames_dir);
tidy(&[&pcm_path]);
discard_frames();
return Err("cancelled".into());
}
if let Some(why) = failed {
discard_frames();
return Err(why);
}
let wav = if audio {
@@ -878,6 +940,146 @@ mod tests {
}
}
/// A writer's folder, removed with the test.
#[cfg(unix)]
struct TakeDir(PathBuf);
#[cfg(unix)]
impl TakeDir {
fn new() -> Self {
static NEXT: std::sync::atomic::AtomicU64 = std::sync::atomic::AtomicU64::new(0);
let n = NEXT.fetch_add(1, std::sync::atomic::Ordering::Relaxed);
let dir = std::env::temp_dir().join(format!(
"horizon-film-take-{}-{n}",
std::process::id()
));
std::fs::create_dir_all(&dir).unwrap();
Self(dir)
}
fn file(&self, name: &str, bytes: &[u8]) -> PathBuf {
let p = self.0.join(name);
std::fs::write(&p, bytes).unwrap();
p
}
fn gone(&self, name: &str) -> bool {
!self.0.join(name).exists()
}
}
#[cfg(unix)]
impl Drop for TakeDir {
fn drop(&mut self) {
let _ = std::fs::remove_dir_all(&self.0);
}
}
/// One take of `name` into `dir` with the frames given, then the ending `end`.
#[cfg(unix)]
fn take(dir: &Path, name: &str, audio: bool, ffmpeg: PathBuf, end: Msg, frames: &[u8]) -> Outcome {
let (tx, rx) = mpsc::sync_channel(8);
for &n in frames {
tx.send(Msg::Frame(frame(n))).unwrap();
}
tx.send(end).unwrap();
let outcome = Arc::new(Mutex::new(None));
write_with(
rx,
dir.to_path_buf(),
name.into(),
30,
[1, 1],
20,
audio,
outcome.clone(),
Some(ffmpeg),
Some(vec!["-c:v".into(), "libx264".into()]),
);
let mut o = outcome.lock().unwrap();
o.take().expect("the writer always reports")
}
/// A failed encoder leaves no picture, raw sound or log of this take, and neither deletes the
/// finished film of an earlier export with the same name nor another take's files.
#[cfg(unix)]
#[test]
fn a_failed_take_removes_its_own_files_and_keeps_the_film() {
if !Path::new("/bin/false").is_file() {
return;
}
for audio in [true, false] {
let take_dir = TakeDir::new();
let earlier = take_dir.file("cap.mp4", b"earlier film");
let other = take_dir.file("other.audio.raw", b"other take");
let other_log = take_dir.file("other.ffmpeg.log", b"other log");
let o = take(&take_dir.0, "cap", audio, PathBuf::from("/bin/false"), Msg::Finish, &[1]);
assert!(!o.ok, "audio={audio}");
assert_eq!(std::fs::read(&earlier).unwrap(), b"earlier film", "audio={audio}");
assert_eq!(std::fs::read(&other).unwrap(), b"other take");
assert_eq!(std::fs::read(&other_log).unwrap(), b"other log");
for name in ["cap.video.mp4", "cap.audio.raw", "cap.ffmpeg.log", "cap_frames"] {
assert!(take_dir.gone(name), "{name} left behind (audio={audio})");
}
}
}
/// The log is removed with the failed take, so its message is carried in the error instead.
#[cfg(unix)]
#[test]
fn an_encoder_error_carries_its_log_and_removes_it() {
use std::os::unix::fs::PermissionsExt;
let take_dir = TakeDir::new();
let encoder = take_dir.file(
"ffmpeg",
b"#!/bin/sh\nfor last; do :; done\ncat > /dev/null\necho 'Unknown encoder libx264' >&2\nexit 1\n",
);
std::fs::set_permissions(&encoder, std::fs::Permissions::from_mode(0o755)).unwrap();
let o = take(&take_dir.0, "cap", true, encoder, Msg::Finish, &[1]);
assert!(!o.ok);
assert!(o.message.contains("Unknown encoder libx264"), "{}", o.message);
assert!(take_dir.gone("cap.ffmpeg.log"));
assert!(take_dir.gone("cap.video.mp4"));
assert!(take_dir.gone("cap.audio.raw"));
}
/// A cancelled take removes the same files, and the earlier film stays as it was.
#[cfg(unix)]
#[test]
fn a_cancelled_take_removes_its_own_files_and_keeps_the_film() {
if !Path::new("/bin/false").is_file() {
return;
}
let take_dir = TakeDir::new();
let earlier = take_dir.file("cap.mp4", b"earlier film");
let o = take(&take_dir.0, "cap", true, PathBuf::from("/bin/false"), Msg::Cancel, &[1]);
assert!(!o.ok);
assert_eq!(std::fs::read(&earlier).unwrap(), b"earlier film");
for name in ["cap.video.mp4", "cap.audio.raw", "cap.ffmpeg.log", "cap_frames"] {
assert!(take_dir.gone(name), "{name} left behind");
}
}
/// A take that finishes keeps the film and drops its temporaries; the film is the only file
/// left for this name.
#[cfg(unix)]
#[test]
fn a_finished_take_keeps_the_film_and_drops_its_temporaries() {
use std::os::unix::fs::PermissionsExt;
let take_dir = TakeDir::new();
let encoder = take_dir.file(
"ffmpeg",
b"#!/bin/sh\nfor last; do :; done\ncat > /dev/null\nprintf 'film' > \"$last\"\nexit 0\n",
);
std::fs::set_permissions(&encoder, std::fs::Permissions::from_mode(0o755)).unwrap();
let o = take(&take_dir.0, "cap", false, encoder, Msg::Finish, &[1, 2]);
assert!(o.ok, "{}", o.message);
assert_eq!(std::fs::read(take_dir.0.join("cap.mp4")).unwrap(), b"film");
for name in ["cap.video.mp4", "cap.audio.raw", "cap.ffmpeg.log", "cap_frames"] {
assert!(take_dir.gone(name), "{name} left behind");
}
}
fn drain(rx: &mpsc::Receiver<Msg>) -> Vec<u8> {
rx.try_iter()
.filter_map(|m| match m {