From f364fc390a05d2b362319c511a76c3344bbd349a Mon Sep 17 00:00:00 2001 From: owenpkent <20529132+owenpkent@users.noreply.github.com> Date: Sun, 30 Aug 2026 19:05:27 -0600 Subject: [PATCH 01/13] Offload from an edit timeline, not just a card A card offload knows its source: everything under one root, and the only question is whether it arrived intact. This is the other job, the one that turns up when a cut comes back from an editor. Which files does this timeline need, and are they all here? offloader resolve --timeline "01 Chairs Row V6.xml" --search-root E:\ChairsDoc offloader offload --timeline "..." --search-root E:\ChairsDoc --dest E:\... Reading timelines is OpenTimelineIO's job and nothing else's, behind a new [timeline] extra. The adapters are not in OTIO's core distribution, so a bare install reads .otio and none of the formats an NLE exports; the extra names otio-fcp-adapter explicitly. OTIO is trusted with media references and not with structure. Measured against a Premiere-exported FCP 7 XML of 913 clipitems, it recovered all 334 unique references with none missed and none invented, including the clipitems whose is an id reference carrying no path of its own. On the same file it reported 29.97 fps and 12 audio tracks for a sequence that declares 24 and 23. So no rate, duration or track count from the adapter reaches a caller. The resolver is the part worth having. Six statuses, and the order of the tests is the design: a generated reference stops at once; anything under a search root with that basename wins whatever path the timeline asked for; a camera original may be satisfied by a proxy if the frame counts agree; only then is a readable file a gap. It refuses to choose between two files that share a name. On the conform this was written against, 20 basenames had more than one copy under the search root. Seven were byte-identical and harmless. Thirteen were different files, eleven of them MISSING MEDIA stand-in slates from an earlier pass sitting beside the real archival footage that arrived later. Relinking by filename picks one at random, and when it picks a slate the clip reports as online. The closest-path candidate is offered as an explicitly labelled guess; it was right all thirteen times, which is a reason to show it and not a reason to act on it unsupervised. Byte-identical duplicates resolve normally and are not copied again. On that job that avoided 1.4 GB of pointless transfer and, more to the point, avoided manufacturing 32 filename collisions on a drive that had none. A proxy standing in for a camera original is checked frame for frame. A proxy one frame short moves every edit point after it. Where the count cannot be established the note says unverified rather than implying agreement; where it is established and disagrees, nothing is substituted. Engine: OffloadOptions.selection takes an explicit file set in place of scanning one root, so a job can span volumes. Each file carries its own destination-relative path and the engine infers no layout. The safety rule had to change, and narrowing it was the point. assert_safe_destinations refuses a destination inside the source, which is right for a card because the source root is the card. It is wrong for a selection, where the search roots are a library being read and collecting a cut's gaps into a folder on that same drive is the ordinary case. A selection is held instead to the property that actually matters and is checkable exactly: no file being read may sit at or beneath somewhere being written. A destination-relative path that is absolute, or climbs out with .., is refused before the job starts rather than partway through it. 33 tests. The eight that exercise the selection path fail without the engine change. Verified end to end against two real timelines: 334 references reconciled file by file against an independent hand-built list, zero missed and zero extra, 15 proxy substitutions all frame-verified. --- CHANGELOG.md | 33 +++ README.md | 53 +++- ROADMAP.md | 14 + docs/timeline.md | 190 +++++++++++++ pyproject.toml | 9 + src/offloader/cli.py | 151 +++++++++- src/offloader/engine.py | 150 +++++++++- src/offloader/timeline.py | 513 ++++++++++++++++++++++++++++++++++ tests/test_timeline.py | 567 ++++++++++++++++++++++++++++++++++++++ 9 files changed, 1663 insertions(+), 17 deletions(-) create mode 100644 docs/timeline.md create mode 100644 src/offloader/timeline.py create mode 100644 tests/test_timeline.py diff --git a/CHANGELOG.md b/CHANGELOG.md index f7bca5e..42e45f5 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,39 @@ project uses [semantic versioning][semver]. ### Added +- **Offloading from an edit timeline.** `offloader resolve --timeline cut.xml + --search-root E:\Media` reports which of a cut's media is already on the + drive and which is not; `offloader offload --timeline ...` copies the + difference, with the same verified copy, checksums and reports as a card. + Timelines are read with OpenTimelineIO (`pip install "offloader[timeline]"`), + covering FCP 7 XML, FCPXML, EDL, AAF and `.otio` — but trusted with media + references only, never with structure: on the file this was measured + against, the adapter recovered all 334 references exactly while reporting + 29.97 fps and 12 audio tracks for a sequence that declares 24 and 23. See + [`docs/timeline.md`](docs/timeline.md). + + The resolver **refuses to choose between two files that share a name.** On + that conform, 20 basenames had more than one copy under the search root: + seven byte-identical and harmless, thirteen genuinely different files, + eleven of those being "MISSING MEDIA" stand-in slates from an earlier pass + sitting beside the real archival footage. A relink by filename picks one at + random and the clip reports as online either way. Byte-identical duplicates + resolve normally and are not copied again, which on that job avoided 1.4 GB + of transfer and, more to the point, avoided manufacturing 32 filename + collisions on a drive that had none. A camera original may be satisfied by a + proxy already on the drive, but only where the frame counts agree — a proxy + one frame short moves every edit point after it. + +- **`OffloadOptions.selection`**, an explicit file set for `engine.run` in + place of scanning one source root. Files may come from any number of volumes + and each carries its own destination-relative path, so the engine infers no + layout. A selection is held to a narrower safety rule than a card's: a card + refuses a destination inside the source, which would wrongly refuse the + ordinary timeline case of collecting a cut's gaps into a folder on the drive + they were resolved against. Instead, no file being read may sit at or beneath + somewhere being written, and a destination-relative path that is absolute or + climbs out with `..` is refused before the job starts. + - **A `data` profile for generic large-data transfers.** The verified copy engine was never camera-specific — it reads every byte once, checksums it, fans it out to N destinations and reads it back — but the metadata layer diff --git a/README.md b/README.md index 7ec969b..2c64956 100644 --- a/README.md +++ b/README.md @@ -91,6 +91,7 @@ offloader verify D:\video\080426\A001 | Command | What it does | | --- | --- | | `offload` | copy and verify a source to one or more destinations | +| `resolve` | report which of an edit timeline's media is already here, copying nothing | | `verify` | re-check an offloaded tree against its manifests | | `report` | regenerate paperwork for an existing tree, copying nothing | | `info` | show tool and environment status | @@ -101,6 +102,7 @@ offloader verify D:\video\080426\A001 | Flag | Meaning | | --- | --- | | `--source PATH` | card or folder to offload | +| `--timeline PATH` | offload an edit timeline's media instead of a folder; see [From an edit timeline](#from-an-edit-timeline) | | `--dest PATH` | destination root; repeat for multiple copies | | `--hash ALGO` | `xxh3-64` (default), `xxh3-128`, `xxh64`, `xxh64be`, `md5`, `sha1`, `sha256`, `c4`, `none` | | `--verify MODE` | `source-only` (default), `full`, `none` | @@ -123,7 +125,55 @@ offloader verify D:\video\080426\A001 | `--quiet` | suppress progress | Exit status is `0` on success, `1` if any file failed verification, `2` on a -usage or I/O error, `3` if a destination was refused as unsafe. +usage or I/O error, `3` if a destination was refused as unsafe, `4` if a +timeline could not be read for want of an adapter. + +### From an edit timeline + +A card offload knows its source. This is the other job: a cut comes back from +an editor, and the question is *which files does this timeline need, and are +they all here?* + +```sh +pip install "offloader[timeline]" + +offloader resolve --timeline "01 Chairs Row V6.xml" --search-root E:\ChairsDoc +offloader offload --timeline "01 Chairs Row V6.xml" --search-root E:\ChairsDoc \ + --dest E:\ChairsDoc\RowV6_Media +``` + +`resolve` copies nothing and prints the answer. `offload` copies only the files +that are not already under a `--search-root`, with the same verified copy, +checksums and reports as a card. + +| Flag | Meaning | +| --- | --- | +| `--timeline PATH` | the cut to read: `.xml` (FCP 7), `.fcpxml`, `.edl`, `.aaf`, `.otio` | +| `--search-root PATH` | where the media already lives; repeatable, and required | +| `--adapter NAME` | override the OpenTimelineIO adapter chosen from the suffix | +| `--no-proxy-substitution` | do not let a proxy on disk stand in for a camera original | +| `--layout mirror\|flat` | keep each file's path below its volume root (default), or put everything in one folder | +| `--csv PATH` | (`resolve`) write the full per-reference table | + +Every reference gets one status: `present`, `substituted`, `gap`, `ambiguous`, +`missing` or `generated`. Only `gap` is copied. `resolve` exits non-zero if +anything is `ambiguous` or `missing`. + +The part worth knowing before you trust it: **it refuses to choose between two +files that share a name.** On the conform it was built against, 20 basenames +had more than one copy under the search root. Seven were byte-identical and +harmless. Thirteen were different files -- eleven of them "MISSING MEDIA" +stand-in slates from an earlier pass, sitting beside the real archival footage +that arrived later. Relinking by filename picks one at random, and when it +picks a slate the clip **reports as online**. So the tool reports every +candidate, offers the closest-path match as an explicitly labelled guess, and +copies nothing. + +OpenTimelineIO is trusted with media references and nothing else. It recovered +all 334 of that file's references exactly, including the clipitems that name +their file by id and carry no path; on the same file it reported 29.97 fps and +12 audio tracks for a sequence declaring 24 fps and 23 tracks. Full account and +the adapter table in [`docs/timeline.md`](docs/timeline.md). ### `verify` @@ -427,6 +477,7 @@ general-purpose tool reports a filename, a size, and a placeholder icon. | [`docs/braw.md`](docs/braw.md) | Blackmagic RAW container parsing, proxy pairing, and the interrupted-recording check | | [`docs/ascmhl.md`](docs/ascmhl.md) | ASC MHL v2.0, and how it was validated against the reference implementation | | [`docs/ixml.md`](docs/ixml.md) | Broadcast WAV chunk walking, the iXML slate, and where sound timecode comes from | +| [`docs/timeline.md`](docs/timeline.md) | Resolving an edit timeline to its media, what OpenTimelineIO is and is not trusted with, and why ambiguity refuses | ## Library diff --git a/ROADMAP.md b/ROADMAP.md index c787aaf..8a30cce 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -19,6 +19,16 @@ an oversight. Engine, CLI, five report formats (PDF, CSV, MHL 1.1, ASC MHL v2.0, HTML), the desktop app, and cross-platform CI. +Offloading from an **edit timeline** rather than a card: `offloader resolve` +and `offload --timeline` read an NLE's interchange file with OpenTimelineIO, +work out which of its media is already under a search root, and offload only +what is not. See [`docs/timeline.md`](docs/timeline.md). It refuses to choose +between two files that share a name, which is not fastidiousness: on the +conform it was written against, thirteen names resolved to *different* files, +eleven of them "MISSING MEDIA" stand-in slates from an earlier pass sitting +beside the real archival footage that arrived later. A relink by filename picks +one at random, and the clip reports as online either way. + The PDF matches a real ShotPut Pro report's geometry, measured from its content streams. ASC MHL is diffed against the reference implementation's own worked example. BRAW metadata comes out of the container because ffprobe cannot read @@ -108,6 +118,10 @@ already does the stem-matching this needs. the destination would do it. - **`--skip-existing` by checksum**, not size. Today it is explicitly a speed option and says so; a checksum variant would make it a safe one. + `timeline._same_file` is the comparison already written: size first, because + it settles almost every case for a stat, then the checksum. What is missing + is only the wiring into the engine's own skip, where the file being skipped + is at the destination rather than under another search root. - **Windows installer and code signing**, so it can be handed to someone who does not have Python. - **Per-job report templates and custom branding.** diff --git a/docs/timeline.md b/docs/timeline.md new file mode 100644 index 0000000..fb52313 --- /dev/null +++ b/docs/timeline.md @@ -0,0 +1,190 @@ +# Offloading from an edit timeline + +A card offload knows its source: everything under one root, and the only +question is whether it arrived intact. This is the other job, the one that +turns up when a cut comes back from an editor. + +**Which files does this timeline actually need, and are they all here?** + +```sh +offloader resolve --timeline "01 Chairs Row V6.xml" --search-root E:\ChairsDoc +offloader offload --timeline "01 Chairs Row V6.xml" --search-root E:\ChairsDoc \ + --dest E:\ChairsDoc\RowV6_Media +``` + +`resolve` copies nothing and answers the question. `offload` answers it and +then copies the files that are missing, with the same verified copy, the same +checksums and the same reports as a card. + +Reading timelines is optional and not installed by default: + +```sh +pip install "offloader[timeline]" +``` + +## What it does, and in what order + +Every reference gets exactly one status. The order the tests run in is the +whole design. + +| Status | Meaning | Copied? | +| --- | --- | --- | +| `present` | A file of that name is under a search root | no | +| `substituted` | A camera original, satisfied by a frame-matched proxy already here | no | +| `gap` | Not here, but readable where the timeline says it is | **yes** | +| `ambiguous` | Several files under the roots share the name and differ | no, refuses | +| `missing` | Not here, and not where the timeline says either | no, cannot | +| `generated` | The timeline carries no media for it at all | no, cannot | + +1. **A generated reference stops immediately.** A Premiere-native graphic, a + title, a generator: there is no file, and no search would ever find one. +2. **Anything under a search root with that basename wins**, whatever path the + timeline asked for. An editor's export addresses media on the editor's + volume, so the path is almost always wrong and the name almost always + right. This is the test that stops a file being copied a second time under + a name the drive already holds. +3. **A camera original may be satisfied by a proxy**, if the frame counts + agree. See below. +4. **Only then is a readable file a gap**, which is the thing worth copying. +5. Anything left is `missing`, and no offload will fix it. + +`resolve` exits non-zero when anything is `ambiguous` or `missing`. A +`generated` reference does not fail it: no offload can supply one, and holding +the exit code hostage to it would only teach a caller to ignore the code. + +## Why `ambiguous` refuses instead of choosing + +This is the part that earns the feature. + +On the conform this was written for, 20 basenames had more than one copy under +the search root. Seven were byte-identical, which is harmless. The other +thirteen were *different files sharing a name*: + +- Eleven were "MISSING MEDIA" stand-in slates left by an earlier conform, + sitting alongside the real archival footage that arrived weeks later. Both + under the same root, both matching the name the timeline asks for. +- Two were the same sound recorder filename from two different cards + (`f2/260223_009.WAV` and `f3/260223_009.WAV`), which are different + recordings that happen to be named alike. + +Relinking by filename picks one of those at random. When it picks a slate, the +clip **reports as online** and the cut looks fine until someone watches it. + +So the resolver refuses. It reports every candidate, and it offers a guess - +the candidate whose trailing path components match the timeline's path for +longest - clearly labelled as a guess. On this material that heuristic picked +the correct file all thirteen times, which is a good reason to show it and not +a good enough reason to act on it unsupervised. + +Byte-identical duplicates are not ambiguous. They resolve to one copy, with the +others recorded in `other_candidates`. + +## Why a proxy may stand in for a camera original + +A timeline that references 55 GB of BRAW usually references it because that is +what the *camera* wrote, not because the cut needs it: the editor cut with +proxies, and the proxies are already on the drive. + +`substituted` means a stem-matched proxy was found under a search root **and +the frame counts agree**. That check is not decoration. Relink a clip to a file +one frame shorter and every edit point after it moves, quietly, and the first +person to notice is whoever watches the export. + +The count comes from `probe.py`, which reads BRAW out of the container because +ffprobe cannot open the format at all. Where a count cannot be established - +no ffprobe, an unreachable original - the substitution is still offered but the +note says `frame counts unverified` rather than implying agreement. Where the +counts are established and **disagree**, the reference is reported `ambiguous` +and nothing is substituted. + +`--no-proxy-substitution` turns the whole step off, and every camera original +becomes an ordinary gap. + +## What OpenTimelineIO is trusted with + +Reading. Nothing else. + +OTIO recovers media references from this material exactly. Measured against a +Premiere-exported FCP 7 XML with 913 clipitems, it found all 334 unique media +references, with none missed and none invented, including the clipitems whose +`` element is an id reference carrying no path of its own. A sweep that +reads only `pathurl` finds a source's first use and silently misses every +later one, which is how a file gets left out of a package. + +Its reading of **structure**, on the same file, is wrong: + +| | the XML says | OTIO reports | +| --- | --- | --- | +| sequence rate | 24, ntsc FALSE | 29.97 | +| duration | 67148 frames | 96231 frames | +| audio tracks | 23 | 12 | + +So nothing in this module reads a rate, a duration, a track or a timecode from +the adapter, and none reaches a caller. If that ever changes, it needs its own +measurement first. + +One more subtlety in the counts: `uses` is **placements in the cut, not +clipitems in the file**. An FCP 7 XML writes one clipitem per audio channel, so +a stereo effect placed ten times appears as twenty clipitems across two tracks. +OTIO reports ten, which is the number a human means. + +## Adapters + +The adapters are not part of OpenTimelineIO's core distribution. A bare +`pip install opentimelineio` reads `.otio` and nothing else - none of the +formats an NLE actually exports. + +| Format | Suffix | Package | +| --- | --- | --- | +| FCP 7 XML (Premiere, Resolve) | `.xml` | `otio-fcp-adapter` | +| Final Cut Pro X | `.fcpxml` | `otio-fcpx-xml-adapter` | +| CMX 3600 EDL | `.edl` | `otio-cmx3600-adapter` | +| AAF (Avid) | `.aaf` | `otio-aaf-adapter` | +| OpenTimelineIO | `.otio` | (core) | + +`offloader[timeline]` installs the first of these, because FCP 7 XML is what +Premiere and Resolve both export for interchange. The others work if installed; +`--adapter` overrides the choice made from the suffix. + +## Layout at the destination + +`--layout mirror` (the default) keeps each file's path below its volume root, +so `D:\ChairsDoc_backup\audio\boom\260828_001.wav` lands at +`ChairsDoc_backup\audio\boom\260828_001.wav`. Longer than a flat folder, and +worth it for two reasons: it records where each file came from, and two files +of the same name from different trees cannot land on each other. + +`--layout flat` puts everything in one folder. If that would put two files on +one path, the offload is refused before a byte moves, the same as for a card. + +## The safety rule is different here, and narrower + +For a card, `assert_safe_destinations` refuses a destination inside the source. +That is right for a card: the source root *is* the card, and writing into it is +how you lose it. + +It is the wrong question for a selection. The search roots are a library being +read, and collecting a cut's gaps into a new folder on that same drive is the +ordinary case: + +```sh +offloader offload --timeline cut.xml --search-root E:\ChairsDoc \ + --dest E:\ChairsDoc\RowV6_Media +``` + +So a selection is checked against a narrower property that is exactly what +matters: **no file being read may sit at or beneath somewhere being written**. +That admits the case above and still refuses the one that destroys data. A +destination-relative path that is absolute, or that climbs out with `..`, is +refused too, before the job starts rather than partway through it. + +## What this deliberately does not do + +- **It does not rewrite the timeline.** Nothing here relinks, conforms, or + emits a repointed XML. Choosing between two files that share a name is a + decision with a person's judgement in it, and a tool that made it silently + would be worse than no tool. `resolve` gives you the table; you relink. +- **It does not trust OTIO with structure.** See above. +- **It does not copy what is already there.** `present` and `substituted` are + not offloaded, because copying them would put a second file of that name on + the drive and manufacture the ambiguity this whole module exists to detect. diff --git a/pyproject.toml b/pyproject.toml index 8db0d68..fc3b3d5 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,6 +41,13 @@ dependencies = [ [project.optional-dependencies] gui = ["PySide6>=6.6"] +# Reading edit timelines. The adapters live outside OpenTimelineIO's core +# distribution -- a bare `pip install opentimelineio` reads none of the +# formats an NLE exports -- so each one is named here explicitly. +timeline = [ + "opentimelineio>=0.18", + "otio-fcp-adapter>=1.0", +] dev = [ "pytest>=7.4", "pytest-cov>=5.0", @@ -48,6 +55,8 @@ dev = [ "hypothesis>=6.100", "ruff>=0.4", "PySide6>=6.6", + "opentimelineio>=0.18", + "otio-fcp-adapter>=1.0", ] [project.urls] diff --git a/src/offloader/cli.py b/src/offloader/cli.py index 0a84ffb..0c291a3 100644 --- a/src/offloader/cli.py +++ b/src/offloader/cli.py @@ -5,9 +5,11 @@ import argparse import sys import time +from dataclasses import replace from pathlib import Path from . import PRODUCT_NAME, __version__, engine, hashers, longpath, probe, retry, thumbs +from . import timeline as timeline_mod from .models import FileStatus, Job, Profile, VerificationMode from .reports import WRITERS from .util import format_elapsed, format_size @@ -193,6 +195,25 @@ def _common_options(parser: argparse.ArgumentParser) -> None: parser.add_argument("--quiet", action="store_true", help="suppress progress") +def _timeline_options(parser: argparse.ArgumentParser) -> None: + parser.add_argument("--search-root", type=Path, action="append", default=[], + dest="search_roots", metavar="PATH", + help="where the media already lives (repeatable). " + "Anything found here is not copied again") + parser.add_argument("--adapter", default=None, metavar="NAME", + help="OpenTimelineIO adapter to read with " + "(default: chosen from the file suffix)") + parser.add_argument("--no-proxy-substitution", dest="substitute_proxies", + action="store_false", default=True, + help="do not let a proxy already on disk stand in for " + "a camera original the timeline references") + parser.add_argument("--layout", choices=("mirror", "flat"), default="mirror", + help="'mirror' (default) keeps each file's path below " + "its volume root, so where it came from stays " + "visible and two same-named files cannot land on " + "each other; 'flat' puts every file in one folder") + + def build_parser() -> argparse.ArgumentParser: parser = argparse.ArgumentParser( prog="offloader", @@ -204,8 +225,12 @@ def build_parser() -> argparse.ArgumentParser: sub = parser.add_subparsers(dest="command", required=True) offload = sub.add_parser("offload", help="copy and verify a source to destinations") - offload.add_argument("--source", type=Path, required=True, - help="card or folder to offload") + what = offload.add_mutually_exclusive_group(required=True) + what.add_argument("--source", type=Path, + help="card or folder to offload") + what.add_argument("--timeline", type=Path, + help="edit timeline whose media to offload; only the " + "files not already under a --search-root are copied") offload.add_argument("--dest", type=Path, action="append", required=True, dest="destinations", metavar="PATH", help="destination root (repeat for multiple copies)") @@ -224,8 +249,21 @@ def build_parser() -> argparse.ArgumentParser: order.add_argument("--originals-first", dest="proxies_first", action="store_false", help="copy in plain tree order instead") + _timeline_options(offload) _common_options(offload) + resolve = sub.add_parser( + "resolve", + help="report which of a timeline's media is already here and which " + "is not, without copying anything") + resolve.add_argument("--timeline", type=Path, required=True, + help="the edit timeline to resolve") + resolve.add_argument("--csv", type=Path, default=None, metavar="PATH", + help="write the full per-reference table here") + resolve.add_argument("--quiet", action="store_true", + help="print the counts only, not the detail") + _timeline_options(resolve) + report = sub.add_parser( "report", help="regenerate reports from an existing tree without copying") report.add_argument("--source", type=Path, required=True, @@ -280,10 +318,111 @@ def _report_dir(args: argparse.Namespace, job: Job, fallback_root: Path) -> Path return fallback_root / f"{job.name}_Reports" +def _resolve_timeline(args: argparse.Namespace) -> timeline_mod.Report: + """Read the timeline and work out what is already here.""" + if not args.search_roots: + # A usage error, routed through main() like every other one so it + # reports as exit 2 rather than leaving by its own door. + raise ValueError( + "--timeline needs at least one --search-root: without one, every " + "reference is a gap and the whole cut would be copied" + ) + references = timeline_mod.read_references(args.timeline, args.adapter) + return timeline_mod.resolve( + references, args.search_roots, + timeline=args.timeline, + excludes=tuple(engine.DEFAULT_EXCLUDES) + tuple(getattr(args, "exclude", [])), + substitute_proxies=args.substitute_proxies, + ) + + +#: Status to the line it prints. Ordered worst-first, so what needs a human +#: is at the bottom of the terminal where it will be read. +_STATUS_LINES = ( + (timeline_mod.Status.PRESENT, "already here"), + (timeline_mod.Status.SUBSTITUTED, "satisfied by a proxy already here"), + (timeline_mod.Status.GAP, "not here, and copyable"), + (timeline_mod.Status.GENERATED, "no media in the timeline to supply"), + (timeline_mod.Status.MISSING, "not here and not where the timeline says"), + (timeline_mod.Status.AMBIGUOUS, "MORE THAN ONE CANDIDATE, not chosen"), +) + + +def _print_resolution(report: timeline_mod.Report, detail: bool = True) -> None: + counts = report.counts + total = len(report.resolutions) + print(f"{report.timeline.name}: {total} media references, " + f"{len(report.search_roots)} search root(s)") + for status, label in _STATUS_LINES: + if counts[status]: + print(f" {counts[status]:5d} {label}") + if report.gaps: + print(f" {format_size(report.gap_bytes)} to copy") + + if not detail: + return + for resolution in report.of(timeline_mod.Status.AMBIGUOUS, + timeline_mod.Status.MISSING): + print(f"\n [{resolution.status.value}] {resolution.name} " + f"(used {resolution.reference.uses}x)") + if resolution.note: + print(f" {resolution.note}") + for candidate in resolution.alternatives: + print(f" candidate: {candidate}") + + +def _write_resolution_csv(report: timeline_mod.Report, path: Path) -> None: + import csv + + path.parent.mkdir(parents=True, exist_ok=True) + with path.open("w", newline="", encoding="utf-8") as handle: + writer = csv.writer(handle) + writer.writerow(["file", "status", "uses", "timeline_path", + "location", "other_candidates", "note"]) + for r in report.resolutions: + writer.writerow([ + r.name, r.status.value, r.reference.uses, + str(r.reference.target) if r.reference.target else "", + str(r.location) if r.location else "", + " | ".join(str(a) for a in r.alternatives), + r.note, + ]) + + +def cmd_resolve(args: argparse.Namespace) -> int: + report = _resolve_timeline(args) + _print_resolution(report, detail=not args.quiet) + if args.csv: + _write_resolution_csv(report, args.csv) + print(f"\nwrote {args.csv}") + + # Exit non-zero on anything a human has to settle. A generated reference is + # reported but does not fail the command: no offload can supply it, so + # holding the exit code hostage to it would only teach a caller to ignore + # the code. + unsettled = report.counts[timeline_mod.Status.AMBIGUOUS] + \ + report.counts[timeline_mod.Status.MISSING] + return 1 if unsettled else 0 + + def cmd_offload(args: argparse.Namespace) -> int: options = _options_from(args, args.destinations) + source = args.source + + if getattr(args, "timeline", None): + report = _resolve_timeline(args) + _print_resolution(report, detail=True) + selection = timeline_mod.selection(report, flat=args.layout == "flat") + if not selection: + print("\nnothing to copy: every reference is already here.") + return 0 + print(f"\ncopying {len(selection)} file(s), " + f"{format_size(report.gap_bytes)}\n") + options = replace(options, selection=selection) + source = args.timeline + progress = _Progress(not args.quiet) - job = engine.run(args.source, options, progress) + job = engine.run(source, options, progress) progress.done() reports = _write_reports( @@ -403,7 +542,8 @@ def cmd_gui(_args: argparse.Namespace) -> int: def main(argv: list[str] | None = None) -> int: args = build_parser().parse_args(argv) handlers = {"offload": cmd_offload, "report": cmd_report, - "verify": cmd_verify, "info": cmd_info, "gui": cmd_gui} + "verify": cmd_verify, "info": cmd_info, "gui": cmd_gui, + "resolve": cmd_resolve} try: return handlers[args.command](args) except KeyboardInterrupt: @@ -412,6 +552,9 @@ def main(argv: list[str] | None = None) -> int: except engine.UnsafeDestination as exc: print(f"refused: {exc}", file=sys.stderr) return 3 + except timeline_mod.TimelineSupportMissing as exc: + print(f"error: {exc}", file=sys.stderr) + return 4 except (OSError, ValueError) as exc: print(f"error: {exc}", file=sys.stderr) return 2 diff --git a/src/offloader/engine.py b/src/offloader/engine.py index 1dfd1c6..65c7da1 100644 --- a/src/offloader/engine.py +++ b/src/offloader/engine.py @@ -109,6 +109,7 @@ def assert_safe_destinations(source_root: Path, destinations: Sequence[Path]) -> def assert_no_destination_collisions( sources: Sequence[Path], source_root: Path, destination_roots: Sequence[Path], preserve_structure: bool, + relatives: dict[Path, Path] | None = None, ) -> None: """Refuse a layout that maps two source files onto one destination path. @@ -122,13 +123,23 @@ def assert_no_destination_collisions( Paths are compared with `os.path.normcase`, so on Windows this also catches two names differing only in case: distinct on the case-sensitive volume they came from, one file on the volume they are going to. + + `relatives` maps a source to its destination-relative path, for a + selection whose layout the engine did not derive. It matters more there + than for a card, not less: a selection is drawn from several volumes at + once, so two files of the same name arriving from different trees is the + normal case rather than the unlucky one. """ claimed: dict[str, Path] = {} collisions: list[tuple[Path, Path, Path]] = [] for source in sources: for root in destination_roots: - target = _destination_for(source, source_root, root, preserve_structure) + if relatives is not None: + target = root / relatives[source] + else: + target = _destination_for(source, source_root, root, + preserve_structure) key = os.path.normcase(str(target)) if key in claimed: collisions.append((claimed[key], source, target)) @@ -204,6 +215,80 @@ class ProgressEvent: ProgressCallback = Callable[[ProgressEvent], None] +@dataclass(frozen=True) +class SelectedFile: + """One file to offload, and where it lands under each destination root. + + A scanned card is a single tree, so a destination path can be derived from + the source root. A selection is not: `timeline.selection` draws files from + several volumes at once, and how they are laid out at the destination is + the selector's decision rather than something the engine can infer. Each + file therefore carries its own destination-relative path, and the engine + infers nothing. + """ + + source: Path + #: Where this file lands beneath each destination root. Relative, and + #: checked to be: see `assert_selection_is_safe`. + relative: Path + #: The tree the file was found in. Reporting only -- `FileEntry.relative` + #: uses it to show a path a reader can place. + root: Path = Path() + + +def assert_selection_is_safe(selection: Sequence[SelectedFile], + destinations: Sequence[Path]) -> None: + """Refuse a selection that could destroy what it is copying. + + `assert_safe_destinations` asks whether the destination is inside the + source, which is the right question for a card: the source root *is* the + card, and writing into it is how you lose it. It is the wrong question for + a selection, where the search roots are a library being read and the + destination is very often a new folder inside it -- resolving a cut + against `E:/ChairsDoc` and collecting its gaps into + `E:/ChairsDoc/RowV6_Media` is the ordinary case, not a mistake. + + The property that actually matters is narrower, and checkable exactly: no + file being read may sit at or beneath somewhere being written. That admits + the ordinary case and still refuses the one that eats data. + + A relative path that is absolute, or that climbs out with `..`, would + escape the destination root and write somewhere the caller never named. + Both are refused here, before any byte moves, rather than at the point of + use where they would leave a half-copied job behind. + """ + resolved_dests: dict[Path, Path] = {} + for destination in destinations: + resolved = Path(destination).resolve() + if resolved in resolved_dests: + raise UnsafeDestination( + f"destinations {resolved_dests[resolved]} and {destination} " + "are the same directory" + ) + resolved_dests[resolved] = Path(destination) + + for entry in selection: + relative = Path(entry.relative) + if relative.is_absolute() or relative.drive or relative.anchor: + raise UnsafeDestination( + f"selection for {entry.source} has an absolute destination " + f"path {relative}; it must be relative to the destination root" + ) + if any(part == os.pardir for part in relative.parts): + raise UnsafeDestination( + f"selection for {entry.source} escapes the destination with " + f"{relative}" + ) + + source = Path(entry.source).resolve() + for resolved, original in resolved_dests.items(): + if source == resolved or resolved in source.parents: + raise UnsafeDestination( + f"{entry.source} is inside the destination {original}; " + "offloading it would copy a file over itself" + ) + + @dataclass class OffloadOptions: destinations: Sequence[Path] @@ -229,6 +314,13 @@ class OffloadOptions: #: How hard to try again when a read fails for a transient-looking reason. #: Marginal cards and readers routinely succeed on a second attempt. retry: retry_mod.RetryPolicy = field(default_factory=retry_mod.RetryPolicy) + #: An explicit file set to offload, in place of scanning the source root. + #: Files may come from any number of volumes and each carries its own + #: destination-relative path, so `preserve_structure` and `excludes` do + #: not apply -- the selector has already decided both. `proxies_first` + #: does not apply either: a selection is transferred in the order given, + #: which is the only order the selector can be held to. + selection: Sequence[SelectedFile] | None = None def __post_init__(self) -> None: # The data profile is defined by the absence of media work, so enforce @@ -520,6 +612,11 @@ def run(source_root: Path, options: OffloadOptions, Pass a `JobControl` to allow pausing or cancelling mid-flight; a cancelled job returns normally, with `Job.cancelled` set and the files it did finish intact. + + With `options.selection` set, the file set is taken from the selection + instead of by scanning, and `source_root` becomes a label for the job + rather than a tree to walk -- it may be a timeline file, which is what the + files were selected from. """ source_root = Path(source_root) if not source_root.exists(): @@ -529,21 +626,44 @@ def run(source_root: Path, options: OffloadOptions, dest_roots = [Path(d) for d in options.destinations] if not dest_roots: raise ValueError("at least one destination is required") - assert_safe_destinations(source_root, dest_roots) - files = scan(source_root, options.excludes) + #: Per-file destination-relative paths, for a selection. None for a card, + #: where the destination is derived from the source root as it always was. + relatives: dict[Path, Path] | None = None + + if options.selection is not None: + assert_selection_is_safe(options.selection, dest_roots) + files = [Path(entry.source) for entry in options.selection] + seen = {os.path.normcase(str(p)) for p in files} + if len(seen) != len(files): + raise ValueError("selection lists the same source file twice") + relatives = {Path(e.source): Path(e.relative) for e in options.selection} + roots = {Path(e.source): Path(e.root) for e in options.selection} + else: + assert_safe_destinations(source_root, dest_roots) + files = scan(source_root, options.excludes) + roots = {} + assert_no_destination_collisions(files, source_root, dest_roots, - options.preserve_structure) + options.preserve_structure, relatives) counters = _Counters(job_bytes_total=sum(p.stat().st_size for p in files)) # `files` stays the tree order the card is laid out in and remains the # authority on what the job contains; `transfer` is only the sequence the # bytes move in. job.files is put back into `files` order before returning. - transfer = order_for_transfer(files, source_root, options.proxies_first) + # A selection is already in the order its selector chose, and reordering + # by proxy directory would only second-guess that. + if options.selection is not None: + transfer = list(files) + else: + transfer = order_for_transfer(files, source_root, options.proxies_first) host = sysinfo.collect() job = Job( - name=options.job_name or source_root.name, + # A selection names its job after the timeline it came from, and a + # timeline is a file: ".xml" in a report header helps nobody. + name=options.job_name or (source_root.stem if source_root.is_file() + else source_root.name), source_root=source_root, destination_roots=dest_roots, verification=options.verification, @@ -576,16 +696,22 @@ def emit(event: ProgressEvent) -> None: stat = source.stat() entry = FileEntry( source=source, - source_root=source_root, + # For a selection this is the tree the file came from, not the job + # label, so the report shows a path a reader can place. + source_root=roots.get(source, source_root), size=stat.st_size, created=getattr(stat, "st_birthtime", stat.st_ctime), modified=stat.st_mtime, ) - targets = [ - _destination_for(source, source_root, root, options.preserve_structure) - for root in dest_roots - ] + if relatives is not None: + targets = [root / relatives[source] for root in dest_roots] + else: + targets = [ + _destination_for(source, source_root, root, + options.preserve_structure) + for root in dest_roots + ] emit(ProgressEvent(index, len(files), source.name, "copy", 0, stat.st_size, @@ -825,7 +951,7 @@ def note_retry(attempt: int, exc: BaseException, pause: float, entry.thumbnail_source = picture elif companions.needs_proxy(source): picture, used_proxy = companions.thumbnail_source( - source, source_root) + source, roots.get(source, source_root)) if used_proxy: entry.thumbnail_source = picture diff --git a/src/offloader/timeline.py b/src/offloader/timeline.py new file mode 100644 index 0000000..12ea48d --- /dev/null +++ b/src/offloader/timeline.py @@ -0,0 +1,513 @@ +"""Resolve an edit timeline to the media it references. + +A card offload knows its source: everything under one root, and the only +question is whether it arrived intact. This module answers the question that +arrives with a cut coming back from an editor -- *which files does this +timeline actually need, and are they all here?* -- and turns the answer into a +file set `engine.run` can offload with the same verified copy and the same +paperwork. + +Reading the timeline is OpenTimelineIO's job and nothing else's. What it is +deliberately not trusted with is structure. Measured against the conform that +prompted this module, the `fcp_xml` adapter recovered all 334 media references +of a Premiere-exported FCP 7 XML exactly -- including the clipitems whose +`` is an id reference carrying no path of its own, which is the trap a +hand-rolled sweep falls into -- while reporting that sequence as 29.97 fps with +12 audio tracks when the file plainly declares 24 fps and 23. So the adapter is +read for `media_reference` and for nothing else, and no rate, duration or track +count from it reaches a caller. See `docs/timeline.md`. + +The resolver is the part worth having. That same timeline produced: + +* 244 references already under the search root, addressed by their real path. +* 32 also already under it, but addressed by a path on another volume: the + same bytes, filed somewhere else. Copying those would have been 1.4 GB of + pointless transfer and, worse, would have manufactured 32 basename + collisions on a drive that had none. +* 15 camera originals -- 55 GB of BRAW -- each with a frame-accurate proxy + already on the drive. +* 43 genuinely absent, which is the offload. +* 20 basenames with more than one copy under the root, 12 of them resolving to + *different* files. Ten were "MISSING MEDIA" stand-in slates left by an + earlier conform, sharing a name with the real archival footage that arrived + later. Relinking by filename picks one of those at random, and the one it + picks reports as online. + +That last number is why `AMBIGUOUS` is a status here rather than a warning a +caller may ignore. A tool that quietly picked the best-looking candidate would +have put a purple slate in a finished film and said "Verified" underneath it. +""" + +from __future__ import annotations + +import os +from collections.abc import Iterable, Sequence +from dataclasses import dataclass, field +from enum import Enum +from pathlib import Path, PurePath +from urllib.parse import unquote + +from . import companions, engine +from .hashers import DEFAULT_ALGORITHM, hash_file + + +class TimelineSupportMissing(RuntimeError): + """OpenTimelineIO, or the adapter this file needs, is not installed.""" + + +#: File suffix to the OTIO adapter that reads it. `.xml` is FCP 7 XML, which +#: is what Premiere and Resolve both export for interchange; Final Cut Pro X's +#: format is a different schema and uses its own suffix. +ADAPTERS = { + ".xml": "fcp_xml", + ".fcpxml": "fcpx_xml", + ".edl": "cmx_3600", + ".aaf": "AAF", + ".otio": "otio_json", +} + +#: The pip extra that supplies each adapter, for the error message. Only +#: `otio_json` ships with OpenTimelineIO itself; every other adapter was moved +#: out of core into its own distribution, so a bare `pip install +#: opentimelineio` reads none of the formats an editor will hand back. +ADAPTER_PACKAGES = { + "fcp_xml": "otio-fcp-adapter", + "fcpx_xml": "otio-fcpx-xml-adapter", + "cmx_3600": "otio-cmx3600-adapter", + "AAF": "otio-aaf-adapter", + "otio_json": "opentimelineio", +} + + +class Status(str, Enum): + """What became of one media reference.""" + + #: Found under a search root. Nothing to copy. + PRESENT = "present" + #: Found nowhere under the roots, but readable where the timeline says it + #: is. This is the offload. + GAP = "gap" + #: The reference is a camera original with no decoder, and a frame-matched + #: proxy is already under a root. Nothing to copy; relink to the proxy. + SUBSTITUTED = "substituted" + #: More than one file under the roots carries this name and they are not + #: the same file. Refuses to choose. + AMBIGUOUS = "ambiguous" + #: Not under the roots and not where the timeline says either. + MISSING = "missing" + #: The timeline carries no media for this at all -- a Premiere-native + #: graphic, a title, a generator. No file exists to supply. + GENERATED = "generated" + + +#: Statuses that leave the caller with something to do before a conform. +UNRESOLVED = (Status.AMBIGUOUS, Status.MISSING, Status.GENERATED) + + +@dataclass(frozen=True) +class Reference: + """One media reference in a timeline, and how often the cut uses it.""" + + name: str + #: Where the timeline says the media is. None for a generated reference. + target: Path | None + #: Placements in the cut, not clipitems in the file. An FCP 7 XML writes + #: one clipitem per audio *channel*, so a stereo effect placed ten times + #: appears as twenty clipitems across two tracks. Counting those would + #: report a sound effect as used twice as often as it is. + uses: int = 1 + generated: bool = False + + +@dataclass +class Resolution: + """A reference, and where -- if anywhere -- it was found.""" + + reference: Reference + status: Status + #: The file that satisfies this reference: where it already is, or the + #: source to copy from. + location: Path | None = None + #: Other files under the roots with the same name. Populated for + #: AMBIGUOUS, and for PRESENT when the copies are byte-identical. + alternatives: list[Path] = field(default_factory=list) + note: str = "" + + @property + def name(self) -> str: + return self.reference.name + + +@dataclass +class Report: + """The whole timeline resolved against a set of search roots.""" + + timeline: Path + search_roots: list[Path] + resolutions: list[Resolution] + + def of(self, *statuses: Status) -> list[Resolution]: + return [r for r in self.resolutions if r.status in statuses] + + @property + def gaps(self) -> list[Resolution]: + return self.of(Status.GAP) + + @property + def unresolved(self) -> list[Resolution]: + """Everything a human has to deal with before the cut will conform.""" + return self.of(*UNRESOLVED) + + @property + def counts(self) -> dict[Status, int]: + return {s: sum(1 for r in self.resolutions if r.status is s) for s in Status} + + @property + def gap_bytes(self) -> int: + total = 0 + for resolution in self.gaps: + if resolution.location is not None: + try: + total += resolution.location.stat().st_size + except OSError: # pragma: no cover - vanished mid-run + pass + return total + + +# -------------------------------------------------------------------------- +# Reading +# -------------------------------------------------------------------------- + +def _url_to_path(url: str) -> Path | None: + """A `file:` URL from a timeline, as a local path. + + NLEs are not consistent here: Premiere writes `file://localhost/E:/...`, + Resolve writes `file:///E:/...`, and a hand-edited XML may carry a bare + path. Percent-escapes are always possible because a clip name may contain + a space. Forward slashes are left alone -- `Path` accepts them on Windows, + and normalising them by hand is how a UNC path loses its leading pair. + """ + if not url: + return None + text = unquote(url) + for prefix in ("file://localhost/", "file:///", "file://"): + if text.startswith(prefix): + text = text[len(prefix):] + break + if not text: + return None + return Path(text) + + +def _adapter_for(path: Path, adapter: str | None = None) -> str: + if adapter: + return adapter + suffix = path.suffix.lower() + if suffix not in ADAPTERS: + raise TimelineSupportMissing( + f"no timeline adapter for {suffix or path.name!r}; " + f"known formats: {', '.join(sorted(ADAPTERS))}" + ) + return ADAPTERS[suffix] + + +def read_references(path: Path, adapter: str | None = None) -> list[Reference]: + """Every distinct piece of media a timeline points at. + + Deduplicated by path and counted, so `uses` is how many times the cut + actually places that file. A reference the timeline carries no media for + -- a Premiere-native graphic, a title, a generator -- comes back with + `generated=True` and no target, because there is no file to supply and + saying so is more useful than dropping it. + """ + path = Path(path) + if not path.is_file(): + raise FileNotFoundError(f"timeline not found: {path}") + name = _adapter_for(path, adapter) + + try: + import opentimelineio as otio + except ImportError as exc: # pragma: no cover - import-guard path + raise TimelineSupportMissing( + "reading timelines needs OpenTimelineIO: " + 'pip install "offloader[timeline]"' + ) from exc + + if name not in set(otio.adapters.available_adapter_names()): + package = ADAPTER_PACKAGES.get(name, "") + hint = f": pip install {package}" if package else "" + raise TimelineSupportMissing( + f"OpenTimelineIO has no {name!r} adapter installed{hint}" + ) + + timeline = otio.adapters.read_from_file(str(path), name) + + order: list[str] = [] + found: dict[str, dict] = {} + generated = 0 + for clip in timeline.find_clips(): + media = clip.media_reference + url = getattr(media, "target_url", None) + resolved = _url_to_path(url) if url else None + if resolved is None: + # A generator, a title, a missing reference: real clips in the cut + # that no file can satisfy. Counted once, reported once. + generated += 1 + continue + key = os.path.normcase(str(resolved)) + if key not in found: + order.append(key) + found[key] = {"path": resolved, "name": resolved.name, "uses": 0} + found[key]["uses"] += 1 + + references = [ + Reference(name=found[k]["name"], target=found[k]["path"], uses=found[k]["uses"]) + for k in order + ] + if generated: + references.append( + Reference(name=f"{generated} timeline-native item(s)", target=None, + uses=generated, generated=True) + ) + return references + + +# -------------------------------------------------------------------------- +# Resolving +# -------------------------------------------------------------------------- + +def index(roots: Iterable[Path], + excludes: Sequence[str] = engine.DEFAULT_EXCLUDES) -> dict[str, list[Path]]: + """Every file under every search root, keyed by lowercased basename. + + Built once and shared across the whole resolve: a timeline of a few hundred + references against a drive of a few thousand files would otherwise be a + few hundred tree walks. + """ + table: dict[str, list[Path]] = {} + for root in roots: + root = Path(root) + if not root.is_dir(): + raise NotADirectoryError(f"search root not found: {root}") + for found in engine.scan(root, excludes): + table.setdefault(found.name.lower(), []).append(found) + return table + + +def _same_file(paths: Sequence[Path], algorithm: str = DEFAULT_ALGORITHM) -> bool: + """Whether every path holds identical bytes. + + Size first, because it settles almost every case for the price of a stat, + and only then the checksum. Two files of the same size are common; two of + the same size and checksum are the same file. + """ + try: + sizes = {p.stat().st_size for p in paths} + except OSError: # pragma: no cover - vanished mid-run + return False + if len(sizes) > 1: + return False + digests = set() + for path in paths: + try: + digests.add(hash_file(path, algorithm)) + except OSError: # pragma: no cover - unreadable + return False + return len(digests) == 1 + + +def _closest(candidates: Sequence[Path], target: Path | None) -> Path: + """The candidate whose tail matches the wanted path for longest. + + A guess, and labelled as one wherever it is used. `.../Sound Effects/ + Wooshes/Ceiling Fan.mp3` scores 3 against the same three trailing + components and 1 against a stand-in of that name sitting loose in another + folder, which is the right ranking and still not proof. + """ + if target is None: + return candidates[0] + wanted = [p.lower() for p in PurePath(target).parts] + + def score(path: Path) -> tuple[int, str]: + parts = [p.lower() for p in PurePath(path).parts] + n = 0 + while n < min(len(wanted), len(parts)) and wanted[-1 - n] == parts[-1 - n]: + n += 1 + return n, str(path) + + return max(candidates, key=score) + + +def _proxy_for(target: Path, table: dict[str, list[Path]]) -> Path | None: + """A proxy under the search roots standing in for a camera original. + + Stem matching, the same rule `companions.find_proxy` uses on a card, but + looking across the indexed roots rather than in the sibling directories: + by the time a cut comes back, the proxy the editor cut with is rarely + filed next to the original any more. + """ + stem = target.stem.lower() + for suffix in companions.PROXY_SUFFIXES: + hits = table.get(f"{stem}{suffix}", []) + if len(hits) == 1: + return hits[0] + if hits: + # More than one proxy of that name is exactly the ambiguity this + # module refuses to guess through. Leave it to the caller. + return None + return None + + +def _frames(path: Path) -> int | None: + """Frame count, or None when it cannot be established. + + Best effort on purpose: ffprobe is an optional dependency here, and BRAW + is read out of the container because ffprobe cannot open it at all. A + number that cannot be got is reported as absent, never as agreement. + """ + try: + from . import probe as probe_mod + return probe_mod.probe(path).frame_count + except Exception: # pragma: no cover - probe unavailable + return None + + +def resolve(references: Sequence[Reference], search_roots: Sequence[Path], *, + timeline: Path | None = None, + excludes: Sequence[str] = engine.DEFAULT_EXCLUDES, + substitute_proxies: bool = True, + algorithm: str = DEFAULT_ALGORITHM, + table: dict[str, list[Path]] | None = None) -> Report: + """Work out, for each reference, whether the media is already here. + + The order of the tests is the whole design: + + 1. A reference the timeline carries no media for is GENERATED and stops + there. No search would ever satisfy it. + 2. Anything under a search root with that basename wins, whatever path the + timeline asked for. This is what stops a file being copied again under + a second name because an editor addressed it on their own volume. + Several copies that are byte-identical are still PRESENT, with the + duplicates recorded; several that differ are AMBIGUOUS and nothing is + chosen. + 3. A camera original with no decoder may be satisfied by a proxy already + under the roots, if the frame counts agree. + 4. Only then, a file readable where the timeline says it is, is a GAP: the + thing actually worth copying. + 5. Anything left is MISSING, and no offload will fix it. + """ + roots = [Path(r) for r in search_roots] + if table is None: + table = index(roots, excludes) + + resolutions: list[Resolution] = [] + for reference in references: + resolutions.append( + _resolve_one(reference, table, substitute_proxies, algorithm) + ) + return Report(timeline=Path(timeline) if timeline else Path(), + search_roots=roots, resolutions=resolutions) + + +def _resolve_one(reference: Reference, table: dict[str, list[Path]], + substitute_proxies: bool, algorithm: str) -> Resolution: + if reference.generated or reference.target is None: + return Resolution(reference, Status.GENERATED, + note="the timeline carries no media for this") + + target = reference.target + candidates = table.get(target.name.lower(), []) + + if len(candidates) == 1: + return Resolution(reference, Status.PRESENT, location=candidates[0]) + + if len(candidates) > 1: + if _same_file(candidates, algorithm): + best = _closest(candidates, target) + others = [c for c in candidates if c != best] + return Resolution( + reference, Status.PRESENT, location=best, alternatives=others, + note=f"{len(candidates)} byte-identical copies; either resolves", + ) + best = _closest(candidates, target) + others = [c for c in candidates if c != best] + return Resolution( + reference, Status.AMBIGUOUS, location=None, + alternatives=[best, *others], + note=(f"{len(candidates)} different files share this name; " + f"closest to the timeline path is {best}"), + ) + + if substitute_proxies and companions.needs_proxy(target): + proxy = _proxy_for(target, table) + if proxy is not None: + original = target if target.is_file() else None + note = _substitution_note(original, proxy) + if note is None: + return Resolution( + reference, Status.AMBIGUOUS, location=None, + alternatives=[proxy], + note=("a proxy of this name is present but its frame count " + "does not match the original; relinking to it would " + "move every edit point"), + ) + return Resolution(reference, Status.SUBSTITUTED, location=proxy, + note=note) + + if target.is_file(): + return Resolution(reference, Status.GAP, location=target) + + return Resolution(reference, Status.MISSING, location=None, + note=f"not under any search root, and not at {target}") + + +def _substitution_note(original: Path | None, proxy: Path) -> str | None: + """Whether `proxy` may stand in for `original`, and why. + + None means refuse. A proxy is only a safe stand-in if it is the same length + as what it replaces, frame for frame: relink a clip to a shorter file and + every edit point after it moves. Where the original cannot be reached the + check cannot be made, and the note says so rather than implying agreement. + """ + if original is None: + return "proxy stands in for an original that is not reachable; frames unverified" + want = _frames(original) + got = _frames(proxy) + if want is None or got is None: + return "proxy stands in for the original; frame counts unverified" + if want != got: + return None + return f"proxy matches the original frame for frame ({want} frames)" + + +# -------------------------------------------------------------------------- +# Handing the gaps to the engine +# -------------------------------------------------------------------------- + +def selection(report: Report, *, flat: bool = False) -> list[engine.SelectedFile]: + """The GAP files, as a file set `engine.run` can offload. + + Only the gaps. PRESENT and SUBSTITUTED are already on the drive and + copying them again would put a second file of the same name on it, which + is the collision this module exists to detect rather than create. + + The default layout keeps each file's path below its volume root, so + `D:/ChairsDoc_backup/audio/boom/260828_001.wav` lands at + `ChairsDoc_backup/audio/boom/260828_001.wav`. That is longer than a flat + folder and worth it: it records where each file came from, and two files + of the same name from different trees cannot land on each other. + """ + files: list[engine.SelectedFile] = [] + for resolution in report.gaps: + source = resolution.location + if source is None: # pragma: no cover - gaps always have one + continue + if flat: + relative = PurePath(source.name) + else: + relative = PurePath(*PurePath(source).parts[1:]) + files.append(engine.SelectedFile(source=source, relative=Path(relative), + root=Path(source.anchor))) + # Proxies first, for the same reason a card offload hoists them: an editor + # can start cutting while the originals are still moving. + return sorted(files, key=lambda f: not engine.is_proxy(f.source, f.root)) diff --git a/tests/test_timeline.py b/tests/test_timeline.py new file mode 100644 index 0000000..8856bb6 --- /dev/null +++ b/tests/test_timeline.py @@ -0,0 +1,567 @@ +"""Resolving an edit timeline to the media it references. + +The resolver tests need no OpenTimelineIO: they build `Reference` objects +directly, because what is being tested is the decision, not the parse. Only +the reader tests need the adapter, and they skip without it. +""" + +from __future__ import annotations + +import csv +import os +from pathlib import Path + +import pytest + +from offloader import cli, engine, timeline +from offloader.models import FileStatus, VerificationMode +from offloader.timeline import Reference, Status + +# -------------------------------------------------------------------------- +# Fixtures +# -------------------------------------------------------------------------- + + +def _write(path: Path, data: bytes) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(data) + return path + + +@pytest.fixture +def library(tmp_path: Path) -> Path: + """A drive with media already on it, in two different folders.""" + root = tmp_path / "library" + _write(root / "package" / "A001_C001.mp4", b"picture one" * 100) + _write(root / "audio" / "260822_001.wav", b"sound one" * 100) + return root + + +def _ref(name: str, target: Path, uses: int = 1) -> Reference: + return Reference(name=name, target=target, uses=uses) + + +#: A path on a volume that is not attached, which is how an editor's export +#: addresses everything. Nothing here should ever resolve by walking it. +def _elsewhere(name: str) -> Path: + return Path(os.sep, "Volumes", "Edit Operational 14TB", "media", name) + + +# -------------------------------------------------------------------------- +# The resolver +# -------------------------------------------------------------------------- + + +def test_present_when_the_name_is_under_a_root(library: Path): + """The editor's path is unreachable; the file is here under another one.""" + report = timeline.resolve([_ref("A001_C001.mp4", _elsewhere("A001_C001.mp4"))], + [library]) + (resolution,) = report.resolutions + assert resolution.status is Status.PRESENT + assert resolution.location == library / "package" / "A001_C001.mp4" + + +def test_gap_when_reachable_but_not_here(library: Path, tmp_path: Path): + other = _write(tmp_path / "elsewhere" / "B002.mov", b"needs copying") + report = timeline.resolve([_ref("B002.mov", other)], [library]) + (resolution,) = report.resolutions + assert resolution.status is Status.GAP + assert resolution.location == other + assert report.gap_bytes == other.stat().st_size + + +def test_missing_when_nowhere_at_all(library: Path): + report = timeline.resolve([_ref("gone.mov", _elsewhere("gone.mov"))], [library]) + (resolution,) = report.resolutions + assert resolution.status is Status.MISSING + assert resolution.location is None + + +def test_identical_duplicates_resolve_and_are_not_a_gap(library: Path): + """The 32-file case: the same bytes filed twice under the search root. + + Copying these again is not merely wasted transfer, it puts a third file of + that name on the drive and makes a name-only relink ambiguous where it was + not. So they must read as PRESENT, and the duplicates must be reported. + """ + same = (library / "package" / "A001_C001.mp4").read_bytes() + _write(library / "backup" / "A001_C001.mp4", same) + + report = timeline.resolve([_ref("A001_C001.mp4", _elsewhere("A001_C001.mp4"))], + [library]) + (resolution,) = report.resolutions + assert resolution.status is Status.PRESENT + assert len(resolution.alternatives) == 1 + assert "byte-identical" in resolution.note + assert report.gaps == [] + + +def test_differing_duplicates_are_ambiguous_and_nothing_is_chosen(library: Path): + """The purple-slate case, and the whole reason this module exists. + + An earlier conform left a stand-in slate of the same name as the real + archival footage that arrived later. Both are under the search root. A + tool that picks one puts a placeholder in a finished film and reports it + as online. + """ + _write(library / "stand-ins" / "88 Paralympics.mp4", b"purple slate") + _write(library / "archival" / "88 Paralympics.mp4", b"the real footage" * 50) + + wanted = Path(library, "archival", "88 Paralympics.mp4") + report = timeline.resolve([_ref("88 Paralympics.mp4", wanted)], [library]) + (resolution,) = report.resolutions + + assert resolution.status is Status.AMBIGUOUS + assert resolution.location is None, "must not choose between different files" + assert len(resolution.alternatives) == 2 + # The closest-path guess is offered, and labelled as a guess in the note. + assert resolution.alternatives[0] == wanted + assert "different files share this name" in resolution.note + assert report.unresolved == [resolution] + + +def test_generated_reference_is_reported_not_dropped(): + report = timeline.resolve( + [Reference(name="3 timeline-native item(s)", target=None, uses=3, + generated=True)], + [], + table={}, + ) + (resolution,) = report.resolutions + assert resolution.status is Status.GENERATED + assert report.gaps == [] + + +# -------------------------------------------------------------------------- +# Proxy substitution +# -------------------------------------------------------------------------- + + +def test_camera_original_is_satisfied_by_a_proxy_already_here( + library: Path, tmp_path: Path, monkeypatch): + """55 GB of BRAW need not move when a frame-matched proxy is on the drive.""" + original = _write(tmp_path / "camera" / "A006_C121.braw", b"raw" * 1000) + proxy = _write(library / "package" / "A006_C121.mp4", b"proxy") + monkeypatch.setattr(timeline, "_frames", lambda path: 1065) + + report = timeline.resolve([_ref("A006_C121.braw", original)], [library]) + (resolution,) = report.resolutions + + assert resolution.status is Status.SUBSTITUTED + assert resolution.location == proxy + assert "1065 frames" in resolution.note + assert report.gaps == [], "the original must not also be copied" + + +def test_proxy_of_the_wrong_length_is_refused(library: Path, tmp_path: Path, + monkeypatch): + """A shorter proxy moves every edit point after it. Never substitute it.""" + original = _write(tmp_path / "camera" / "A006_C121.braw", b"raw" * 1000) + _write(library / "package" / "A006_C121.mp4", b"proxy") + lengths = {"braw": 1065, "mp4": 900} + monkeypatch.setattr(timeline, "_frames", + lambda path: lengths[Path(path).suffix.lstrip(".")]) + + report = timeline.resolve([_ref("A006_C121.braw", original)], [library]) + (resolution,) = report.resolutions + + assert resolution.status is Status.AMBIGUOUS + assert resolution.location is None + assert "does not match" in resolution.note + + +def test_unverifiable_frame_count_says_so_rather_than_implying_agreement( + library: Path, tmp_path: Path, monkeypatch): + original = _write(tmp_path / "camera" / "A006_C121.braw", b"raw" * 1000) + _write(library / "package" / "A006_C121.mp4", b"proxy") + monkeypatch.setattr(timeline, "_frames", lambda path: None) + + report = timeline.resolve([_ref("A006_C121.braw", original)], [library]) + (resolution,) = report.resolutions + assert resolution.status is Status.SUBSTITUTED + assert "unverified" in resolution.note + + +def test_substitution_can_be_switched_off(library: Path, tmp_path: Path): + original = _write(tmp_path / "camera" / "A006_C121.braw", b"raw" * 1000) + _write(library / "package" / "A006_C121.mp4", b"proxy") + + report = timeline.resolve([_ref("A006_C121.braw", original)], [library], + substitute_proxies=False) + (resolution,) = report.resolutions + assert resolution.status is Status.GAP + + +# -------------------------------------------------------------------------- +# Turning a report into a file set +# -------------------------------------------------------------------------- + + +def test_selection_carries_only_the_gaps(library: Path, tmp_path: Path): + gap = _write(tmp_path / "elsewhere" / "B002.mov", b"needs copying") + references = [ + _ref("A001_C001.mp4", _elsewhere("A001_C001.mp4")), # present + _ref("B002.mov", gap), # gap + _ref("gone.mov", _elsewhere("gone.mov")), # missing + ] + selection = timeline.selection(timeline.resolve(references, [library])) + assert [f.source for f in selection] == [gap] + + +def test_mirror_layout_keeps_the_path_below_the_volume(library: Path, + tmp_path: Path): + gap = _write(tmp_path / "elsewhere" / "deep" / "B002.mov", b"x") + (entry,) = timeline.selection( + timeline.resolve([_ref("B002.mov", gap)], [library])) + assert entry.relative == Path(*gap.parts[1:]) + assert not entry.relative.is_absolute() + + +def test_flat_layout_is_the_basename(library: Path, tmp_path: Path): + gap = _write(tmp_path / "elsewhere" / "deep" / "B002.mov", b"x") + (entry,) = timeline.selection( + timeline.resolve([_ref("B002.mov", gap)], [library]), flat=True) + assert entry.relative == Path("B002.mov") + + +# -------------------------------------------------------------------------- +# The engine, offloading a selection +# -------------------------------------------------------------------------- + + +def _options(tmp_path: Path, **overrides) -> engine.OffloadOptions: + defaults = dict( + destinations=[tmp_path / "dest"], + algorithm="xxh3-64", + verification=VerificationMode.FULL, + thumbnail_count=0, + extra_probe=False, + ) + defaults.update(overrides) + return engine.OffloadOptions(**defaults) + + +def test_selection_offloads_files_from_several_volumes(tmp_path: Path): + """The change this whole feature rests on: one job, many source trees. + + Without the selection path this cannot be expressed at all -- `run` scans + a single root, so the second file could only be reached by a second job + with its own report. + """ + one = _write(tmp_path / "treeA" / "clips" / "A.mov", b"a" * 500) + two = _write(tmp_path / "treeB" / "sound" / "B.wav", b"b" * 500) + selection = [ + engine.SelectedFile(one, Path("treeA", "clips", "A.mov"), tmp_path), + engine.SelectedFile(two, Path("treeB", "sound", "B.wav"), tmp_path), + ] + + job = engine.run(tmp_path / "treeA", _options(tmp_path, selection=selection)) + + assert job.total_files == 2 + assert all(f.status is FileStatus.VERIFIED for f in job.files) + assert (tmp_path / "dest" / "treeA" / "clips" / "A.mov").read_bytes() == b"a" * 500 + assert (tmp_path / "dest" / "treeB" / "sound" / "B.wav").read_bytes() == b"b" * 500 + + +def test_selection_ignores_the_scan_and_its_excludes(tmp_path: Path): + """Only the selected file moves, not everything beside it.""" + one = _write(tmp_path / "tree" / "A.mov", b"a" * 100) + _write(tmp_path / "tree" / "B.mov", b"b" * 100) + selection = [engine.SelectedFile(one, Path("A.mov"), tmp_path / "tree")] + + job = engine.run(tmp_path / "tree", _options(tmp_path, selection=selection)) + + assert job.total_files == 1 + assert not (tmp_path / "dest" / "B.mov").exists() + + +def test_a_destination_inside_a_search_root_is_allowed(tmp_path: Path): + """The ordinary case for this feature, and the one the card rule refuses. + + Collecting a cut's gaps into a new folder on the same drive the cut was + resolved against is normal. `assert_safe_destinations` would reject it, + because for a card "destination inside source" means you are about to eat + the card. + """ + library = tmp_path / "library" + source = _write(tmp_path / "other" / "A.mov", b"a" * 100) + dest = library / "collected" + dest.mkdir(parents=True) + selection = [engine.SelectedFile(source, Path("A.mov"), tmp_path / "other")] + + job = engine.run(library, _options(tmp_path, destinations=[dest], + selection=selection)) + assert job.total_files == 1 + assert (dest / "A.mov").exists() + + +def test_a_source_inside_the_destination_is_refused(tmp_path: Path): + """The narrower rule that replaces it: never read a file you are writing.""" + dest = tmp_path / "dest" + inside = _write(dest / "already" / "A.mov", b"a" * 100) + selection = [engine.SelectedFile(inside, Path("already", "A.mov"), dest)] + + with pytest.raises(engine.UnsafeDestination, match="inside the destination"): + engine.run(tmp_path, _options(tmp_path, selection=selection)) + + +def test_a_relative_path_that_climbs_out_is_refused(tmp_path: Path): + source = _write(tmp_path / "tree" / "A.mov", b"a" * 100) + selection = [engine.SelectedFile(source, Path("..", "escaped.mov"), + tmp_path / "tree")] + + with pytest.raises(engine.UnsafeDestination, match="escapes the destination"): + engine.run(tmp_path, _options(tmp_path, selection=selection)) + assert not (tmp_path / "escaped.mov").exists() + + +def test_an_absolute_relative_path_is_refused(tmp_path: Path): + source = _write(tmp_path / "tree" / "A.mov", b"a" * 100) + elsewhere = tmp_path / "not-the-destination" / "A.mov" + selection = [engine.SelectedFile(source, elsewhere, tmp_path / "tree")] + + with pytest.raises(engine.UnsafeDestination, match="absolute destination"): + engine.run(tmp_path, _options(tmp_path, selection=selection)) + assert not elsewhere.exists() + + +def test_two_selected_files_landing_on_one_path_are_refused(tmp_path: Path): + """Flattening two trees is exactly how a selection loses a file. + + Both would be copied, both verified as they landed, and the report would + attest to two files where the drive holds one. + """ + one = _write(tmp_path / "treeA" / "A.mov", b"a" * 100) + two = _write(tmp_path / "treeB" / "A.mov", b"b" * 100) + selection = [ + engine.SelectedFile(one, Path("A.mov"), tmp_path / "treeA"), + engine.SelectedFile(two, Path("A.mov"), tmp_path / "treeB"), + ] + + with pytest.raises(engine.UnsafeDestination, match="silently overwrite"): + engine.run(tmp_path, _options(tmp_path, selection=selection)) + + +def test_the_same_file_twice_in_a_selection_is_refused(tmp_path: Path): + one = _write(tmp_path / "tree" / "A.mov", b"a" * 100) + selection = [ + engine.SelectedFile(one, Path("A.mov"), tmp_path / "tree"), + engine.SelectedFile(one, Path("copy", "A.mov"), tmp_path / "tree"), + ] + with pytest.raises(ValueError, match="same source file twice"): + engine.run(tmp_path, _options(tmp_path, selection=selection)) + + +def test_report_shows_a_placeable_path_for_each_tree(tmp_path: Path): + one = _write(tmp_path / "treeA" / "clips" / "A.mov", b"a" * 100) + selection = [engine.SelectedFile(one, Path("treeA", "clips", "A.mov"), + tmp_path)] + job = engine.run(tmp_path, _options(tmp_path, selection=selection)) + assert job.files[0].relative == Path("treeA", "clips", "A.mov") + + +def test_a_job_named_after_a_timeline_drops_the_suffix(tmp_path: Path): + """The job name reaches a report header, where ".xml" helps nobody.""" + source = _write(tmp_path / "tree" / "A.mov", b"a" * 100) + cut = _timeline_file(tmp_path, "file://localhost/X/A.mov", + "file://localhost/X/B.wav") + selection = [engine.SelectedFile(source, Path("A.mov"), tmp_path / "tree")] + + job = engine.run(cut, _options(tmp_path, selection=selection)) + assert job.name == "cut" + + +# -------------------------------------------------------------------------- +# Reading a real timeline +# -------------------------------------------------------------------------- + +FCP_XML = """ + + + + Test Sequence + 240 + 24FALSE + + + + + +""" + + +def _timeline_file(tmp_path: Path, first: str, second: str) -> Path: + path = tmp_path / "cut.xml" + path.write_text(FCP_XML.format(first=first, second=second), encoding="utf-8") + return path + + +def _has_adapter() -> bool: + try: + import opentimelineio as otio + return "fcp_xml" in set(otio.adapters.available_adapter_names()) + except ImportError: + return False + + +needs_otio = pytest.mark.skipif( + not _has_adapter(), reason='needs pip install "offloader[timeline]"') + + +@needs_otio +def test_reads_every_reference_including_id_only_clipitems(tmp_path: Path): + """A clipitem may name its file by id and carry no path of its own. + + Both of the first clip's uses must be counted against one reference. A + sweep that only reads `pathurl` finds a source's first use and silently + misses every later one, which is how a file gets left out of a package. + """ + path = _timeline_file(tmp_path, + "file://localhost/X/media/A001_C001.mp4", + "file://localhost/X/audio/260822_001.wav") + references = timeline.read_references(path) + + by_name = {r.name: r for r in references} + assert set(by_name) == {"A001_C001.mp4", "260822_001.wav"} + assert by_name["A001_C001.mp4"].uses == 2 + assert by_name["260822_001.wav"].uses == 1 + + +@needs_otio +def test_percent_escapes_in_a_path_are_decoded(tmp_path: Path): + path = _timeline_file(tmp_path, + "file://localhost/X/media/Shop%20Machines.mp4", + "file://localhost/X/audio/260822_001.wav") + names = {r.name for r in timeline.read_references(path)} + assert "Shop Machines.mp4" in names + + +@needs_otio +def test_read_then_resolve_end_to_end(tmp_path: Path, library: Path): + """The whole path: an unreachable export, resolved against a real drive.""" + path = _timeline_file(tmp_path, + "file://localhost/Volumes/Edit%2014TB/A001_C001.mp4", + "file://localhost/Volumes/Edit%2014TB/260822_001.wav") + report = timeline.resolve(timeline.read_references(path), [library], + timeline=path) + assert report.counts[Status.PRESENT] == 2 + assert report.gaps == [] + + +def test_unknown_suffix_is_refused_with_the_formats_it_knows(tmp_path: Path): + path = tmp_path / "cut.premiereproj" + path.write_text("not interchange", encoding="utf-8") + with pytest.raises(timeline.TimelineSupportMissing, match="no timeline adapter"): + timeline.read_references(path) + + +def test_a_missing_timeline_says_so(tmp_path: Path): + with pytest.raises(FileNotFoundError): + timeline.read_references(tmp_path / "nope.xml") + + +# -------------------------------------------------------------------------- +# Through the command line +# -------------------------------------------------------------------------- + + +def test_resolve_without_a_search_root_is_a_usage_error(tmp_path: Path, capsys): + """Without one, every reference is a gap and the whole cut gets copied.""" + cut = _timeline_file(tmp_path, "file://localhost/X/A.mov", + "file://localhost/X/B.wav") + code = cli.main(["resolve", "--timeline", str(cut)]) + assert code == 2 + assert "--search-root" in capsys.readouterr().err + + +@needs_otio +def test_resolve_exits_non_zero_when_something_is_unsettled( + tmp_path: Path, library: Path, capsys): + _write(library / "stand-ins" / "A001_C001.mp4", b"a different file") + cut = _timeline_file(tmp_path, + "file://localhost/X/A001_C001.mp4", + "file://localhost/X/260822_001.wav") + + code = cli.main(["resolve", "--timeline", str(cut), + "--search-root", str(library)]) + assert code == 1 + assert "MORE THAN ONE CANDIDATE" in capsys.readouterr().out + + +@needs_otio +def test_resolve_is_clean_when_everything_is_here(tmp_path: Path, library: Path): + cut = _timeline_file(tmp_path, + "file://localhost/X/A001_C001.mp4", + "file://localhost/X/260822_001.wav") + assert cli.main(["resolve", "--timeline", str(cut), + "--search-root", str(library)]) == 0 + + +@needs_otio +def test_resolve_writes_a_row_per_reference(tmp_path: Path, library: Path): + cut = _timeline_file(tmp_path, + "file://localhost/X/A001_C001.mp4", + "file://localhost/X/260822_001.wav") + out = tmp_path / "resolved.csv" + cli.main(["resolve", "--timeline", str(cut), "--search-root", str(library), + "--csv", str(out), "--quiet"]) + + rows = list(csv.DictReader(out.open(encoding="utf-8"))) + assert {r["file"] for r in rows} == {"A001_C001.mp4", "260822_001.wav"} + assert all(r["status"] == "present" for r in rows) + + +@needs_otio +def test_offload_from_a_timeline_copies_only_the_gap(tmp_path: Path, + library: Path): + gap = _write(tmp_path / "elsewhere" / "260822_001.wav", b"not on the drive") + (library / "audio" / "260822_001.wav").unlink() + cut = _timeline_file(tmp_path, + "file://localhost/X/A001_C001.mp4", + gap.as_uri()) + dest = tmp_path / "dest" + + code = cli.main(["offload", "--timeline", str(cut), + "--search-root", str(library), "--dest", str(dest), + "--report", "csv", "--no-probe", "--quiet"]) + + assert code == 0 + landed = list(dest.rglob("260822_001.wav")) + assert len(landed) == 1 + assert landed[0].read_bytes() == b"not on the drive" + # The file already under the search root is not copied again. + assert not list(dest.rglob("A001_C001.mp4")) From 95b73944d94fe931469c7999bb859b9e7b0aa333 Mon Sep 17 00:00:00 2001 From: owenpkent <20529132+owenpkent@users.noreply.github.com> Date: Sun, 30 Aug 2026 20:37:04 -0600 Subject: [PATCH 02/13] Keep a file: URL's leading slash unless a drive letter follows CI on macOS and Linux caught this; Windows passed. After the scheme and any localhost authority, what is left always begins with a slash: /E:/Media/a.mov, or /Volumes/Edit/a.mov. That slash belongs to a POSIX path and must stay, but on a Windows path it sits in front of the drive letter and must go. Stripping it unconditionally turned /Volumes/Edit/a.mov into the relative path Volumes/Edit/a.mov. The failure is close to invisible, which is why it earns a long comment. A relative path still resolves by basename, so a resolve run looks entirely correct; what breaks is every test of whether a file is where the timeline says it is, so a reachable file reads as missing rather than as a gap. An editor cutting on a Mac addresses every clip that way, so this is the common case, not an edge. Tested as a pure function over both platforms' forms, including UNC and percent-escapes, so the coverage no longer depends on which runner it lands on. Re-checked against the two production timelines: 327 references, zero status changes. --- src/offloader/timeline.py | 40 +++++++++++++++++++++++++++++++++------ tests/test_timeline.py | 29 ++++++++++++++++++++++++++++ 2 files changed, 63 insertions(+), 6 deletions(-) diff --git a/src/offloader/timeline.py b/src/offloader/timeline.py index 12ea48d..8f44911 100644 --- a/src/offloader/timeline.py +++ b/src/offloader/timeline.py @@ -41,6 +41,7 @@ from __future__ import annotations import os +import re from collections.abc import Iterable, Sequence from dataclasses import dataclass, field from enum import Enum @@ -184,16 +185,43 @@ def _url_to_path(url: str) -> Path | None: NLEs are not consistent here: Premiere writes `file://localhost/E:/...`, Resolve writes `file:///E:/...`, and a hand-edited XML may carry a bare path. Percent-escapes are always possible because a clip name may contain - a space. Forward slashes are left alone -- `Path` accepts them on Windows, - and normalising them by hand is how a UNC path loses its leading pair. + a space. + + The leading slash is the trap, and it is worth spelling out because + getting it wrong is invisible on one platform. After the scheme and any + `localhost` authority, what is left always begins with `/`: + + file://localhost/E:/Media/a.mov -> /E:/Media/a.mov + file:///Volumes/Edit/a.mov -> /Volumes/Edit/a.mov + + That slash belongs to a POSIX path and must stay, but on a Windows path it + sits in front of the drive letter and must go. Stripping it unconditionally + turns `/Volumes/Edit/a.mov` into the *relative* path `Volumes/Edit/a.mov`, + which still resolves by basename and so looks fine, while every "is it + where the timeline says?" test quietly answers no. An editor cutting on a + Mac addresses every clip that way, so this is the common case, not an edge. + + Forward slashes are otherwise left alone: `Path` accepts them on Windows, + and rewriting them by hand is how a UNC path loses its leading pair. """ if not url: return None text = unquote(url) - for prefix in ("file://localhost/", "file:///", "file://"): - if text.startswith(prefix): - text = text[len(prefix):] - break + + if text.lower().startswith("file:"): + text = text[len("file:"):] + if text.startswith("//"): + rest = text[2:] + if rest.lower().startswith("localhost/"): + rest = rest[len("localhost"):] # keeps the leading slash + elif not rest.startswith("/"): + # file://server/share/... is a UNC path; the pair is the point. + return Path("//" + rest) if rest else None + text = rest + + # A drive letter behind the leading slash means this is a Windows path. + if re.match(r"^/[A-Za-z]:", text): + text = text[1:] if not text: return None return Path(text) diff --git a/tests/test_timeline.py b/tests/test_timeline.py index 8856bb6..0a631a7 100644 --- a/tests/test_timeline.py +++ b/tests/test_timeline.py @@ -482,6 +482,35 @@ def test_read_then_resolve_end_to_end(tmp_path: Path, library: Path): assert report.gaps == [] +@pytest.mark.parametrize("url, expected", [ + # Premiere on Windows, and Resolve's variant of the same. + ("file://localhost/E:/Media/a.mov", "E:/Media/a.mov"), + ("file:///E:/Media/a.mov", "E:/Media/a.mov"), + # An editor cutting on a Mac. The leading slash is part of the path and + # must survive: dropping it yields a *relative* path that still resolves + # by basename, so nothing looks broken while every "is it where the + # timeline says?" test quietly answers no. + ("file:///Volumes/Edit/a.mov", "/Volumes/Edit/a.mov"), + ("file://localhost/Volumes/Edit/a.mov", "/Volumes/Edit/a.mov"), + # Percent-escapes, because clip names contain spaces. + ("file:///Volumes/Edit%2014TB/Shop%20Machines.mp4", + "/Volumes/Edit 14TB/Shop Machines.mp4"), + # A bare path, from a hand-edited file. + ("E:/Media/a.mov", "E:/Media/a.mov"), +]) +def test_file_urls_survive_both_platforms(url: str, expected: str): + """Pure function, so it runs identically on every runner.""" + got = timeline._url_to_path(url) + assert got is not None + assert got.as_posix() == expected + + +def test_a_unc_url_keeps_its_leading_pair(): + got = timeline._url_to_path("file://server/share/a.mov") + assert got is not None + assert got.as_posix().startswith("//server/share") + + def test_unknown_suffix_is_refused_with_the_formats_it_knows(tmp_path: Path): path = tmp_path / "cut.premiereproj" path.write_text("not interchange", encoding="utf-8") From fb59094f85281873bc5aac5198f5c9c63cd0d776 Mon Sep 17 00:00:00 2001 From: owenpkent <20529132+owenpkent@users.noreply.github.com> Date: Sun, 30 Aug 2026 20:37:18 -0600 Subject: [PATCH 03/13] Pause a running offload from another terminal JobControl has existed since the desktop app needed transport buttons, and run() checks it once per 8 MiB chunk, but cli.py never passed one. So a job started in a terminal could only be killed: the capability was there and nothing reached it. --control-file PATH wires a FileControl to a file, and "offloader control PATH --pause|--resume|--cancel" drives it from anywhere. A file rather than a signal or a keypress because Windows has almost no signal support beyond SIGINT, a 1.9 TB offload is started detached or over ssh with no console to type into, and a file needs no port and no daemon, survives the terminal closing, and can be read to see what a job is doing. The failure modes are the design. The file holds one word; anything else - empty, garbled, half-written, or momentarily unreadable because another process has it open - is no opinion, and leaves the job in whatever state it is already in. Inferring cancel from a damaged control file would let a stray byte stop an offload that is nine hours in, and a small text file is exactly what a sync client or an editor rewrites in two steps. The control command writes through a staging file and renames it into place, so a job polling between chunks cannot read a truncated instruction. Starting a job claims the path by writing run to it first, or a stale pause from a previous job would stop the next one before it copied a byte with nothing on screen to say why. A paused job keeps polling, so it can still be cancelled; JobControl alone would block inside Event.wait() and never see it. 11 tests. The cancel tests drive the transition from the progress callback rather than after a sleep, because a dozen 400 KB files finish faster than any wall-clock guess; the suite was run five times over to confirm it. One of them also covers the handler never being registered in main(), which argparse hides: --help works from the parser alone, so the command looks fine until it is actually run, which is how that shipped broken for an hour here. --- CHANGELOG.md | 27 +++++ README.md | 43 +++++++ src/offloader/cli.py | 63 ++++++++++- src/offloader/engine.py | 100 +++++++++++++++++ tests/test_control.py | 241 ++++++++++++++++++++++++++++++++++++++++ 5 files changed, 472 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 42e45f5..aaec2b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,6 +10,25 @@ project uses [semantic versioning][semver]. ### Added +- **Pause, resume and cancel from the command line.** `JobControl` has existed + since the desktop app needed transport buttons, and is checked once per 8 MiB + chunk, but the CLI never passed one — so a job started in a terminal could + only be killed. `--control-file PATH` wires one to a file, and + `offloader control PATH --pause|--resume|--cancel` drives it from anywhere. + A file rather than a signal or a keypress because Windows has almost no + signal support, a detached job has no console to type into, and a file needs + no port or daemon, survives the terminal closing, and can be read to see what + a job is doing. + + The failure modes are the interesting part. The file holds one word; + **anything else — empty, garbled, half-written, momentarily locked — is "no + opinion" and leaves the job in the state it is already in**, because + inferring `cancel` from a damaged control file would let a stray byte stop an + offload that is nine hours in. `offloader control` writes through a staging + file and renames it into place, so a job polling between chunks cannot read a + truncated instruction. Starting a job writes `run` to the path first, so a + stale `pause` from a previous job cannot silently stop the next one. + - **Offloading from an edit timeline.** `offloader resolve --timeline cut.xml --search-root E:\Media` reports which of a cut's media is already on the drive and which is not; `offloader offload --timeline ...` copies the @@ -33,6 +52,14 @@ project uses [semantic versioning][semver]. proxy already on the drive, but only where the frame counts agree — a proxy one frame short moves every edit point after it. + A `file:` URL keeps its leading slash unless a drive letter follows it. + Stripping it unconditionally turns `/Volumes/Edit/a.mov` into the *relative* + path `Volumes/Edit/a.mov`, which still resolves by basename and so looks + correct on Windows, while every "is it where the timeline says?" test quietly + answers no. An editor cutting on a Mac addresses every clip that way, so it + is the common case rather than an edge; CI on macOS and Linux is what caught + it. + - **`OffloadOptions.selection`**, an explicit file set for `engine.run` in place of scanning one source root. Files may come from any number of volumes and each carries its own destination-relative path, so the engine infers no diff --git a/README.md b/README.md index 2c64956..dceaaa1 100644 --- a/README.md +++ b/README.md @@ -92,6 +92,7 @@ offloader verify D:\video\080426\A001 | --- | --- | | `offload` | copy and verify a source to one or more destinations | | `resolve` | report which of an edit timeline's media is already here, copying nothing | +| `control` | pause, resume or cancel a running offload from another terminal | | `verify` | re-check an offloaded tree against its manifests | | `report` | regenerate paperwork for an existing tree, copying nothing | | `info` | show tool and environment status | @@ -117,6 +118,7 @@ offloader verify D:\video\080426\A001 | `--exclude GLOB` | extra filename pattern to skip; repeatable | | `--flat` | do not recreate the source folder structure; refused if two files would land on one path | | `--skip-existing` | skip files already present at matching size | +| `--control-file PATH` | make the job pausable from another terminal; see [Pausing a running job](#pausing-a-running-job) | | `--proxies-first` | copy the camera's proxy folders before the originals (default) | | `--originals-first` | copy in plain tree order instead | | `--retries N` | attempts per failing read on a transient error (default 3, 1 disables) | @@ -128,6 +130,47 @@ Exit status is `0` on success, `1` if any file failed verification, `2` on a usage or I/O error, `3` if a destination was refused as unsafe, `4` if a timeline could not be read for want of an adapter. +### Pausing a running job + +A long offload gets started detached, over ssh, or by a scheduler, and the +person who wants it paused is rarely sitting at that terminal. Start it with a +control file and it can be driven from anywhere: + +```sh +offloader offload --source E:\ --dest D:\A001 --control-file D:\A001\job.control + +# from any other terminal +offloader control D:\A001\job.control --pause +offloader control D:\A001\job.control --resume +offloader control D:\A001\job.control --cancel +offloader control D:\A001\job.control # what state is it in? +``` + +The job reads the file **once per 8 MiB chunk**, so a pause takes effect inside +a second even in the middle of a 14 GB clip. Paused, it holds its place with +the file still in flight; resumed, it carries on from that chunk rather than +restarting the file. A cancel keeps every finished file and discards the one in +flight, which never had its real name. + +The file holds one word: `run`, `pause` or `cancel`. Deleting it releases the +job. **Anything else is "no opinion" and leaves the job exactly as it is** — +empty, garbled, half-written, or momentarily unreadable because another process +has it open. That asymmetry is deliberate: inferring `cancel` from a damaged +control file would let a stray byte stop an offload that is nine hours in, and +a small text file is precisely what a sync client rewrites in two steps. +`offloader control` writes through a staging file and renames it into place, so +a job polling between chunks can never read a half-written instruction. + +Starting a job also *claims* the path by writing `run` to it. A control file +left saying `pause` by a previous job would otherwise stop the next one before +it copied a byte, with nothing on screen to explain why. + +Why a file rather than a signal or a keypress: Windows has almost no signal +support beyond SIGINT, a detached job has no console to press a key in, and a +file needs no port and no daemon, survives the terminal going away, and can be +read to see what a job is doing. The desktop app drives the same `JobControl` +through its transport buttons. + ### From an edit timeline A card offload knows its source. This is the other job: a cut comes back from diff --git a/src/offloader/cli.py b/src/offloader/cli.py index 0c291a3..440a875 100644 --- a/src/offloader/cli.py +++ b/src/offloader/cli.py @@ -3,6 +3,7 @@ from __future__ import annotations import argparse +import os import sys import time from dataclasses import replace @@ -249,9 +250,31 @@ def build_parser() -> argparse.ArgumentParser: order.add_argument("--originals-first", dest="proxies_first", action="store_false", help="copy in plain tree order instead") + offload.add_argument("--control-file", type=Path, default=None, metavar="PATH", + help="make the job pausable from another terminal: " + "it reads this file between chunks. Drive it with " + "'offloader control PATH --pause|--resume|--cancel'") _timeline_options(offload) _common_options(offload) + control = sub.add_parser( + "control", + help="pause, resume or cancel a running offload through its " + "--control-file") + control.add_argument("path", type=Path, + help="the --control-file the job was started with") + state = control.add_mutually_exclusive_group() + state.add_argument("--pause", action="store_const", dest="state", + const=engine.FileControl.PAUSE, + help="hold the job at the next 8 MiB chunk") + state.add_argument("--resume", action="store_const", dest="state", + const=engine.FileControl.RUN, help="let it continue") + state.add_argument("--cancel", action="store_const", dest="state", + const=engine.FileControl.CANCEL, + help="stop it; finished files are kept, the file in " + "flight is discarded") + control.set_defaults(state=None) + resolve = sub.add_parser( "resolve", help="report which of a timeline's media is already here and which " @@ -389,6 +412,29 @@ def _write_resolution_csv(report: timeline_mod.Report, path: Path) -> None: ]) +def cmd_control(args: argparse.Namespace) -> int: + """Read or set a running job's control file.""" + control = engine.FileControl(args.path) + if args.state is None: + if not args.path.exists(): + print(f"no control file at {args.path}", file=sys.stderr) + return 2 + state = control.read() + print(state if state is not None + else "unreadable, so the job continues in its current state") + return 0 + + # Written whole and moved into place: a job polling between chunks can + # otherwise read a file that has been truncated but not yet rewritten, + # which is precisely the "no opinion" case this avoids needing. + args.path.parent.mkdir(parents=True, exist_ok=True) + staging = args.path.with_name(args.path.name + ".writing") + staging.write_text(args.state + "\n", encoding="utf-8") + os.replace(staging, args.path) + print(f"{args.state} -> {args.path}") + return 0 + + def cmd_resolve(args: argparse.Namespace) -> int: report = _resolve_timeline(args) _print_resolution(report, detail=not args.quiet) @@ -421,8 +467,21 @@ def cmd_offload(args: argparse.Namespace) -> int: options = replace(options, selection=selection) source = args.timeline + control = None + if args.control_file: + def announce(state: str) -> None: + # From the reader thread, and the progress line is written from + # this one, so start on a fresh line rather than overwriting it. + print(f"\n[{state}] {args.control_file}", file=sys.stderr, flush=True) + + control = engine.FileControl(args.control_file, on_change=announce) + control.claim() + if not args.quiet: + print(f"pausable: offloader control \"{args.control_file}\" --pause", + file=sys.stderr) + progress = _Progress(not args.quiet) - job = engine.run(source, options, progress) + job = engine.run(source, options, progress, control) progress.done() reports = _write_reports( @@ -543,7 +602,7 @@ def main(argv: list[str] | None = None) -> int: args = build_parser().parse_args(argv) handlers = {"offload": cmd_offload, "report": cmd_report, "verify": cmd_verify, "info": cmd_info, "gui": cmd_gui, - "resolve": cmd_resolve} + "resolve": cmd_resolve, "control": cmd_control} try: return handlers[args.command](args) except KeyboardInterrupt: diff --git a/src/offloader/engine.py b/src/offloader/engine.py index 65c7da1..2685112 100644 --- a/src/offloader/engine.py +++ b/src/offloader/engine.py @@ -198,6 +198,106 @@ def checkpoint(self) -> None: raise JobCancelled() +class FileControl(JobControl): + """A `JobControl` driven by a file, so another process can pause a job. + + The desktop app has transport buttons and a `JobControl` to wire them to. + The CLI has neither, and usually has no console to press a key in either: a + 1.9 TB offload is started detached, over ssh, or by a scheduler, and the + person who wants it paused is not sitting at that terminal. So the + instruction has to arrive from outside the process. + + A file is the smallest thing that works everywhere. It needs no signal + support -- Windows has almost none beyond SIGINT, and this tool is + Windows-first -- no port, no daemon and no shared memory. It survives the + terminal going away, any user with write access can set it, and it can be + *read* to see what state a job is in. + + The file holds one word: `run`, `pause` or `cancel`. Anything else -- + empty, garbled, half-written, or unreadable because another process has it + open at that moment -- is **no opinion**, and leaves the job in whatever + state it is already in. That asymmetry is deliberate. Inferring `cancel` + from a damaged control file would let a stray byte stop an offload that is + hours in, and a control file is exactly the kind of thing that gets + clobbered by a sync client or a text editor writing in two steps. + + Polling is rate-limited by `poll` seconds because `checkpoint()` runs once + per 8 MiB chunk -- around 16 times a second at 130 MB/s -- and the control + file may well be on a network path. + """ + + RUN = "run" + PAUSE = "pause" + CANCEL = "cancel" + STATES = (RUN, PAUSE, CANCEL) + + def __init__(self, path: Path, poll: float = 0.5, + on_change: Callable[[str], None] | None = None) -> None: + super().__init__() + self.path = Path(path) + self.poll = max(0.05, poll) + self._on_change = on_change + self._state = self.RUN + self._checked = 0.0 + self._lock = threading.Lock() + + @property + def state(self) -> str: + return self._state + + def claim(self) -> None: + """Write `run`, taking ownership of the path for this job. + + A control file left saying `pause` by a previous job would otherwise + stop the next one before it copied a byte, and the reason would not be + obvious to anyone watching. A job starts by saying what it is doing. + """ + self.path.parent.mkdir(parents=True, exist_ok=True) + self.path.write_text(self.RUN + "\n", encoding="utf-8") + + def read(self) -> str | None: + """The word in the file, or None for "no opinion".""" + try: + text = self.path.read_text(encoding="utf-8") + except FileNotFoundError: + # Deleting the file releases the job rather than stranding it. + return self.RUN + except OSError: + return None + word = text.strip().lower().split() + if not word or word[0] not in self.STATES: + return None + return word[0] + + def sync(self, force: bool = False) -> None: + with self._lock: + now = time.monotonic() + if not force and now - self._checked < self.poll: + return + self._checked = now + state = self.read() + if state is None or state == self._state: + return + self._state = state + if state == self.PAUSE: + self.pause() + elif state == self.RUN: + self.resume() + elif state == self.CANCEL: + self.cancel() + if self._on_change is not None: + self._on_change(state) + + def checkpoint(self) -> None: + self.sync() + # Poll while held, or a `resume` written to the file would never be + # seen: JobControl.checkpoint would be blocked inside Event.wait(). + while self.paused and not self.cancelled: + time.sleep(self.poll) + self.sync(force=True) + super().checkpoint() + + @dataclass class ProgressEvent: """Emitted as the job runs, for CLI progress bars and (later) the GUI.""" diff --git a/tests/test_control.py b/tests/test_control.py index 2e794b1..404fd26 100644 --- a/tests/test_control.py +++ b/tests/test_control.py @@ -136,3 +136,244 @@ def test_control_state_flags(): control.cancel() assert control.cancelled + + +# -------------------------------------------------------------------------- +# FileControl: pausing a CLI job from another process +# -------------------------------------------------------------------------- + + +def test_file_control_reads_each_state(tmp_path: Path): + path = tmp_path / "job.control" + control = engine.FileControl(path, poll=0.0) + + assert control.read() == "run", "an absent file releases the job" + for word in ("pause", "run", "cancel"): + path.write_text(word + "\n", encoding="utf-8") + assert control.read() == word + path.write_text(" PAUSE \n", encoding="utf-8") + assert control.read() == "pause", "case and whitespace are not the point" + + +def test_a_damaged_control_file_is_no_opinion_not_a_cancel(tmp_path: Path): + """A stray byte must never stop an offload that is hours in. + + A control file is exactly the kind of small text file a sync client or an + editor rewrites in two steps, so a half-written read has to leave the job + alone rather than be guessed at. + """ + path = tmp_path / "job.control" + control = engine.FileControl(path, poll=0.0) + + for junk in ("", " ", "\x00\x00", "stop", "resume please", "PAUS"): + path.write_text(junk, encoding="utf-8") + assert control.read() is None, f"{junk!r} should be no opinion" + + # And no opinion really does leave the state alone. + path.write_text("pause", encoding="utf-8") + control.sync(force=True) + assert control.paused + path.write_text("nonsense", encoding="utf-8") + control.sync(force=True) + assert control.paused, "a damaged file must not release a paused job either" + + +def test_claim_resets_a_stale_pause_from_a_previous_job(tmp_path: Path): + """Otherwise the next job stops dead and nobody can see why.""" + path = tmp_path / "job.control" + path.write_text("pause\n", encoding="utf-8") + + control = engine.FileControl(path, poll=0.0) + control.claim() + + assert path.read_text(encoding="utf-8").strip() == "run" + control.sync(force=True) + assert not control.paused + + +def test_file_control_pauses_and_resumes_a_running_offload(tmp_path: Path): + source = tmp_path / "card" + source.mkdir() + for index in range(10): + (source / f"clip{index:02d}.mov").write_bytes(b"\0" * 400_000) + + path = tmp_path / "job.control" + control = engine.FileControl(path, poll=0.02) + control.claim() + result: dict[str, object] = {} + worker = threading.Thread( + target=lambda: result.update(job=engine.run(source, _options(tmp_path), + control=control)), + daemon=True, + ) + worker.start() + + path.write_text("pause\n", encoding="utf-8") + deadline = time.monotonic() + 5 + while not control.paused and time.monotonic() < deadline: + time.sleep(0.02) + assert control.paused, "the job never saw the pause" + + # It must actually stop moving, not merely set a flag. + settled = len(list((tmp_path / "dest").rglob("*.mov"))) + time.sleep(0.3) + assert len(list((tmp_path / "dest").rglob("*.mov"))) == settled + assert worker.is_alive() + + path.write_text("run\n", encoding="utf-8") + worker.join(timeout=20) + assert not worker.is_alive(), "the job never saw the resume" + assert result["job"].total_files == 10 + assert not result["job"].cancelled + + +def test_deleting_the_control_file_releases_a_paused_job(tmp_path: Path): + path = tmp_path / "job.control" + control = engine.FileControl(path, poll=0.0) + path.write_text("pause\n", encoding="utf-8") + control.sync(force=True) + assert control.paused + + path.unlink() + control.sync(force=True) + assert not control.paused + + +def test_file_control_cancels_a_running_offload(tmp_path: Path): + source = tmp_path / "card" + source.mkdir() + for index in range(12): + (source / f"clip{index:02d}.mov").write_bytes(b"\0" * 400_000) + + path = tmp_path / "job.control" + control = engine.FileControl(path, poll=0.0) + control.claim() + + # Written from the progress callback rather than after a sleep: a dozen + # 400 KB files finish faster than any wall-clock guess, and what is under + # test is the cancel, not the race. + seen: set[str] = set() + + def progress(event: engine.ProgressEvent) -> None: + seen.add(event.file_name) + if len(seen) == 3: + path.write_text("cancel\n", encoding="utf-8") + + job = engine.run(source, _options(tmp_path), progress, control) + + assert job.cancelled + assert 0 < len(job.files) < 12 + # Whatever it did record actually landed, whole. + for entry in job.files: + for destination in entry.destinations: + assert destination.path.exists() + assert destination.path.stat().st_size == entry.size + partials = list((tmp_path / "dest").rglob("*" + engine.PARTIAL_SUFFIX)) + assert partials == [], "a cancel must not leave a half-written file" + + +def test_cancel_reaches_a_job_that_is_already_paused(tmp_path: Path): + """A paused job polls, so it can still be told to stop.""" + source = tmp_path / "card" + source.mkdir() + for index in range(10): + (source / f"clip{index:02d}.mov").write_bytes(b"\0" * 400_000) + + path = tmp_path / "job.control" + control = engine.FileControl(path, poll=0.02) + control.claim() + result: dict[str, object] = {} + worker = threading.Thread( + target=lambda: result.update(job=engine.run(source, _options(tmp_path), + control=control)), + daemon=True, + ) + worker.start() + path.write_text("pause\n", encoding="utf-8") + deadline = time.monotonic() + 5 + while not control.paused and time.monotonic() < deadline: + time.sleep(0.02) + assert control.paused + + path.write_text("cancel\n", encoding="utf-8") + worker.join(timeout=20) + assert not worker.is_alive() + assert result["job"].cancelled + + +def test_state_changes_are_announced_once(tmp_path: Path): + path = tmp_path / "job.control" + seen: list[str] = [] + control = engine.FileControl(path, poll=0.0, on_change=seen.append) + + for word in ("pause", "pause", "pause", "run", "run"): + path.write_text(word, encoding="utf-8") + control.sync(force=True) + assert seen == ["pause", "run"], "only transitions are worth reporting" + + +# -------------------------------------------------------------------------- +# Through the command line +# -------------------------------------------------------------------------- + + +def test_control_command_writes_each_state(tmp_path: Path, capsys): + """Also catches the handler never being registered, which argparse hides: + `--help` works from the parser alone, so the command looks fine until it + is actually run.""" + from offloader import cli + + path = tmp_path / "job.control" + for flag, word in (("--pause", "pause"), ("--resume", "run"), + ("--cancel", "cancel")): + assert cli.main(["control", str(path), flag]) == 0 + assert path.read_text(encoding="utf-8").strip() == word + capsys.readouterr() + + assert cli.main(["control", str(path)]) == 0 + assert capsys.readouterr().out.strip() == "cancel" + + +def test_control_command_on_a_path_with_no_job(tmp_path: Path, capsys): + from offloader import cli + + assert cli.main(["control", str(tmp_path / "nope.control")]) == 2 + assert "no control file" in capsys.readouterr().err + + +def test_control_command_writes_atomically(tmp_path: Path): + """A job polls between chunks, so it must never read a truncated file.""" + from offloader import cli + + path = tmp_path / "job.control" + cli.main(["control", str(path), "--pause"]) + assert not list(tmp_path.glob("*.writing")), "staging file left behind" + assert path.read_text(encoding="utf-8").strip() == "pause" + + +def test_offload_cli_is_pausable_with_a_control_file(tmp_path: Path): + """End to end: the flag wires a FileControl into the run.""" + from offloader import cli + + source = tmp_path / "card" + source.mkdir() + for index in range(10): + (source / f"clip{index:02d}.mov").write_bytes(b"\0" * 400_000) + path = tmp_path / "job.control" + path.write_text("pause\n", encoding="utf-8") # stale, must be reset + + result: dict[str, object] = {} + worker = threading.Thread( + target=lambda: result.update(code=cli.main([ + "offload", "--source", str(source), "--dest", str(tmp_path / "dest"), + "--control-file", str(path), "--no-probe", "--report", "csv", + "--quiet", + ])), + daemon=True, + ) + worker.start() + worker.join(timeout=30) + + assert not worker.is_alive(), "claim() did not clear the stale pause" + assert result["code"] == 0 + assert len(list((tmp_path / "dest").rglob("*.mov"))) == 10 From a7cab0717204fee4a34dfe8e3f6cb69601e7901e Mon Sep 17 00:00:00 2001 From: owenpkent <20529132+owenpkent@users.noreply.github.com> Date: Mon, 21 Sep 2026 20:39:48 -0400 Subject: [PATCH 04/13] Read the whole control word, and stop racing the poll interval Two ways a damaged control file could stop a running offload, both of them the opposite of what the file's own docstring promises. read() compared word[0] after splitting, so "cancel pending upload" - the kind of thing an editor or a sync client leaves behind - read as a cancellation. It now compares the whole stripped, case-folded value against the three states, so anything with a token after it is no opinion. read_text(encoding="utf-8") raises UnicodeDecodeError, which is not an OSError and escaped the handler beside it. A control file holding two bytes of FF FE aborted the checkpoint instead of being read as damage. Caught now alongside the read errors, leaving the job in whatever state it was in. The pause/cancel integration tests were failing on Ubuntu 3.10, Ubuntu 3.13, Ubuntu with ffmpeg and macOS while passing on Windows, which is timing rather than behaviour. poll has a deliberate 50 ms floor, so poll=0.0 did not mean what those tests assumed: sync() skips the read until the interval elapses, and ten 400 KB files can finish between two reads without ever seeing the word written midway. FileControl now takes an injectable clock, defaulting to time.monotonic, so a test can decide the limiter has elapsed instead of hoping the machine is slow enough. Production behaviour is unchanged and the floor stays. The pause tests also write their instruction from the progress callback rather than from the main thread after start, the way the cancel test already did, and write it once: several events fire per file, and rewriting pause after the resume put the job straight back to sleep. Five regressions: a trailing token after each of the three valid words, invalid UTF-8, and a paused job held through both kinds of damage without cancelling. The control suite runs in 2.1s rather than 22.5s, which is the wall-clock the races were costing, and was run five times over to confirm it. --- src/offloader/engine.py | 26 +++++++++--- tests/test_control.py | 93 +++++++++++++++++++++++++++++++++++++---- 2 files changed, 107 insertions(+), 12 deletions(-) diff --git a/src/offloader/engine.py b/src/offloader/engine.py index 2685112..0ff2a89 100644 --- a/src/offloader/engine.py +++ b/src/offloader/engine.py @@ -232,7 +232,8 @@ class FileControl(JobControl): STATES = (RUN, PAUSE, CANCEL) def __init__(self, path: Path, poll: float = 0.5, - on_change: Callable[[str], None] | None = None) -> None: + on_change: Callable[[str], None] | None = None, + clock: Callable[[], float] = time.monotonic) -> None: super().__init__() self.path = Path(path) self.poll = max(0.05, poll) @@ -240,6 +241,12 @@ def __init__(self, path: Path, poll: float = 0.5, self._state = self.RUN self._checked = 0.0 self._lock = threading.Lock() + # Injectable so a test can decide when the rate limiter has elapsed. + # `poll` has a 50 ms floor, deliberately: a caller asking for 0 on a + # network control path would otherwise stat it sixteen times a second. + # That floor also means a small fixture can finish between two reads, + # which is a race in the test rather than a behaviour worth having. + self._clock = clock @property def state(self) -> str: @@ -264,14 +271,23 @@ def read(self) -> str | None: return self.RUN except OSError: return None - word = text.strip().lower().split() - if not word or word[0] not in self.STATES: + except UnicodeDecodeError: + # Not an OSError, so it escaped the handler above and aborted the + # checkpoint. A control file being written underneath us, or one an + # editor saved in another encoding, is damaged content: the + # documented answer to that is no opinion, not a stopped transfer. return None - return word[0] + # The whole value, not its first token: `cancel pending upload` is + # something an editor or a sync left behind, not an instruction to stop + # a running offload. + word = text.strip().lower() + if word not in self.STATES: + return None + return word def sync(self, force: bool = False) -> None: with self._lock: - now = time.monotonic() + now = self._clock() if not force and now - self._checked < self.poll: return self._checked = now diff --git a/tests/test_control.py b/tests/test_control.py index 404fd26..fabb5d1 100644 --- a/tests/test_control.py +++ b/tests/test_control.py @@ -2,14 +2,31 @@ from __future__ import annotations +import itertools import threading import time from pathlib import Path +import pytest + from offloader import engine from offloader.models import FileStatus, VerificationMode +def _every_checkpoint_reads(): + """A clock that has always elapsed, so `sync()` never skips a read. + + `FileControl.poll` has a 50 ms floor, so a fixture of a few small files can + finish between two reads of the control file and never see an instruction + written midway. That is a race in the test, not behaviour worth asserting: + these tests are about what happens once the word is read. Driving the rate + limiter instead of racing it is what makes them mean the same thing on a + fast Linux runner as on a slow Windows one. + """ + ticks = itertools.count(0.0, 3600.0) + return lambda: next(ticks) + + def _options(tmp_path: Path, **overrides) -> engine.OffloadOptions: defaults = dict( destinations=[tmp_path / "dest"], @@ -198,17 +215,31 @@ def test_file_control_pauses_and_resumes_a_running_offload(tmp_path: Path): (source / f"clip{index:02d}.mov").write_bytes(b"\0" * 400_000) path = tmp_path / "job.control" - control = engine.FileControl(path, poll=0.02) + control = engine.FileControl(path, poll=0.02, clock=_every_checkpoint_reads()) control.claim() + + # Written from the progress callback rather than from this thread after the + # worker starts: ten 400 KB files can be finished before the write lands, + # and what is under test is the pause, not that race. + seen: set[str] = set() + asked = threading.Event() + + def progress(event: engine.ProgressEvent) -> None: + seen.add(event.file_name) + # Once only: several events fire per file, and rewriting `pause` after + # the resume below would put the job straight back to sleep. + if len(seen) == 3 and not asked.is_set(): + asked.set() + path.write_text("pause\n", encoding="utf-8") + result: dict[str, object] = {} worker = threading.Thread( target=lambda: result.update(job=engine.run(source, _options(tmp_path), - control=control)), + progress, control)), daemon=True, ) worker.start() - path.write_text("pause\n", encoding="utf-8") deadline = time.monotonic() + 5 while not control.paused and time.monotonic() < deadline: time.sleep(0.02) @@ -246,7 +277,7 @@ def test_file_control_cancels_a_running_offload(tmp_path: Path): (source / f"clip{index:02d}.mov").write_bytes(b"\0" * 400_000) path = tmp_path / "job.control" - control = engine.FileControl(path, poll=0.0) + control = engine.FileControl(path, poll=0.0, clock=_every_checkpoint_reads()) control.claim() # Written from the progress callback rather than after a sleep: a dozen @@ -280,16 +311,24 @@ def test_cancel_reaches_a_job_that_is_already_paused(tmp_path: Path): (source / f"clip{index:02d}.mov").write_bytes(b"\0" * 400_000) path = tmp_path / "job.control" - control = engine.FileControl(path, poll=0.02) + control = engine.FileControl(path, poll=0.02, clock=_every_checkpoint_reads()) control.claim() + seen: set[str] = set() + asked = threading.Event() + + def progress(event: engine.ProgressEvent) -> None: + seen.add(event.file_name) + if len(seen) == 3 and not asked.is_set(): + asked.set() + path.write_text("pause\n", encoding="utf-8") + result: dict[str, object] = {} worker = threading.Thread( target=lambda: result.update(job=engine.run(source, _options(tmp_path), - control=control)), + progress, control)), daemon=True, ) worker.start() - path.write_text("pause\n", encoding="utf-8") deadline = time.monotonic() + 5 while not control.paused and time.monotonic() < deadline: time.sleep(0.02) @@ -301,6 +340,46 @@ def test_cancel_reaches_a_job_that_is_already_paused(tmp_path: Path): assert result["job"].cancelled +@pytest.mark.parametrize("word", ["run", "pause", "cancel"]) +def test_a_valid_word_with_anything_after_it_is_no_opinion(tmp_path: Path, + word: str): + """REGRESSION. Only the first token was compared, so `cancel pending upload` + left behind by an editor or a sync read as a cancellation.""" + path = tmp_path / "job.control" + control = engine.FileControl(path, poll=0.0) + + path.write_text(f"{word} pending upload\n", encoding="utf-8") + assert control.read() is None + + +def test_invalid_utf8_in_the_control_file_is_no_opinion(tmp_path: Path): + """REGRESSION. `UnicodeDecodeError` is not an `OSError`, so it escaped the + handler and aborted the checkpoint instead of being read as damage.""" + path = tmp_path / "job.control" + control = engine.FileControl(path, poll=0.0) + + path.write_bytes(b"\xff\xfe") + assert control.read() is None + + +def test_a_damaged_control_file_leaves_a_paused_job_paused(tmp_path: Path): + path = tmp_path / "job.control" + control = engine.FileControl(path, poll=0.0) + path.write_text("pause\n", encoding="utf-8") + control.sync(force=True) + assert control.paused + + path.write_bytes(b"\xff\xfe") + control.sync(force=True) + assert control.paused, "damaged content must not change the state" + assert not control.cancelled + + path.write_text("cancel upload now\n", encoding="utf-8") + control.sync(force=True) + assert control.paused + assert not control.cancelled, "a trailing token must not cancel a job" + + def test_state_changes_are_announced_once(tmp_path: Path): path = tmp_path / "job.control" seen: list[str] = [] From bb99e8fd28eea456282e833b7b3b59df34c9e40e Mon Sep 17 00:00:00 2001 From: owenpkent <20529132+owenpkent@users.noreply.github.com> Date: Tue, 22 Sep 2026 11:37:19 -0400 Subject: [PATCH 05/13] Record what a control file reads as damage in the changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index aaec2b9..faef86b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -29,6 +29,13 @@ project uses [semantic versioning][semver]. truncated instruction. Starting a job writes `run` to the path first, so a stale `pause` from a previous job cannot silently stop the next one. + "One word" means the whole value, not its first token: `cancel pending + upload`, the kind of thing an editor or a sync client leaves behind, is + damaged content and not an instruction to stop. Contents that are not valid + UTF-8 are damage too — that raises a `UnicodeDecodeError` rather than an + `OSError`, so it used to abort the checkpoint instead of being read as no + opinion. + - **Offloading from an edit timeline.** `offloader resolve --timeline cut.xml --search-root E:\Media` reports which of a cut's media is already on the drive and which is not; `offloader offload --timeline ...` copies the From 6172dc2e477b2f47cc64f1a207f69c9b51a6c78b Mon Sep 17 00:00:00 2001 From: Owen Kent Date: Thu, 8 Oct 2026 20:22:17 -0400 Subject: [PATCH 06/13] Update test counts --- CONTRIBUTING.md | 2 +- README.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index a21de54..7168bed 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,7 +37,7 @@ pip install -e ".[dev]" `ffmpeg` and `ffprobe` on `PATH` are optional — the suite runs without them. ```sh -pytest # 680 tests, about 20s +pytest # 737 tests, about 20s pytest --fuzz # property tests at 3000 examples each, about 3 min ruff check src tests pytest --cov=offloader --cov-report=term-missing diff --git a/README.md b/README.md index a070e5c..0424f66 100644 --- a/README.md +++ b/README.md @@ -586,13 +586,13 @@ what makes the report layer testable without moving bytes. ```sh pip install -e ".[dev]" -pytest # 680 tests, ~20s +pytest # 737 tests, ~20s pytest --fuzz # same suite, 3000 examples per property (~3 min) ruff check src tests pytest --cov=offloader --cov-report=term-missing ``` -680 tests at 86% line coverage. They cover formatting against the reference's +737 tests at 86% line coverage. They cover formatting against the reference's exact strings, checksum vectors and streaming equivalence, copy/verify behaviour including simulated destination corruption, pause/resume/cancel concurrency, retry discrimination, BRAW container parsing, ffprobe parsing, From 7c38c60eccf68057dbafe36f938ed85feb961d2c Mon Sep 17 00:00:00 2001 From: owenpkent <20529132+owenpkent@users.noreply.github.com> Date: Sun, 11 Oct 2026 15:28:57 -0600 Subject: [PATCH 07/13] Update test counts --- CONTRIBUTING.md | 2 +- README.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index 7af1dd2..9b00e13 100644 --- a/CONTRIBUTING.md +++ b/CONTRIBUTING.md @@ -37,7 +37,7 @@ pip install -e ".[dev]" `ffmpeg` and `ffprobe` on `PATH` are optional — the suite runs without them. ```sh -pytest # 741 tests, about 20s +pytest # 744 tests, about 20s pytest --fuzz # property tests at 3000 examples each, about 3 min ruff check src tests pytest --cov=offloader --cov-report=term-missing diff --git a/README.md b/README.md index 7c18f02..a9e0f8c 100644 --- a/README.md +++ b/README.md @@ -591,13 +591,13 @@ what makes the report layer testable without moving bytes. ```sh pip install -e ".[dev]" -pytest # 741 tests, ~20s +pytest # 744 tests, ~20s pytest --fuzz # same suite, 3000 examples per property (~3 min) ruff check src tests pytest --cov=offloader --cov-report=term-missing ``` -741 tests at 86% line coverage. They cover formatting against the reference's +744 tests at 86% line coverage. They cover formatting against the reference's exact strings, checksum vectors and streaming equivalence, copy/verify behaviour including simulated destination corruption, pause/resume/cancel concurrency, retry discrimination, BRAW container parsing, ffprobe parsing, From b212a85757e31a226e71a5662c570ccc082b8f0e Mon Sep 17 00:00:00 2001 From: owenpkent <20529132+owenpkent@users.noreply.github.com> Date: Sun, 11 Oct 2026 15:50:02 -0600 Subject: [PATCH 08/13] Refuse a control-file path that aliases a source, a good copy, or a promotion target FileControl.claim() wrote the run state to whatever path --control-file named, before engine.run() had validated anything. A path that aliased a source clip truncated it to five bytes before it was hashed; the engine then copied and verified the replacement text, reported Verified, and exited 0. The same write would destroy an existing good destination file. claim() now creates a missing file exclusively and overwrites an existing one only when it is empty or holds a control word. A new guard, run by both the engine and the CLI before anything is written, refuses a control path inside the source, equal to a selected media file or a destination root, or equal to any planned final or partial destination path. Symlinks are followed on every side. The CLI reports the refusal and exits 3 without copying. --- CHANGELOG.md | 8 ++ README.md | 6 +- docs/data-safety.md | 1 + src/offloader/cli.py | 10 ++- src/offloader/engine.py | 103 +++++++++++++++++++++++++- tests/test_control.py | 157 ++++++++++++++++++++++++++++++++++++++++ tests/test_timeline.py | 15 ++++ 7 files changed, 297 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 5edcff5..9d47201 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,14 @@ project uses [semantic versioning][semver]. ### Fixed +- **`--control-file` could overwrite a source or destination file.** Starting a + job wrote `run` to the control path before anything was validated, so a path + that named a source clip (or a good copy) truncated it to five bytes, and the + job then copied and reported the replacement as Verified. The path is now + checked first: it may not be inside the source, be a selected timeline file, + be a destination root, or be where a copy would land, and an existing file is + only overwritten if it is empty or holds `run`, `pause` or `cancel`. + - **Pause and cancel are honoured during rereads.** A cancel issued during the `--paranoid` source reread or the full-verification destination reread was ignored, and the file was promoted and reported Verified. Both rereads now diff --git a/README.md b/README.md index a9e0f8c..c7ae782 100644 --- a/README.md +++ b/README.md @@ -164,7 +164,11 @@ a job polling between chunks can never read a half-written instruction. Starting a job also *claims* the path by writing `run` to it. A control file left saying `pause` by a previous job would otherwise stop the next one before -it copied a byte, with nothing on screen to explain why. +it copied a byte, with nothing on screen to explain why. The path is validated +first and claiming never truncates anything that is not a control file: a path +inside the source, on a selected media file, on a destination root, or naming an +existing file that is not empty or a control word is refused before anything is +written. Why a file rather than a signal or a keypress: Windows has almost no signal support beyond SIGINT, a detached job has no console to press a key in, and a diff --git a/docs/data-safety.md b/docs/data-safety.md index ca3a5d2..5a63cf9 100644 --- a/docs/data-safety.md +++ b/docs/data-safety.md @@ -183,6 +183,7 @@ exists precisely so it can be checked independently. | Card pulled mid-copy | `OSError` fails that file; the partial is deleted; other files are unaffected. | | Quit during an offload | Confirmed, then cancelled cleanly, partials removed. | | Cancel | Deletes the in-flight partial rather than leaving plausible-looking media. Pause and cancel are also honoured during the `--paranoid` source reread and the full-verification destination reread, so a cancel there never promotes the file. | +| `--control-file` pointing at real data | The control path is validated before anything is written. It may not be inside the source, alias selected timeline media or a destination root, be where a copy would land, or point (symlinks followed) at an existing file that is not empty or one of `run`, `pause`, `cancel`. A control file inside a destination root is allowed. Claiming never truncates anything that is not a control file; the job is refused with `UnsafeControlFile`. | | Report writer crashing | Caught; a bad PDF never invalidates a good copy. | | Corrupt config file | Treated as empty. Config must never stand between someone and their card. | | Two source files mapping to one destination path | Refused before a byte moves, naming both sources. See below. | diff --git a/src/offloader/cli.py b/src/offloader/cli.py index d2f90fd..f47fdc9 100644 --- a/src/offloader/cli.py +++ b/src/offloader/cli.py @@ -484,6 +484,14 @@ def announce(state: str) -> None: print(f"\n[{state}] {args.control_file}", file=sys.stderr, flush=True) control = engine.FileControl(args.control_file, on_change=announce) + # Validate before claim() writes anything: the path is user input and + # may alias a source clip or a good copy. + control.guard( + source, + [Path(e.source) for e in options.selection] + if options.selection is not None else None, + options.destinations, + ) control.claim() if not args.quiet: print(f"pausable: offloader control \"{args.control_file}\" --pause", @@ -621,7 +629,7 @@ def main(argv: list[str] | None = None) -> int: except KeyboardInterrupt: print("\ninterrupted", file=sys.stderr) return 130 - except engine.UnsafeDestination as exc: + except (engine.UnsafeDestination, engine.UnsafeControlFile) as exc: print(f"refused: {exc}", file=sys.stderr) return 3 except timeline_mod.TimelineSupportMissing as exc: diff --git a/src/offloader/engine.py b/src/offloader/engine.py index 3b43e27..a380d6b 100644 --- a/src/offloader/engine.py +++ b/src/offloader/engine.py @@ -67,6 +67,10 @@ class UnsafeDestination(ValueError): """A destination that could destroy the source it is copying.""" +class UnsafeControlFile(ValueError): + """A control-file path that would overwrite something that is not one.""" + + def assert_safe_destinations(source_root: Path, destinations: Sequence[Path]) -> None: """Refuse destinations that can eat the source. @@ -230,6 +234,8 @@ class FileControl(JobControl): PAUSE = "pause" CANCEL = "cancel" STATES = (RUN, PAUSE, CANCEL) + #: Largest existing file `claim` will overwrite; a control word is shorter. + MAX_BYTES = 64 def __init__(self, path: Path, poll: float = 0.5, on_change: Callable[[str], None] | None = None, @@ -258,9 +264,73 @@ def claim(self) -> None: A control file left saying `pause` by a previous job would otherwise stop the next one before it copied a byte, and the reason would not be obvious to anyone watching. A job starts by saying what it is doing. + + Claiming must never truncate a file that is not a control file. The + path comes from a command line, and a mistyped one can name a source + clip or a good copy; overwriting it with `run` would destroy it before + anything is hashed, and the job would then verify the replacement. So + an existing path is only overwritten when it is empty or holds one of + the control words, and a missing one is created exclusively. """ self.path.parent.mkdir(parents=True, exist_ok=True) - self.path.write_text(self.RUN + "\n", encoding="utf-8") + target = Path(os.path.realpath(self.path)) + data = (self.RUN + "\n").encode("utf-8") + # Before the exclusive create: Windows reports an existing directory + # there as PermissionError rather than FileExistsError. + if target.is_dir(): + raise UnsafeControlFile(f"control file {self.path} is a directory") + try: + fd = os.open(target, os.O_CREAT | os.O_EXCL | os.O_WRONLY, 0o666) + except FileExistsError: + pass + else: + with os.fdopen(fd, "wb") as handle: + handle.write(data) + return + + size = target.stat().st_size + if size > self.MAX_BYTES: + raise UnsafeControlFile( + f"control file {self.path} is {size} bytes and is not a " + "control file; refusing to overwrite it") + try: + content = target.read_bytes().decode("utf-8").strip().lower() + except UnicodeDecodeError: + content = None + if content is None or (content and content not in self.STATES): + raise UnsafeControlFile( + f"control file {self.path} holds data that is not a control " + "word; refusing to overwrite it") + with open(target, "wb") as handle: + handle.write(data) + + def guard(self, source_root: Path, + selection: Iterable[Path] | None = None, + destinations: Iterable[Path] = ()) -> None: + """Refuse a control path that aliases data this job reads or writes. + + The job rewrites the control path (`claim`) and later promotes copies + over final names, so the path must not be: inside the source tree (or + the source itself, which for a timeline job is the timeline file), one + of the selected media files, or a destination root. Symlinks are + followed on every side, so a link to a clip is the clip. A control + file *inside* a destination root is fine and is the documented usage. + """ + control = Path(os.path.realpath(self.path)) + source = Path(os.path.realpath(source_root)) + if control == source or source in control.parents: + raise UnsafeControlFile( + f"control file {self.path} is inside the source {source_root}; " + "choose a path outside it") + for media in selection or (): + if control == Path(os.path.realpath(media)): + raise UnsafeControlFile( + f"control file {self.path} is the selected media file " + f"{media}") + for destination in destinations: + if control == Path(os.path.realpath(destination)): + raise UnsafeControlFile( + f"control file {self.path} is the destination {destination}") def read(self) -> str | None: """The word in the file, or None for "no opinion".""" @@ -870,6 +940,26 @@ def _discard(targets: Iterable[Path]) -> None: pass +def _assert_control_not_a_target( + control: FileControl, files: Sequence[Path], source_root: Path, + destination_roots: Sequence[Path], preserve_structure: bool, + relatives: dict[Path, Path] | None, +) -> None: + """Refuse a job whose promotion would replace the control file.""" + path = Path(os.path.realpath(control.path)) + for source in files: + for root in destination_roots: + target = (root / relatives[source] if relatives is not None + else _destination_for(source, source_root, root, + preserve_structure)) + for candidate in (target, + target.with_name(target.name + PARTIAL_SUFFIX)): + if Path(os.path.realpath(candidate)) == path: + raise UnsafeControlFile( + f"control file {control.path} is where {source} would " + "be copied; choose another control path") + + def run(source_root: Path, options: OffloadOptions, progress: ProgressCallback | None = None, control: JobControl | None = None) -> Job: @@ -893,6 +983,14 @@ def run(source_root: Path, options: OffloadOptions, if not dest_roots: raise ValueError("at least one destination is required") + if isinstance(control, FileControl): + control.guard( + source_root, + [Path(e.source) for e in options.selection] + if options.selection is not None else None, + dest_roots, + ) + #: Per-file destination-relative paths, for a selection. None for a card, #: where the destination is derived from the source root as it always was. relatives: dict[Path, Path] | None = None @@ -912,6 +1010,9 @@ def run(source_root: Path, options: OffloadOptions, assert_no_destination_collisions(files, source_root, dest_roots, options.preserve_structure, relatives) + if isinstance(control, FileControl): + _assert_control_not_a_target(control, files, source_root, dest_roots, + options.preserve_structure, relatives) counters = _Counters(job_bytes_total=sum(p.stat().st_size for p in files)) # `files` stays the tree order the card is laid out in and remains the diff --git a/tests/test_control.py b/tests/test_control.py index 9647ad7..812622d 100644 --- a/tests/test_control.py +++ b/tests/test_control.py @@ -535,3 +535,160 @@ def progress(event: engine.ProgressEvent) -> None: progress, control) _assert_nothing_promoted(job, tmp_path) + + +# ------------------------------------------------ an unsafe control-file path +# +# `--control-file` is user input, and `claim()` writes to it before the engine +# has validated anything else. Each of these used to truncate the named file +# to the five bytes of `run\n`, and the job then copied and "verified" it. + +_CLIP = bytes(range(256)) * 64 # 16,384 bytes of recognisable content + + +def _card(tmp_path: Path) -> Path: + card = tmp_path / "card" + card.mkdir() + (card / "clip.mov").write_bytes(_CLIP) + return card + + +def _offload_with_control(card: Path, dest: Path, control: Path) -> int: + from offloader import cli + + return cli.main([ + "offload", "--source", str(card), "--dest", str(dest), + "--control-file", str(control), "--no-probe", "--report", "csv", + "--quiet", + ]) + + +def _nothing_copied(dest: Path) -> None: + assert not list(dest.rglob("*.mov")) + assert not list(dest.rglob("*" + engine.PARTIAL_SUFFIX)) + + +def test_control_file_that_is_a_source_file_is_refused(tmp_path: Path, capsys): + card = _card(tmp_path) + dest = tmp_path / "dest" + + code = _offload_with_control(card, dest, card / "clip.mov") + + assert code != 0 + assert (card / "clip.mov").read_bytes() == _CLIP + _nothing_copied(dest) + assert "control file" in capsys.readouterr().err + + +def test_control_file_that_is_a_good_destination_file_is_refused(tmp_path: Path): + card = _card(tmp_path) + dest = tmp_path / "dest" + dest.mkdir() + good = dest / "clip.mov" + good.write_bytes(b"an earlier good copy" * 100) + + code = _offload_with_control(card, dest, good) + + assert code != 0 + assert good.read_bytes() == b"an earlier good copy" * 100 + assert not list(dest.rglob("*" + engine.PARTIAL_SUFFIX)) + + +def test_control_path_inside_the_source_is_refused_before_it_is_created( + tmp_path: Path, +): + card = _card(tmp_path) + dest = tmp_path / "dest" + path = card / "job.control" + + assert _offload_with_control(card, dest, path) != 0 + + assert not path.exists() + _nothing_copied(dest) + + +def test_control_path_that_is_a_symlink_to_a_source_file_is_refused( + tmp_path: Path, +): + card = _card(tmp_path) + dest = tmp_path / "dest" + link = tmp_path / "job.control" + try: + link.symlink_to(card / "clip.mov") + except (OSError, NotImplementedError): + pytest.skip("symlinks are not permitted on this machine") + + assert _offload_with_control(card, dest, link) != 0 + + assert (card / "clip.mov").read_bytes() == _CLIP + _nothing_copied(dest) + + +def test_library_run_refuses_a_control_file_inside_the_source(tmp_path: Path): + card = _card(tmp_path) + control = engine.FileControl(card / "clip.mov") + + with pytest.raises(engine.UnsafeControlFile): + engine.run(card, _options(tmp_path), control=control) + + assert (card / "clip.mov").read_bytes() == _CLIP + assert not (tmp_path / "dest").exists() + + +def test_library_run_refuses_a_control_file_that_is_a_promotion_target( + tmp_path: Path, +): + card = _card(tmp_path) + dest = tmp_path / "dest" + dest.mkdir() + (dest / "clip.mov").write_bytes(b"x" * 10) + control = engine.FileControl(dest / "clip.mov") + + with pytest.raises(engine.UnsafeControlFile): + engine.run(card, _options(tmp_path), control=control) + + assert (dest / "clip.mov").read_bytes() == b"x" * 10 + + +def test_control_file_inside_a_destination_root_is_still_allowed(tmp_path: Path): + card = _card(tmp_path) + dest = tmp_path / "dest" + + assert _offload_with_control(card, dest, dest / "job.control") == 0 + + assert (dest / "clip.mov").read_bytes() == _CLIP + + +def test_claim_refuses_content_that_is_not_a_control_word(tmp_path: Path): + path = tmp_path / "notes.txt" + path.write_bytes(b"precious" * 100) + + with pytest.raises(engine.UnsafeControlFile): + engine.FileControl(path).claim() + + assert path.read_bytes() == b"precious" * 100 + + +def test_claim_refuses_a_directory(tmp_path: Path): + with pytest.raises(engine.UnsafeControlFile): + engine.FileControl(tmp_path).claim() + + +@pytest.mark.parametrize("before", [b"", b"pause\n", b" CANCEL \r\n"]) +def test_claim_overwrites_an_empty_or_stale_control_file( + tmp_path: Path, before: bytes, +): + path = tmp_path / "job.control" + path.write_bytes(before) + + engine.FileControl(path).claim() + + assert path.read_text(encoding="utf-8").strip() == "run" + + +def test_claim_creates_a_missing_file_and_its_parent(tmp_path: Path): + path = tmp_path / "new" / "job.control" + + engine.FileControl(path).claim() + + assert path.read_text(encoding="utf-8").strip() == "run" diff --git a/tests/test_timeline.py b/tests/test_timeline.py index 0a631a7..b491035 100644 --- a/tests/test_timeline.py +++ b/tests/test_timeline.py @@ -594,3 +594,18 @@ def test_offload_from_a_timeline_copies_only_the_gap(tmp_path: Path, assert landed[0].read_bytes() == b"not on the drive" # The file already under the search root is not copied again. assert not list(dest.rglob("A001_C001.mp4")) + + +def test_control_file_that_is_a_selected_media_file_is_refused(tmp_path: Path): + """The selection is drawn from several trees, none of them the job root.""" + one = _write(tmp_path / "treeA" / "clips" / "A.mov", b"a" * 500) + selection = [engine.SelectedFile(one, Path("treeA", "clips", "A.mov"), + tmp_path)] + cut = _write(tmp_path / "cuts" / "edit.xml", b"") + + with pytest.raises(engine.UnsafeControlFile): + engine.run(cut, _options(tmp_path, selection=selection), + control=engine.FileControl(one)) + + assert one.read_bytes() == b"a" * 500 + assert not (tmp_path / "dest").exists() From 280518c3bfcef29f8e2b1c73f506456bc37cbf43 Mon Sep 17 00:00:00 2001 From: owenpkent <20529132+owenpkent@users.noreply.github.com> Date: Sun, 11 Oct 2026 16:12:27 -0600 Subject: [PATCH 09/13] Never claim a timeline offload is complete while references are unresolved A timeline offload could print "nothing to copy: every reference is already here", end Verified and exit 0 while references were missing, ambiguous, offline or unsupported, and the reports listed only what was copied. One same-named file under the search root satisfied several distinct references, so the second recording in the docs' own example was never copied; a readable original was declared present at a different-size stand-in; offline media and image sequences were lumped into "generated"; a control-file cancel exited 0; an unverifiable proxy stood in for the original without a word in the terminal; and relative paths resolved against the working directory rather than the timeline. The job now carries its unresolved references: the final status is Incomplete, the exit code is 1, the terminal lists them, and the CSV, HTML and PDF reports gain an unresolved section. Distinct references sharing a name must resolve to distinct files or stay ambiguous, a readable original is compared with its candidate by size then hash, offline and unsupported references keep their names and fail the verdict, a proxy with no frame count does not stand in, a cancel exits 130 with the not-attempted count, and relative paths anchor to the timeline's folder while non-file URLs are reported as external. --- CHANGELOG.md | 30 ++ README.md | 16 +- docs/data-safety.md | 3 +- docs/timeline.md | 87 +++- src/offloader/cli.py | 92 ++++- src/offloader/engine.py | 1 + src/offloader/models.py | 11 + src/offloader/reports/csv_report.py | 13 + src/offloader/reports/html.py | 13 + src/offloader/reports/pdf.py | 60 +++ src/offloader/timeline.py | 388 +++++++++++++++--- tests/test_timeline.py | 8 +- tests/test_timeline_completeness.py | 602 ++++++++++++++++++++++++++++ 13 files changed, 1223 insertions(+), 101 deletions(-) create mode 100644 tests/test_timeline_completeness.py diff --git a/CHANGELOG.md b/CHANGELOG.md index 9d47201..77d9ee7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,36 @@ project uses [semantic versioning][semver]. ### Fixed +- **A timeline offload could call a partial cut complete.** With nothing to + copy it printed "every reference is already here" and exited 0 while + references were missing or ambiguous, and with gaps to copy it reported + "Verified" and exited 0 while the reports listed only the copied files. Now + every unresolved reference is printed, recorded on the job and listed in the + CSV, HTML and PDF reports; the job reads `Incomplete` and exits 1, the same + code `resolve` uses. +- **One file could satisfy several different timeline references.** Two + recordings sharing a name (`f2/260223_009.WAV`, `f3/260223_009.WAV`) both + resolved to the single copy on the drive. They are now `ambiguous` unless + their bytes show which is which. A readable original is compared with the + same-named file under the roots (size, then checksum), so a "MISSING MEDIA" + slate no longer stands in for real footage the timeline can reach; the + original is copied instead, with a warning naming the slate. +- **Offline clips and image sequences were reported as generated media.** An + OTIO `MissingReference`, an FCP 7 clip without a `` and an image + sequence are now reported by name as `offline` or `unsupported`, count as + unresolved and fail the exit code. A generator, title or matte is still + `generated`. +- **A proxy could stand in without any check.** A same-stem file that yields no + frame count no longer satisfies a camera original. A proxy whose count + cannot be compared with the original's is shown in the terminal with its + `frames unverified` note, carried as a job warning, and keeps the job from + reading `Verified`. +- **Relative timeline paths were read against the working directory.** They + are now read against the timeline file's folder, so an unrelated file of the + same relative path is never copied. Non-file URLs (`https`, `s3` and so on) + are reported as `external` and never matched to a local file by name. +- **A `--control-file` cancel exited 0.** A cancelled job now exits 130, as + Ctrl+C does, and the summary says how many files were not attempted. - **`--control-file` could overwrite a source or destination file.** Starting a job wrote `run` to the control path before anything was validated, so a path that named a source clip (or a good copy) truncated it to five bytes, and the diff --git a/README.md b/README.md index c7ae782..2b66502 100644 --- a/README.md +++ b/README.md @@ -127,9 +127,11 @@ offloader verify D:\video\080426\A001 | `--no-probe` | skip ffprobe metadata and thumbnails | | `--quiet` | suppress progress | -Exit status is `0` on success, `1` if any file failed verification, `2` on a -usage or I/O error, `3` if a destination was refused as unsafe, `4` if a -timeline could not be read for want of an adapter. +Exit status is `0` on success, `1` if any file failed verification or a +timeline offload left references unresolved, `2` on a usage or I/O error, `3` +if a destination was refused as unsafe, `4` if a timeline could not be read +for want of an adapter, and `130` if the job was cancelled (Ctrl+C or +`offloader control --cancel`). ### Pausing a running job @@ -204,8 +206,12 @@ checksums and reports as a card. | `--csv PATH` | (`resolve`) write the full per-reference table | Every reference gets one status: `present`, `substituted`, `gap`, `ambiguous`, -`missing` or `generated`. Only `gap` is copied. `resolve` exits non-zero if -anything is `ambiguous` or `missing`. +`missing`, `offline`, `unsupported`, `external` or `generated`. Only `gap` is +copied. While anything is unresolved (every status but `present`, `gap` and +`generated`, plus a proxy whose frame counts could not be checked), `resolve` +exits 1 and `offload` copies the gaps, lists what it could not supply in the +terminal and every report, calls the job `Incomplete` and exits 1. See +[docs/timeline.md](docs/timeline.md). The part worth knowing before you trust it: **it refuses to choose between two files that share a name.** On the conform it was built against, 20 basenames diff --git a/docs/data-safety.md b/docs/data-safety.md index 5a63cf9..c58cd74 100644 --- a/docs/data-safety.md +++ b/docs/data-safety.md @@ -182,7 +182,8 @@ exists precisely so it can be checked independently. | Re-offloading a card already pulled | Fingerprinted by source name plus the listing's names and sizes, warned with the earlier job's name and date. Only successful offloads count. | | Card pulled mid-copy | `OSError` fails that file; the partial is deleted; other files are unaffected. | | Quit during an offload | Confirmed, then cancelled cleanly, partials removed. | -| Cancel | Deletes the in-flight partial rather than leaving plausible-looking media. Pause and cancel are also honoured during the `--paranoid` source reread and the full-verification destination reread, so a cancel there never promotes the file. | +| Cancel | Deletes the in-flight partial rather than leaving plausible-looking media. Pause and cancel are also honoured during the `--paranoid` source reread and the full-verification destination reread, so a cancel there never promotes the file. A CLI job cancelled through `--control-file` exits 130, as Ctrl+C does, and its summary counts the files not attempted. | +| Timeline offload with references it cannot supply | Missing, ambiguous, offline, unsupported and non-file references, and proxies whose frame counts could not be checked, are listed in the terminal and in the CSV, HTML and PDF reports; the job reads `Incomplete`, never `Verified`, and exits 1. See [timeline.md](timeline.md). | | `--control-file` pointing at real data | The control path is validated before anything is written. It may not be inside the source, alias selected timeline media or a destination root, be where a copy would land, or point (symlinks followed) at an existing file that is not empty or one of `run`, `pause`, `cancel`. A control file inside a destination root is allowed. Claiming never truncates anything that is not a control file; the job is refused with `UnsafeControlFile`. | | Report writer crashing | Caught; a bad PDF never invalidates a good copy. | | Corrupt config file | Treated as empty. Config must never stand between someone and their card. | diff --git a/docs/timeline.md b/docs/timeline.md index fb52313..c0d44a7 100644 --- a/docs/timeline.md +++ b/docs/timeline.md @@ -29,28 +29,65 @@ whole design. | Status | Meaning | Copied? | | --- | --- | --- | -| `present` | A file of that name is under a search root | no | -| `substituted` | A camera original, satisfied by a frame-matched proxy already here | no | +| `present` | A file of that name is under a search root (and, when the timeline's own path is readable, holds the same bytes) | no | +| `substituted` | A camera original, satisfied by a proxy already here | no | | `gap` | Not here, but readable where the timeline says it is | **yes** | -| `ambiguous` | Several files under the roots share the name and differ | no, refuses | +| `ambiguous` | Several files under the roots share the name and differ, or several different references share a name the roots hold only one file of | no, refuses | | `missing` | Not here, and not where the timeline says either | no, cannot | +| `offline` | The timeline names the clip but carries no path for it (an OTIO `MissingReference`, an FCP 7 `` without ``) | no, cannot | +| `unsupported` | A media reference this module does not resolve, such as an image sequence | no, cannot | +| `external` | A non-file URL (`http`, `https`, `s3` and so on) | no, never | | `generated` | The timeline carries no media for it at all | no, cannot | -1. **A generated reference stops immediately.** A Premiere-native graphic, a - title, a generator: there is no file, and no search would ever find one. +1. **A reference with no local file stops immediately.** A Premiere-native + graphic, a title, a generator is `generated`: there is no file, and no + search would ever find one. Real media without a usable path is reported + by name as `offline`, `unsupported` or `external`; files under the roots + with an offline clip's name are listed as candidates and never chosen. 2. **Anything under a search root with that basename wins**, whatever path the timeline asked for. An editor's export addresses media on the editor's volume, so the path is almost always wrong and the name almost always right. This is the test that stops a file being copied a second time under - a name the drive already holds. -3. **A camera original may be satisfied by a proxy**, if the frame counts - agree. See below. + a name the drive already holds. Two limits apply: + - When the timeline's own path is readable, it is the truth. The file under + the root counts only if it holds the same bytes (size, then checksum); + otherwise the original is a `gap`, copied, and the other file is named in + the note and in a job warning. + - Different references that share a basename (two different paths) must + resolve to different files. When the roots hold only one file of that + name and nothing shows which reference it is, every one of them is + `ambiguous` rather than all of them mapping to the one file. +3. **A camera original may be satisfied by a proxy**. See below. 4. **Only then is a readable file a gap**, which is the thing worth copying. 5. Anything left is `missing`, and no offload will fix it. -`resolve` exits non-zero when anything is `ambiguous` or `missing`. A -`generated` reference does not fail it: no offload can supply one, and holding -the exit code hostage to it would only teach a caller to ignore the code. +A relative path in a timeline is read relative to the timeline file's own +folder, never the directory the command was run from. + +## Exit codes and "complete" + +A reference is *unresolved* when it is `ambiguous`, `missing`, `offline`, +`unsupported` or `external`, or when it is `substituted` by a proxy whose +frame counts could not be compared. While any reference is unresolved, the +cut is not complete and nothing says otherwise: + +- `resolve` exits 1. +- `offload` still copies every `gap`, because the reachable part of a cut is + worth having, but it lists every unresolved reference before and after the + copy, records them on the job, writes them into the CSV, HTML and PDF + reports under "Unresolved timeline references, NOT supplied by this job", + reports the job as `Incomplete` rather than `Verified`, and exits 1. +- With no gap to copy, `offload` prints "nothing to copy: every reference is + already here" only when that is true. Otherwise it prints "nothing + copyable" with the unresolved count and exits 1. + +A `generated` reference does not fail either command: no offload can supply +one, and holding the exit code hostage to it would only teach a caller to +ignore the code. + +A job cancelled with `offloader control PATH --cancel` exits 130, the same as +Ctrl+C, and its summary says how many files were not attempted. A failed file +still exits 1. ## Why `ambiguous` refuses instead of choosing @@ -85,17 +122,27 @@ A timeline that references 55 GB of BRAW usually references it because that is what the *camera* wrote, not because the cut needs it: the editor cut with proxies, and the proxies are already on the drive. -`substituted` means a stem-matched proxy was found under a search root **and -the frame counts agree**. That check is not decoration. Relink a clip to a file -one frame shorter and every edit point after it moves, quietly, and the first -person to notice is whoever watches the export. +`substituted` means a stem-matched proxy was found under a search root and +nothing contradicts it. Proof means **the frame counts agree**. That check is +not decoration. Relink a clip to a file one frame shorter and every edit point +after it moves, quietly, and the first person to notice is whoever watches the +export. The count comes from `probe.py`, which reads BRAW out of the container because -ffprobe cannot open the format at all. Where a count cannot be established - -no ffprobe, an unreachable original - the substitution is still offered but the -note says `frame counts unverified` rather than implying agreement. Where the -counts are established and **disagree**, the reference is reported `ambiguous` -and nothing is substituted. +ffprobe cannot open the format at all. The outcomes: + +- **Both counts known and equal**: `substituted`, settled. +- **Both known and different**: `ambiguous`, nothing substituted. +- **The proxy's count known, the original's not** (an unreachable original, or + one probe cannot read): still `substituted`, but *unverified*. The terminal + prints the reference with the note `frames unverified`, the job carries a + warning naming the file, and it counts as unresolved: the job is + `Incomplete` and the exit code is 1. +- **The proxy itself yields no frame count**, which includes a same-stem file + that is not media at all: it does not stand in. A readable original is then + an ordinary `gap` and is copied; an unreachable one is `missing`, with the + rejected file named in the note. Without ffprobe no `.mov` or `.mp4` proxy + can be counted, so none stands in. `--no-proxy-substitution` turns the whole step off, and every camera original becomes an ordinary gap. diff --git a/src/offloader/cli.py b/src/offloader/cli.py index f47fdc9..580c8f0 100644 --- a/src/offloader/cli.py +++ b/src/offloader/cli.py @@ -145,6 +145,16 @@ def _summarize(job: Job, reports: list[Path]) -> None: print(f" -> {destination}") for report in reports: print(f" report: {report}") + if job.cancelled: + print(f" cancelled: {job.not_attempted} file(s) not attempted") + if job.unresolved: + # Every one, uncapped: this is the list of media the job did not + # supply, and a truncated version of it is a false "that was all". + print(file=sys.stderr) + print(f" {len(job.unresolved)} timeline reference(s) NOT supplied; " + "the cut is not complete:", file=sys.stderr) + for line in job.unresolved: + print(f" - {line}", file=sys.stderr) if job.warnings: print(file=sys.stderr) print(f" {len(job.warnings)} warning(s):", file=sys.stderr) @@ -375,10 +385,22 @@ def _resolve_timeline(args: argparse.Namespace) -> timeline_mod.Report: (timeline_mod.Status.SUBSTITUTED, "satisfied by a proxy already here"), (timeline_mod.Status.GAP, "not here, and copyable"), (timeline_mod.Status.GENERATED, "no media in the timeline to supply"), + (timeline_mod.Status.EXTERNAL, "NOT A LOCAL FILE (URL), not searched"), + (timeline_mod.Status.UNSUPPORTED, "UNSUPPORTED media reference, check by hand"), + (timeline_mod.Status.OFFLINE, "OFFLINE in the timeline, no media path"), (timeline_mod.Status.MISSING, "not here and not where the timeline says"), (timeline_mod.Status.AMBIGUOUS, "MORE THAN ONE CANDIDATE, not chosen"), ) +#: What `resolve` and a timeline `offload` return while any reference is +#: unresolved, so a caller gating on the exit code never reads a partial cut +#: as a complete one. +EXIT_UNRESOLVED = 1 +#: A job stopped by a cancel (Ctrl+C or `offloader control --cancel`): the +#: same code either way, so a detached run's caller sees an interruption +#: rather than the 0 of a finished offload. +EXIT_CANCELLED = 130 + def _print_resolution(report: timeline_mod.Report, detail: bool = True) -> None: counts = report.counts @@ -388,15 +410,31 @@ def _print_resolution(report: timeline_mod.Report, detail: bool = True) -> None: for status, label in _STATUS_LINES: if counts[status]: print(f" {counts[status]:5d} {label}") + if report.unconfirmed: + print(f" {len(report.unconfirmed):5d} of those proxies, frame counts " + "UNVERIFIED") if report.gaps: print(f" {format_size(report.gap_bytes)} to copy") if not detail: return - for resolution in report.of(timeline_mod.Status.AMBIGUOUS, - timeline_mod.Status.MISSING): - print(f"\n [{resolution.status.value}] {resolution.name} " + # Every reference that keeps the cut from being complete, plus the gaps + # copied despite a different same-named file under a root. The notes are + # the point: "frames unverified" must be read, not left in a CSV. + shown = {id(r) for r in report.incomplete + report.differing} + for resolution in report.resolutions: + if id(resolution) not in shown: + continue + label = ("unverified proxy" if resolution.unverified + else resolution.status.value) + print(f"\n [{label}] {resolution.name} " f"(used {resolution.reference.uses}x)") + if resolution.reference.target is not None: + print(f" timeline path: {resolution.reference.target}") + elif resolution.reference.url: + print(f" timeline target: {resolution.reference.url}") + if resolution.status is timeline_mod.Status.SUBSTITUTED and resolution.location: + print(f" proxy: {resolution.location}") if resolution.note: print(f" {resolution.note}") for candidate in resolution.alternatives: @@ -414,7 +452,7 @@ def _write_resolution_csv(report: timeline_mod.Report, path: Path) -> None: for r in report.resolutions: writer.writerow([ r.name, r.status.value, r.reference.uses, - str(r.reference.target) if r.reference.target else "", + str(r.reference.target) if r.reference.target else r.reference.url, str(r.location) if r.location else "", " | ".join(str(a) for a in r.alternatives), r.note, @@ -451,24 +489,27 @@ def cmd_resolve(args: argparse.Namespace) -> int: _write_resolution_csv(report, args.csv) print(f"\nwrote {args.csv}") - # Exit non-zero on anything a human has to settle. A generated reference is - # reported but does not fail the command: no offload can supply it, so - # holding the exit code hostage to it would only teach a caller to ignore - # the code. - unsettled = report.counts[timeline_mod.Status.AMBIGUOUS] + \ - report.counts[timeline_mod.Status.MISSING] - return 1 if unsettled else 0 + # Exit non-zero on anything a human has to settle, including a proxy whose + # frames could not be checked. A generated reference is reported but does + # not fail the command: no offload can supply it, so holding the exit code + # hostage to it would only teach a caller to ignore the code. + return EXIT_UNRESOLVED if report.incomplete else 0 def cmd_offload(args: argparse.Namespace) -> int: options = _options_from(args, args.destinations) source = args.source + report: timeline_mod.Report | None = None if getattr(args, "timeline", None): report = _resolve_timeline(args) _print_resolution(report, detail=True) selection = timeline_mod.selection(report, flat=args.layout == "flat") if not selection: + if report.incomplete: + print(f"\nnothing copyable, and {len(report.incomplete)} " + "reference(s) unresolved: the cut is NOT complete.") + return EXIT_UNRESOLVED print("\nnothing to copy: every reference is already here.") return 0 print(f"\ncopying {len(selection)} file(s), " @@ -501,6 +542,20 @@ def announce(state: str) -> None: job = engine.run(source, options, progress, control) progress.done() + if report is not None: + # Recorded before the reports are written, so every one of them shows + # what this job did not supply and none reads as the whole cut. + job.unresolved = [timeline_mod.describe(r) for r in report.incomplete] + for resolution in report.unconfirmed: + job.warnings.append( + f"{resolution.name}: treated as satisfied by proxy " + f"{resolution.location}, frame counts unverified") + for resolution in report.differing: + others = ", ".join(str(a) for a in resolution.alternatives) + job.warnings.append( + f"{resolution.name}: copied from {resolution.location}; a " + f"different file of this name is under the search roots ({others})") + reports = _write_reports( job, args.report if isinstance(args.report, list) else _parse_reports(args.report), @@ -509,7 +564,18 @@ def announce(state: str) -> None: args.footer, ) _summarize(job, reports) - return 1 if any(f.status is FileStatus.FAILED for f in job.files) else 0 + return _exit_code(job) + + +def _exit_code(job: Job) -> int: + """0 only for a job that finished and supplied everything it was asked for.""" + if any(f.status is FileStatus.FAILED for f in job.files): + return 1 + if job.cancelled: + return EXIT_CANCELLED + if job.unresolved: + return EXIT_UNRESOLVED + return 0 def cmd_report(args: argparse.Namespace) -> int: @@ -628,7 +694,7 @@ def main(argv: list[str] | None = None) -> int: return handlers[args.command](args) except KeyboardInterrupt: print("\ninterrupted", file=sys.stderr) - return 130 + return EXIT_CANCELLED except (engine.UnsafeDestination, engine.UnsafeControlFile) as exc: print(f"refused: {exc}", file=sys.stderr) return 3 diff --git a/src/offloader/engine.py b/src/offloader/engine.py index a380d6b..83ea0bf 100644 --- a/src/offloader/engine.py +++ b/src/offloader/engine.py @@ -1391,6 +1391,7 @@ def copy_once(_src=source, _partials=partials, _idx=index, if job.cancelled: not_attempted = len(files) - len(job.files) + job.not_attempted = max(0, not_attempted) if not_attempted > 0: counters.errors.append( f"cancelled — {not_attempted} file(s) not attempted") diff --git a/src/offloader/models.py b/src/offloader/models.py index b92ac12..0cbddba 100644 --- a/src/offloader/models.py +++ b/src/offloader/models.py @@ -302,6 +302,15 @@ class Job: #: erasing a card — empty files, verifications that may have been served #: from cache. warnings: list[str] = field(default_factory=list) + #: Media the job was asked to supply and did not: for a timeline offload, + #: every reference that is missing, ambiguous, offline, unsupported, a + #: non-file URL, or a proxy standing in without proof. One line each, + #: written into every report. Any entry makes the job "Incomplete", so + #: copying the reachable part of a cut never reads as the whole of it. + unresolved: list[str] = field(default_factory=list) + #: Files in the job's file set that were never started because the job + #: was cancelled first. + not_attempted: int = 0 @property def total_bytes(self) -> int: @@ -340,6 +349,8 @@ def final_status(self) -> str: return "Failed" if self.cancelled: return "Cancelled" + if self.unresolved: + return "Incomplete" if self.verification is VerificationMode.NONE: return "Copied" if all(f.status is FileStatus.VERIFIED for f in self.files) and self.files: diff --git a/src/offloader/reports/csv_report.py b/src/offloader/reports/csv_report.py index c5cc9a6..f079874 100644 --- a/src/offloader/reports/csv_report.py +++ b/src/offloader/reports/csv_report.py @@ -91,6 +91,10 @@ def _audio_columns(media) -> list: ] +#: The heading every report gives the media a timeline job did not supply. +UNRESOLVED_TITLE = "Unresolved timeline references, NOT supplied by this job" + + def write_csv(job: Job, path: Path, *, delimiter: str = ",", **_options) -> Path: path = Path(path) path.parent.mkdir(parents=True, exist_ok=True) @@ -167,4 +171,13 @@ def write_csv(job: Job, path: Path, *, delimiter: str = ",", **_options) -> Path + tail + [destination.error or "", belongs_to] ) + + if job.unresolved: + # After the file rows, so a reader that takes row 3 as the header + # and stops at the first comment still parses, and labelled so + # nobody mistakes the table above for the whole of the cut. + writer.writerow([]) + writer.writerow([f"# {UNRESOLVED_TITLE} ({len(job.unresolved)})"]) + for line in job.unresolved: + writer.writerow([f"# {line}"]) return path diff --git a/src/offloader/reports/html.py b/src/offloader/reports/html.py index fa9fd73..fffffde 100644 --- a/src/offloader/reports/html.py +++ b/src/offloader/reports/html.py @@ -23,6 +23,7 @@ format_sample_rate, format_size, ) +from .csv_report import UNRESOLVED_TITLE _STYLE = """ :root { @@ -67,6 +68,9 @@ td.name { color: var(--label); font-weight: bold; white-space: nowrap; } .status-Verified { color: var(--ok); font-weight: bold; } .status-Failed { color: var(--bad); font-weight: bold; } +.unresolved { border: 2px solid var(--bad); padding: 8px 12px; margin-top: 14px; } +.unresolved h2 { color: var(--bad); font-size: 16px; font-weight: bold; margin: 0 0 6px; } +.unresolved li { font-size: 11px; word-break: break-all; } footer { margin-top: 28px; border-top: 1px solid var(--rule); padding-top: 8px; font-size: 10px; color: var(--muted); } .wrap { overflow-x: auto; } @@ -243,6 +247,14 @@ def write_html(job: Job, path: Path, *, thumbnails: bool = True, **_options) -> for i, root in enumerate(job.destination_roots, start=1) ) + # Straight under the summary, above the clips: what the job did not + # supply is the first thing a reader of a partial cut needs to see. + unresolved = "" + if job.unresolved: + items = "".join(f"
  • {_esc(line)}
    • " for line in job.unresolved) + unresolved = (f'

    {_esc(UNRESOLVED_TITLE)} ' + f'({len(job.unresolved)})

      {items}
    ') + document = f""" @@ -252,6 +264,7 @@ def write_html(job: Job, path: Path, *, thumbnails: bool = True, **_options) ->
    {''.join(f'
    {_esc(k)}: {_esc(v)}
    ' for k, v in summary if v)}
    +{unresolved} {''.join(clips)}

    All file details for root source: {_esc(job.name)}

    Full Path: {_esc(job.source_root)}
    {destinations}
    diff --git a/src/offloader/reports/pdf.py b/src/offloader/reports/pdf.py index 653e0f6..52ddcc1 100644 --- a/src/offloader/reports/pdf.py +++ b/src/offloader/reports/pdf.py @@ -29,6 +29,7 @@ format_size, ) from . import fonts, icons, layout +from .csv_report import UNRESOLVED_TITLE @dataclass(frozen=True) @@ -548,6 +549,62 @@ def _draw_detail_entry(self, entry: FileEntry, index: int) -> None: self.y = top + height + # ------------------------------------------------------------ unresolved + def _draw_unresolved(self) -> None: + """What a timeline job did not supply, after everything it did. + + Only drawn when there is something to say, so a card's report keeps + the reference layout exactly. "Final Status: Incomplete" in the header + points here. + """ + lines = self.job.unresolved + size = layout.SIZE_META + pitch = size + 2.5 + self._ensure(layout.DETAIL_TITLE_GAP + layout.DETAIL_PITCH) + baseline = self.y + layout.DETAIL_TITLE_GAP + self._draw_text(layout.DETAIL_TITLE_X, baseline, + f"{UNRESOLVED_TITLE} ({len(lines)})", + layout.SIZE_HEADER, bold=True, color=layout.COLOR_LABEL) + self.y = baseline + 4.0 + width = layout.DETAIL_MAX_X - layout.DETAIL_NAME_X + for line in lines: + # Wrapped, never elided: the middle of a path is often the part + # that says which of two same-named files is meant. + for index, piece in enumerate(self._wrap(line, size, width)): + self._ensure(pitch) + self.y += pitch + x = layout.DETAIL_NAME_X + (0.0 if index == 0 else 8.0) + self._draw_text(x, self.y, piece, size) + self.y += 1.5 + + def _wrap(self, text: str, size: float, width: float) -> list[str]: + """Greedy word wrap at `size`, hard-splitting a word wider than a line.""" + font = self._font(False) + + def fits(candidate: str) -> bool: + return pdfmetrics.stringWidth(candidate, font, size) <= width - 8.0 + + lines: list[str] = [] + current = "" + for word in text.split(" "): + candidate = f"{current} {word}" if current else word + if fits(candidate): + current = candidate + continue + if current: + lines.append(current) + current = "" + while word and not fits(word): + cut = len(word) + while cut > 1 and not fits(word[:cut]): + cut -= 1 + lines.append(word[:cut]) + word = word[cut:] + current = word + if current: + lines.append(current) + return lines or [""] + # ------------------------------------------------------------ build def build(self) -> Path: self._draw_header() @@ -560,6 +617,9 @@ def build(self) -> Path: for index, entry in enumerate(self.job.files): self._draw_detail_entry(entry, index) + if self.job.unresolved: + self._draw_unresolved() + self._draw_footer() self.canvas.save() return self.path diff --git a/src/offloader/timeline.py b/src/offloader/timeline.py index 8f44911..4d673d4 100644 --- a/src/offloader/timeline.py +++ b/src/offloader/timeline.py @@ -43,10 +43,10 @@ import os import re from collections.abc import Iterable, Sequence -from dataclasses import dataclass, field +from dataclasses import dataclass, field, replace from enum import Enum from pathlib import Path, PurePath -from urllib.parse import unquote +from urllib.parse import unquote, urlparse from . import companions, engine from .hashers import DEFAULT_ALGORITHM, hash_file @@ -96,13 +96,35 @@ class Status(str, Enum): AMBIGUOUS = "ambiguous" #: Not under the roots and not where the timeline says either. MISSING = "missing" + #: The timeline names a clip but carries no path for it: an OTIO + #: MissingReference, or an FCP 7 `` with a name and no ``. + #: Real media the editor had offline. Nothing can be found or copied by + #: path, so a human relinks it. + OFFLINE = "offline" + #: A media reference this module does not resolve, such as an image + #: sequence. Real media, reported by name, never counted as satisfied. + UNSUPPORTED = "unsupported" + #: A non-file URL (http, https, s3 and so on). Never treated as a local + #: path, never searched by basename, never copied. + EXTERNAL = "external" #: The timeline carries no media for this at all -- a Premiere-native #: graphic, a title, a generator. No file exists to supply. GENERATED = "generated" -#: Statuses that leave the caller with something to do before a conform. -UNRESOLVED = (Status.AMBIGUOUS, Status.MISSING, Status.GENERATED) +#: Statuses that leave real media unsupplied: the cut will not conform until +#: a human settles each one, so no offload may call itself complete while any +#: remain. GENERATED is not among them: there is no file to supply. +UNRESOLVED = (Status.AMBIGUOUS, Status.MISSING, Status.OFFLINE, + Status.UNSUPPORTED, Status.EXTERNAL) + +#: `Reference.kind` values. Only FILE references carry a local target. +KIND_FILE = "file" +KIND_GENERATED = "generated" +KIND_OFFLINE = "offline" +KIND_SEQUENCE = "sequence" +KIND_EXTERNAL = "external" +KIND_UNSUPPORTED = "unsupported" @dataclass(frozen=True) @@ -118,6 +140,13 @@ class Reference: #: report a sound effect as used twice as often as it is. uses: int = 1 generated: bool = False + #: What sort of reference this is; see the KIND_* constants. Anything + #: but KIND_FILE has no local target and can never be PRESENT or a GAP. + kind: str = KIND_FILE + #: The timeline's own text for a reference with no local target: a URL, + #: an image sequence's base, or the media reference's type. Shown to the + #: user so an unresolved item can be found by hand. + url: str = "" @dataclass @@ -133,6 +162,10 @@ class Resolution: #: AMBIGUOUS, and for PRESENT when the copies are byte-identical. alternatives: list[Path] = field(default_factory=list) note: str = "" + #: A SUBSTITUTED proxy whose frame count could not be compared with the + #: original's. It may well be right, but nothing has shown it, so it + #: counts against completeness. + unverified: bool = False @property def name(self) -> str: @@ -156,9 +189,34 @@ def gaps(self) -> list[Resolution]: @property def unresolved(self) -> list[Resolution]: - """Everything a human has to deal with before the cut will conform.""" + """References whose media is not supplied: see `UNRESOLVED`.""" return self.of(*UNRESOLVED) + @property + def unconfirmed(self) -> list[Resolution]: + """Proxy substitutions whose frame counts could not be compared.""" + return [r for r in self.resolutions + if r.status is Status.SUBSTITUTED and r.unverified] + + @property + def incomplete(self) -> list[Resolution]: + """Everything a human has to deal with before the cut will conform. + + Unresolved references, and proxies standing in without proof. While + this is non-empty neither `resolve` nor `offload` may report the cut + as complete, and both exit non-zero. + """ + return [r for r in self.resolutions + if r.status in UNRESOLVED + or (r.status is Status.SUBSTITUTED and r.unverified)] + + @property + def differing(self) -> list[Resolution]: + """GAPs copied from the timeline's own path although a *different* + file of the same name is under a search root. Worth a warning: after + the copy the drive holds two files of that name.""" + return [r for r in self.gaps if r.alternatives] + @property def counts(self) -> dict[Status, int]: return {s: sum(1 for r in self.resolutions if r.status is s) for s in Status} @@ -227,6 +285,35 @@ def _url_to_path(url: str) -> Path | None: return Path(text) +_SCHEME = re.compile(r"^([A-Za-z][A-Za-z0-9+.\-]*):") + + +def _url_scheme(url: str) -> str: + """The URL scheme, lowercased, or "" for a bare path. + + A single letter is a Windows drive (`E:/Media/a.mov`), not a scheme. + """ + match = _SCHEME.match(url.strip()) + if not match or len(match.group(1)) == 1: + return "" + return match.group(1).lower() + + +def _anchored(target: Path, base: Path) -> Path: + """`target`, resolved against `base` when the timeline wrote it relative. + + A relative target means "relative to the timeline", never "relative to + wherever the command happened to be run": resolved against the working + directory, `media/clip.mov` would find, and copy, an unrelated file of + that name. `anchor` rather than `is_absolute()` decides, because on + Windows a Mac editor's `/Volumes/Edit/a.mov` has a root but no drive and + is not relative to anything here. + """ + if target.anchor: + return target + return Path(os.path.normpath(base / target)) + + def _adapter_for(path: Path, adapter: str | None = None) -> str: if adapter: return adapter @@ -247,6 +334,11 @@ def read_references(path: Path, adapter: str | None = None) -> list[Reference]: -- a Premiere-native graphic, a title, a generator -- comes back with `generated=True` and no target, because there is no file to supply and saying so is more useful than dropping it. + + Real media the module cannot resolve by path is never folded in with + those. An offline clip (no path), an image sequence, a non-file URL and + any other kind of media reference each come back by name, with a `kind` + that makes the resolver report it as unresolved. """ path = Path(path) if not path.is_file(): @@ -269,33 +361,84 @@ def read_references(path: Path, adapter: str | None = None) -> list[Reference]: ) timeline = otio.adapters.read_from_file(str(path), name) + base = Path(os.path.abspath(path)).parent + schema = otio.schema - order: list[str] = [] - found: dict[str, dict] = {} + order: list[tuple[str, str]] = [] + found: dict[tuple[str, str], dict] = {} generated = 0 + + def add(kind: str, key: str, label: str, target: Path | None = None, + url: str = "") -> None: + slot = (kind, key) + if slot not in found: + order.append(slot) + found[slot] = {"name": label, "target": target, "url": url, + "kind": kind, "uses": 0} + found[slot]["uses"] += 1 + for clip in timeline.find_clips(): media = clip.media_reference - url = getattr(media, "target_url", None) - resolved = _url_to_path(url) if url else None - if resolved is None: - # A generator, a title, a missing reference: real clips in the cut - # that no file can satisfy. Counted once, reported once. + clip_name = (getattr(clip, "name", "") or "").strip() + media_name = (getattr(media, "name", "") or "").strip() + + if isinstance(media, schema.GeneratorReference): + # A generator, a title, a matte: real clips in the cut that no + # file can satisfy. Counted once, reported once. generated += 1 continue - key = os.path.normcase(str(resolved)) - if key not in found: - order.append(key) - found[key] = {"path": resolved, "name": resolved.name, "uses": 0} - found[key]["uses"] += 1 + + if isinstance(media, schema.ImageSequenceReference): + padding = "#" * max(1, media.frame_zero_padding) + stem = f"{media.name_prefix}{padding}{media.name_suffix}" + url = media.target_url_base or "" + add(KIND_SEQUENCE, os.path.normcase(url + stem), stem, url=url) + continue + + if media is None or isinstance(media, schema.MissingReference): + label = media_name or clip_name or "unnamed offline clip" + add(KIND_OFFLINE, os.path.normcase(label), label) + continue + + if not isinstance(media, schema.ExternalReference): + kind_name = type(media).__name__ + label = media_name or clip_name or kind_name + add(KIND_UNSUPPORTED, f"{kind_name}:{os.path.normcase(label)}", label, + url=kind_name) + continue + + url = (media.target_url or "").strip() + if not url: + label = media_name or clip_name or "unnamed offline clip" + add(KIND_OFFLINE, os.path.normcase(label), label) + continue + + scheme = _url_scheme(url) + if scheme and scheme != "file": + # Not a local path, whatever its last component looks like. + # Matching https://host/clip.mov by basename would "find" an + # unrelated local clip.mov. + label = Path(unquote(urlparse(url).path)).name or url + add(KIND_EXTERNAL, url, label, url=url) + continue + + resolved = _url_to_path(url) + if resolved is None: + label = media_name or clip_name or "unnamed offline clip" + add(KIND_OFFLINE, os.path.normcase(label), label) + continue + resolved = _anchored(resolved, base) + add(KIND_FILE, os.path.normcase(str(resolved)), resolved.name, resolved) references = [ - Reference(name=found[k]["name"], target=found[k]["path"], uses=found[k]["uses"]) + Reference(name=found[k]["name"], target=found[k]["target"], + uses=found[k]["uses"], kind=found[k]["kind"], url=found[k]["url"]) for k in order ] if generated: references.append( Reference(name=f"{generated} timeline-native item(s)", target=None, - uses=generated, generated=True) + uses=generated, generated=True, kind=KIND_GENERATED) ) return references @@ -411,13 +554,23 @@ def resolve(references: Sequence[Reference], search_roots: Sequence[Path], *, The order of the tests is the whole design: 1. A reference the timeline carries no media for is GENERATED and stops - there. No search would ever satisfy it. + there. No search would ever satisfy it. Real media with no usable + local path (offline, an image sequence, a URL) is OFFLINE, + UNSUPPORTED or EXTERNAL and stops there too, but counts as unresolved. 2. Anything under a search root with that basename wins, whatever path the timeline asked for. This is what stops a file being copied again under a second name because an editor addressed it on their own volume. Several copies that are byte-identical are still PRESENT, with the duplicates recorded; several that differ are AMBIGUOUS and nothing is - chosen. + chosen. Two limits on "wins": + + * When the timeline's own path is readable it is the truth. The file + under the root counts only if it holds the same bytes; otherwise the + original is a GAP and the other file is reported as differing. + * Distinct timeline references that share a basename must resolve to + distinct files. Without content evidence one file cannot be shown to + be any particular one of them, so each such reference is AMBIGUOUS + rather than all of them mapping to the one file. 3. A camera original with no decoder may be satisfied by a proxy already under the roots, if the frame counts agree. 4. Only then, a file readable where the timeline says it is, is a GAP: the @@ -428,50 +581,135 @@ def resolve(references: Sequence[Reference], search_roots: Sequence[Path], *, if table is None: table = index(roots, excludes) - resolutions: list[Resolution] = [] + base = Path(os.path.abspath(timeline)).parent if timeline else None + prepared: list[Reference] = [] for reference in references: + if (base is not None and reference.target is not None + and not reference.target.anchor): + reference = replace(reference, target=_anchored(reference.target, base)) + prepared.append(reference) + + # How many *distinct* files the timeline asks for under each basename. + # Lowercased like the index, so A.MOV and a.mov from a case-sensitive + # volume count as two references competing for one name. + wanted: dict[str, set[str]] = {} + for reference in prepared: + if _is_file_reference(reference): + wanted.setdefault(reference.target.name.lower(), set()).add( + os.path.normcase(str(reference.target))) + + resolutions: list[Resolution] = [] + for reference in prepared: + siblings = (len(wanted.get(reference.target.name.lower(), ())) + if _is_file_reference(reference) else 1) resolutions.append( - _resolve_one(reference, table, substitute_proxies, algorithm) + _resolve_one(reference, table, substitute_proxies, algorithm, siblings) ) return Report(timeline=Path(timeline) if timeline else Path(), search_roots=roots, resolutions=resolutions) +def _is_file_reference(reference: Reference) -> bool: + return (not reference.generated and reference.kind == KIND_FILE + and reference.target is not None) + + +def _matches(target: Path, candidate: Path, algorithm: str) -> bool: + """Whether a readable timeline target and a candidate hold the same bytes. + + The same file is trivially a match and is not hashed: a reference + addressed by its real path under the root is the common case. + """ + try: + if os.path.samefile(target, candidate): + return True + except OSError: + return False + return _same_file([target, candidate], algorithm) + + def _resolve_one(reference: Reference, table: dict[str, list[Path]], - substitute_proxies: bool, algorithm: str) -> Resolution: - if reference.generated or reference.target is None: + substitute_proxies: bool, algorithm: str, + siblings: int = 1) -> Resolution: + if reference.generated or reference.kind == KIND_GENERATED: return Resolution(reference, Status.GENERATED, note="the timeline carries no media for this") + if reference.kind == KIND_SEQUENCE: + return Resolution( + reference, Status.UNSUPPORTED, + note=(f"image sequence at {reference.url or 'an unknown location'}; " + "sequences are not resolved, check it by hand")) + + if reference.kind == KIND_EXTERNAL: + return Resolution( + reference, Status.EXTERNAL, + note=f"not a local file: {reference.url}; not searched or copied") + + if reference.kind == KIND_UNSUPPORTED: + return Resolution( + reference, Status.UNSUPPORTED, + note=f"{reference.url or 'unknown'} media reference is not supported") + + if reference.kind == KIND_OFFLINE or reference.target is None: + # Searching by the clip's name would be a guess at which file the + # editor meant; offering the hits is useful, choosing one is not. + hits = table.get(reference.name.lower(), []) + note = "the timeline has no media path for this clip (offline)" + if hits: + note += (f"; {len(hits)} file(s) of this name under the search " + "roots, not chosen") + return Resolution(reference, Status.OFFLINE, alternatives=list(hits), + note=note) + target = reference.target candidates = table.get(target.name.lower(), []) + readable = target.is_file() - if len(candidates) == 1: - return Resolution(reference, Status.PRESENT, location=candidates[0]) + if candidates: + best = _closest(candidates, target) + others = [c for c in candidates if c != best] + if others and not _same_file(candidates, algorithm): + return Resolution( + reference, Status.AMBIGUOUS, location=None, + alternatives=[best, *others], + note=(f"{len(candidates)} different files share this name; " + f"closest to the timeline path is {best}"), + ) + copies = (f"{len(candidates)} byte-identical copies; either resolves" + if others else "") + + if readable: + if _matches(target, best, algorithm): + return Resolution(reference, Status.PRESENT, location=best, + alternatives=others, note=copies) + # The "MISSING MEDIA" slate case: the real file is reachable and + # the one under the root of that name is something else. + return Resolution( + reference, Status.GAP, location=target, + alternatives=list(candidates), + note=(f"a different file of this name is under the search roots " + f"({best}); the timeline's original is copied"), + ) - if len(candidates) > 1: - if _same_file(candidates, algorithm): - best = _closest(candidates, target) - others = [c for c in candidates if c != best] + if siblings > 1: return Resolution( - reference, Status.PRESENT, location=best, alternatives=others, - note=f"{len(candidates)} byte-identical copies; either resolves", + reference, Status.AMBIGUOUS, location=None, + alternatives=[best, *others], + note=(f"{siblings} different timeline references are named " + f"{target.name} but the search roots hold only one file " + "of that name; not chosen"), ) - best = _closest(candidates, target) - others = [c for c in candidates if c != best] - return Resolution( - reference, Status.AMBIGUOUS, location=None, - alternatives=[best, *others], - note=(f"{len(candidates)} different files share this name; " - f"closest to the timeline path is {best}"), - ) + return Resolution(reference, Status.PRESENT, location=best, + alternatives=others, note=copies) + proxy_note = "" if substitute_proxies and companions.needs_proxy(target): proxy = _proxy_for(target, table) if proxy is not None: - original = target if target.is_file() else None - note = _substitution_note(original, proxy) - if note is None: + original = target if readable else None + verdict, note = _substitution(original, proxy) + if verdict == "mismatch": return Resolution( reference, Status.AMBIGUOUS, location=None, alternatives=[proxy], @@ -479,33 +717,61 @@ def _resolve_one(reference: Reference, table: dict[str, list[Path]], "does not match the original; relinking to it would " "move every edit point"), ) - return Resolution(reference, Status.SUBSTITUTED, location=proxy, - note=note) + if verdict in ("verified", "unverified"): + return Resolution(reference, Status.SUBSTITUTED, location=proxy, + note=note, unverified=verdict == "unverified") + # "unusable": the stand-in could not be read as media at all. + proxy_note = note - if target.is_file(): - return Resolution(reference, Status.GAP, location=target) + if readable: + return Resolution(reference, Status.GAP, location=target, note=proxy_note) - return Resolution(reference, Status.MISSING, location=None, - note=f"not under any search root, and not at {target}") + note = f"not under any search root, and not at {target}" + if proxy_note: + note += f"; {proxy_note}" + return Resolution(reference, Status.MISSING, location=None, note=note) -def _substitution_note(original: Path | None, proxy: Path) -> str | None: +def _substitution(original: Path | None, proxy: Path) -> tuple[str, str]: """Whether `proxy` may stand in for `original`, and why. - None means refuse. A proxy is only a safe stand-in if it is the same length - as what it replaces, frame for frame: relink a clip to a shorter file and - every edit point after it moves. Where the original cannot be reached the - check cannot be made, and the note says so rather than implying agreement. + Returns a verdict and a note. The verdict is "verified" (frame counts + agree), "unverified" (the proxy is readable media but the original's count + could not be got), "mismatch" (the counts disagree: refuse), or + "unusable" (the proxy itself yields no frame count, so nothing shows it is + media at all, let alone this media: it must not stand in). + + A proxy is only a safe stand-in if it is the same length as what it + replaces, frame for frame: relink a clip to a shorter file and every edit + point after it moves. """ + got = _frames(proxy) + if got is None: + return "unusable", (f"same-stem file {proxy} could not be read as media " + "(no frame count), so it does not stand in") if original is None: - return "proxy stands in for an original that is not reachable; frames unverified" + return "unverified", ("proxy stands in for an original that is not " + "reachable; frames unverified") want = _frames(original) - got = _frames(proxy) - if want is None or got is None: - return "proxy stands in for the original; frame counts unverified" + if want is None: + return "unverified", "proxy stands in for the original; frame counts unverified" if want != got: - return None - return f"proxy matches the original frame for frame ({want} frames)" + return "mismatch", "" + return "verified", f"proxy matches the original frame for frame ({want} frames)" + + +def describe(resolution: Resolution) -> str: + """One line for a report or a summary: status, name, uses and why.""" + label = ("unverified proxy" if resolution.unverified + else resolution.status.value) + text = f"{label}: {resolution.name} (used {resolution.reference.uses}x)" + if resolution.status is Status.SUBSTITUTED and resolution.location is not None: + text += f", stood in for by {resolution.location}" + elif resolution.reference.target is not None: + text += f", timeline path {resolution.reference.target}" + if resolution.note: + text += f"; {resolution.note}" + return text # -------------------------------------------------------------------------- diff --git a/tests/test_timeline.py b/tests/test_timeline.py index b491035..061b2f5 100644 --- a/tests/test_timeline.py +++ b/tests/test_timeline.py @@ -174,12 +174,18 @@ def test_unverifiable_frame_count_says_so_rather_than_implying_agreement( library: Path, tmp_path: Path, monkeypatch): original = _write(tmp_path / "camera" / "A006_C121.braw", b"raw" * 1000) _write(library / "package" / "A006_C121.mp4", b"proxy") - monkeypatch.setattr(timeline, "_frames", lambda path: None) + # The proxy reads as media; the original's count cannot be got. + lengths = {"braw": None, "mp4": 1065} + monkeypatch.setattr(timeline, "_frames", + lambda path: lengths[Path(path).suffix.lstrip(".")]) report = timeline.resolve([_ref("A006_C121.braw", original)], [library]) (resolution,) = report.resolutions assert resolution.status is Status.SUBSTITUTED assert "unverified" in resolution.note + # Offered, but not counted as settled: see test_timeline_completeness. + assert resolution.unverified + assert report.incomplete == [resolution] def test_substitution_can_be_switched_off(library: Path, tmp_path: Path): diff --git a/tests/test_timeline_completeness.py b/tests/test_timeline_completeness.py new file mode 100644 index 0000000..30f323a --- /dev/null +++ b/tests/test_timeline_completeness.py @@ -0,0 +1,602 @@ +"""A timeline offload must never call a partial cut complete. + +Every test here is a way the cut could be short of media while the terminal, +the exit code or the paperwork said otherwise: references that are missing, +ambiguous, offline, unsupported or remote; two different recordings sharing a +name; a stand-in slate where the real file was reachable; a proxy nobody +could check; a cancel; a relative path read against the wrong directory. + +Timeline files are written with OpenTimelineIO's own `otio_json` adapter, so +the reader under test is the real one. +""" + +from __future__ import annotations + +import csv +from pathlib import Path + +import pytest + +from offloader import cli, engine, timeline +from offloader.models import ( + Destination, + FileEntry, + FileStatus, + Job, + VerificationMode, +) +from offloader.reports import write_csv, write_html, write_pdf +from offloader.timeline import Reference, Status + +try: + import opentimelineio as otio +except ImportError: # pragma: no cover - optional extra + otio = None + +needs_otio = pytest.mark.skipif(otio is None, reason="needs opentimelineio") + +#: The heading every report gives what a timeline job did not supply. Spelled +#: out rather than imported: it is what a reader of the paperwork sees. +UNRESOLVED_TITLE = "Unresolved timeline references, NOT supplied by this job" + + +# -------------------------------------------------------------------------- +# Fixtures +# -------------------------------------------------------------------------- + + +def _write(path: Path, data: bytes) -> Path: + path.parent.mkdir(parents=True, exist_ok=True) + path.write_bytes(data) + return path + + +def _otio(path: Path, refs: list, clip_names: list[str] | None = None) -> Path: + """An .otio cut with one clip per entry of `refs`. + + A str or Path entry becomes an ExternalReference: an absolute path is + written as a file URL, anything else verbatim (a relative path, an + https URL). Any other entry is used as the media reference itself. + """ + cut = otio.schema.Timeline(name="cut") + track = otio.schema.Track(kind=otio.schema.TrackKind.Video) + cut.tracks.append(track) + span = otio.opentime.TimeRange(otio.opentime.RationalTime(0, 24), + otio.opentime.RationalTime(24, 24)) + for index, ref in enumerate(refs): + if isinstance(ref, (str, Path)): + url = Path(ref).as_uri() if Path(ref).is_absolute() else str(ref) + media = otio.schema.ExternalReference(target_url=url) + else: + media = ref + name = clip_names[index] if clip_names else f"clip{index}" + track.append(otio.schema.Clip(name=name, media_reference=media, + source_range=span)) + path.parent.mkdir(parents=True, exist_ok=True) + otio.adapters.write_to_file(cut, str(path)) + return path + + +def _offload(cut: Path, root: Path, dest: Path, *extra: str) -> int: + return cli.main(["offload", "--timeline", str(cut), "--search-root", str(root), + "--dest", str(dest), "--report", "csv", "--no-probe", + "--quiet", "--retries", "1", *extra]) + + +def _csv_status(dest: Path) -> str: + (report,) = list(dest.rglob("*.csv")) + header = report.read_text(encoding="utf-8").splitlines()[1] + return next(cell for cell in header.split(",") if cell.startswith("Status:")) + + +@pytest.fixture +def s1(tmp_path: Path): + """present.mov here, missing.mov nowhere, amb.mov twice and different.""" + root = tmp_path / "drive" + _write(root / "present.mov", b"P" * 1000) + _write(root / "a" / "amb.mov", b"A" * 1000) + _write(root / "b" / "amb.mov", b"B" * 1000) + editor = tmp_path / "editor" + return root, [editor / "present.mov", editor / "missing.mov", editor / "amb.mov"] + + +# -------------------------------------------------------------------------- +# 1. "nothing to copy" and "Verified" with unresolved references +# -------------------------------------------------------------------------- + + +@needs_otio +def test_nothing_to_copy_with_unresolved_references_fails_and_names_them( + tmp_path: Path, s1, capsys): + root, refs = s1 + cut = _otio(tmp_path / "cut.otio", refs) + + code = _offload(cut, root, tmp_path / "dest") + + out = capsys.readouterr().out + assert code == 1 + assert "every reference is already here" not in out + assert "NOT complete" in out + assert "[missing] missing.mov" in out + assert "[ambiguous] amb.mov" in out + + +@needs_otio +def test_nothing_to_copy_when_everything_is_present_still_exits_zero( + tmp_path: Path, capsys): + root = tmp_path / "drive" + _write(root / "present.mov", b"P" * 1000) + cut = _otio(tmp_path / "cut.otio", [tmp_path / "editor" / "present.mov"]) + + assert _offload(cut, root, tmp_path / "dest") == 0 + assert "every reference is already here" in capsys.readouterr().out + + +@needs_otio +def test_offload_copies_the_gap_lists_the_unresolved_and_is_not_verified( + tmp_path: Path, s1, capsys): + root, refs = s1 + gap = _write(tmp_path / "elsewhere" / "gap.mov", b"G" * 5000) + cut = _otio(tmp_path / "cut.otio", [gap, *refs[1:]]) + dest = tmp_path / "dest" + + code = _offload(cut, root, dest) + + captured = capsys.readouterr() + text = captured.out + captured.err + assert code == 1 + landed = list(dest.rglob("gap.mov")) + assert len(landed) == 1 and landed[0].read_bytes() == b"G" * 5000 + assert ": Incomplete" in captured.out + assert "2 timeline reference(s) NOT supplied" in text + assert "missing: missing.mov" in text and "ambiguous: amb.mov" in text + assert _csv_status(dest) == "Status: Incomplete" + + +def test_a_job_with_unresolved_references_is_never_verified(tmp_path: Path): + entry = FileEntry(source=tmp_path / "a.mov", source_root=tmp_path, size=1, + created=0.0, modified=0.0) + entry.destinations.append(Destination(root=tmp_path / "d", + path=tmp_path / "d" / "a.mov", + status=FileStatus.VERIFIED)) + job = Job(name="cut", source_root=tmp_path, destination_roots=[tmp_path / "d"], + files=[entry]) + assert job.final_status == "Verified" + job.unresolved = ["missing: b.mov (used 1x)"] + assert job.final_status == "Incomplete" + job.verification = VerificationMode.NONE + assert job.final_status == "Incomplete" + + +# -------------------------------------------------------------------------- +# 2. Reports list what was not supplied +# -------------------------------------------------------------------------- + +_LINES = ["missing: missing.mov (used 2x); not under any search root", + "ambiguous: amb.mov (used 1x); 2 different files share this name"] + + +def test_csv_report_lists_unresolved_references(sample_job: Job, tmp_path: Path): + sample_job.unresolved = list(_LINES) + text = write_csv(sample_job, tmp_path / "r.csv").read_text(encoding="utf-8") + assert f"# {UNRESOLVED_TITLE} (2)" in text + for line in _LINES: + assert line in text + assert "Status: Incomplete" in text + # The file table above is untouched: the header row is still row 3. + rows = list(csv.reader((tmp_path / "r.csv").open(encoding="utf-8"))) + assert rows[2][0] == "File Name" + + +def test_html_report_lists_unresolved_references(sample_job: Job, tmp_path: Path): + sample_job.unresolved = [*_LINES, "offline: