#1743: check final storage overflow destinations for queued outputs

This commit is contained in:
Kristian Myram committed 2026-10-30 09:55:49 +01:00
1 parent f558305795
commit b7d97c20e9
7 files changed
+175 -26

No files matched your search

+82 -18
View File
@@ -312,6 +312,7 @@ struct Clash<'a> {
#[derive(Default)]
struct Writes {
all: Vec<Write>,
path_bytes: usize,
/// The first output to write a file, by name.
files: HashMap<String, usize>,
/// ... and by what the file is on its volume.
@@ -374,7 +375,12 @@ impl Writes {
}
}
fn push(&mut self, write: Write) {
fn push(&mut self, write: Write) -> Result<(), String> {
let path_bytes = self.path_bytes.saturating_add(write.path.len());
if self.all.len() >= 65_536 || path_bytes > 16 * 1024 * 1024 {
return Err("render output plan exceeds 65536 paths or 16 MiB of path names; split the queue".into());
}
self.path_bytes = path_bytes;
let at = self.all.len();
match &write.target {
Target::File { key, identity } => {
@@ -386,6 +392,7 @@ impl Writes {
Target::Frames { .. } => {}
}
self.all.push(write);
Ok(())
}
}
@@ -459,8 +466,11 @@ impl Session {
if seen.contains(&item.id) {
continue;
}
let reason = match self.check_output(&protected, item, path) {
Ok(reason) => reason,
let reason = match self.write_paths(item, path) {
Ok(paths) => paths.iter().find_map(|path| match self.check_output(&protected, item, path) {
Ok(reason) => reason,
Err(reason) => Some(reason),
}),
Err(reason) => Some(reason),
};
if let Some(reason) = reason {
@@ -472,18 +482,55 @@ impl Session {
}
/// What the job writes more than once. An item in `refused` writes nothing, so it takes no part.
pub(crate) fn duplicate_outputs(&self, items: &[Vec<(RenderQueueItem, String)>], refused: &[Refused]) -> Vec<DuplicateOutput> {
pub(crate) fn duplicate_outputs(&self, items: &[Vec<(RenderQueueItem, String)>], refused: &[Refused]) -> Result<Vec<DuplicateOutput>, String> {
let skip: HashSet<u64> = refused.iter().map(|r| r.item).collect();
let mut writes = Writes::default();
let mut duplicates = Vec::new();
for (m, path) in items.iter().flatten().filter(|(m, _)| !skip.contains(&m.id)) {
let write = self.write_of(m, path, true);
if let Some(clash) = writes.clash(&write) {
duplicates.push(self.duplicate_of(&write, &clash));
let paths = self.write_paths(m, path)?;
// Alternative destinations of one output cannot collide with themselves.
for path in &paths {
let write = self.write_of(m, path, true);
if let Some(clash) = writes.clash(&write) {
duplicates.push(self.duplicate_of(&write, &clash));
break;
}
}
for path in paths {
writes.push(self.write_of(m, &path, true))?;
}
writes.push(write);
}
duplicates
Ok(duplicates)
}
/// Every possible destination, since quota decisions can change during rendering.
fn write_paths<'a>(&self, item: &RenderQueueItem, path: &'a str) -> Result<Vec<std::borrow::Cow<'a, str>>, String> {
let overflow = &self.project.render_prefs.overflow_folders;
const PATH_BYTES: usize = 1024 * 1024;
if path.len() > PATH_BYTES {
return Err("render output destinations exceed 1 MiB of path names".into());
}
let mut paths = vec![std::borrow::Cow::Borrowed(path)];
let mut bytes = path.len();
if item.settings.storage_overflow && self.storage_quota.is_some() {
if overflow.len() > 256 {
return Err("render output planning supports at most 256 storage overflow folders".into());
}
if let Some(name) = Path::new(path).file_name() {
for folder in overflow {
if bytes.saturating_add(folder.len()).saturating_add(name.len()).saturating_add(1) > PATH_BYTES {
return Err("render output destinations exceed 1 MiB of path names".into());
}
let destination = Path::new(folder).join(name).to_string_lossy().into_owned();
bytes = bytes.saturating_add(destination.len());
if bytes > PATH_BYTES {
return Err("render output destinations exceed 1 MiB of path names".into());
}
paths.push(std::borrow::Cow::Owned(destination));
}
}
}
Ok(paths)
}
/// What `path`, an output of `m`, writes. `by_identity`: see [`Target::file`].
@@ -494,7 +541,7 @@ impl Session {
} else {
Target::file(Path::new(path), by_identity)
};
let skips = m.settings.skip_existing && self.exporter.as_ref().is_some_and(|exporter| exporter.supports_skip_existing());
let skips = m.settings.skip_existing && self.exporter.as_ref().is_some_and(|exporter| exporter.supports_skip_existing(&m.settings));
Write { item: m.id, path: path.to_string(), target, skips }
}
@@ -548,7 +595,7 @@ impl Session {
// Native source protection stays with its filesystem guard; virtual web outputs still
// need distinct queue names because the browser stores completed files by path.
let protected = if cfg!(target_arch = "wasm32") { Protected::default() } else { self.protected_files() };
let queue = self.queue_writes();
let Ok(queue) = self.queue_writes() else { return };
if protected.is_empty() && queue.is_empty() {
return;
}
@@ -566,7 +613,11 @@ impl Session {
}
Ok(None) => {}
}
in_the_way |= queue.clash(&self.write_of(module, path, false)).is_some();
let Ok(paths) = self.write_paths(module, path) else {
item.output.output = template;
return;
};
in_the_way |= paths.iter().any(|path| queue.clash(&self.write_of(module, path, false)).is_some());
}
if !in_the_way {
return;
@@ -577,14 +628,16 @@ impl Session {
/// What the items of the queue write (by name: a new item is not compared with what is on
/// disk).
fn queue_writes(&self) -> Writes {
fn queue_writes(&self) -> Result<Writes, String> {
let mut writes = Writes::default();
for outputs in self.project.render_queue.iter().filter_map(|it| self.outputs_of(it, None).ok()) {
for (m, path) in &outputs {
writes.push(self.write_of(m, path, false));
for path in self.write_paths(m, path)? {
writes.push(self.write_of(m, &path, false))?;
}
}
}
writes
Ok(writes)
}
}
@@ -602,11 +655,11 @@ mod tests {
s.execute("renderQueue.add", json!({"format": "h264", "output": "/virtual/shared.mp4"})).unwrap();
}
let (outputs, _) = s.queued_outputs(&effectcraft_project::render_queue::OutputFormat::ALL);
let duplicates = s.duplicate_outputs(&outputs, &[]);
let duplicates = s.duplicate_outputs(&outputs, &[]).unwrap();
assert_eq!(duplicates.len(), 1);
assert!(duplicates[0].path.ends_with("shared.mp4"));
let refused = [Refused { item: duplicates[0].item, reason: "already refused".into() }];
assert!(s.duplicate_outputs(&outputs, &refused).is_empty());
assert!(s.duplicate_outputs(&outputs, &refused).unwrap().is_empty());
}
/// An output of queue item `item` that is a file, known by its name alone.
@@ -622,11 +675,22 @@ mod tests {
fn job_of(writes: impl IntoIterator<Item = Write>) -> Writes {
let mut job = Writes::default();
for write in writes {
job.push(write);
job.push(write).unwrap();
}
job
}
#[test]
fn output_plan_path_budget_returns_an_error() {
let mut job = Writes::default();
let path = "x".repeat(1024 * 1024);
for _ in 0..16 {
job.push(movie(1, &path)).unwrap();
}
assert!(job.push(movie(2, "one-too-many")).is_err());
assert_eq!(job.all.len(), 16);
}
/// The item an output replaces a file of, how many files, and the first.
fn clash(job: &Writes, later: &Write) -> Option<(u64, u64, i64)> {
job.clash(later).map(|c| (c.earlier.item, c.files, c.first))
+3 -2
View File
@@ -59,7 +59,8 @@ pub trait Exporter: Send + Sync {
fn formats(&self) -> Vec<OutputFormat>;
/// Whether Skip Existing Files preserves sequence frames written by earlier items. Exporters
/// without an existence check (such as memory sinks) must keep the conservative default.
fn supports_skip_existing(&self) -> bool {
/// The check must cover the final destination for these settings, including any overflow.
fn supports_skip_existing(&self, _settings: &effectcraft_project::render_queue::RenderSettings) -> bool {
false
}
/// Render and write one item (blocking). `progress(done, total)` returns `false` to cancel;
@@ -500,7 +501,7 @@ impl Session {
let refused = self.check_outputs(&items);
// Two outputs of this job on one file: the render would replace its own output, so it is
// refused before anything is written (the items refused above take no part).
if let Some(refusal) = crate::output_guard::duplicate_refusal(&self.duplicate_outputs(&items, &refused)) {
if let Some(refusal) = crate::output_guard::duplicate_refusal(&self.duplicate_outputs(&items, &refused)?) {
return Err(refusal);
}
items.retain(|modules| modules.first().is_none_or(|(it, _)| !refused.iter().any(|r| r.item == it.id)));
+38 -1
View File
@@ -28,7 +28,7 @@ impl Exporter for Writer {
fn formats(&self) -> Vec<OutputFormat> {
OutputFormat::ALL.to_vec()
}
fn supports_skip_existing(&self) -> bool {
fn supports_skip_existing(&self, _: &effectcraft_project::render_queue::RenderSettings) -> bool {
true
}
fn export(&self, job: &ExportJob, _: &mut dyn FnMut(u64, u64) -> bool) -> Result<ExportResult, String> {
@@ -633,6 +633,7 @@ impl Fixture {
let duplicates: Vec<Value> = self
.s
.duplicate_outputs(&items, &refused)
.unwrap()
.iter()
.map(|d| {
json!({
@@ -658,6 +659,42 @@ fn span(pattern: &str, start: f64, end: f64) -> Value {
json!({"format": "png", "output": pattern, "timeSpan": "custom", "start": start, "end": end})
}
fn with_overflow(f: &mut Fixture) {
struct Room;
impl effectcraft_project::render_queue::StorageQuota for Room {
fn has_room(&self, _: &str, _: u64) -> bool {
true
}
}
f.s.storage_quota = Some(Arc::new(Room));
std::fs::create_dir_all(f.path("overflow")).unwrap();
Arc::make_mut(&mut f.s.project).render_prefs.overflow_folders = vec![f.text("overflow")];
}
#[test]
fn default_names_avoid_possible_storage_overflow_collisions() {
let mut f = Fixture::new("overflow-default");
f.comp("Main");
with_overflow(&mut f);
f.add(json!({"output": f.text("other/Main.mp4"), "storageOverflow": true}));
f.add(json!({"storageOverflow": true}));
assert_eq!(f.output(1), f.path("Main_1.mp4"));
assert!(f.clashes().is_empty());
}
#[test]
fn possible_storage_overflow_destinations_cannot_replace_footage() {
let mut f = Fixture::new("overflow-footage");
f.comp("Main");
with_overflow(&mut f);
f.footage("overflow/Main.mp4");
f.add(json!({"output": f.text("other/Main.mp4"), "storageOverflow": true}));
let error = f.render(json!({})).unwrap_err().to_string();
assert!(error.contains("footage") && error.contains("Main.mp4"), "{error}");
assert!(f.exported().is_empty());
assert_eq!(f.read("overflow/Main.mp4"), SOURCE);
}
/// After Effects numbers the second item of one composition (`Comp 1_1.mov`). A default output
/// name that another item of the queue already has is numbered the same way.
#[test]
+3 -2
View File
@@ -33,8 +33,9 @@ impl Exporter for FileExporter {
fn formats(&self) -> Vec<OutputFormat> {
effectcraft_export::available_formats()
}
fn supports_skip_existing(&self) -> bool {
self.sink.is_none() && !cfg!(target_arch = "wasm32")
fn supports_skip_existing(&self, settings: &effectcraft_project::render_queue::RenderSettings) -> bool {
// The exporter's existence check precedes storage-overflow redirection.
self.sink.is_none() && !cfg!(target_arch = "wasm32") && !settings.storage_overflow
}
fn export(&self, job: &ExportJob, progress: &mut dyn FnMut(u64, u64) -> bool) -> Result<ExportResult, String> {
effectcraft_engine::render::passes::block_on(self.export_async(job, progress))
+44
View File
@@ -1,5 +1,49 @@
use serde_json::json;
#[test]
#[cfg(not(target_arch = "wasm32"))]
fn storage_overflow_cannot_bypass_duplicate_output_protection() {
for same_folder in [true, false] {
refuse_overflow_collision(same_folder);
}
}
#[cfg(not(target_arch = "wasm32"))]
fn refuse_overflow_collision(same_folder: bool) {
use std::{path::PathBuf, sync::Arc};
struct OverflowOnly(PathBuf);
impl effectcraft_project::render_queue::StorageQuota for OverflowOnly {
fn has_room(&self, path: &str, _: u64) -> bool {
std::path::Path::new(path).starts_with(&self.0)
}
}
let dir = std::env::temp_dir().join(format!("ec-duplicate-overflow-{}-{same_folder}", std::process::id()));
let _ = std::fs::remove_dir_all(&dir);
let overflow = dir.join("overflow");
std::fs::create_dir_all(&overflow).unwrap();
let mut s = effectcraft_engine::Session {
exporter: Some(Arc::new(super::FileExporter::default())),
storage_quota: Some(Arc::new(OverflowOnly(overflow.clone()))),
..Default::default()
};
Arc::make_mut(&mut s.project).render_prefs.overflow_folders = vec![overflow.to_string_lossy().into_owned(); 2];
s.execute("comp.new", json!({"width": 16, "height": 16, "frameRate": 10, "duration": 0.2})).unwrap();
for index in 0..2 {
let folder = if same_folder { dir.clone() } else { dir.join(index.to_string()) };
s.execute("renderQueue.add", json!({"format": "png", "output": folder.join("frames_[#####].png"), "skipExisting": true, "storageOverflow": true}))
.unwrap();
}
let result = s.execute("renderQueue.render", json!({}));
let written = std::fs::read_dir(&overflow).unwrap().count();
assert!(result.is_err(), "overflow destinations cannot skip previous frames: {result:?}");
assert_eq!(written, 0, "refused before any overflow file is written");
s.execute("renderQueue.remove", json!({"index": 2})).unwrap();
let result = s.execute("renderQueue.render", json!({})).unwrap();
assert_eq!(result["items"][0]["status"], "Done", "one output's alternatives are not separate outputs: {result}");
assert_eq!(std::fs::read_dir(&overflow).unwrap().count(), 2);
std::fs::remove_dir_all(&dir).unwrap();
}
#[test]
fn memory_sink_cannot_overwrite_a_previous_output_even_with_skip_existing() {
use std::sync::{
+2 -1
View File
@@ -323,7 +323,8 @@ build real projects through these interfaces; they are worked examples of everyt
`renderQueue.duplicate` copy, output modules, segments) name the same file, the whole render is
refused before anything is written, naming both items. Sequences clash only on frames both
write. A later item with Skip Existing Files leaves the earlier one's frames alone only when
the exporter can check existing files; browser and other memory sinks still report collisions. Default
the exporter can check the final destination. Browser/memory sinks and storage-overflow renders still report collisions.
Preflight compares every configured overflow destination, since available space can change during a render. Default
names are numbered like footage collisions when another queue item writes them
(`renderQueue.add`, `render.preRender`, `comp.saveFrameAs {queue}`, `file.createProxy`: a second
`Main` renders to `Main_1.mp4`); a name you give is kept, and `renderQueue.duplicate` copies the
+3 -2
View File
@@ -59,8 +59,9 @@ Two outputs of one job that write the same file (two items with one Output To, a
output modules or segments that meet) refuse the whole `renderQueue.render` before anything is
written: the reply is `isError: true`, naming both items, and the queue is left as it was.
Sequences clash only on the frames both write. Skip Existing Files leaves earlier frames alone
only when the exporter can check existing files; browser and other memory sinks still report
collisions. A new item's default output name is numbered (`Main.mp4`,
only when the exporter can check the final destination; browser/memory sinks and storage-overflow
renders still report collisions. Preflight checks every configured overflow destination, since free
space can change during a render. A new item's default output name is numbered (`Main.mp4`,
`Main_1.mp4`) when another item of the queue writes it.
## Progress and cancellation