diff --git a/CHANGELOG.md b/CHANGELOG.md index 0ee5eea..1cf965e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,8 +8,71 @@ project uses [semantic versioning][semver]. ## [Unreleased] +### Changed + +- **Full verification is the default.** The read-back is the only mode that + proves what is on the destination device, and the default should be the one + whose "Verified" means the most. Applies to the engine's options, new + presets, Simple mode and the CLI's `--verify`; `source-only` remains one + flag or one dropdown away for the run that is racing a deadline. The cost + is one extra read of each copy at the destination's own read speed. +- **Jobs offloaded from a card's root are named after the volume label.** A + root has no folder name, so the job — and every report named after it — + was called "Offload". It is now called what the operator calls the card: + the label, e.g. "A003". Applies to the engine, the queue's naming + templates (`{card}`), and Simple mode's placeholder. +- **The PDF's document title carries the route and the date.** A stack of + reports in a file manager all read "Offload Job Report"; the title is now + "A003 Job Report — E:\ → D:\skate video — 2026-08-09". + +### Fixed + +- **A decoder ffmpeg lacks is probed once per job, not per clip.** Extracting + thumbnails from BRAW with a stock ffmpeg fails identically for every clip; + each one still paid four doomed process spawns. The first clip of a suffix + that produces no frames now marks that suffix dead for the rest of the job + (camera proxies, being ordinary MP4/MOV, are unaffected). + +- **The queue's throughput and ETA measure the last five seconds, not the life + of the job.** The old figure was `bytes / total elapsed`, which folds the + pre-copy card scan and every between-file probe stall into the number + forever — a real offload read 3.5 MB/s while clips were demonstrably flying + past, and the ETA was wrong in the same direction. The rate now comes from a + trailing window, decays visibly during a stall instead of freezing, and + survives the copy→verify counter reset. + +- **The drive panel no longer waits on the slowest network share.** Volume + probes run concurrently instead of serially — the refresh costs the slowest + probe, not the sum — and local drives are delivered before network shares, + so the card reader next to the machine never queues behind an SMB + round-trip. While the shares are still answering, the rows from the last + scan stay up rather than flickering out, and the Refresh button says + "Scanning…" instead of looking like a button that does nothing. +- **A running job is visible as one.** The queue panel now carries a summary + line — stage, current file, percent, live rate and ETA — instead of leaving + the evidence in a thin strip of 30 px rows. The progress bar gained a + percent label and colours that survive the row being selected (the running + row is auto-selected, and an accent bar on the accent selection was + invisible on exactly the row that mattered). The rate and progress columns + are fixed-width, so updating values no longer shove the numbers being read. + A once-a-second repaint lets the displayed rate visibly decay during a + stall instead of freezing at its last healthy value. +- **Simple mode's form rows no longer clip.** Inputs and checkboxes declare + the height their styling actually needs; at fractional display scales + (125%) the computed hint fell short and every field's text was sliced at + the bottom. + ### Added +- **"Start offload" says "Add to queue" when that is what it does.** Jobs run + one at a time; while one is running the button enqueues, and the ready line + says the job runs after the current one. +- **Checksum pickers say what the choice costs.** MD5 sat in the same list as + XXHash3-64 looking like an equal choice; on the copy path, where every byte + is hashed once per stream, it is ~40x slower and can cap copy speed. The + desktop pickers, `--hash` help and `offloader info` now carry a speed note + per algorithm ("fastest", "~40x slower, legacy compatibility only", …). + - **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 @@ -21,6 +84,77 @@ project uses [semantic versioning][semver]. profile is a first-class field on `OffloadOptions`, `Job` and saved presets, and is selectable in the desktop app's Simple mode and preset editor. This is a one-way verified transfer, not two-way sync — see `ROADMAP.md`. +- **`--paranoid` reads every source file twice and compares.** The gap it + closes: a read that returns wrong bytes *without raising*. The checksum is + computed from whatever came back, so the destination faithfully matches a + corrupted source and verifies clean at every level — file hashes, directory + hashes, the lot. Nothing but reading twice can see it. A disagreement is + retried rather than adjudicated, because there is no basis for deciding which + read was the true one; a source that will not read the same twice fails the + file and leaves nothing behind. The page cache is dropped before the second + read, and the job says so when it could not be, since a re-read served from + memory compares the first read against itself. Costs a full second pass, which + is why it is opt-in. +- **Sidecars and proxies are grouped with the clip they belong to.** A + `.sidecar` carries a BRAW's grade; delivered without its clip it is nothing, + and a clip delivered without it has silently lost the grade. Matching is by + stem, reusing what proxy pairing already did, and an ambiguous stem is left + unlinked rather than guessed at. A clip that copies while a file belonging to + it does not is now a job warning instead of two rows twenty lines apart. The + HTML report shows them together and the CSV gains a `Companion Of` column. + Media profile only: a companion is a file belonging to a *clip*, and under + `--profile data` nothing is a clip, so a dataset is not told that + `capture.xmp` belongs to `capture.h5` on the strength of a shared stem. +- **`offloader verify` now re-checks the ASC MHL directory hashes**, which were + written from the start and never read back. A rename or a moved file leaves + every individual file hashing exactly as recorded, so no file-level check can + object to it; the structure hash exists precisely to catch that, and now does. + Content matching while structure does not is reported as `RENAMED`, which is a + much stronger statement than the "not in manifest" line it used to produce. + + Verifying this way means hashing files the manifest does not list — that is + what proves a rename is only a rename — while honouring the manifest's own + `ignore` patterns. Directory hashes that a failed file already accounts for + say so rather than repeating themselves up to the root. + + A manifest now records where the job's reports went, alongside `ascmhl`. They + are written into the destination after it, so they are on disk when a verifier + recomputes but were never in what it recomputes against — without the pattern, + a card that had just been copied reported its own `JobReport.pdf` as a change + to the tree. The path is recorded rather than the conventional name, since + `--report-dir` moves it; histories written before it was recorded are read with + `*_Reports` allowed for. + +### Changed + +- **The preset editor is grouped into Preset, Copying and Reports.** Sixteen + fields in one flat column read as a wall, and the two or three bearing on any + given change were never next to each other. Checkboxes now sit together under + one label instead of each taking a blank one, `Job name` is called `Job name + template` to distinguish it from Simple mode's literal job name, and + `Skip files already present at matching size` carries a tooltip saying what it + does not compare. +- **A transient read failure is retried at the chunk that failed, not by + restarting the file.** Recovering a bad sector near the end of a 79 GB clip + used to mean re-reading all 79 GB; it now costs one 8 MiB re-read. This turned + out not to need the hasher rewind it looked like it would: a chunk is only + hashed once it has been delivered whole, so a failed read has produced no + state to unwind. The source is reopened and sought back to the failed offset, + since a reader that dropped off the bus needs its handle re-established. + Writes still restart the whole file — a write that fails part-way leaves the + destination at a length the copy loop does not know. Once a chunk has had + every attempt the policy allows, the whole-file retry no longer repeats them + against the same fault. +- **A verify report that failed only on its directory hashes says so first.** + It used to open with the file tally — `3 checked: 3 ok` — on a report that did + not pass, which reads as a pass to anyone scanning. That combination is now + stated as what it is: the bytes are intact and the tree is not. Reports with + file failures are unchanged; they already led with them. +- **Recovered reads are reported once per file, not once per chunk.** A card + failing over a contiguous stretch produced one warning every 8 MiB, burying + every other warning in the job. A single bad sector still names its offset + exactly, because there the byte is the useful fact; a run of them is bounded + by the first and the last, because there it is not. ### Fixed diff --git a/CONTRIBUTING.md b/CONTRIBUTING.md index c08f130..2cb1dfc 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 # ~400 tests, about 33s +pytest # 453 tests pytest --fuzz # property tests at 3000 examples each, about 2 min ruff check src tests pytest --cov=offloader --cov-report=term-missing @@ -50,6 +50,16 @@ test files, but if you run Qt code by hand: QT_QPA_PLATFORM=offscreen python -m pytest tests/test_gui.py ``` +The README's screenshots are generated, not captured, so a change to the +interface can bring them along with it: + +```sh +python tools/screenshots.py # rewrites docs/images/ +``` + +It runs the real app against a throwaway config directory and invented volumes, +so it neither reads your presets nor puts your drive labels in the README. + ## Testing without a camera card Almost nobody has a 27 GB BRAW clip and a failing card reader to hand, so the diff --git a/README.md b/README.md index 291b30b..3eaeb7f 100644 --- a/README.md +++ b/README.md @@ -90,18 +90,19 @@ offloader verify D:\video\080426\A001 | `--source PATH` | card or folder to offload | | `--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` | +| `--verify MODE` | `full` (default), `source-only`, `none` | | `--profile P` | `media` (default: ffprobe, thumbnails, BRAW) or `data` (generic transfer, no media probing) | | `--generic` | shorthand for `--profile data` | | `--report FMT[,FMT]` | `pdf` (default), `csv`, `mhl`, `ascmhl`, `html` | | `--report-dir PATH` | override the report location | | `--thumbs N` | frames per clip, 0 to disable (default 4) | -| `--name NAME` | job name; defaults to the source folder name | +| `--name NAME` | job name; defaults to the source folder name, or the volume label for a card offloaded from its root | | `--logo PATH` | image for the PDF header | | `--footer TEXT` | footer line for the PDF | | `--exclude GLOB` | extra filename pattern to skip; repeatable | | `--flat` | do not recreate the source folder structure | | `--skip-existing` | skip files already present at matching size | +| `--paranoid` | read each source file twice and compare (offload only) | | `--retries N` | attempts per file on a transient read failure (default 3, 1 disables) | | `--retry-wait SECONDS` | pause before the first retry, backing off after (default 2) | | `--no-probe` | skip ffprobe metadata and thumbnails | @@ -121,16 +122,32 @@ manifest lists and exits non-zero if anything is off, so a format script can gat on it. `--allow-cache` skips the page-cache eviction — faster, and may verify memory rather than the device. +For an ASC MHL history it also recomputes the directory content and structure +hashes, which is the only check that catches a rename or a moved file — every +file involved still hashes exactly as recorded. See +[`docs/ascmhl.md`](docs/ascmhl.md#directory-hashes). + ### Verification modes | Mode | What it does | Catches | | --- | --- | --- | | `none` | copy only | nothing | | `source-only` | hashes the source as it is read and the bytes as they are written | corruption in transit | -| `full` | additionally re-reads each destination file off disk and hashes it | the above, plus bad media and lying write caches | - -`full` is the honest one: it is the only mode that proves what is actually on -the destination, at the cost of reading everything twice. +| `full` (default) | additionally re-reads each destination file off disk and hashes it | the above, plus bad media and lying write caches | + +`full` is the honest one — the only mode that proves what is actually on the +destination — which is why it is the default. The cost is one extra read of +each copy at the destination's own speed: a fast SSD destination adds a few +percent to the job, a spinning disk can approach doubling it. `source-only` +is there for the run that is racing a deadline. + +`--paranoid` is orthogonal to all three. Every mode above compares against the +source's checksum, which is computed from whatever the read returned — so a read +that hands back wrong bytes *without raising* produces a destination that +faithfully matches a corrupted source and verifies clean everywhere. Reading the +source a second time is the only thing that sees it. It costs a full extra pass, +which is why it is opt-in. See +[`docs/data-safety.md`](docs/data-safety.md#reading-the-source-twice). ## Reports @@ -150,6 +167,11 @@ the destination, at the cost of reading everything twice. - **HTML** — self-contained; thumbnails inlined as data URIs, light and dark themes, no external requests. +Sidecars and proxies are shown with the clip they belong to rather than as +unrelated files, matched by stem. A clip that copies while a file belonging to it +does not is a job warning: a BRAW delivered without its `.sidecar` has silently +lost its grade. + ## Generic data transfers The copy engine has never been camera-specific: it streams the source once, @@ -172,7 +194,9 @@ offloader offload \ ``` Nothing is treated as a clip, ffmpeg is never invoked, and the run does not need -it installed. What you still get is the whole point of the tool: every byte +it installed — including the sidecar and proxy grouping above, which links a +file to the clip it belongs to by stem and would otherwise announce that +`capture.xmp` belongs to `capture.h5` on no more evidence than a shared name. What you still get is the whole point of the tool: every byte read once and fanned out, both copies verified off disk, a checksum manifest beside each one, and `offloader verify` to re-check the archive months later for bit rot. The PDF, CSV, MHL, ASC MHL and HTML reports all render a plain file @@ -189,6 +213,8 @@ partial-file updates. See [`ROADMAP.md`](ROADMAP.md). offloader-gui # or: offloader gui ``` +![Preset mode: the drive panel, saved presets and the job queue](docs/images/app-preset-mode.png) + Two modes, switched from the header: - **Preset mode** — saved workflows, each with its own destinations, checksum, @@ -198,6 +224,14 @@ Two modes, switched from the header: - **Simple mode** — source, destinations and options on one screen, for a one-off where building a preset would be more work than the job. +![Simple mode: source, destinations and options on one screen](docs/images/app-simple-mode.png) + +A preset is edited in three blocks — what it is, how it copies, what paperwork +it leaves: + +The preset editor, grouped into Preset, Copying and Reports + Down the left is the **drive panel**: every mounted volume with a capacity bar (amber past 80 %, red past 95 %) and one-click *Source* / *Destination* buttons. Volumes that look like camera media are badged `CARD` and sorted to the top — @@ -316,13 +350,13 @@ what makes the report layer testable without moving bytes. ```sh pip install -e ".[dev]" -pytest # 409 tests, ~33s +pytest # 453 tests pytest --fuzz # same suite, 3000 examples per property (~2 min) ruff check src tests pytest --cov=offloader --cov-report=term-missing ``` -409 tests at 82% line coverage. They cover formatting against the reference's +453 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, @@ -373,9 +407,9 @@ that file. Names are now sanitised into the XML character range. already documented in `docs/`, not from a wishlist. It also says what this deliberately will **not** become. -Nearest up: verifying the ASC MHL directory hashes that are already written (so -a rename is a mismatch rather than a footnote), an optional second read of the -source, and chunk-level rather than whole-file retry for marginal cards. +Nearest up: making `--skip-existing` compare checksums rather than sizes, +writing `previousPath` so a rename survives a generation, and a lock file so two +instances pointed at one destination know about each other. ## Contributing diff --git a/ROADMAP.md b/ROADMAP.md index c6e964b..277e04f 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -17,55 +17,55 @@ an oversight. ## Done Engine, CLI, five report formats (PDF, CSV, MHL 1.1, ASC MHL v2.0, HTML), the -desktop app, and cross-platform CI. +desktop app, and cross-platform CI. A first live card offload then drove a +round of fixes: full verification as the default, windowed throughput/ETA, +checksum-cost labels in every picker, a running-job summary line, the +local-first parallel drive scan, jobs named after the volume label, and a PDF +title that tells reports apart. 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 the format at all, and has been run over 510 real clips from two camera bodies. -## Next - -### Verify what is already written - -`offloader verify` checks file hashes. ASC MHL also records **directory content -and structure hashes**, and those are written but never re-checked. The -structure hash exists precisely to catch a rename or a moved file — a change no -file hash can see, because every file is individually fine. +`offloader verify` re-checks the ASC MHL directory content and structure hashes, +not only the file hashes — so a rename or a moved file, which every file hash +agrees is fine, is reported as the structure-hash mismatch it is. See +[`docs/ascmhl.md`](docs/ascmhl.md#directory-hashes). -Today a rename shows up only as a "not in manifest" line. It should be a -structure-hash mismatch, which is a much stronger statement. +`--paranoid` reads every source file a second time and compares, which is the +only thing that catches a read returning wrong bytes without reporting an error. +Retry works at the chunk that failed rather than restarting the file. Sidecars +and proxies are grouped with the clip they belong to, so a clip separated from +its grade is a warning rather than two unrelated rows. -*Where:* `verify.py`, using the directory-hash code already in `ascmhl.py`. - -### Read the source twice, optionally +## Next -A source read that returns wrong bytes without raising is rare, but nothing -currently catches it: the checksum is computed from what was read, so a bad read -produces a destination that faithfully matches a corrupted source and verifies -clean. +### `--skip-existing` by checksum, not size -A `--paranoid` mode reading the source twice and comparing would close it, at -the cost of a second pass. Worth having as an option for irreplaceable material, -not as a default. +Today it is explicitly a speed option and says so: a destination file of the +right length is assumed to be the right file. That is the one place the tool +takes something on trust, and it is listed under "What is still not protected" +in [`docs/data-safety.md`](docs/data-safety.md) for that reason. A checksum +variant would make it a safe option rather than a fast one. -*Where:* `engine.py`, alongside the existing verification modes. +*Where:* `engine.py`, in the `skip_existing` branch. -### Retry the source, not just the read +### `previousPath`, so a rename survives a generation -Retry currently restarts the whole file on a transient error. For a marginal -card that fails at one sector, re-reading the entire 79 GB clip to recover a few -bytes is expensive. Retrying at the chunk level would need the hasher state -rewound to a chunk boundary — doable, and worth it on failing media. +Verifying directory hashes made a rename visible; it did not make it +*explicable*. A renamed directory reports as one `MISSING` line and one +`RENAMED` parent, with nothing saying the first became the second. The format +has `previousPath` for exactly this and it is not written. -*Where:* `engine.py` `_copy_fanout`, with `retry.py` unchanged. +*Where:* `ascmhl.py`, and `verify.py` to read it back. -### `.sidecar` and companion grouping +### Coordination between instances -BRAW `.sidecar` files carry colour metadata. They are copied like any other -file, but nothing links them to their clip, so a missing one is not flagged and -a report does not show them together. The proxy pairing in `companions.py` -already does the stem-matching this needs. +One app instance serialises its queue. Two pointed at the same destination do +not know about each other, which is a documented gap in +[`docs/data-safety.md`](docs/data-safety.md). A lock file in the destination +would close it. ## Later @@ -85,8 +85,6 @@ already does the stem-matching this needs. - **Nested histories** — an `ascmhl` folder further down the tree with its own chain, and a parent taking a child's root hash as its directory hash. -- **`previousPath`** so a rename is tracked across generations rather than - reading as a new `original` plus a missing path. - **The flatten operation**, consolidating a history into one manifest. - **Several hash formats per manifest**, which the format allows. @@ -94,11 +92,6 @@ already does the stem-matching this needs. - **Email or SMS on completion.** ShotPut Pro has it; a DIT running a long offload wants to leave the cart. -- **Coordination between instances.** One app instance serialises its queue; two - pointed at the same destination do not know about each other. A lock file in - 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. - **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/ascmhl.md b/docs/ascmhl.md index c997cda..89e51ab 100644 --- a/docs/ascmhl.md +++ b/docs/ascmhl.md @@ -52,6 +52,38 @@ in `processinfo/roothash`. Both are built by Appendix G: sort the hashes lexicographically, write their raw bytes into a fresh generator, digest. +`offloader verify` recomputes both from what is on disk and compares. The two +answers separate two different failures: + +| content | structure | verdict | what happened | +| --- | --- | --- | --- | +| matches | matches | `ok` | — | +| matches | differs | `RENAMED` | every byte is intact; a name changed or a file moved | +| differs | differs | `CHANGED` | the bytes under this directory are not what was recorded | + +This is the only check that can see a rename. Every file involved still hashes +correctly, so no file hash — and no amount of re-reading — will ever object. + +Recomputing means hashing the files the manifest does *not* list, since a +renamed file is unlisted under its new name and its hash is what proves the +rename is all that happened. Files matching a recorded `ignore` pattern are left +out, exactly as the writer left them out. + +That is why the writer records where the job's own paperwork went. The PDF, CSV +and thumbnails are written into the destination *after* the manifest, so they +are on disk when a verifier recomputes but were never in what it recomputes +against — and folding them in reports the tool's own output as a change to the +tree. The manifest carries the report directory as an `ignore` pattern for the +same reason it carries `ascmhl`: neither is managed data. A path rather than an +assumed name, because `--report-dir` moves it. A history written before this was +recorded is read with the conventional `*_Reports` allowed for, which is a name +and not a fact — a current manifest states its own layout. + +A directory whose mismatch is already accounted for by a file that failed on its +own hash says so, rather than reporting a fresh problem for every directory +between that file and the root. A directory that gained an unexpected file is +never counted as accounted for — no file verdict can report an arrival. + ## C4 The chain file identifies each manifest by its C4 ID (SMPTE ST 2114): a SHA-512 @@ -105,10 +137,10 @@ the evidence the format exists to carry. ## Limits -- **Directory hashes are written but not verified.** `offloader verify` checks - file hashes; it does not recompute directory hashes, so a pure rename inside - an already-verified tree is reported through the "not in manifest" list rather - than as a structure-hash mismatch. +- **A renamed directory is reported as two facts, not one.** The old name reads + as `MISSING` and its parent as `RENAMED`; nothing states that the one became + the other. `previousPath` is what the format has for that, and it is not + written. - **No nested histories.** The spec allows an `ascmhl` folder further down the tree with its own history, and permits a parent to take a child's root hash as its directory hash. One history per destination root is written here. diff --git a/docs/data-safety.md b/docs/data-safety.md index b91965e..8eea8cd 100644 --- a/docs/data-safety.md +++ b/docs/data-safety.md @@ -71,7 +71,7 @@ Three modes, and it is worth being precise about what each proves. | --- | --- | --- | | `none` | source once | nothing | | `source-only` | source once | the bytes written matched the bytes read | -| `full` | source once, destination again | the bytes **on the destination** match the source | +| `full` (default) | source once, destination again | the bytes **on the destination** match the source | `source-only` hashes the source as it is read and hashes each buffer as it is handed to `write()`. It catches corruption in transit. It cannot catch anything @@ -131,6 +131,35 @@ checksum finds that. It also reports files present on disk that the manifest does not list, and it evicts each file before reading so a freshly written tree is read off the device. +Where the manifest is an ASC MHL history, the directory hashes are recomputed +too, and they catch the one class of change no checksum can. Rename a clip and +every file still hashes exactly as recorded — the bytes did not move. The +structure hash folds each name in with its hash, so it does not agree: + +``` + 3 checked: 1 missing, 2 ok; 2 of 2 directory hashes differ + MISSING ...\Clips\A001_C001.mov + RENAMED (root) + every file still hashes as recorded, so a name changed or a file moved + RENAMED Clips/ + every file still hashes as recorded, so a name changed or a file moved + not in manifest: ...\Clips\A001_C001_take2.mov + +NOT VERIFIED — do not erase the source +``` + +Whether that matters depends on the delivery. A tree whose bytes are intact but +whose names are not is still wrong to hand to an archive that will look for them +by path. + +Recomputing costs more than checking the listed files. Proving a rename is only +a rename means hashing what the manifest does *not* list, so a destination root +holding anything besides this job is read too — and that happens whether or not +the unlisted files were asked for, since the hashes are what the comparison +needs. The job's own reports are excluded, because the manifest records where +they went, but a shared archive drive is not. `--allow-cache` removes the +eviction, not the reading. + ### The manifest has to travel An MHL that records absolute paths is useless the moment the drive gets a @@ -164,8 +193,10 @@ known. - **Controller and drive caches.** As above: `full` verification proves the operating system is not lying. It cannot prove the drive is not. -- **Source read errors that return garbage instead of raising.** Very rare, and - only a second independent read of the source would catch it. Not implemented. +- **Source read errors that return garbage instead of raising** — unless + `--paranoid` is on, which reads the source a second time and compares. Off by + default because it costs a full extra pass over the card. See "Reading the + source twice" below. - **`skip_existing` compares size, not checksum.** It is a speed option, not a safety one, and should not be used on a tree whose integrity is in question. - **Concurrent instances.** One app instance serialises its queue. Two instances @@ -185,14 +216,61 @@ delay, so only errors with a plausible transient cause qualify: `EIO`, `EBUSY`, `ERROR_SHARING_VIOLATION` (usually antivirus, usually brief) and `ERROR_IO_DEVICE`. `ENOENT` and `ENOSPC` fail immediately. -A retry restarts the whole file rather than resuming, because a partial read -leaves the running checksum meaningless. The partial is discarded and the -progress it claimed is given back, so a retry cannot push the job past 100 %. +A failed *read* is retried at the chunk it failed on, not by restarting the +file. A chunk is only hashed once it has arrived whole, so a read that failed +produced no checksum state to unwind — recovering a bad sector near the end of a +79 GB clip costs one 8 MiB re-read rather than 79 GB. The source is reopened and +sought back to the offset, because a reader that dropped off the bus needs its +handle re-established. Once a chunk has had every attempt the policy allows, the +file is not started again from the top: that would only repeat the same attempts +against the same fault. + +A failed *write* does restart the whole file, because a write that fails +part-way leaves the destination at a length the copy loop does not know. The +partial is discarded and the progress it claimed is given back, so a retry +cannot push the job past 100 %. **A recovered file is still reported.** A card that reads on the third attempt today is a card to stop using, so the job carries a warning naming it. Silent recovery would be the wrong outcome. +## Reading the source twice + +Everything above compares the destination against the source's checksum. That +checksum is computed from whatever the read returned — so a read that hands back +wrong bytes *without raising* produces a destination which faithfully matches a +corrupted source, and verifies clean at every level: file hashes, directory +hashes, read-back off the platter, all of it. The copy is a perfect reproduction +of something that was never on the card. + +Nothing detects that except reading the source again: + +```sh +offloader offload --source E:\ --dest D:\A001 --verify full --paranoid +``` + +The page cache is dropped before the second read, or it would compare the first +read against itself; where the platform cannot drop it the job says so rather +than claim the guarantee. A disagreement is not adjudicated — there is no basis +for deciding which read was true — so it is retried, and a source that will not +read the same twice fails that file and leaves nothing behind. + +It costs a second full pass over the card, which is why it is opt-in rather than +the default. For irreplaceable material it is the strongest statement available. + +## Files that belong together + +A BRAW `.sidecar` carries the clip's grade. Delivered without its clip it is +nothing; the clip delivered without it has silently lost the grade. The same +goes for the proxy a camera writes beside the original. + +These are matched to their clip by stem — the only relationship cameras actually +record — and a clip that copies while a file belonging to it does not is a job +warning, not two rows twenty lines apart in a table nobody reads to the end. An +ambiguous stem, two takes of the same name in different folders, is left +unlinked: naming the wrong clip would be worse than saying nothing, because the +only value of the link is that it can be trusted. + ## Long paths Windows caps a path at 260 characters unless the caller opts out with the diff --git a/docs/images/app-preset-editor.png b/docs/images/app-preset-editor.png new file mode 100644 index 0000000..4ec3850 Binary files /dev/null and b/docs/images/app-preset-editor.png differ diff --git a/docs/images/app-preset-mode.png b/docs/images/app-preset-mode.png new file mode 100644 index 0000000..992f11b Binary files /dev/null and b/docs/images/app-preset-mode.png differ diff --git a/docs/images/app-simple-mode.png b/docs/images/app-simple-mode.png new file mode 100644 index 0000000..5908984 Binary files /dev/null and b/docs/images/app-simple-mode.png differ diff --git a/src/offloader/ascmhl.py b/src/offloader/ascmhl.py index 61e3934..09ffd17 100644 --- a/src/offloader/ascmhl.py +++ b/src/offloader/ascmhl.py @@ -108,19 +108,25 @@ def _build_tree(entries: list[tuple[Path, str, str]]) -> _Node: return root -def _directory_hashes(node: _Node, algorithm_key: str) -> tuple[str, str]: +def _directory_hashes(node: _Node, algorithm_key: str, prefix: str = ".", + sink: dict[str, tuple[str, str]] | None = None, + ) -> tuple[str, str]: """(content, structure) for a directory, computed bottom-up. Only hashes that stand as evidence contribute: a `failed` hash means the file is not what it was, so folding it in would produce a directory hash that certifies a known-bad tree. + + `sink`, when given, collects every directory's pair on the way back up, + keyed by relative POSIX path. """ content_inputs: list[str] = [] structure_inputs: list[str] = [] for name in sorted(node.directories): child_content, child_structure = _directory_hashes( - node.directories[name], algorithm_key) + node.directories[name], algorithm_key, + name if prefix == "." else f"{prefix}/{name}", sink) content_inputs.append(child_content) structure_inputs.append( _structure_entry(name, child_structure, algorithm_key)) @@ -132,8 +138,31 @@ def _directory_hashes(node: _Node, algorithm_key: str) -> tuple[str, str]: content_inputs.append(digest) structure_inputs.append(_structure_entry(name, digest, algorithm_key)) - return (hash_of_hashes(content_inputs, algorithm_key), + pair = (hash_of_hashes(content_inputs, algorithm_key), hash_of_hashes(structure_inputs, algorithm_key)) + if sink is not None: + sink[prefix] = pair + return pair + + +def directory_hashes(entries: list[tuple[Path, str]], + algorithm_key: str) -> dict[str, tuple[str, str]]: + """Every directory's (content, structure) pair, keyed by relative POSIX + path, with `"."` for the root of the managed data. + + Exposed so a verifier can recompute from what is on disk what the writer + recorded. Only directories holding at least one file appear, which is + exactly the set the writer emits a `directoryhash` for. + + Pass only hashes that stand as evidence — excluding a failed one is the + caller's job here, the writer having already done it through the `action` + it recorded. + """ + sink: dict[str, tuple[str, str]] = {} + tree = _build_tree([(relative, digest, ACTION_ORIGINAL) + for relative, digest in entries]) + _directory_hashes(tree, algorithm_key, ".", sink) + return sink # ------------------------------------------------------------------ history @@ -143,6 +172,34 @@ def ascmhl_dir(root: Path) -> Path: return Path(root) / ASCMHL_DIRNAME +def _default_ignores(job: Job, root: Path, report_dir: Path | None) -> list[str]: + """The history's own folder, plus the job's paperwork. + + Reports are written into the destination *after* this manifest, and they + are not managed data: nothing lists them, and the directory hashes here do + not cover them. Recording them as ignored is what stops a verifier folding + the tool's own output back into a recomputed hash and reporting the report + it just wrote as a change to the tree. + + Two entries rather than one because they can differ: `--report-dir` moves + the PDF and CSV, while thumbnails always land in `_Reports`. A report + directory outside this copy needs no pattern, and one that *is* the copy + cannot have one — ignoring `.` would ignore everything. + """ + root = Path(root) + patterns = [ASCMHL_DIRNAME] + for candidate in (report_dir, root / f"{job.name}_Reports"): + if candidate is None: + continue + try: + relative = Path(candidate).resolve().relative_to(root.resolve()).as_posix() + except (ValueError, OSError): + continue + if relative not in (".", "") and relative not in patterns: + patterns.append(relative) + return patterns + + def existing_manifests(root: Path) -> list[Path]: """Manifests already in this history, in sequence order.""" directory = ascmhl_dir(root) @@ -232,6 +289,7 @@ def write_manifest(job: Job, root: Path, *, destination_index: int = 0, process: str = PROCESS_TRANSFER, algorithm_key: str | None = None, ignore_patterns: list[str] | None = None, + report_dir: Path | None = None, directory_hashes: bool = True, when: _dt.datetime | None = None) -> Path: """Write one ASC MHL generation for the copy at `root`, and update the chain. @@ -292,7 +350,8 @@ def write_manifest(job: Job, root: Path, *, destination_index: int = 0, root_hash = ET.SubElement(info, "roothash") _hash_pair(root_hash, tag, content, structure, moment) - patterns = ignore_patterns if ignore_patterns is not None else [ASCMHL_DIRNAME] + patterns = (ignore_patterns if ignore_patterns is not None + else _default_ignores(job, root, report_dir)) if patterns: ignore = ET.SubElement(info, "ignore") for pattern in patterns: @@ -375,14 +434,17 @@ def write_ascmhl(job: Job, path: Path, *, destination_index: int = 0, `path` is the conventional report location; ASC MHL ignores it and writes into `ascmhl/` at the root of the copy, which is where the format requires - a history to live. + a history to live. Its *folder* is still worth knowing: that is where the + rest of the paperwork lands, and the manifest records it as ignored so a + verifier does not mistake it for managed data. """ roots = job.destination_roots or [job.source_root] index = min(destination_index, len(roots) - 1) # The writer interface is shared with the PDF, which takes logo/footer. # Accept and drop anything that does not apply here. - accepted = {"process", "algorithm_key", "ignore_patterns", + accepted = {"process", "algorithm_key", "ignore_patterns", "report_dir", "directory_hashes", "when"} + options.setdefault("report_dir", Path(path).parent) return write_manifest( job, roots[index], destination_index=index, **{k: v for k, v in options.items() if k in accepted}, diff --git a/src/offloader/cli.py b/src/offloader/cli.py index 6773f22..dee3e95 100644 --- a/src/offloader/cli.py +++ b/src/offloader/cli.py @@ -128,7 +128,8 @@ def _summarize(job: Job, reports: list[Path]) -> None: video = f" ({job.video_files} video)" if job.profile.probes_media else "" print(f" {job.total_files} files, {format_size(job.total_bytes)}" f" in {format_elapsed(job.elapsed_sec)}{video}") - print(f" Verification: {job.verification_label}") + print(f" Verification: {job.verification_label}" + f"{' + second source read' if job.paranoid else ''}") for destination in job.destination_roots: print(f" -> {destination}") for report in reports: @@ -150,7 +151,10 @@ def _summarize(job: Job, reports: list[Path]) -> None: def _common_options(parser: argparse.ArgumentParser) -> None: parser.add_argument("--hash", default=hashers.DEFAULT_ALGORITHM, choices=sorted(hashers.algorithm_keys()), - help="checksum algorithm (default: %(default)s)") + help="checksum algorithm (default: %(default)s; the " + "engine hashes every byte on the copy path, so a " + "slow choice caps copy speed — md5 is ~40x slower " + "than the default; see 'offloader info')") parser.add_argument("--report", type=_parse_reports, default=DEFAULT_REPORTS, metavar="FMT[,FMT...]", help=f"report formats: {', '.join(WRITERS)} (default: pdf)") @@ -201,13 +205,17 @@ def build_parser() -> argparse.ArgumentParser: offload.add_argument("--dest", type=Path, action="append", required=True, dest="destinations", metavar="PATH", help="destination root (repeat for multiple copies)") - offload.add_argument("--verify", default=VerificationMode.SOURCE_ONLY.value, + offload.add_argument("--verify", default=VerificationMode.FULL.value, choices=[m.value for m in VerificationMode], help="verification depth (default: %(default)s)") offload.add_argument("--flat", action="store_true", help="do not recreate the source folder structure") offload.add_argument("--skip-existing", action="store_true", help="skip files already present with a matching size") + offload.add_argument("--paranoid", action="store_true", + help="read each source file twice and compare, to " + "catch a read that returned wrong bytes without " + "reporting an error (costs a second pass)") _common_options(offload) report = sub.add_parser( @@ -242,7 +250,7 @@ def _options_from(args: argparse.Namespace, destinations: list[Path]) -> engine. return engine.OffloadOptions( destinations=destinations, algorithm=args.hash, - verification=VerificationMode(getattr(args, "verify", "source-only")), + verification=VerificationMode(getattr(args, "verify", "full")), thumbnail_count=0 if args.no_probe else max(0, args.thumbs), excludes=tuple(engine.DEFAULT_EXCLUDES) + tuple(args.exclude), preserve_structure=not args.flat, @@ -252,6 +260,7 @@ def _options_from(args: argparse.Namespace, destinations: list[Path]) -> engine. profile=profile, retry=retry.RetryPolicy(attempts=max(1, args.retries), delay=max(0.0, args.retry_wait)), + paranoid=getattr(args, "paranoid", False), ) @@ -335,6 +344,8 @@ def progress(index: int, total: int, path: Path) -> None: print(f" {report.summary()}") for verdict in report.failures: print(f" {verdict.describe()}") + for verdict in report.directory_failures: + print(f" {verdict.describe()}") for extra in report.unlisted[:20]: print(f" not in manifest: {extra}") if len(report.unlisted) > 20: @@ -368,7 +379,9 @@ def cmd_info(_args: argparse.Namespace) -> int: else "required for destinations past 260 characters") print(f" long paths: Windows support {'on' if enabled else 'off'};" f" {prefix} prefix {note}") - print(f" checksums: {', '.join(sorted(hashers.algorithm_keys()))}") + print(" checksums: " + "; ".join( + f"{key} ({alg.speed})" if alg.speed else key + for key, alg in sorted(hashers.ALGORITHMS.items()))) print(f" reports: {', '.join(WRITERS)}") print(f" profiles: {', '.join(p.value for p in Profile)} " f"(--profile; 'data' skips media probing for generic transfers)") diff --git a/src/offloader/companions.py b/src/offloader/companions.py index b1f1723..f062ff2 100644 --- a/src/offloader/companions.py +++ b/src/offloader/companions.py @@ -7,10 +7,18 @@ Blackmagic's layout is `A001/A001_08041254_C001.braw` beside `A001/Proxy/A001_08041254_C001.mp4` — same stem, sibling directory. + +The same stem-matching answers a second question: which files have no meaning +on their own. A `.sidecar` is a clip's grade; delivered without its clip it is +nothing, and a clip delivered without it has silently lost the grade. Copying +both and reporting them as two unrelated files is how that goes unnoticed, so +`group` links them and the engine refuses to let them end up with different +verdicts quietly. """ from __future__ import annotations +from collections.abc import Iterable from pathlib import Path #: Camera originals ffmpeg cannot decode without a vendor SDK. @@ -24,6 +32,16 @@ #: Container suffixes a proxy might use, in preference order. PROXY_SUFFIXES = (".mp4", ".mov", ".m4v", ".mxf") +#: Suffixes worn by a file that describes a clip rather than being one. Each is +#: a format a camera or a grading tool writes beside the original, matched to it +#: by stem. Deliberately short: a file wrongly called a companion is reported as +#: belonging to something it does not. +COMPANION_SUFFIXES = { + ".sidecar", # Blackmagic RAW — colour metadata, written when a grade is set + ".rmd", # RED metadata + ".xmp", # Adobe sidecar metadata +} + def needs_proxy(path: Path) -> bool: """Whether this file needs a stand-in to produce a thumbnail.""" @@ -73,3 +91,49 @@ def thumbnail_source(source: Path, if proxy is not None: return proxy, True return Path(source), False + + +def is_companion(path: Path) -> bool: + """Whether this file describes a clip rather than being one.""" + return Path(path).suffix.lower() in COMPANION_SUFFIXES + + +def in_proxy_directory(path: Path) -> bool: + return Path(path).parent.name in PROXY_DIRECTORIES + + +def group(paths: Iterable[Path]) -> dict[Path, Path]: + """Map each companion file to the clip it belongs to. + + Two kinds qualify: a sidecar carrying a clip's metadata, and a proxy the + camera filed in its own directory. Both are matched by stem, which is the + only relationship cameras actually record. + + An ambiguous stem — two clips of the same name in different folders, one + sidecar — is left unlinked rather than guessed at. Claiming a `.sidecar` + belongs to the wrong take would be worse than saying nothing, because the + whole point of the link is that someone trusts it. + """ + files = [Path(p) for p in paths] + clips = [p for p in files if not is_companion(p) and not in_proxy_directory(p)] + + by_stem: dict[str, list[Path]] = {} + for clip in clips: + by_stem.setdefault(clip.stem, []).append(clip) + + linked: dict[Path, Path] = {} + for candidate in files: + if not (is_companion(candidate) or in_proxy_directory(candidate)): + continue + matches = by_stem.get(candidate.stem, []) + if not matches: + continue + # A clip in the same folder wins; a proxy's clip is the folder above. + near = [c for c in matches + if c.parent == candidate.parent + or c.parent == candidate.parent.parent] + if len(near) == 1: + linked[candidate] = near[0] + elif not near and len(matches) == 1: + linked[candidate] = matches[0] + return linked diff --git a/src/offloader/engine.py b/src/offloader/engine.py index 6660ceb..cfc309c 100644 --- a/src/offloader/engine.py +++ b/src/offloader/engine.py @@ -18,7 +18,7 @@ from pathlib import Path from . import braw as braw_mod -from . import companions, integrity, longpath, sysinfo, thumbs +from . import companions, integrity, longpath, sysinfo, thumbs, volumes from . import probe as probe_mod from . import retry as retry_mod from .hashers import get_algorithm, hash_file, new_hasher @@ -167,7 +167,11 @@ class ProgressEvent: class OffloadOptions: destinations: Sequence[Path] algorithm: str = "xxh3-64" - verification: VerificationMode = VerificationMode.SOURCE_ONLY + # FULL by default: the read-back is the only mode that proves what is on + # the destination device, and an offload tool's default should be the one + # whose "Verified" means the most. The cost is one extra read of each copy + # at the destination's own speed; anyone racing a deadline can opt down. + verification: VerificationMode = VerificationMode.FULL thumbnail_count: int = 4 excludes: Sequence[str] = DEFAULT_EXCLUDES #: Preserve the source tree under each destination root. @@ -184,6 +188,10 @@ 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) + #: Read every source file a second time and compare. Costs a full extra + #: pass over the card, and is the only thing that catches a read which + #: returned wrong bytes without the operating system noticing. + paranoid: bool = False def __post_init__(self) -> None: # The data profile is defined by the absence of media work, so enforce @@ -231,9 +239,37 @@ def _destination_for(source: Path, source_root: Path, dest_root: Path, return dest_root / source.name +def _close_quietly(handle: object | None) -> None: + """Close a file handle, swallowing anything it raises. + + Deliberately not just `OSError`. This runs on the way out of a failure and + must never *become* the failure: a raise from here would skip the sentinel + the reader thread owes its consumer, and the copy would hang rather than + report the error that actually happened. + """ + if handle is None: + return + try: + handle.close() + except Exception: + pass + + +@dataclass +class _CopyResult: + """What one pass of `_copy_fanout` produced.""" + + source_checksum: str + destination_checksums: list[str] + #: (offset, attempts) for every chunk that did not read first time. The copy + #: succeeded, but a card that needs these is a card on its way out. + recovered_reads: list[tuple[int, int]] = field(default_factory=list) + + def _copy_fanout(source: Path, targets: Sequence[Path], algorithm: str, on_chunk: Callable[[int], None], - control: JobControl | None = None) -> tuple[str, list[str]]: + control: JobControl | None = None, + retry: retry_mod.RetryPolicy = retry_mod.NO_RETRY) -> _CopyResult: """Stream `source` into every target at once. `targets` are the *in-flight* paths — the caller renames them into place @@ -243,6 +279,11 @@ def _copy_fanout(source: Path, targets: Sequence[Path], algorithm: str, Returns the source checksum plus one checksum per target, computed from the bytes actually handed to each write() call. + + `retry` applies to *source reads only*, chunk by chunk. Writes are left to + the caller's whole-file retry: a write that fails part-way leaves the + destination at a length nothing here knows, whereas a failed read has + produced nothing at all. """ source = Path(source) src_hasher = new_hasher(algorithm) @@ -261,27 +302,64 @@ def _copy_fanout(source: Path, targets: Sequence[Path], algorithm: str, chunks: queue.Queue = queue.Queue(maxsize=READ_AHEAD) stop = threading.Event() failure: list[BaseException] = [] + recovered: list[tuple[int, int]] = [] def read_ahead() -> None: - """Keep the queue fed so the next read overlaps the current write.""" + """Keep the queue fed so the next read overlaps the current write. + + A transient read failure is retried *here*, at the chunk that failed, + rather than by restarting the file. Nothing has been hashed yet — the + hashers only ever see a chunk once it has been delivered whole — so + there is no checksum state to unwind, and recovering a bad sector costs + one 8 MiB re-read instead of a re-read of everything before it. On a + 79 GB clip that is the difference between seconds and a quarter of an + hour. + """ + reader = None + offset = 0 try: - with longpath.open_binary(source, "rb") as reader: + reader = longpath.open_binary(source, "rb") + + def read_one() -> bytes: + return reader.read(CHUNK_SIZE) + + def recover() -> None: + # Reopen rather than seek alone: a reader that dropped off the + # bus needs its handle re-established, which restarting the + # whole file used to get for free. + nonlocal reader + _close_quietly(reader) + reader = longpath.open_binary(source, "rb") + reader.seek(offset) + + while not stop.is_set(): + if control is not None: + control.checkpoint() + try: + chunk, attempts = retry_mod.call(read_one, retry, + before_retry=recover) + except OSError as exc: + if retry.enabled and retry_mod.is_transient(exc): + raise retry_mod.Exhausted( + f"read failed at offset {offset} after " + f"{retry.attempts} attempts: {exc}") from exc + raise + if attempts > 1: + recovered.append((offset, attempts)) + if not chunk: + break + offset += len(chunk) + # Time-boxed so a consumer that died still lets us exit. while not stop.is_set(): - if control is not None: - control.checkpoint() - chunk = reader.read(CHUNK_SIZE) - if not chunk: + try: + chunks.put(chunk, timeout=0.2) break - # Time-boxed so a consumer that died still lets us exit. - while not stop.is_set(): - try: - chunks.put(chunk, timeout=0.2) - break - except queue.Full: - continue + except queue.Full: + continue except BaseException as exc: # re-raised on the calling thread failure.append(exc) finally: + _close_quietly(reader) # The sentinel must be delivered, not attempted: if the queue # happens to be full at EOF a dropped sentinel leaves the consumer # blocked on get() forever. Only give up once `stop` is set, which @@ -334,7 +412,82 @@ def read_ahead() -> None: for handle in handles: handle.close() - return src_hasher.hexdigest(), [h.hexdigest() for h in dst_hashers] + return _CopyResult(src_hasher.hexdigest(), + [h.hexdigest() for h in dst_hashers], + recovered) + + +def _confirm_source(source: Path, expected: str, algorithm: str) -> bool: + """Read `source` a second time and insist it hashes the same. + + The gap this closes: a read that returns wrong bytes *without raising*. The + checksum is computed from whatever was read, so a bad read produces a + destination that faithfully matches a corrupted source and verifies clean at + every level — file hashes, directory hashes, the lot. Nothing but reading + twice can see it. + + Raises `UnstableRead` on a disagreement rather than choosing a winner: there + is no basis for deciding which of the two reads was the true one. + + Returns whether the page cache was actually dropped first. A second read + served out of memory compares the first read against itself, so a caller + that cannot evict has to say so rather than claim the guarantee. + """ + evicted = integrity.evict_from_cache(source) + again = hash_file(source, algorithm) + if again != expected: + raise retry_mod.UnstableRead( + f"two reads of {source.name} disagreed ({expected} then {again}) — " + "the source did not return the same bytes twice" + ) + return evicted + + +def _describe_recovery(recovered: list[tuple[int, int]]) -> str: + """What a file's recovered reads amount to, in one line. + + A single bad sector is worth naming exactly; a run of them is worth + bounding, because the useful fact stops being *which* byte and becomes how + much of the file would not read first time. + """ + if len(recovered) == 1: + offset, attempts = recovered[0] + return f"recovered a failed read at byte {offset} on attempt {attempts}" + offsets = [offset for offset, _ in recovered] + worst = max(attempts for _, attempts in recovered) + return (f"recovered {len(recovered)} failed reads between byte " + f"{min(offsets)} and byte {max(offsets)}, the worst on " + f"attempt {worst}") + + +def _invert_companions(belongs_to: dict[Path, Path]) -> dict[Path, list[Path]]: + """clip -> its companions, from companion -> its clip.""" + owns: dict[Path, list[Path]] = {} + for companion, clip in belongs_to.items(): + owns.setdefault(clip, []).append(companion) + for paths in owns.values(): + paths.sort() + return owns + + +def _warn_on_split_companions(job: Job) -> None: + """A clip and the files that belong to it have to share a fate. + + A graded BRAW delivered without its `.sidecar` has lost the grade, and a + per-file table showing one Verified row and one Failed row twenty lines + apart is not how anyone finds that out. + """ + by_source = {entry.source: entry for entry in job.files} + for entry in job.files: + if entry.companion_of is None or entry.status is not FileStatus.FAILED: + continue + clip = by_source.get(entry.companion_of) + if clip is None or clip.status is FileStatus.FAILED: + continue + job.warnings.append( + f"{entry.name} did not copy but {clip.name} did — the clip has " + "been separated from a file that belongs with it" + ) def _discard(targets: Iterable[Path]) -> None: @@ -371,7 +524,10 @@ def run(source_root: Path, options: OffloadOptions, host = sysinfo.collect() job = Job( - name=options.job_name or source_root.name, + # A card offloaded from its root has no folder name; its volume + # label is what the operator calls it. + name=(options.job_name or source_root.name + or volumes.volume_label(source_root) or "Offload"), source_root=source_root, destination_roots=dest_roots, verification=options.verification, @@ -381,17 +537,33 @@ def run(source_root: Path, options: OffloadOptions, os_version=host.os_version, processors=host.processors, system_ram=host.system_ram, + paranoid=options.paranoid, ) thumb_dir = options.thumbnail_dir or (dest_roots[0] / f"{job.name}_Reports" / "thumbs") + # A sidecar belongs to a *clip*. Under the data profile nothing is a clip, + # so stem-matching a dataset would announce that `run_1440.xmp` belongs to + # `run_1440.h5` on no evidence beyond a shared name. + belongs_to = (companions.group(files) if options.profile.probes_media + else {}) + owns = _invert_companions(belongs_to) + #: Whether the "cache could not be evicted" limitation has been reported. + #: Said once per job, not once per clip: where the platform has no eviction + #: call at all (macOS has no posix_fadvise), repeating it per file would + #: bury the warnings that are about actual media. evict_noted = False + reread_noted = False def emit(event: ProgressEvent) -> None: if progress: progress(event) + # One memo per job: the first clip of a suffix this ffmpeg cannot decode + # pays the failed extraction, the remaining clips skip it. + thumb_memo = thumbs.DecoderMemo() + for index, source in enumerate(files): # Between files is the cheapest place to honour a pause or cancel. if control is not None: @@ -408,6 +580,8 @@ def emit(event: ProgressEvent) -> None: size=stat.st_size, created=getattr(stat, "st_birthtime", stat.st_ctime), modified=stat.st_mtime, + companion_of=belongs_to.get(source), + companions=owns.get(source, []), ) targets = [ @@ -470,17 +644,53 @@ def note_retry(attempt: int, exc: BaseException, pause: float, f"{_src.name}: read failed ({exc}); " f"attempt {attempt} of {options.retry.attempts}") - (src_sum, dst_sums), used = retry_mod.call( - lambda _src=source, _partials=partials: _copy_fanout( - _src, _partials, options.algorithm, on_chunk, control), - options.retry, on_retry=note_retry, before_retry=rewind, + def copy_once(_src=source, _partials=partials, _idx=index, + _st=stat) -> _CopyResult: + nonlocal reread_noted + result = _copy_fanout(_src, _partials, options.algorithm, + on_chunk, control, options.retry) + if not options.paranoid: + return result + emit(ProgressEvent(_idx, len(files), _src.name, "reread", + 0, _st.st_size, + counters.job_bytes_done, + counters.job_bytes_total)) + # Raises UnstableRead on a disagreement, which the retry around + # this call treats as transient: the honest response to a source + # that read differently twice is to read it again, not to guess + # which of the two was right. + evicted = _confirm_source(_src, result.source_checksum, + options.algorithm) + if not evicted and not reread_noted: + reread_noted = True + job.warnings.append( + "could not evict files from the page cache on this " + "platform, so the second read may have come from memory " + "rather than the device — --paranoid proved less than " + "it appears to" + ) + return result + + result, used = retry_mod.call( + copy_once, options.retry, + on_retry=note_retry, before_retry=rewind, ) + src_sum, dst_sums = result.source_checksum, result.destination_checksums if used > 1: # Not a failure, but a card that needs retries today is a card # to stop using. job.warnings.append( f"{source.name} copied on attempt {used} of " f"{options.retry.attempts} — the source may be failing") + if result.recovered_reads: + # Recovered without restarting the file, which is why the copy + # succeeded at all — but the sectors that needed it are real. + # Said once per file: a card failing over a contiguous stretch + # produces one of these every 8 MiB, and a warning list that + # long is one nobody reads to the end. + job.warnings.append( + f"{source.name}: {_describe_recovery(result.recovered_reads)}" + " — the source may be failing") entry.checksum = src_sum or None except JobCancelled: _discard(partials) @@ -521,10 +731,6 @@ def note_retry(attempt: int, exc: BaseException, pause: float, # Evict first, or the read-back is served from the page # cache and verifies our own memory against itself. if not integrity.evict_from_cache(partial) and not evict_noted: - # Said once per job, not once per clip: where the - # platform has no eviction call at all (macOS has no - # posix_fadvise), repeating it per file would bury the - # warnings that are about actual media. evict_noted = True job.warnings.append( "could not evict files from the page cache on this " @@ -623,6 +829,7 @@ def note_retry(attempt: int, exc: BaseException, pause: float, entry.thumbnails = thumbs.extract( picture, entry.media, thumb_dir, options.thumbnail_count, + memo=thumb_memo, ) job.files.append(entry) @@ -633,6 +840,7 @@ def note_retry(attempt: int, exc: BaseException, pause: float, if not_attempted > 0: counters.errors.append( f"cancelled — {not_attempted} file(s) not attempted") + _warn_on_split_companions(job) job.notes = "; ".join(counters.errors) return job @@ -651,7 +859,10 @@ def rescan(source_root: Path, destination_roots: Sequence[Path], host = sysinfo.collect() job = Job( - name=options.job_name or source_root.name, + # A card offloaded from its root has no folder name; its volume + # label is what the operator calls it. + name=(options.job_name or source_root.name + or volumes.volume_label(source_root) or "Offload"), source_root=source_root, destination_roots=[Path(d) for d in destination_roots] or [source_root], verification=options.verification, @@ -663,9 +874,17 @@ def rescan(source_root: Path, destination_roots: Sequence[Path], system_ram=host.system_ram, ) thumb_dir = options.thumbnail_dir or (source_root / f"{job.name}_Reports" / "thumbs") + thumb_memo = thumbs.DecoderMemo() total = sum(p.stat().st_size for p in files) done = 0 + # A sidecar belongs to a *clip*. Under the data profile nothing is a clip, + # so stem-matching a dataset would announce that `run_1440.xmp` belongs to + # `run_1440.h5` on no evidence beyond a shared name. + belongs_to = (companions.group(files) if options.profile.probes_media + else {}) + owns = _invert_companions(belongs_to) + for index, source in enumerate(files): stat = source.stat() entry = FileEntry( @@ -674,6 +893,8 @@ def rescan(source_root: Path, destination_roots: Sequence[Path], size=stat.st_size, created=getattr(stat, "st_birthtime", stat.st_ctime), modified=stat.st_mtime, + companion_of=belongs_to.get(source), + companions=owns.get(source, []), ) if progress: progress(ProgressEvent(index, len(files), source.name, "verify", @@ -703,7 +924,8 @@ def rescan(source_root: Path, destination_roots: Sequence[Path], entry.media = probe_mod.probe(source) if options.thumbnail_count > 0 and entry.media.is_video: entry.thumbnails = thumbs.extract( - source, entry.media, thumb_dir, options.thumbnail_count + source, entry.media, thumb_dir, options.thumbnail_count, + memo=thumb_memo, ) job.files.append(entry) diff --git a/src/offloader/gui/drives.py b/src/offloader/gui/drives.py index 42a6a5b..d088bc9 100644 --- a/src/offloader/gui/drives.py +++ b/src/offloader/gui/drives.py @@ -17,15 +17,33 @@ ) from ..util import format_size -from ..volumes import Volume, list_volumes +from ..volumes import Volume, list_roots, order_volumes, probe_many from . import theme from .widgets import CapacityBar, button, label, row REFRESH_MS = 4000 +def scan_batches(): + """Yield (volumes, final) — local drives first, network shares after. + + A network probe is a synchronous SMB round-trip that can take seconds; the + fixed and removable drives — the ones an offload actually uses — must not + wait behind it. Each phase probes its roots concurrently, so the wait per + batch is the slowest probe, not the sum. + """ + roots = list_roots() + local = [(root, kind) for root, kind in roots if kind != "network"] + remote = [(root, kind) for root, kind in roots if kind == "network"] + found = probe_many(local) + if remote: + yield order_volumes(found), False + found = found + probe_many(remote) + yield order_volumes(found), True + + class _ScanSignals(QObject): - done = Signal(list) + batch = Signal(list, bool) # volumes, final class _ScanTask(QRunnable): @@ -42,11 +60,14 @@ def __init__(self, signals: _ScanSignals) -> None: def run(self) -> None: # noqa: D102 - QRunnable entry point try: - volumes = list_volumes() + for volumes, final in scan_batches(): + self._emit(volumes, final) except Exception: - volumes = [] + self._emit([], True) + + def _emit(self, volumes: list, final: bool) -> None: try: - self._signals.done.emit(volumes) + self._signals.batch.emit(volumes, final) except RuntimeError: # The window closed while this scan was in flight; nothing to tell. pass @@ -56,6 +77,7 @@ class VolumeWatcher(QObject): """Polls for mounted volumes and reports changes.""" volumesChanged = Signal(list) + scanningChanged = Signal(bool) def __init__(self, parent: QObject | None = None) -> None: super().__init__(parent) @@ -64,7 +86,7 @@ def __init__(self, parent: QObject | None = None) -> None: self._stopped = False # Parented, so its lifetime is the watcher's rather than a task's. self._signals = _ScanSignals(self) - self._signals.done.connect(self._on_scanned) + self._signals.batch.connect(self._on_batch) self._timer = QTimer(self) self._timer.setInterval(REFRESH_MS) self._timer.timeout.connect(self.refresh) @@ -86,14 +108,30 @@ def refresh(self) -> None: if self._busy or self._stopped: return self._busy = True + self.scanningChanged.emit(True) QThreadPool.globalInstance().start(_ScanTask(self._signals)) - def _on_scanned(self, volumes: list) -> None: - self._busy = False + def _on_batch(self, volumes: list, final: bool) -> None: + if final: + self._busy = False if self._stopped: return + if not final: + # The local half of a scan whose network shares are still being + # probed. Keep the shares from the previous scan rather than + # tearing their rows down for a few seconds every poll. Dedup by + # root, not resolved root — resolving a network path is itself a + # round-trip, and this runs on the UI thread. + known = {v.root: v for v in self._volumes + if v.drive_type == "network"} + for volume in volumes: + known[volume.root] = volume + volumes = sorted(known.values(), + key=lambda v: (not v.is_camera_card, str(v.root))) self._volumes = volumes self.volumesChanged.emit(volumes) + if final: + self.scanningChanged.emit(False) class VolumeRow(QFrame): @@ -164,10 +202,11 @@ def __init__(self, parent: QWidget | None = None) -> None: self.watcher = VolumeWatcher(self) self.watcher.volumesChanged.connect(self._rebuild) + self.watcher.scanningChanged.connect(self._on_scanning) - refresh = button("Refresh", flat=True) - refresh.clicked.connect(self.watcher.refresh) - header = row(label("Drives", "heading"), None, refresh) + self._refresh = button("Refresh", flat=True) + self._refresh.clicked.connect(self.watcher.refresh) + header = row(label("Drives", "heading"), None, self._refresh) self._container = QWidget() self._container_layout = QVBoxLayout(self._container) @@ -193,6 +232,12 @@ def start(self) -> None: def stop(self) -> None: self.watcher.stop() + def _on_scanning(self, scanning: bool) -> None: + # The busy state the panel was missing: without it a slow network + # share made "Refresh" look like a button that does nothing. + self._refresh.setEnabled(not scanning) + self._refresh.setText("Scanning…" if scanning else "Refresh") + def _rebuild(self, volumes: list) -> None: same_set = ( len(volumes) == len(self._rows) diff --git a/src/offloader/gui/main_window.py b/src/offloader/gui/main_window.py index de8ad90..8eab0ff 100644 --- a/src/offloader/gui/main_window.py +++ b/src/offloader/gui/main_window.py @@ -52,6 +52,8 @@ def __init__(self) -> None: self.controller.jobStarted.connect(lambda _: self._update_status()) self.controller.itemsChanged.connect(self._update_status) self.controller.itemChanged.connect(lambda _: self._update_status()) + self.controller.itemsChanged.connect(self._sync_queue_busy) + self.controller.itemChanged.connect(lambda _: self._sync_queue_busy()) # ----------------------------------------------------------- panels self.simple = SimpleModePanel() @@ -295,6 +297,10 @@ def _check_duplicate(self, source: Path) -> bool: return answer == QMessageBox.Yes # ---------------------------------------------------------------- events + def _sync_queue_busy(self) -> None: + self.simple.set_queue_busy( + any(not i.state.is_terminal for i in self.controller.items)) + def _update_status(self) -> None: running = [i for i in self.controller.items if i.state is JobState.RUNNING] queued = [i for i in self.controller.items if i.state is JobState.QUEUED] diff --git a/src/offloader/gui/preset_editor.py b/src/offloader/gui/preset_editor.py index 68499fa..d161b45 100644 --- a/src/offloader/gui/preset_editor.py +++ b/src/offloader/gui/preset_editor.py @@ -26,7 +26,7 @@ from ..presets import PRESET_COLORS, Preset from ..reports import WRITERS from . import theme -from .widgets import DestinationList, button, label, row +from .widgets import DestinationList, button, column, label, row VERIFICATION_LABELS = { VerificationMode.NONE: "None — copy only", @@ -34,6 +34,37 @@ VerificationMode.FULL: "Full — re-read each destination from disk", } +#: Said the same way in the preset editor and in Simple mode, because it is the +#: one option here whose cost is not obvious from its name. +PARANOID_LABEL = "Read each source file twice and compare" +PARANOID_TOOLTIP = ( + "Catches a read that returned the wrong bytes without reporting an error — " + "the one fault no checksum can see, because the checksum is taken from what " + "the read returned.\n\nCosts a second full pass over the card." +) + + +def _form() -> QFormLayout: + form = QFormLayout() + form.setSpacing(10) + form.setLabelAlignment(Qt.AlignRight | Qt.AlignVCenter) + return form + + +def _section(title: str, form: QFormLayout) -> QWidget: + """A titled block of rows. + + There are sixteen fields here. In one flat list they read as a wall, and the + two or three that bear on any given change are never next to each other. + """ + box = QWidget() + layout = QVBoxLayout(box) + layout.setContentsMargins(0, 0, 0, 0) + layout.setSpacing(8) + layout.addWidget(label(title, "heading")) + layout.addLayout(form) + return box + def _color_icon(color: str, size: int = 14) -> QIcon: pixmap = QPixmap(size, size) @@ -75,7 +106,7 @@ def __init__(self, preset: Preset | None = None, parent: QWidget | None = None) self._algorithm = QComboBox() for key, algorithm in ALGORITHMS.items(): - self._algorithm.addItem(algorithm.label, key) + self._algorithm.addItem(algorithm.picker_label, key) self._algorithm.setCurrentIndex( max(0, self._algorithm.findData(self._source.algorithm))) @@ -121,6 +152,12 @@ def __init__(self, preset: Preset | None = None, parent: QWidget | None = None) self._preserve.setChecked(self._source.preserve_structure) self._skip = QCheckBox("Skip files already present at matching size") self._skip.setChecked(self._source.skip_existing) + self._skip.setToolTip( + "Compares size only, never contents. A speed option, not a safety " + "one — do not use it on a tree whose integrity is in question.") + self._paranoid = QCheckBox(PARANOID_LABEL) + self._paranoid.setChecked(self._source.paranoid) + self._paranoid.setToolTip(PARANOID_TOOLTIP) self._excludes = QLineEdit(", ".join(self._source.excludes)) self._excludes.setPlaceholderText("*.tmp, *.thm") @@ -133,25 +170,29 @@ def __init__(self, preset: Preset | None = None, parent: QWidget | None = None) self._footer = QLineEdit(self._source.footer or "") self._footer.setPlaceholderText("Offloader Version 0.1.0") - form = QFormLayout() - form.setSpacing(10) - form.setLabelAlignment(Qt.AlignRight | Qt.AlignVCenter) - form.addRow("Name", self._name) - form.addRow("Colour", self._color) - form.addRow("Destinations", self._destinations) - form.addRow("", row(add, remove, None)) - form.addRow("Profile", self._profile) - form.addRow("Checksum", self._algorithm) - form.addRow("Verification", self._verification) - form.addRow("Thumbnails", self._thumbnails) - form.addRow("Reports", row(*report_row)) - form.addRow("Job name", self._naming) - form.addRow("", tokens) - form.addRow("Exclude", self._excludes) - form.addRow("PDF logo", row(self._logo, browse_logo)) - form.addRow("PDF footer", self._footer) - form.addRow("", self._preserve) - form.addRow("", self._skip) + identity = _form() + identity.addRow("Name", self._name) + identity.addRow("Colour", self._color) + identity.addRow("Destinations", self._destinations) + identity.addRow("", row(add, remove, None)) + + # Profile leads: it decides whether there is any media work to do at + # all, which is what every control under it is then qualified by. + copying = _form() + copying.addRow("Profile", self._profile) + copying.addRow("Checksum", self._algorithm) + copying.addRow("Verification", self._verification) + copying.addRow("Exclude", self._excludes) + copying.addRow("Options", + column(self._preserve, self._skip, self._paranoid)) + + paperwork = _form() + paperwork.addRow("Reports", row(*report_row)) + paperwork.addRow("Thumbnails", self._thumbnails) + paperwork.addRow("Job name template", self._naming) + paperwork.addRow("", tokens) + paperwork.addRow("PDF logo", row(self._logo, browse_logo)) + paperwork.addRow("PDF footer", self._footer) buttons = QDialogButtonBox(QDialogButtonBox.Save | QDialogButtonBox.Cancel) buttons.accepted.connect(self._accept) @@ -159,8 +200,10 @@ def __init__(self, preset: Preset | None = None, parent: QWidget | None = None) layout = QVBoxLayout(self) layout.setContentsMargins(18, 18, 18, 18) - layout.setSpacing(14) - layout.addLayout(form) + layout.setSpacing(18) + layout.addWidget(_section("Preset", identity)) + layout.addWidget(_section("Copying", copying)) + layout.addWidget(_section("Reports", paperwork)) layout.addWidget(buttons) def _on_profile_changed(self) -> None: @@ -193,6 +236,7 @@ def _accept(self) -> None: naming_template=self._naming.text().strip() or "{card}", preserve_structure=self._preserve.isChecked(), skip_existing=self._skip.isChecked(), + paranoid=self._paranoid.isChecked(), excludes=excludes, logo=Path(logo_text) if logo_text else None, footer=footer_text or None, diff --git a/src/offloader/gui/queue_view.py b/src/offloader/gui/queue_view.py index 4e59983..0c9abd9 100644 --- a/src/offloader/gui/queue_view.py +++ b/src/offloader/gui/queue_view.py @@ -6,11 +6,12 @@ import sys from pathlib import Path -from PySide6.QtCore import QAbstractTableModel, QModelIndex, Qt +from PySide6.QtCore import QAbstractTableModel, QModelIndex, Qt, QTimer from PySide6.QtGui import QColor, QPainter from PySide6.QtWidgets import ( QAbstractItemView, QHeaderView, + QStyle, QStyledItemDelegate, QTableView, QVBoxLayout, @@ -25,6 +26,15 @@ COLUMNS = ("Job", "Source", "Preset", "Status", "Progress", "Throughput") COL_STATUS = 3 COL_PROGRESS = 4 +COL_THROUGHPUT = 5 + +#: What each engine stage is doing, in the operator's words. +STAGE_VERBS = { + "copy": "Copying", + "verify": "Verifying", + "probe": "Reading metadata", + "thumbs": "Extracting thumbnails", +} def _throughput(item: QueueItem) -> str: @@ -45,6 +55,19 @@ def _throughput(item: QueueItem) -> str: return "" +def _active_summary(item: QueueItem) -> str: + """The running job in one line: what, on which file, how fast.""" + if item.state is JobState.PAUSED: + return f"Paused — {item.name} at {item.fraction * 100:.0f}%" + verb = STAGE_VERBS.get(item.stage, item.stage.capitalize() or "Running") + text = f"{verb} {item.current_file}" if item.current_file else verb + text += f" — {item.fraction * 100:.0f}%" + rate = _throughput(item) + if rate: + text += f" · {rate}" + return text + + class QueueModel(QAbstractTableModel): def __init__(self, controller: QueueController, parent=None) -> None: super().__init__(parent) @@ -117,6 +140,15 @@ def _refresh_one(self, identifier: int) -> None: self.dataChanged.emit(self.index(index, 0), self.index(index, len(COLUMNS) - 1)) + def refresh_throughput(self) -> None: + """Repaint the rate column without a progress event. During a stall no + events arrive, which is exactly when the displayed rate must be seen + to fall rather than freeze at its last healthy value.""" + rows = len(self.controller.items) + if rows: + self.dataChanged.emit(self.index(0, COL_THROUGHPUT), + self.index(rows - 1, COL_THROUGHPUT)) + def item_at(self, index: QModelIndex) -> QueueItem | None: if not index.isValid(): return None @@ -124,7 +156,16 @@ def item_at(self, index: QModelIndex) -> QueueItem | None: class ProgressDelegate(QStyledItemDelegate): - """Draws the progress column as a bar rather than a number.""" + """Draws the progress column as a bar with its percentage beside it. + + The colours are chosen against both grounds the cell can have: on the + normal row the old accent-on-near-black bar read fine, but the running row + is auto-selected, and an accent bar on the accent selection colour was + invisible — the operator's own job was the one row without a readable bar. + """ + + TEXT_WIDTH = 40 + BAR_HEIGHT = 10 def paint(self, painter: QPainter, option, index) -> None: fraction = index.data(Qt.UserRole) @@ -132,22 +173,34 @@ def paint(self, painter: QPainter, option, index) -> None: super().paint(painter, option, index) return + selected = bool(option.state & QStyle.State_Selected) + painter.save() + if selected: + painter.fillRect(option.rect, option.palette.highlight()) + rect = option.rect.adjusted(6, 0, -6, 0) - height = 8 - bar = rect.adjusted(0, (rect.height() - height) // 2, 0, - -(rect.height() - height) // 2) + bar = rect.adjusted(0, (rect.height() - self.BAR_HEIGHT) // 2, + -self.TEXT_WIDTH, + -(rect.height() - self.BAR_HEIGHT) // 2) - painter.save() painter.setRenderHint(QPainter.Antialiasing) painter.setPen(Qt.NoPen) - painter.setBrush(QColor(theme.BG)) + painter.setBrush(QColor(0, 0, 0, 90) if selected + else QColor(theme.PROGRESS_TRACK)) painter.drawRoundedRect(bar, 4, 4) - width = int(bar.width() * max(0.0, min(1.0, float(fraction)))) + clamped = max(0.0, min(1.0, float(fraction))) + width = int(bar.width() * clamped) if width > 0: filled = bar.adjusted(0, 0, width - bar.width(), 0) - painter.setBrush(QColor(theme.ACCENT)) + painter.setBrush(QColor("#ffffff") if selected + else QColor(theme.ACCENT)) painter.drawRoundedRect(filled, 4, 4) + + text_rect = rect.adjusted(rect.width() - self.TEXT_WIDTH + 6, 0, 0, 0) + painter.setPen(QColor("#ffffff") if selected else QColor(theme.FG)) + painter.drawText(text_rect, Qt.AlignRight | Qt.AlignVCenter, + f"{clamped * 100:.0f}%") painter.restore() @@ -190,8 +243,12 @@ def __init__(self, controller: QueueController, parent=None) -> None: header.setSectionResizeMode(2, QHeaderView.ResizeToContents) header.setSectionResizeMode(3, QHeaderView.ResizeToContents) header.setSectionResizeMode(4, QHeaderView.Fixed) - header.setSectionResizeMode(5, QHeaderView.ResizeToContents) - header.resizeSection(4, 150) + header.resizeSection(4, 170) + # Fixed, not ResizeToContents: the rate string changes width on every + # update, and letting it drive the layout shoved the column — the one + # being read — around as values grew and shrank. + header.setSectionResizeMode(COL_THROUGHPUT, QHeaderView.Fixed) + header.resizeSection(COL_THROUGHPUT, 190) self._pause = button("Pause", flat=True) self._cancel = button("Cancel", flat=True) @@ -212,10 +269,24 @@ def __init__(self, controller: QueueController, parent=None) -> None: self._empty = label("Nothing queued. Drop a card on a preset to start.", "muted") + # The running job promoted to where the eye already is: the queue rows + # are 30 px tall at the bottom of the window, and a job in flight + # looked almost identical to an idle queue. + self._active = label("", "heading") + self._active.setStyleSheet(f"color: {theme.ACCENT};") + self._active.setVisible(False) + + # Repaints the decaying rate during stalls, when no progress events + # arrive to do it. Runs only while a job is active. + self._ticker = QTimer(self) + self._ticker.setInterval(1000) + self._ticker.timeout.connect(self._tick) + layout = QVBoxLayout(self) layout.setContentsMargins(0, 0, 0, 0) layout.setSpacing(8) layout.addWidget(row(label("Queue", "heading"), None, self._clear)) + layout.addWidget(self._active) layout.addWidget(self._empty) layout.addWidget(self.table, 1) layout.addWidget(row(self._pause, self._cancel, 12, self._up, self._down, @@ -227,6 +298,25 @@ def __init__(self, controller: QueueController, parent=None) -> None: controller.jobStarted.connect(self._select_job) self._sync_buttons() + def _active_item(self) -> QueueItem | None: + return next((i for i in self.controller.items + if i.state in (JobState.RUNNING, JobState.PAUSED)), None) + + def _update_active(self) -> None: + item = self._active_item() + if item is None: + self._active.setVisible(False) + self._ticker.stop() + return + self._active.setText(_active_summary(item)) + self._active.setVisible(True) + if not self._ticker.isActive(): + self._ticker.start() + + def _tick(self) -> None: + self._update_active() + self.model.refresh_throughput() + def _select_job(self, identifier: int) -> None: """Follow the running job, so the transport controls act on it without the operator having to click the row first.""" @@ -249,6 +339,7 @@ def _sync_buttons(self) -> None: has_rows = bool(self.controller.items) self._empty.setVisible(not has_rows) self.table.setVisible(has_rows) + self._update_active() item = self._selected() running = item is not None and item.state is JobState.RUNNING diff --git a/src/offloader/gui/simple_mode.py b/src/offloader/gui/simple_mode.py index 9253dc8..ca9f227 100644 --- a/src/offloader/gui/simple_mode.py +++ b/src/offloader/gui/simple_mode.py @@ -23,8 +23,9 @@ from ..models import Profile, VerificationMode from ..presets import Preset from ..reports import WRITERS -from .preset_editor import VERIFICATION_LABELS -from .widgets import DestinationList, SourceDropZone, button, label, row +from ..volumes import volume_label +from .preset_editor import PARANOID_LABEL, PARANOID_TOOLTIP, VERIFICATION_LABELS +from .widgets import DestinationList, SourceDropZone, button, column, label, row class SimpleModePanel(QWidget): @@ -34,6 +35,7 @@ class SimpleModePanel(QWidget): def __init__(self, parent: QWidget | None = None) -> None: super().__init__(parent) + self._queue_busy = False self.drop_zone = SourceDropZone() self.drop_zone.pathChosen.connect(self._on_source_chosen) @@ -51,14 +53,14 @@ def __init__(self, parent: QWidget | None = None) -> None: self._algorithm = QComboBox() for key, algorithm in ALGORITHMS.items(): - self._algorithm.addItem(algorithm.label, key) + self._algorithm.addItem(algorithm.picker_label, key) self._algorithm.setCurrentIndex(max(0, self._algorithm.findData("xxh3-64"))) self._verification = QComboBox() for mode, text in VERIFICATION_LABELS.items(): self._verification.addItem(text, mode.value) self._verification.setCurrentIndex( - max(0, self._verification.findData(VerificationMode.SOURCE_ONLY.value))) + max(0, self._verification.findData(VerificationMode.FULL.value))) self._profile = QComboBox() self._profile.addItem("Media — camera card", Profile.MEDIA.value) @@ -84,6 +86,8 @@ def __init__(self, parent: QWidget | None = None) -> None: self._preserve = QCheckBox("Recreate the source folder structure") self._preserve.setChecked(True) + self._paranoid = QCheckBox(PARANOID_LABEL) + self._paranoid.setToolTip(PARANOID_TOOLTIP) form = QFormLayout() form.setSpacing(10) @@ -94,7 +98,7 @@ def __init__(self, parent: QWidget | None = None) -> None: form.addRow("Verification", self._verification) form.addRow("Thumbnails", self._thumbnails) form.addRow("Reports", row(*report_row)) - form.addRow("", self._preserve) + form.addRow("Options", column(self._preserve, self._paranoid)) self._start = button("Start offload", accent=True) self._start.clicked.connect(self._start_clicked) @@ -123,7 +127,25 @@ def add_destination(self, path: Path) -> None: def _on_source_chosen(self, path: Path) -> None: if not self._name.text().strip(): - self._name.setPlaceholderText(Path(path).name or "Offload") + # Preview what the job will actually be called: folder name, or + # for a card offloaded from its root, the volume label ("A003"). + self._name.setPlaceholderText(self._default_name(path)) + self._sync() + + @staticmethod + def _default_name(source: Path) -> str: + source = Path(source) + return source.name or volume_label(source) or "Offload" + + def set_queue_busy(self, busy: bool) -> None: + """Tell the panel whether the queue is already working. Jobs run one + at a time, so while one is running the button cannot start anything — + it enqueues. It should say so, rather than promise an immediate + offload it cannot deliver.""" + if busy == self._queue_busy: + return + self._queue_busy = busy + self._start.setText("Add to queue" if busy else "Start offload") self._sync() def _sync(self) -> None: @@ -142,7 +164,10 @@ def _sync(self) -> None: self._hint.setText("A destination sits inside the source — pick another.") else: copies = f"{len(destinations)} cop{'ies' if len(destinations) > 1 else 'y'}" - self._hint.setText(f"Ready: {source} → {copies}") + ready = f"Ready: {source} → {copies}" + if self._queue_busy: + ready += " — runs after the current job" + self._hint.setText(ready) @staticmethod def _overlaps(source: Path, destination: Path) -> bool: @@ -171,11 +196,12 @@ def build_preset(self) -> Preset: thumbnail_count=self._thumbnails.value(), reports=[key for key, box in self._reports.items() if box.isChecked()], preserve_structure=self._preserve.isChecked(), + paranoid=self._paranoid.isChecked(), ) def _start_clicked(self) -> None: source = self.drop_zone.path if source is None: return - name = self._name.text().strip() or (Path(source).name or "Offload") + name = self._name.text().strip() or self._default_name(source) self.runRequested.emit(source, self.build_preset(), name) diff --git a/src/offloader/gui/theme.py b/src/offloader/gui/theme.py index a45dcbb..9072bd2 100644 --- a/src/offloader/gui/theme.py +++ b/src/offloader/gui/theme.py @@ -17,6 +17,9 @@ FG_MUTED = "#9aa0a6" ACCENT = "#5577b0" ACCENT_HOVER = "#6688c4" +#: Unfilled part of a progress bar. Distinct from BG, which vanished against +#: the table's own background. +PROGRESS_TRACK = "#31363f" OK = "#4caf7d" WARN = "#d8a13c" BAD = "#e0645c" @@ -131,6 +134,9 @@ def apply(app) -> None: border-radius: 6px; padding: 5px 8px; selection-background-color: {ACCENT}; + /* Without this the size hint under-reports the styled height and text + clips at its bottom edge at fractional DPI scales (125%). */ + min-height: 18px; }} QLineEdit:focus, QComboBox:focus, QSpinBox:focus, QPlainTextEdit:focus {{ border-color: {ACCENT}; @@ -179,6 +185,7 @@ def apply(app) -> None: QTabBar::tab:selected {{ color: {FG}; border-bottom-color: {ACCENT}; }} QTabWidget::pane {{ border: none; }} +QCheckBox, QRadioButton {{ min-height: 20px; }} QCheckBox::indicator, QRadioButton::indicator {{ width: 15px; height: 15px; border: 1px solid {BORDER}; diff --git a/src/offloader/gui/widgets.py b/src/offloader/gui/widgets.py index 97a9873..1f8a3bb 100644 --- a/src/offloader/gui/widgets.py +++ b/src/offloader/gui/widgets.py @@ -56,6 +56,18 @@ def row(*children, spacing: int = 8) -> QWidget: return widget +def column(*children: QWidget, spacing: int = 6) -> QWidget: + """A vertical stack, for grouping several checkboxes under one form label + rather than giving each its own blank one.""" + widget = QWidget() + layout = QVBoxLayout(widget) + layout.setContentsMargins(0, 0, 0, 0) + layout.setSpacing(spacing) + for child in children: + layout.addWidget(child) + return widget + + def _directories_from(event) -> list[Path]: """Directories in a drag payload. Files are mapped to their parent, so dropping a clip on a destination means "put it in that folder".""" diff --git a/src/offloader/gui/worker.py b/src/offloader/gui/worker.py index 8786498..770b3e4 100644 --- a/src/offloader/gui/worker.py +++ b/src/offloader/gui/worker.py @@ -8,6 +8,7 @@ from __future__ import annotations import time +from collections import deque from dataclasses import dataclass, field from enum import Enum from pathlib import Path @@ -18,6 +19,11 @@ from ..models import Job from ..presets import Preset from ..reports import WRITERS +from ..volumes import volume_label + +#: Trailing window over which throughput is measured. Long enough to smooth +#: per-chunk jitter, short enough that a stall shows up within seconds. +RATE_WINDOW_SEC = 5.0 REPORT_FILENAMES = { "pdf": "JobReport.pdf", @@ -62,6 +68,8 @@ class QueueItem: reports: list[Path] = field(default_factory=list) error: str | None = None control: engine.JobControl = field(default_factory=engine.JobControl) + #: (monotonic time, job bytes done) samples inside the trailing window. + _samples: deque = field(default_factory=deque, repr=False) @property def elapsed(self) -> float: @@ -69,10 +77,36 @@ def elapsed(self) -> float: return 0.0 return (self.finished_at or time.monotonic()) - self.started_at + def record_progress(self, bytes_done: int) -> None: + """Feed the throughput window. Called on every progress event.""" + now = time.monotonic() + if self._samples and bytes_done < self._samples[-1][1]: + # The counter went backwards — a new stage started counting from + # zero. A delta across that boundary would be negative garbage. + self._samples.clear() + self._samples.append((now, bytes_done)) + while self._samples and now - self._samples[0][0] > RATE_WINDOW_SEC: + self._samples.popleft() + @property def rate_bytes_per_sec(self) -> float: - elapsed = self.elapsed - return self.bytes_done / elapsed if elapsed > 0.5 else 0.0 + """Throughput over the trailing window, not the life of the job. + + A lifetime average (`bytes_done / elapsed`) folds the pre-copy scan and + every between-file probe stall into the number forever: a slow first + minute reads as a slow job for the rest of the offload, and the ETA + derived from it is wrong in the same direction. The window forgets. + Measured against `now` rather than the newest sample, so a stall shows + as a rate falling toward zero instead of freezing at its last value. + """ + if not self._samples: + return 0.0 + now = time.monotonic() + oldest_time, oldest_bytes = self._samples[0] + span = now - oldest_time + if span < 0.5 or now - self._samples[-1][0] > RATE_WINDOW_SEC: + return 0.0 + return max(0, self.bytes_done - oldest_bytes) / span @property def eta_seconds(self) -> float | None: @@ -190,7 +224,8 @@ def __init__(self, parent: QObject | None = None) -> None: def enqueue(self, source: Path, preset: Preset, name: str | None = None) -> QueueItem: source = Path(source) resolved = name or naming.build( - preset.naming_template, source, taken=self._taken_names() + preset.naming_template, source, + volume_label=volume_label(source), taken=self._taken_names() ) item = QueueItem( identifier=self._next_id, @@ -312,6 +347,7 @@ def _on_progress(self, identifier: int, fraction: float, stage: str, item.current_file = filename item.bytes_done = done item.bytes_total = total + item.record_progress(done) self.itemChanged.emit(identifier) def _on_completed(self, identifier: int, job: Job | None, diff --git a/src/offloader/hashers.py b/src/offloader/hashers.py index c9a5290..7ff001d 100644 --- a/src/offloader/hashers.py +++ b/src/offloader/hashers.py @@ -28,10 +28,22 @@ class Algorithm: factory: Callable[[], Hasher] | None #: MHL/ASC-MHL element name, or None if the format has no slot for it. mhl_tag: str | None = None + #: What choosing this costs, shown wherever the algorithm is picked. The + #: ratios are single-thread throughput against XXHash3-64, measured with + #: 8 MiB blocks; exact numbers vary by CPU, the ordering does not. They + #: matter because the engine hashes every byte once per stream — source + #: plus each destination — on the copy path, so a slow hash is a ceiling + #: on copy speed, not an afterthought. + speed: str = "" def new(self) -> Hasher | None: return self.factory() if self.factory else None + @property + def picker_label(self) -> str: + """Label with the cost attached, e.g. "MD5 — ~40x slower".""" + return f"{self.label} — {self.speed}" if self.speed else self.label + #: Base58 alphabet used by C4 (SMPTE ST 2114) — no 0, O, I or l. _C4_ALPHABET = "123456789ABCDEFGHJKLMNPQRSTUVWXYZabcdefghijkmnopqrstuvwxyz" @@ -74,15 +86,24 @@ def hexdigest(self) -> str: # noqa: D102 ALGORITHMS: dict[str, Algorithm] = { - "xxh3-64": Algorithm("xxh3-64", "XXHash3-64", xxhash.xxh3_64, "xxh3"), - "xxh3-128": Algorithm("xxh3-128", "XXHash3-128", xxhash.xxh3_128, "xxh3-128"), - "xxh64": Algorithm("xxh64", "XXHash-64", xxhash.xxh64, "xxh64"), - "xxh64be": Algorithm("xxh64be", "XXHash-64BE", xxhash.xxh64, "xxh64be"), - "md5": Algorithm("md5", "MD5", hashlib.md5, "md5"), - "sha1": Algorithm("sha1", "SHA-1", hashlib.sha1, "sha1"), - "sha256": Algorithm("sha256", "SHA-256", hashlib.sha256, "sha256"), - "c4": Algorithm("c4", "C4", C4Hasher, "c4"), - "none": Algorithm("none", "None", None, None), + "xxh3-64": Algorithm("xxh3-64", "XXHash3-64", xxhash.xxh3_64, "xxh3", + speed="fastest"), + "xxh3-128": Algorithm("xxh3-128", "XXHash3-128", xxhash.xxh3_128, "xxh3-128", + speed="fastest"), + "xxh64": Algorithm("xxh64", "XXHash-64", xxhash.xxh64, "xxh64", + speed="fast"), + "xxh64be": Algorithm("xxh64be", "XXHash-64BE", xxhash.xxh64, "xxh64be", + speed="fast"), + "md5": Algorithm("md5", "MD5", hashlib.md5, "md5", + speed="~40x slower, legacy compatibility only"), + "sha1": Algorithm("sha1", "SHA-1", hashlib.sha1, "sha1", + speed="~14x slower"), + "sha256": Algorithm("sha256", "SHA-256", hashlib.sha256, "sha256", + speed="~15x slower, tamper-evident"), + "c4": Algorithm("c4", "C4", C4Hasher, "c4", + speed="~40x slower, tamper-evident"), + "none": Algorithm("none", "None", None, None, + speed="no checksum, nothing verified"), } DEFAULT_ALGORITHM = "xxh3-64" diff --git a/src/offloader/models.py b/src/offloader/models.py index e07f3c4..0c9da07 100644 --- a/src/offloader/models.py +++ b/src/offloader/models.py @@ -176,6 +176,11 @@ class FileEntry: #: original could not be decoded. thumbnail_source: Path | None = None destinations: list[Destination] = field(default_factory=list) + #: The clip this file belongs to, when it is a sidecar or a proxy rather + #: than a take in its own right. + companion_of: Path | None = None + #: The sidecars and proxies that belong to this clip. + companions: list[Path] = field(default_factory=list) @property def name(self) -> str: @@ -221,6 +226,10 @@ class Job: system_ram: str = "" notes: str = "" cancelled: bool = False + #: Every source file was read a second time and the two reads compared. + #: Recorded because it changes what "Verified" is worth, and a delivery + #: should be able to say which one it got. + paranoid: bool = False #: Things that did not fail the job but that a human should see before #: erasing a card — empty files, verifications that may have been served #: from cache. diff --git a/src/offloader/naming.py b/src/offloader/naming.py index 0d670c9..208d475 100644 --- a/src/offloader/naming.py +++ b/src/offloader/naming.py @@ -45,7 +45,11 @@ def context(source: Path, volume_label: str | None = None, except Exception: # pragma: no cover - no login name in some containers user = "unknown" return { - "card": source.name or source.anchor.strip("\\/:") or "Offload", + # A source with no folder name is a card offloaded from its root, and + # what the operator calls that card is its volume label — "A003", not + # "E". The drive letter stays as the last resort. + "card": source.name or volume_label or source.anchor.strip("\\/:") + or "Offload", "volume": volume_label or source.name or "", "date": f"{moment:%Y%m%d}", "time": f"{moment:%H%M%S}", diff --git a/src/offloader/presets.py b/src/offloader/presets.py index edaaf70..698840a 100644 --- a/src/offloader/presets.py +++ b/src/offloader/presets.py @@ -39,12 +39,14 @@ class Preset: name: str destinations: list[Path] = field(default_factory=list) algorithm: str = "xxh3-64" - verification: VerificationMode = VerificationMode.SOURCE_ONLY + verification: VerificationMode = VerificationMode.FULL profile: Profile = Profile.MEDIA thumbnail_count: int = 4 reports: list[str] = field(default_factory=lambda: ["pdf"]) preserve_structure: bool = True skip_existing: bool = False + #: Read every source file twice and compare. For irreplaceable material. + paranoid: bool = False excludes: list[str] = field(default_factory=list) naming_template: str = DEFAULT_TEMPLATE retry_attempts: int = 3 @@ -110,6 +112,7 @@ def to_options(self, job_name: str | None = None) -> OffloadOptions: profile=self.profile, retry=RetryPolicy(attempts=max(1, self.retry_attempts), delay=max(0.0, self.retry_wait)), + paranoid=self.paranoid, ) # ---------------------------------------------------------------- codec @@ -124,6 +127,7 @@ def to_dict(self) -> dict: "reports": list(self.reports), "preserve_structure": self.preserve_structure, "skip_existing": self.skip_existing, + "paranoid": self.paranoid, "excludes": list(self.excludes), "naming_template": self.naming_template, "retry_attempts": self.retry_attempts, @@ -165,9 +169,9 @@ def as_list(key): return list(got) if isinstance(got, (list, tuple)) else [] try: - verification = VerificationMode(value("verification", "source-only")) + verification = VerificationMode(value("verification", "full")) except (ValueError, TypeError): - verification = VerificationMode.SOURCE_ONLY + verification = VerificationMode.FULL try: profile = Profile(value("profile", "media")) @@ -192,6 +196,7 @@ def as_list(key): if data.get("reports") is not None else ["pdf"]), preserve_structure=bool(value("preserve_structure", True)), skip_existing=bool(value("skip_existing", False)), + paranoid=bool(value("paranoid", False)), excludes=[e for e in as_list("excludes") if isinstance(e, str)], naming_template=str(value("naming_template", DEFAULT_TEMPLATE)), retry_attempts=as_int("retry_attempts", 3), diff --git a/src/offloader/reports/csv_report.py b/src/offloader/reports/csv_report.py index 3ebb994..466eb80 100644 --- a/src/offloader/reports/csv_report.py +++ b/src/offloader/reports/csv_report.py @@ -37,6 +37,9 @@ "Good Take", "Colour Science", "Error", + # Appended rather than slotted in beside the file columns, so an existing + # consumer reading by index is unaffected. + "Companion Of", ] @@ -85,11 +88,13 @@ def write_csv(job: Job, path: Path, *, delimiter: str = ",", **_options) -> Path media.camera.colour_science or "", ] + belongs_to = entry.companion_of.name if entry.companion_of else "" + if not entry.destinations: writer.writerow(base + ["", "", "", "Skipped", format_file_datetime(entry.created), format_file_datetime(entry.modified)] - + tail + [""]) + + tail + ["", belongs_to]) continue for number, destination in enumerate(entry.destinations, start=1): @@ -104,6 +109,6 @@ def write_csv(job: Job, path: Path, *, delimiter: str = ",", **_options) -> Path format_file_datetime(entry.modified), ] + tail - + [destination.error or ""] + + [destination.error or "", belongs_to] ) return path diff --git a/src/offloader/reports/html.py b/src/offloader/reports/html.py index f26bb05..c2f1525 100644 --- a/src/offloader/reports/html.py +++ b/src/offloader/reports/html.py @@ -50,8 +50,8 @@ .clip .meta b { color: var(--fg); font-weight: bold; } .strip { display: flex; flex: 1 1 auto; gap: 0; min-width: 0; } .strip img { width: 25%; height: auto; object-fit: contain; background: #000; } -.proxy-note { font-size: 10px; color: var(--muted); font-style: italic; - margin-top: 3px; } +.proxy-note, .companion-note { font-size: 10px; color: var(--muted); + font-style: italic; margin-top: 3px; } .noimg { flex: 1 1 auto; min-height: 78px; background: #1c1c1c; border-radius: 4px; color: #f0a92b; display: flex; align-items: center; justify-content: center; font-size: 12px; letter-spacing: 2px; } @@ -153,7 +153,10 @@ def write_html(job: Job, path: Path, *, thumbnails: bool = True, **_options) -> ("Size of offload", format_size(job.total_bytes)), ("Offload Finish Date", format_job_datetime(finished)), ("Processors", str(job.processors) if job.processors else ""), - ("Verification Type", job.verification_label), + # The PDF's header string is pinned to the reference report's wording, + # so the extra pass is said here rather than folded into it. + ("Verification Type", job.verification_label + + (" + second source read" if job.paranoid else "")), ("Total Time", format_elapsed(job.elapsed_sec)), ("System Ram", job.system_ram), ("Total Files", str(job.total_files)), @@ -176,6 +179,14 @@ def write_html(job: Job, path: Path, *, thumbnails: bool = True, **_options) -> if entry.thumbnail_source is not None: provenance = (f'
Frames from proxy: ' f'{_esc(entry.thumbnail_source.name)}
') + # Sidecars and proxies are shown with the clip they belong to. Copied + # and listed as unrelated files, a missing one is invisible. + if entry.companions: + names = ", ".join(_esc(p.name) for p in entry.companions) + provenance += f'
With: {names}
' + elif entry.companion_of is not None: + provenance += (f'
Belongs to: ' + f'{_esc(entry.companion_of.name)}
') clips.append( f'
{_clip_meta(job, entry)}' f'{provenance}
{strip}
' diff --git a/src/offloader/reports/pdf.py b/src/offloader/reports/pdf.py index fedad2b..100f431 100644 --- a/src/offloader/reports/pdf.py +++ b/src/offloader/reports/pdf.py @@ -69,7 +69,14 @@ def __init__( fonts.register() self.canvas = Canvas(str(self.path), pagesize=layout.PAGE_SIZE) - self.canvas.setTitle(f"{job.name} Job Report") + # The document title is what a stack of reports is told apart by in a + # file manager or browser tab, so it carries the route and the date, + # not just a job name that may be as generic as "Offload". + destination = (str(job.destination_roots[0]) + if job.destination_roots else "") + route = f" — {job.source_root} → {destination}" if destination else "" + self.canvas.setTitle( + f"{job.name} Job Report{route} — {job.started:%Y-%m-%d}") self.canvas.setAuthor(PRODUCT_NAME) self.canvas.setSubject("Verified offload report") diff --git a/src/offloader/retry.py b/src/offloader/retry.py index 97224fe..c2edb03 100644 --- a/src/offloader/retry.py +++ b/src/offloader/retry.py @@ -47,6 +47,24 @@ 1167, # ERROR_DEVICE_NOT_CONNECTED } +class UnstableRead(OSError): + """Two reads of the same bytes disagreed. + + The operating system reported no error at all — this is only visible to a + caller that read twice and compared. It is still the marginal-media signal, + and deserves the same second attempt, so it is transient by construction. + """ + + +class Exhausted(OSError): + """A failure that has already been retried as far as it is going to be. + + Retrying it again at a coarser level would only repeat the same attempts + against the same fault, at the cost of re-reading everything that already + succeeded. Raised by a fine-grained retry loop to close the one above it. + """ + + #: Never retried: retrying cannot help and the delay hides the real fault. _PERMANENT_ERRNO = { errno.ENOENT, # the file is gone @@ -84,6 +102,12 @@ def is_transient(exc: BaseException) -> bool: """Whether this failure has a plausible chance of not recurring.""" if not isinstance(exc, OSError): return False + # Both are decided by what raised them, not by an errno, and `Exhausted` is + # checked first because it may well be wrapping something transient. + if isinstance(exc, Exhausted): + return False + if isinstance(exc, UnstableRead): + return True winerror = getattr(exc, "winerror", None) if winerror is not None: return winerror in _TRANSIENT_WINERROR diff --git a/src/offloader/thumbs.py b/src/offloader/thumbs.py index cdf200a..4f5bd11 100644 --- a/src/offloader/thumbs.py +++ b/src/offloader/thumbs.py @@ -23,6 +23,28 @@ def ffmpeg_path() -> str | None: return shutil.which("ffmpeg") +class DecoderMemo: + """Remembers, per file suffix, that this ffmpeg produced no frames. + + A decoder ffmpeg lacks — BRAW without the Blackmagic SDK is the common + case — fails identically for every clip, and paying four doomed process + spawns per clip is a real cost on a several-hundred-clip card. One memo + lives for one job: the first clip of a suffix pays the probe, the rest + skip. Scoped to the job rather than the process so a swapped-in ffmpeg + gets a fresh chance, and a single corrupt file can mute at most one + offload's contact sheet, never the tool's. + """ + + def __init__(self) -> None: + self._dead: set[str] = set() + + def is_dead(self, source: Path) -> bool: + return source.suffix.lower() in self._dead + + def record_failure(self, source: Path) -> None: + self._dead.add(source.suffix.lower()) + + def _sample_offsets(duration: float, count: int) -> list[float]: """Evenly spaced sample points, biased off the very start and end so we don't grab slates or black frames.""" @@ -41,12 +63,18 @@ def extract( out_dir: Path, count: int = 4, timeout: float = 60.0, + memo: DecoderMemo | None = None, ) -> list[Path]: """Grab `count` thumbnails. Returns [] if ffmpeg is missing, the file has - no video stream, or extraction fails — thumbnails are never load-bearing.""" + no video stream, or extraction fails — thumbnails are never load-bearing. + + With a `memo`, a suffix whose every extraction failed is skipped for the + rest of that memo's lifetime instead of re-spawning ffmpeg per clip.""" exe = ffmpeg_path() if exe is None or not media.is_video or not media.duration_sec: return [] + if memo is not None and memo.is_dead(source): + return [] out_dir.mkdir(parents=True, exist_ok=True) stem = source.stem @@ -81,4 +109,9 @@ def extract( if proc.returncode == 0 and target.exists() and target.stat().st_size > 0: results.append(target) + if memo is not None and not results: + # Every sample offset failed after real attempts (ffmpeg present, a + # video stream, a duration). The overwhelmingly likely cause is a + # decoder this ffmpeg does not have, which the next clip has too. + memo.record_failure(source) return results diff --git a/src/offloader/verify.py b/src/offloader/verify.py index 3975139..794a27d 100644 --- a/src/offloader/verify.py +++ b/src/offloader/verify.py @@ -8,10 +8,16 @@ the moment it was copied. Re-hashing later and comparing is the only way to detect corruption that happened *after* the offload — bit rot, a failing drive, a bad cable on the way to the archive. + +An ASC MHL history records more than file hashes: every directory carries a +content hash and a structure hash. Those are re-checked here too, because they +catch what no file hash can — a rename, or a file moved between folders, where +every individual file is still perfectly intact. """ from __future__ import annotations +import fnmatch import os from collections.abc import Callable, Iterator from dataclasses import dataclass, field @@ -21,9 +27,15 @@ from .ascmhl import ASCMHL_DIRNAME from .ascmhl import NAMESPACE as ASCMHL_NAMESPACE +from .ascmhl import directory_hashes as ascmhl_directory_hashes from .hashers import ALGORITHMS, hash_file from .integrity import evict_from_cache +#: Where this tool files its own paperwork, as a pattern. Only used to read +#: histories written before the writer recorded the directory itself; a current +#: manifest carries the real path, which handles `--report-dir` as this cannot. +REPORT_DIRECTORY_GLOB = "*_Reports" + class EntryResult(str, Enum): OK = "ok" @@ -61,6 +73,58 @@ def describe(self) -> str: f" actual {self.actual}") +class DirectoryResult(str, Enum): + OK = "ok" + RENAMED = "renamed" # the same bytes under a different name or layout + CHANGED = "changed" # the content itself no longer hashes the same + MISSING = "missing" # nothing of the directory is left on disk + + +@dataclass +class DirectoryVerdict: + """One directory's recorded hashes against what its contents hash to now. + + The two hashes answer different questions. Content covers the file hashes + alone, so it survives a rename. Structure folds each name in with its hash, + so it does not. Content matching while structure does not is therefore a + precise statement: nothing was corrupted, something was renamed or moved. + """ + + path: Path + #: Relative to the root of the managed data; `"."` is that root. + relative: str + result: DirectoryResult + expected_content: str | None = None + actual_content: str | None = None + expected_structure: str | None = None + actual_structure: str | None = None + #: A file inside already failed on its own hash, which is enough to account + #: for this. Without it a single corrupt file reads as one failure per + #: directory between it and the root. + explained_by_files: bool = False + + @property + def ok(self) -> bool: + return self.result is DirectoryResult.OK + + def describe(self) -> str: + label = f"{self.relative}/" if self.relative != "." else "(root)" + if self.result is DirectoryResult.OK: + return f"ok {label}" + if self.result is DirectoryResult.MISSING: + return f"MISSING {label}" + if self.result is DirectoryResult.RENAMED: + return (f"RENAMED {label}\n" + " every file still hashes as recorded, so a " + "name changed or a file moved") + tail = ("\n (accounted for by the file failures above)" + if self.explained_by_files else "") + actual = self.actual_content or "(no files remain)" + return (f"CHANGED {label}\n" + f" expected {self.expected_content}\n" + f" actual {actual}{tail}") + + @dataclass class VerifyReport: manifest: Path @@ -68,6 +132,8 @@ class VerifyReport: verdicts: list[FileVerdict] = field(default_factory=list) #: Files present on disk that the manifest does not mention. unlisted: list[Path] = field(default_factory=list) + #: One per directory hash the manifest recorded. Only ASC MHL has these. + directories: list[DirectoryVerdict] = field(default_factory=list) @property def checked(self) -> int: @@ -77,9 +143,14 @@ def checked(self) -> int: def failures(self) -> list[FileVerdict]: return [v for v in self.verdicts if not v.ok] + @property + def directory_failures(self) -> list[DirectoryVerdict]: + return [v for v in self.directories if not v.ok] + @property def passed(self) -> bool: - return bool(self.verdicts) and not self.failures + return (bool(self.verdicts) and not self.failures + and not self.directory_failures) def counts(self) -> dict[str, int]: tally: dict[str, int] = {} @@ -91,9 +162,25 @@ def summary(self) -> str: if not self.verdicts: return "manifest listed no files with checksums" if self.passed: + if self.directories: + return (f"all {self.checked} files and all " + f"{len(self.directories)} directory hashes match the " + "manifest") return f"all {self.checked} files match the manifest" + if not self.failures and self.directory_failures: + # Every file matched and the report still did not pass. Listing the + # file tally first would open "N checked: N ok", which reads as a + # pass to anyone scanning — and this is the one verdict where the + # file hashes agreeing is the point rather than the reassurance. + return (f"{len(self.directory_failures)} of {len(self.directories)} " + f"directory hashes differ; all {self.checked} files match, " + "so the bytes are intact and the tree is not") parts = [f"{count} {name}" for name, count in sorted(self.counts().items())] - return f"{self.checked} checked: " + ", ".join(parts) + line = f"{self.checked} checked: " + ", ".join(parts) + if self.directory_failures: + line += (f"; {len(self.directory_failures)} of " + f"{len(self.directories)} directory hashes differ") + return line #: MHL element name -> our algorithm key. @@ -163,25 +250,72 @@ def verify_manifest( progress: Callable[[int, int, Path], None] | None = None, bypass_cache: bool = True, find_unlisted: bool = True, + check_directories: bool = True, ) -> VerifyReport: """Re-hash everything an MHL describes and compare. `bypass_cache` evicts each file before reading it, so a freshly written tree is read off the device rather than out of memory. + + `check_directories` recomputes an ASC MHL manifest's directory hashes from + what is on disk. That means hashing the files the manifest does *not* list + as well: a renamed file is unlisted under its new name, and hashing it is + what turns "one file missing, one file unexpected" into the far stronger + "these are the same bytes, the name changed". """ manifest = Path(manifest) entries = list(_entries(manifest)) report = VerifyReport(manifest=manifest, algorithm=next((e[1] for e in entries if e[1]), "unknown")) + # ASC MHL keeps its manifests in `ascmhl/` at the root of the managed data, + # so the folder above is what the recorded paths are relative to. + managed_root = manifest.parent.parent + ascmhl_root = _ascmhl_root(manifest) if check_directories else None + recorded: dict[str, tuple[str, str]] = {} + if ascmhl_root is not None: + recorded, directory_algorithm = _ascmhl_directory_hashes(ascmhl_root) + # Recomputing is only meaningful when the directory hashes and the file + # hashes were taken with the same algorithm. + if not recorded or directory_algorithm != report.algorithm: + ascmhl_root, recorded = None, {} + + #: Relative POSIX path -> what it hashes to now, feeding the directory pass. + on_disk: dict[str, str] = {} + #: Paths that can no longer stand as evidence, so that one bad file is not + #: re-reported as a fresh failure for every directory above it. + unsound: set[str] = set() + #: Paths on disk the manifest never mentioned. A directory that gained one + #: is *not* explained by its file failures, however many it has. + unexpected: set[str] = set() + + def note(path: Path, digest: str | None) -> str | None: + """Record what a file hashes to now — a `digest` of `None` meaning it + cannot stand as evidence at all. Returns the key it was filed under.""" + if ascmhl_root is None: + return None + try: + relative = path.relative_to(managed_root).as_posix() + except ValueError: + return None + if digest is None: + unsound.add(relative) + else: + on_disk[relative] = digest + return relative + for index, (path, algorithm_key, expected, expected_size) in enumerate(entries): if progress: progress(index, len(entries), path) if algorithm_key is None or not expected: + # Including a hash the manifest itself disowned would contradict the + # writer, which left it out of the directory hashes for the same + # reason. Deliberately not noted either way. report.verdicts.append(FileVerdict(path, EntryResult.NO_CHECKSUM)) continue if not path.exists(): + note(path, None) report.verdicts.append( FileVerdict(path, EntryResult.MISSING, expected=expected, expected_size=expected_size)) @@ -193,12 +327,16 @@ def verify_manifest( evict_from_cache(path) actual = hash_file(path, algorithm_key) except OSError as exc: + note(path, None) report.verdicts.append( FileVerdict(path, EntryResult.UNREADABLE, expected=expected, detail=str(exc))) continue matched = actual == expected + relative = note(path, actual) + if not matched and relative is not None: + unsound.add(relative) report.verdicts.append(FileVerdict( path, EntryResult.OK if matched else EntryResult.MISMATCH, @@ -206,20 +344,183 @@ def verify_manifest( expected_size=expected_size, actual_size=actual_size, )) - if find_unlisted: + # The directory pass needs the unexpected files hashed, so it scans even + # when the caller did not ask for them to be reported. + if find_unlisted or ascmhl_root is not None: listed = {p.resolve() for p, _, _, _ in entries} - for candidate in _described_root(manifest, listed).rglob("*"): + patterns = _ignore_patterns(ascmhl_root) if ascmhl_root is not None else [] + scan_root = (managed_root if ascmhl_root is not None + else _described_root(manifest, listed)) + # Manifests written before the report directory was recorded as ignored + # say nothing about it, and folding the tool's own paperwork into a + # recomputed hash reports the report as a change to the tree. Keyed off + # the absence of any recorded pattern but the history's own folder, so + # it stops applying the moment a manifest describes its own layout. + legacy_paperwork = not [p for p in patterns if p != ASCMHL_DIRNAME] + + for candidate in scan_root.rglob("*"): if not candidate.is_file() or candidate.suffix.lower() == ".mhl": continue # The history's own bookkeeping is not managed data. if ASCMHL_DIRNAME in candidate.parts: continue - if candidate.resolve() not in listed: + if candidate.resolve() in listed: + continue + relative = candidate.relative_to(scan_root).as_posix() + if _ignored(relative, patterns): + continue + + if find_unlisted: report.unlisted.append(candidate) + if ascmhl_root is not None: + if legacy_paperwork and _ignored(relative, [REPORT_DIRECTORY_GLOB]): + continue + unexpected.add(relative) + try: + if bypass_cache: + evict_from_cache(candidate) + on_disk[relative] = hash_file(candidate, report.algorithm) + except OSError: + unsound.add(relative) + + if recorded: + report.directories = _directory_verdicts( + managed_root, recorded, report.algorithm, on_disk, + unsound, unexpected) return report +def _ascmhl_root(manifest: Path) -> ET.Element | None: + """The parsed manifest, but only if it is an ASC MHL one. + + Classic MHL 1.1 has no concept of a directory hash, so there is nothing to + re-check and nothing to read. + """ + root = ET.parse(manifest).getroot() + return root if root.tag == f"{{{ASCMHL_NAMESPACE}}}hashlist" else None + + +def _ascmhl_directory_hashes( + root: ET.Element) -> tuple[dict[str, tuple[str, str]], str | None]: + """The recorded (content, structure) per directory, and their algorithm. + + The root of the managed data is written as `` inside + `` rather than as one of the `` elements, so it + is keyed `"."` here to line up with what `ascmhl.directory_hashes` returns. + """ + prefix = f"{{{ASCMHL_NAMESPACE}}}" + recorded: dict[str, tuple[str, str]] = {} + algorithm_key: str | None = None + + def pair(element: ET.Element) -> tuple[str, str] | None: + nonlocal algorithm_key + sides: list[str] = [] + for side in ("content", "structure"): + holder = element.find(f"{prefix}{side}") + if holder is None: + return None + for child in holder: + name = child.tag.split("}")[-1] + if name in _TAG_TO_ALGORITHM and child.text: + sides.append(child.text.strip()) + algorithm_key = algorithm_key or _TAG_TO_ALGORITHM[name] + break + return (sides[0], sides[1]) if len(sides) == 2 else None + + for element in root.iter(f"{prefix}roothash"): + values = pair(element) + if values: + recorded["."] = values + + for element in root.iter(f"{prefix}directoryhash"): + path_element = element.find(f"{prefix}path") + if path_element is None or not path_element.text: + continue + values = pair(element) + if values: + recorded[path_element.text.strip().strip("/")] = values + + return recorded, algorithm_key + + +def _ignore_patterns(root: ET.Element) -> list[str]: + """What the writer recorded as excluded from the managed data. + + Honouring it matters more here than for the unlisted list: a file the + manifest deliberately ignored is not evidence, and folding it into a + recomputed directory hash would make every directory above it mismatch. + """ + prefix = f"{{{ASCMHL_NAMESPACE}}}" + return [element.text.strip() + for info in root.iter(f"{prefix}processinfo") + for element in info.iter(f"{prefix}pattern") + if element.text and element.text.strip()] + + +def _ignored(relative: str, patterns: list[str]) -> bool: + """A pattern matches the whole relative path or any one component of it.""" + parts = relative.split("/") + for pattern in patterns: + if fnmatch.fnmatch(relative, pattern): + return True + if any(fnmatch.fnmatch(part, pattern) for part in parts): + return True + return False + + +def _within(relative: str, directory: str) -> bool: + return (directory == "." + or relative == directory + or relative.startswith(f"{directory}/")) + + +def _directory_verdicts(base: Path, recorded: dict[str, tuple[str, str]], + algorithm_key: str, on_disk: dict[str, str], + unsound: set[str], + unexpected: set[str]) -> list[DirectoryVerdict]: + """Every recorded directory hash against the tree as it stands now.""" + computed = ascmhl_directory_hashes( + [(Path(relative), digest) for relative, digest in on_disk.items()], + algorithm_key) + + verdicts: list[DirectoryVerdict] = [] + for relative in sorted(recorded): + expected_content, expected_structure = recorded[relative] + path = base if relative == "." else base / relative + # Only fully explained when every difference underneath was already + # reported file by file. A file that arrived was not. + explained = (any(_within(bad, relative) for bad in unsound) + and not any(_within(extra, relative) for extra in unexpected)) + found = computed.get(relative) + + if found is None: + # Nothing hashable is left underneath. Whether the folder itself + # survives is the difference between emptied and gone. + verdicts.append(DirectoryVerdict( + path, relative, + DirectoryResult.CHANGED if path.is_dir() else DirectoryResult.MISSING, + expected_content=expected_content, + expected_structure=expected_structure, + explained_by_files=explained)) + continue + + actual_content, actual_structure = found + if actual_content != expected_content: + result = DirectoryResult.CHANGED + elif actual_structure != expected_structure: + result = DirectoryResult.RENAMED + else: + result = DirectoryResult.OK + + verdicts.append(DirectoryVerdict( + path, relative, result, + expected_content=expected_content, actual_content=actual_content, + expected_structure=expected_structure, actual_structure=actual_structure, + explained_by_files=explained and result is not DirectoryResult.OK)) + return verdicts + + def _ascmhl_entries(manifest: Path, root: ET.Element): """ASC MHL paths are relative to the root of the managed data, which is the parent of the `ascmhl` folder the manifest sits in.""" diff --git a/src/offloader/volumes.py b/src/offloader/volumes.py index 324b677..78ab2d3 100644 --- a/src/offloader/volumes.py +++ b/src/offloader/volumes.py @@ -10,6 +10,7 @@ import platform import shutil import string +from concurrent.futures import ThreadPoolExecutor from dataclasses import dataclass from pathlib import Path @@ -131,50 +132,62 @@ def _usage(root: Path) -> tuple[int, int]: return 0, 0 -def _windows_volumes() -> list[Volume]: +def _windows_roots() -> list[tuple[Path, str]]: + """Every drive letter and its type. Cheap — `GetDriveTypeW` reads a flag + the mount manager already holds, it does not touch the volume.""" import ctypes kernel32 = ctypes.windll.kernel32 - # Stop Windows popping "insert a disk" dialogs for empty card readers. - previous = kernel32.SetErrorMode(0x0001 | 0x0002) - found: list[Volume] = [] + roots: list[tuple[Path, str]] = [] + bitmask = kernel32.GetLogicalDrives() + for index, letter in enumerate(string.ascii_uppercase): + if not bitmask & (1 << index): + continue + root = f"{letter}:\\" + drive_type = _WINDOWS_DRIVE_TYPES.get(kernel32.GetDriveTypeW(root), "unknown") + if drive_type in ("no-root", "unknown"): + continue + roots.append((Path(root), drive_type)) + return roots + + +def _windows_probe(root: Path, drive_type: str) -> Volume | None: + """Label, filesystem, usage and card detection for one root — the part + that actually talks to the device, and for a network share is a synchronous + SMB round-trip. Runs on a pool thread; error mode is set per-thread so an + empty card reader cannot pop an "insert a disk" dialog.""" + import ctypes + + kernel32 = ctypes.windll.kernel32 + previous = ctypes.c_uint(0) + kernel32.SetThreadErrorMode(0x0001 | 0x0002, ctypes.byref(previous)) try: - bitmask = kernel32.GetLogicalDrives() - for index, letter in enumerate(string.ascii_uppercase): - if not bitmask & (1 << index): - continue - root = f"{letter}:\\" - drive_type = _WINDOWS_DRIVE_TYPES.get(kernel32.GetDriveTypeW(root), "unknown") - if drive_type in ("no-root", "unknown"): - continue + label_buffer = ctypes.create_unicode_buffer(261) + fs_buffer = ctypes.create_unicode_buffer(261) + ok = kernel32.GetVolumeInformationW( + ctypes.c_wchar_p(str(root)), label_buffer, 261, + None, None, None, fs_buffer, 261, + ) + if not ok and drive_type == "optical": + return None # empty drive - label_buffer = ctypes.create_unicode_buffer(261) - fs_buffer = ctypes.create_unicode_buffer(261) - ok = kernel32.GetVolumeInformationW( - ctypes.c_wchar_p(root), label_buffer, 261, - None, None, None, fs_buffer, 261, - ) - if not ok and drive_type == "optical": - continue # empty drive - - total, free = _usage(Path(root)) - if total == 0 and drive_type != "removable": - continue - found.append(Volume( - root=Path(root), - label=label_buffer.value, - filesystem=fs_buffer.value, - total_bytes=total, - free_bytes=free, - drive_type=drive_type, - is_camera_card=detect_camera_card(Path(root), drive_type), - )) + total, free = _usage(root) + if total == 0 and drive_type != "removable": + return None + return Volume( + root=root, + label=label_buffer.value, + filesystem=fs_buffer.value, + total_bytes=total, + free_bytes=free, + drive_type=drive_type, + is_camera_card=detect_camera_card(root, drive_type), + ) finally: - kernel32.SetErrorMode(previous) - return found + kernel32.SetThreadErrorMode(previous.value, None) -def _posix_volumes() -> list[Volume]: +def _posix_roots() -> list[tuple[Path, str]]: candidates: list[Path] = [Path("/")] for parent in (Path("/Volumes"), Path("/media"), Path("/mnt"), Path("/run/media") / (Path.home().name)): @@ -183,40 +196,67 @@ def _posix_volumes() -> list[Volume]: except OSError: continue - found: list[Volume] = [] + roots: list[tuple[Path, str]] = [] seen: set[Path] = set() for root in candidates: if root in seen: continue seen.add(root) - total, free = _usage(root) - if total == 0: - continue - drive_type = "fixed" if root == Path("/") else "removable" - found.append(Volume( - root=root, - label=root.name or str(root), - filesystem="", - total_bytes=total, - free_bytes=free, - drive_type=drive_type, - is_camera_card=detect_camera_card(root, drive_type), - )) - return found - + roots.append((root, "fixed" if root == Path("/") else "removable")) + return roots + + +def _posix_probe(root: Path, drive_type: str) -> Volume | None: + total, free = _usage(root) + if total == 0: + return None + return Volume( + root=root, + label=root.name or str(root), + filesystem="", + total_bytes=total, + free_bytes=free, + drive_type=drive_type, + is_camera_card=detect_camera_card(root, drive_type), + ) + + +def list_roots() -> list[tuple[Path, str]]: + """Every mount point and its drive type, without touching any volume.""" + try: + return (_windows_roots() if platform.system() == "Windows" + else _posix_roots()) + except Exception: + return [] -def list_volumes() -> list[Volume]: - """Every mounted volume, cards first so they are easy to spot. - Deduplicated by resolved root: macOS reaches the boot volume through both - `/` and `/Volumes/Macintosh HD`, and listing it twice would be noise. - """ +def probe_volume(root: Path, drive_type: str) -> Volume | None: + """Full details for one root, or None if it should not be listed.""" try: - volumes = (_windows_volumes() if platform.system() == "Windows" - else _posix_volumes()) + return (_windows_probe(root, drive_type) + if platform.system() == "Windows" + else _posix_probe(root, drive_type)) except Exception: - volumes = [] - + return None + + +def probe_many(roots: list[tuple[Path, str]]) -> list[Volume]: + """Probe roots concurrently, so a sleeping USB drive or a slow network + share costs its own probe rather than delaying every drive after it — + serial probing made the panel's refresh wait the *sum* of every + round-trip; this waits only the slowest.""" + if not roots: + return [] + with ThreadPoolExecutor(max_workers=min(len(roots), 12), + thread_name_prefix="volscan") as pool: + results = pool.map(lambda pair: probe_volume(*pair), roots) + return [volume for volume in results if volume is not None] + + +def order_volumes(volumes: list[Volume]) -> list[Volume]: + """Cards first so they are easy to spot; deduplicated by resolved root, + because macOS reaches the boot volume through both `/` and + `/Volumes/Macintosh HD`, and listing it twice would be noise.""" seen: set[Path] = set() unique: list[Volume] = [] for volume in sorted(volumes, key=lambda v: len(str(v.root))): @@ -232,6 +272,32 @@ def list_volumes() -> list[Volume]: return sorted(unique, key=lambda v: (not v.is_camera_card, str(v.root))) +def list_volumes() -> list[Volume]: + """Every mounted volume, cards first so they are easy to spot.""" + return order_volumes(probe_many(list_roots())) + + +def volume_label(path: Path) -> str | None: + """The label of the volume holding `path`, probing only that volume. + + Exists for job naming: a card offloaded from its root has no folder name + to be named after, and the volume label — A003 — is what the operator + calls the card. `find_volume` would answer too, but it probes every + mounted volume including network shares; this touches one. + """ + if platform.system() != "Windows": + return None # POSIX mounts carry their label as the directory name + import ctypes + + kernel32 = ctypes.windll.kernel32 + buffer = ctypes.create_unicode_buffer(261) + ok = kernel32.GetVolumeInformationW( + ctypes.c_wchar_p(Path(path).anchor or str(path)), buffer, 261, + None, None, None, None, 0, + ) + return (buffer.value or None) if ok else None + + def find_volume(path: Path) -> Volume | None: """The volume a path sits on, for labelling a chosen source.""" path = Path(path).resolve() diff --git a/tests/test_ascmhl.py b/tests/test_ascmhl.py index 80094f8..eec4fab 100644 --- a/tests/test_ascmhl.py +++ b/tests/test_ascmhl.py @@ -15,7 +15,7 @@ import pytest -from offloader import ascmhl, engine, verify +from offloader import ascmhl, cli, engine, verify from offloader.hashers import c4_of_bytes, hash_file from offloader.models import VerificationMode @@ -97,6 +97,19 @@ def test_directory_hashes_match_the_reference_implementation(): assert ascmhl.hash_of_hashes(structure, "xxh64") == REF_CLIPS_STRUCTURE +def test_directory_hashes_is_keyed_by_path_and_agrees_with_the_reference(): + """The verifier recomputes through this entry point, so it has to produce + what the writer records — including for the root, which the manifest keeps + under `roothash` rather than as a `directoryhash`.""" + hashes = ascmhl.directory_hashes( + [(Path(name), digest) for name, digest in REF_DIGESTS.items()], "xxh64") + + assert hashes["Clips"] == (REF_CLIPS_CONTENT, REF_CLIPS_STRUCTURE) + assert set(hashes) == {".", "Clips"} + # The root folds in `Clips`, so it is not the same pair. + assert hashes["."] != hashes["Clips"] + + def test_hash_of_hashes_is_order_independent(): """The list is sorted before hashing, so discovery order cannot change it.""" digests = ["ffffffffffffffff", "0000000000000000", "aaaaaaaaaaaaaaaa"] @@ -328,7 +341,224 @@ def test_classic_mhl_still_verifies(tmp_path: Path): job, destination = _offload(tmp_path, {"a.mov": b"aaaa"}) manifest = write_mhl(job, destination / "A001_Reports" / "JobReport.mhl") - assert verify.verify_manifest(manifest).passed + report = verify.verify_manifest(manifest) + assert report.passed + # MHL 1.1 has no directory hashes, so there is nothing to re-check. + assert report.directories == [] + + +# ------------------------------------------------- verifying directory hashes + + +def _directory(report: verify.VerifyReport, relative: str): + return next(v for v in report.directories if v.relative == relative) + + +def test_verify_rechecks_the_recorded_directory_hashes(history): + _job, destination, _manifest = history + report = verify.verify_manifest(verify.find_manifests(destination)[0]) + + # The root, written as `roothash`, and `Clips`, the one subdirectory. + assert sorted(v.relative for v in report.directories) == [".", "Clips"] + assert all(v.result is verify.DirectoryResult.OK for v in report.directories) + assert "directory hashes" in report.summary() + + +def test_a_rename_is_a_structure_mismatch_not_a_content_one(history): + """The whole reason the structure hash exists: every file is individually + fine, and the tree is still not what was recorded.""" + _job, destination, _manifest = history + clips = destination / "Clips" + (clips / "A002C006_141024_R2EC.mov").rename(clips / "A002C099_141024_R2EC.mov") + + report = verify.verify_manifest(verify.find_manifests(destination)[0]) + assert not report.passed + + verdict = _directory(report, "Clips") + assert verdict.result is verify.DirectoryResult.RENAMED + # Nothing was corrupted — the bytes under `Clips` hash exactly as recorded. + assert verdict.actual_content == verdict.expected_content + assert verdict.actual_structure != verdict.expected_structure + # And it propagates: the root cannot certify a tree it no longer describes. + assert _directory(report, ".").result is verify.DirectoryResult.RENAMED + # The `MISSING` line for the old name does not account for this. Something + # arrived under a new one, which no file verdict can say. + assert not verdict.explained_by_files + + +def test_a_file_moved_between_directories_is_caught(history): + """A move keeps the bytes but changes which directory owns them, so the + content hash moves with it.""" + _job, destination, _manifest = history + (destination / "Sidecar.txt").rename(destination / "Clips" / "Sidecar.txt") + + report = verify.verify_manifest(verify.find_manifests(destination)[0]) + assert not report.passed + assert _directory(report, "Clips").result is verify.DirectoryResult.CHANGED + assert _directory(report, ".").result is verify.DirectoryResult.CHANGED + + +def test_a_new_file_changes_the_directory_that_gained_it(history): + _job, destination, _manifest = history + (destination / "Clips" / "extra.mov").write_bytes(b"not in the manifest\n") + + report = verify.verify_manifest(verify.find_manifests(destination)[0]) + assert _directory(report, "Clips").result is verify.DirectoryResult.CHANGED + assert any(p.name == "extra.mov" for p in report.unlisted) + + # Every listed file still matches, so the summary must not open with the + # file tally: "3 checked: 3 ok" leading a report that did not pass is how + # an arrival gets waved through. + summary = report.summary() + assert summary.startswith("2 of 2 directory hashes differ"), summary + assert "the bytes are intact and the tree is not" in summary + + +def test_a_deleted_directory_reads_as_missing(history): + _job, destination, _manifest = history + clips = destination / "Clips" + for child in clips.iterdir(): + child.unlink() + clips.rmdir() + + report = verify.verify_manifest(verify.find_manifests(destination)[0]) + assert _directory(report, "Clips").result is verify.DirectoryResult.MISSING + + +def test_a_corrupt_file_is_not_re_reported_for_every_directory_above_it(history): + """A flipped bit invalidates every directory hash up to the root. Saying so + three times over would bury the one line that matters.""" + _job, destination, _manifest = history + victim = destination / "Clips" / "A002C006_141024_R2EC.mov" + payload = bytearray(victim.read_bytes()) + payload[0] ^= 0x01 + victim.write_bytes(bytes(payload)) + + report = verify.verify_manifest(verify.find_manifests(destination)[0]) + verdict = _directory(report, "Clips") + assert verdict.result is verify.DirectoryResult.CHANGED + assert verdict.explained_by_files + assert "accounted for by the file failures" in verdict.describe() + + +def test_an_ignored_file_is_kept_out_of_the_recomputation(tmp_path: Path): + """A file the manifest was told to ignore is not evidence. Folding it in + would fail every directory above it for a file nobody claimed to have + copied.""" + job, destination = _offload(tmp_path, dict(PLACEHOLDER_FILES)) + (destination / "Clips" / ".DS_Store").write_bytes(b"finder droppings\n") + ascmhl.write_manifest(job, destination, when=WHEN, + ignore_patterns=["ascmhl", ".DS_Store"]) + + report = verify.verify_manifest(verify.find_manifests(destination)[0]) + assert report.passed + assert not report.unlisted + + +def test_directory_checking_can_be_turned_off(history): + _job, destination, _manifest = history + (destination / "Clips" / "A002C006_141024_R2EC.mov").rename( + destination / "Clips" / "renamed.mov") + + report = verify.verify_manifest(verify.find_manifests(destination)[0], + check_directories=False) + assert report.directories == [] + + +# --------------------------------------------------- the job's own paperwork + + +def _cli_offload(tmp_path: Path, reports: str, extra: list[str] | None = None) -> Path: + """A real offload through the CLI, which is what writes the reports. + + The fixtures above call `write_manifest` directly, so the destination they + build has no `_Reports` folder in it — which is exactly why nothing + caught the tool's own output being counted as a change to the tree. + """ + source = tmp_path / "card" / "Clips" + source.mkdir(parents=True) + (source / "A001_C001.mov").write_bytes(b"footage " * 500) + (tmp_path / "card" / "readme.txt").write_bytes(b"shot notes\n") + + destination = tmp_path / "dest" + assert cli.main(["offload", "--source", str(tmp_path / "card"), + "--dest", str(destination), "--name", "A001", + "--report", reports, "--quiet", *(extra or [])]) == 0 + return destination + + +def test_a_freshly_offloaded_card_verifies_clean(tmp_path: Path): + """REGRESSION. The reports land inside the destination after the manifest + is written, so recomputing the root hash over everything on disk folded the + tool's own paperwork in and reported it as a change. A card that was just + copied has to verify.""" + destination = _cli_offload(tmp_path, "ascmhl,csv,pdf") + + report = verify.verify_manifest(verify.find_manifests(destination)[0]) + assert report.passed, report.summary() + assert all(v.ok for v in report.directories), \ + [v.describe() for v in report.directories] + + +def test_the_manifest_records_where_the_paperwork_went(tmp_path: Path): + destination = _cli_offload(tmp_path, "ascmhl,csv") + text = verify.find_manifests(destination)[0].read_text(encoding="utf-8") + + assert "ascmhl" in text + assert "A001_Reports" in text + + +def test_a_relocated_report_directory_is_recorded_too(tmp_path: Path): + """`--report-dir` is why the path is recorded rather than assumed: the + conventional name is not where these went.""" + destination = _cli_offload(tmp_path, "ascmhl,csv", + ["--report-dir", str(tmp_path / "dest" / "paperwork")]) + text = verify.find_manifests(destination)[0].read_text(encoding="utf-8") + assert "paperwork" in text + + report = verify.verify_manifest(verify.find_manifests(destination)[0]) + assert report.passed, report.summary() + + +def test_reports_sent_outside_the_copy_need_no_pattern(tmp_path: Path): + destination = _cli_offload(tmp_path, "ascmhl,csv", + ["--report-dir", str(tmp_path / "elsewhere")]) + text = verify.find_manifests(destination)[0].read_text(encoding="utf-8") + + assert "elsewhere" not in text + assert verify.verify_manifest(verify.find_manifests(destination)[0]).passed + + +def test_paperwork_beside_a_manifest_that_never_recorded_it_is_tolerated( + tmp_path: Path +): + """A history written before the writer recorded its own report folder. The + files are still reported as unlisted — that much was always true — but they + are not counted as a change to a directory the manifest never covered.""" + job, destination = _offload(tmp_path, dict(PLACEHOLDER_FILES)) + ascmhl.write_manifest(job, destination, when=WHEN, + ignore_patterns=[ascmhl.ASCMHL_DIRNAME]) + reports = destination / "A002R2EC_Reports" + reports.mkdir() + (reports / "JobReport.pdf").write_bytes(b"%PDF-1.4 paperwork\n") + (reports / "thumbs").mkdir() + (reports / "thumbs" / "A002C006.jpg").write_bytes(b"jpeg") + + report = verify.verify_manifest(verify.find_manifests(destination)[0]) + assert report.passed, report.summary() + assert any(p.name == "JobReport.pdf" for p in report.unlisted) + + +def test_the_tolerance_stops_at_anything_that_is_not_paperwork(tmp_path: Path): + """It is scoped to the one directory the tool writes itself. A stray file + anywhere else still moves the hash it belongs to.""" + job, destination = _offload(tmp_path, dict(PLACEHOLDER_FILES)) + ascmhl.write_manifest(job, destination, when=WHEN, + ignore_patterns=[ascmhl.ASCMHL_DIRNAME]) + (destination / "Clips" / "extra.mov").write_bytes(b"not in the manifest\n") + + report = verify.verify_manifest(verify.find_manifests(destination)[0]) + assert _directory(report, "Clips").result is verify.DirectoryResult.CHANGED # --------------------------------------------------------------- reference diff --git a/tests/test_braw.py b/tests/test_braw.py index e25775b..524b441 100644 --- a/tests/test_braw.py +++ b/tests/test_braw.py @@ -348,6 +348,66 @@ def test_needs_proxy_covers_the_undecodable_formats(): assert not companions.needs_proxy(Path("clip.mp4")) +# ------------------------------------------------------------- grouping + + +def test_a_sidecar_is_grouped_with_its_clip(tmp_path: Path): + clip = write_braw(tmp_path / "A001_C001.braw") + sidecar = tmp_path / "A001_C001.sidecar" + sidecar.write_bytes(b"colour metadata") + + assert companions.group([clip, sidecar]) == {sidecar: clip} + + +def test_a_proxy_is_grouped_with_the_clip_a_folder_up(tmp_path: Path): + card = tmp_path / "A001" + (card / "Proxy").mkdir(parents=True) + clip = write_braw(card / "A001_C001.braw") + proxy = card / "Proxy" / "A001_C001.mp4" + proxy.write_bytes(b"proxy") + + assert companions.group([clip, proxy]) == {proxy: clip} + + +def test_an_ambiguous_stem_is_left_ungrouped(tmp_path: Path): + """Two takes of the same name in different folders, one sidecar between + them. Naming the wrong clip would be worse than saying nothing, because the + only value of the link is that it can be trusted.""" + first = write_braw(tmp_path / "Day1" / "A001_C001.braw") + second = write_braw(tmp_path / "Day2" / "A001_C001.braw") + sidecar = tmp_path / "A001_C001.sidecar" + sidecar.write_bytes(b"whose?") + + assert companions.group([first, second, sidecar]) == {} + + +def test_a_sidecar_beside_one_of_two_takes_picks_the_near_one(tmp_path: Path): + first = write_braw(tmp_path / "Day1" / "A001_C001.braw") + second = write_braw(tmp_path / "Day2" / "A001_C001.braw") + sidecar = tmp_path / "Day2" / "A001_C001.sidecar" + sidecar.write_bytes(b"day two") + + assert companions.group([first, second, sidecar]) == {sidecar: second} + + +def test_an_orphan_sidecar_is_not_invented_a_clip(tmp_path: Path): + sidecar = tmp_path / "A001_C009.sidecar" + sidecar.write_bytes(b"no clip here") + clip = write_braw(tmp_path / "A001_C001.braw") + + assert companions.group([clip, sidecar]) == {} + + +def test_clips_are_never_companions_of_each_other(tmp_path: Path): + """Two ordinary takes sharing a stem are two takes, not a pair.""" + one = tmp_path / "A001_C001.mov" + two = tmp_path / "A001_C001.mp4" + for path in (one, two): + path.write_bytes(b"a take") + + assert companions.group([one, two]) == {} + + # ------------------------------------------------------------------ real file #: A real Blackmagic PYXIS 6K still, if one happens to be around. The synthetic diff --git a/tests/test_config_thumbs.py b/tests/test_config_thumbs.py index 4037650..dbe0c0e 100644 --- a/tests/test_config_thumbs.py +++ b/tests/test_config_thumbs.py @@ -133,3 +133,50 @@ def test_collect_returns_usable_host_facts(): def test_ram_reading_is_plausible(): total = sysinfo._ram_bytes() assert total == 0 or 256 * 1024**2 < total < 8 * 1024**4 + + +def test_memo_skips_a_suffix_that_never_decoded(tmp_path: Path, monkeypatch): + """BRAW without the Blackmagic SDK fails identically for every clip; the + first clip pays the failed probe, the rest of the job skips the four + doomed spawns each.""" + monkeypatch.setattr(thumbs, "ffmpeg_path", lambda: "ffmpeg") + calls: list = [] + + class _Failed: + returncode = 1 + stdout = "" + stderr = "decoder not found" + + monkeypatch.setattr(thumbs.subprocess, "run", + lambda cmd, **kwargs: calls.append(cmd) or _Failed()) + media = MediaInfo(width=4096, height=2160, duration_sec=10.0) + memo = thumbs.DecoderMemo() + + assert thumbs.extract(tmp_path / "A001.braw", media, tmp_path / "out", + 4, memo=memo) == [] + assert len(calls) == 4 + + # Same suffix, case-insensitive: not one more spawn. + assert thumbs.extract(tmp_path / "A002.BRAW", media, tmp_path / "out", + 4, memo=memo) == [] + assert len(calls) == 4 + + # A different suffix still gets its chance. + thumbs.extract(tmp_path / "C001.mov", media, tmp_path / "out", 4, memo=memo) + assert len(calls) == 8 + + +def test_memo_learns_nothing_from_a_skip(tmp_path: Path, monkeypatch): + """No ffmpeg and no video stream never attempted a decode, so they prove + nothing about the decoder.""" + media = MediaInfo(width=1920, height=1080, duration_sec=10.0) + memo = thumbs.DecoderMemo() + + monkeypatch.setattr(thumbs, "ffmpeg_path", lambda: None) + thumbs.extract(tmp_path / "clip.braw", media, tmp_path / "out", memo=memo) + assert not memo.is_dead(tmp_path / "clip.braw") + + monkeypatch.setattr(thumbs, "ffmpeg_path", lambda: "ffmpeg") + audio = MediaInfo(duration_sec=10.0) + thumbs.extract(tmp_path / "take.wav", audio, tmp_path / "out", memo=memo) + assert not memo.is_dead(tmp_path / "take.wav") diff --git a/tests/test_data_safety.py b/tests/test_data_safety.py index ae60845..8ecf39c 100644 --- a/tests/test_data_safety.py +++ b/tests/test_data_safety.py @@ -16,7 +16,7 @@ import pytest -from offloader import engine, integrity +from offloader import engine, integrity, retry from offloader.models import FileStatus, VerificationMode PAYLOAD = b"IRREPLACEABLE FOOTAGE " * 5000 @@ -34,6 +34,11 @@ def _options(tmp_path: Path, **overrides) -> engine.OffloadOptions: return engine.OffloadOptions(**defaults) +def _fast() -> retry.RetryPolicy: + """The real retry counts, without sitting out the real backoff.""" + return retry.RetryPolicy(attempts=3, delay=0) + + def _card(tmp_path: Path, name: str = "A001_C001.mov") -> Path: root = tmp_path / "card" root.mkdir(parents=True, exist_ok=True) @@ -303,3 +308,139 @@ def test_eviction_does_not_damage_the_file(tmp_path: Path): target.write_bytes(PAYLOAD) integrity.evict_from_cache(target) assert target.read_bytes() == PAYLOAD + + +# ------------------------------------------------------------ reading twice + + +class _AlternatingReader: + """A source that returns different bytes on alternate opens. + + Simulates the fault nothing else here can catch: a read that returns wrong + bytes and reports no error at all. The checksum is computed from whatever + came back, so the destination faithfully matches a corrupted source and + verifies clean at every level. + """ + + def __init__(self, handle, opens: dict): + self._handle = handle + opens["n"] += 1 + self._lie = opens["n"] % 2 == 0 + + def read(self, size=-1): + data = self._handle.read(size) + return bytes(len(data)) if self._lie else data + + def seek(self, offset, whence=0): + return self._handle.seek(offset, whence) + + def close(self): + self._handle.close() + + def __enter__(self): + return self + + def __exit__(self, *args): + self._handle.close() + + +def _patch_alternating_source(monkeypatch, card: Path, opens: dict) -> None: + real_open = builtins.open + + def fake_open(path, mode="r", *args, **kwargs): + handle = real_open(path, mode, *args, **kwargs) + try: + inside = Path(path).resolve().is_relative_to(card.resolve()) + except (OSError, ValueError): + inside = False + if inside and "r" in str(mode) and "b" in str(mode): + return _AlternatingReader(handle, opens) + return handle + + monkeypatch.setattr(builtins, "open", fake_open) + + +def test_paranoid_catches_a_source_that_does_not_read_the_same_twice( + tmp_path: Path, monkeypatch +): + """The gap --paranoid exists to close. Every layer below agrees the copy is + perfect, because every layer is comparing against the same bad read.""" + card = _card(tmp_path) + _patch_alternating_source(monkeypatch, card, {"n": 0}) + job = engine.run(card, _options(tmp_path, paranoid=True, retry=_fast())) + monkeypatch.undo() + + assert job.final_status == "Failed" + assert not (tmp_path / "dest" / "A001_C001.mov").exists() + + +def test_without_paranoid_the_same_source_verifies_clean(tmp_path: Path, + monkeypatch): + """The point of the test above: this is what happens today. A copy that + matches its source's checksum exactly, of bytes that were never on the + card. Only reading twice can tell.""" + card = _card(tmp_path) + _patch_alternating_source(monkeypatch, card, {"n": 0}) + job = engine.run(card, _options(tmp_path)) + monkeypatch.undo() + + assert job.final_status == "Verified" + + +def test_paranoid_leaves_nothing_behind_when_it_fails(tmp_path: Path, + monkeypatch): + card = _card(tmp_path) + _patch_alternating_source(monkeypatch, card, {"n": 0}) + engine.run(card, _options(tmp_path, paranoid=True, retry=_fast())) + monkeypatch.undo() + + assert list((tmp_path / "dest").rglob(f"*{engine.PARTIAL_SUFFIX}")) == [] + + +def test_a_sound_source_passes_paranoid_and_says_so(tmp_path: Path): + card = _card(tmp_path) + job = engine.run(card, _options(tmp_path, paranoid=True)) + + assert job.final_status == "Verified" + assert job.paranoid + assert (tmp_path / "dest" / "A001_C001.mov").read_bytes() == PAYLOAD + + +def test_paranoid_is_off_by_default_and_reads_the_source_once(tmp_path: Path, + monkeypatch): + """It costs a second full pass over the card, which is only worth paying + deliberately.""" + card = _card(tmp_path) + reads: list[Path] = [] + real_open = builtins.open + + def counting_open(path, mode="r", *args, **kwargs): + if "r" in str(mode) and "b" in str(mode): + try: + if Path(path).resolve().is_relative_to(card.resolve()): + reads.append(Path(path)) + except (OSError, ValueError): + pass + return real_open(path, mode, *args, **kwargs) + + assert engine.OffloadOptions(destinations=[tmp_path / "d"]).paranoid is False + + monkeypatch.setattr(builtins, "open", counting_open) + job = engine.run(card, _options(tmp_path)) + monkeypatch.undo() + + assert job.final_status == "Verified" + assert len(reads) == 1, f"the source was opened for reading {len(reads)} times" + + +def test_paranoid_says_so_when_it_could_not_drop_the_cache(tmp_path: Path, + monkeypatch): + """A second read served out of memory compares the first read against + itself. That is worth nothing, and claiming otherwise is the failure mode + this whole file exists to prevent.""" + card = _card(tmp_path) + monkeypatch.setattr(integrity, "evict_from_cache", lambda _p: False) + job = engine.run(card, _options(tmp_path, paranoid=True)) + + assert any("second read" in w and "memory" in w for w in job.warnings), \ + job.warnings diff --git a/tests/test_engine.py b/tests/test_engine.py index 9c19897..c8d56ae 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -262,4 +262,101 @@ def run() -> None: assert not worker.is_alive(), "copy deadlocked waiting for the sentinel" assert (tmp_path / "out.bin").stat().st_size == engine.CHUNK_SIZE * 3 - assert result["digest"][0] == hashers.hash_file(source, "xxh3-64") + assert result["digest"].source_checksum == hashers.hash_file(source, "xxh3-64") + + +def test_a_reader_that_will_not_close_still_delivers_the_sentinel(tmp_path: Path, + monkeypatch): + """REGRESSION. The reader thread owes its consumer an end-of-file sentinel. + Closing the source runs on the way out, including out of a failure, so a + close that raises must not skip it — the consumer would block on get() + forever and the copy would hang instead of reporting the real error.""" + import builtins + import threading + + real_open = builtins.open + source = tmp_path / "clip.mov" + source.write_bytes(b"payload " * 500) + + class WontClose: + def __init__(self, handle): + self._handle = handle + + def read(self, size=-1): + return self._handle.read(size) + + def close(self): + raise RuntimeError("close is broken") + + def fake_open(path, mode="r", *args, **kwargs): + handle = real_open(path, mode, *args, **kwargs) + if Path(path).name == source.name and "b" in str(mode): + return WontClose(handle) + return handle + + monkeypatch.setattr(builtins, "open", fake_open) + + done = threading.Event() + + def run() -> None: + try: + engine._copy_fanout(source, [tmp_path / "out.bin"], "xxh3-64", + lambda _n: None) + finally: + done.set() + + worker = threading.Thread(target=run, daemon=True) + worker.start() + assert done.wait(timeout=20), "copy hung after the source failed to close" + monkeypatch.undo() + + +# ------------------------------------------------------------- companions + + +def _card_with_sidecar(tmp_path: Path) -> Path: + card = tmp_path / "card" + card.mkdir() + (card / "A001_C001.braw").write_bytes(b"a clip " * 400) + (card / "A001_C001.sidecar").write_bytes(b"the grade") + return card + + +def test_a_sidecar_is_linked_to_its_clip_in_the_job(tmp_path: Path): + job = engine.run(_card_with_sidecar(tmp_path), _options(tmp_path)) + + clip = next(f for f in job.files if f.name == "A001_C001.braw") + sidecar = next(f for f in job.files if f.name == "A001_C001.sidecar") + + assert sidecar.companion_of == clip.source + assert clip.companions == [sidecar.source] + assert clip.companion_of is None + + +def test_a_clip_separated_from_its_sidecar_is_a_warning(tmp_path: Path, + monkeypatch): + """A graded BRAW delivered without its .sidecar has lost the grade. One + Verified row and one Failed row twenty lines apart is not how anyone finds + that out.""" + import builtins + import errno + + card = _card_with_sidecar(tmp_path) + real_open = builtins.open + + def fake_open(path, mode="r", *args, **kwargs): + if Path(path).suffix == ".sidecar" and "b" in str(mode): + raise OSError(errno.EACCES, "permission denied") + return real_open(path, mode, *args, **kwargs) + + monkeypatch.setattr(builtins, "open", fake_open) + job = engine.run(card, _options(tmp_path)) + monkeypatch.undo() + + assert any("A001_C001.sidecar" in w and "belongs with it" in w + for w in job.warnings), job.warnings + + +def test_a_clean_offload_says_nothing_about_companions(tmp_path: Path): + job = engine.run(_card_with_sidecar(tmp_path), _options(tmp_path)) + assert not any("belongs with it" in w for w in job.warnings) diff --git a/tests/test_gui.py b/tests/test_gui.py index 48b88fd..cd81ea0 100644 --- a/tests/test_gui.py +++ b/tests/test_gui.py @@ -249,7 +249,7 @@ def test_simple_mode_builds_a_preset_from_its_controls(qapp, tmp_path): assert preset.destinations == [tmp_path / "dest"] assert preset.reports == ["pdf"] - assert preset.verification is VerificationMode.SOURCE_ONLY + assert preset.verification is VerificationMode.FULL def test_preset_panel_disables_run_for_a_preset_without_destinations(qapp, tmp_path): @@ -289,3 +289,188 @@ def test_source_drop_zone_reports_its_path(qapp, tmp_path): assert zone.path is None zone.set_path(tmp_path / "A001") assert zone.path == tmp_path / "A001" + + +# ------------------------------------------------------------- the UI sweep + + +def test_simple_mode_offers_the_second_read_and_it_reaches_the_engine(qapp, + tmp_path): + """A preset field the interface never exposes is a field nobody can use.""" + panel = SimpleModePanel() + assert panel.build_preset().paranoid is False + + panel._paranoid.setChecked(True) + assert panel.build_preset().to_options().paranoid is True + + +def test_the_preset_editor_round_trips_the_second_read(qapp): + from offloader.gui.preset_editor import PresetEditor + + editor = PresetEditor(Preset(name="Irreplaceable", paranoid=True)) + assert editor._paranoid.isChecked() + + editor._paranoid.setChecked(False) + editor._accept() + assert editor.result_preset.paranoid is False + + +def test_the_preset_editor_is_grouped_rather_than_one_flat_list(qapp): + """Sixteen fields in a single column read as a wall. The sections are the + difference between scanning for a setting and hunting for it.""" + from PySide6.QtWidgets import QLabel + + from offloader.gui.preset_editor import PresetEditor + + editor = PresetEditor(Preset(name="p")) + headings = [w.text() for w in editor.findChildren(QLabel) + if w.property("role") == "heading"] + assert headings == ["Preset", "Copying", "Reports"] + + # Regrouping a form is exactly the change that silently drops a field. + for name in ("_name", "_color", "_destinations", "_algorithm", + "_verification", "_thumbnails", "_naming", "_excludes", + "_logo", "_footer", "_preserve", "_skip", "_paranoid"): + assert getattr(editor, name).parent() is not None, f"{name} is orphaned" + + +# ---------------------------------------------------------------- throughput + +def _running_item(**kwargs): + from offloader.gui.worker import QueueItem + + item = QueueItem(identifier=1, source=Path("card"), name="job", + preset=Preset(name="p"), **kwargs) + item.state = JobState.RUNNING + return item + + +def test_rate_is_windowed_not_a_lifetime_average(monkeypatch): + """A slow first minute must not read as a slow job forever. The regression + this pins: a card scan plus early probe stalls dragged the lifetime average + to 3.5 MB/s while clips were demonstrably flying past.""" + from offloader.gui import worker as worker_mod + + clock = {"now": 1000.0} + monkeypatch.setattr(worker_mod.time, "monotonic", lambda: clock["now"]) + + item = _running_item() + item.started_at = clock["now"] + + # Ten dead seconds of scanning, then a steady 100 MB/s. + clock["now"] += 10.0 + for _ in range(10): + clock["now"] += 1.0 + item.bytes_done += 100_000_000 + item.record_progress(item.bytes_done) + + lifetime = item.bytes_done / item.elapsed # 50 MB/s — the old lie + windowed = item.rate_bytes_per_sec + assert windowed == pytest.approx(100_000_000, rel=0.05) + assert windowed > 1.8 * lifetime + + +def test_rate_decays_during_a_stall_instead_of_freezing(monkeypatch): + from offloader.gui import worker as worker_mod + + clock = {"now": 0.0} + monkeypatch.setattr(worker_mod.time, "monotonic", lambda: clock["now"]) + + item = _running_item() + for _ in range(5): + clock["now"] += 1.0 + item.bytes_done += 100_000_000 + item.record_progress(item.bytes_done) + flowing = item.rate_bytes_per_sec + + clock["now"] += 3.0 # stall: no new bytes + assert item.rate_bytes_per_sec < flowing + clock["now"] += 10.0 # window fully drained + assert item.rate_bytes_per_sec == 0.0 + assert item.eta_seconds is None + + +def test_rate_survives_a_counter_reset_between_stages(monkeypatch): + """Copy and verify each count job bytes from zero; a delta computed across + that boundary would be negative garbage.""" + from offloader.gui import worker as worker_mod + + clock = {"now": 0.0} + monkeypatch.setattr(worker_mod.time, "monotonic", lambda: clock["now"]) + + item = _running_item() + for _ in range(3): + clock["now"] += 1.0 + item.bytes_done += 100_000_000 + item.record_progress(item.bytes_done) + + item.bytes_done = 0 # verify stage begins + item.record_progress(0) + for _ in range(2): + clock["now"] += 1.0 + item.bytes_done += 50_000_000 + item.record_progress(item.bytes_done) + assert item.rate_bytes_per_sec == pytest.approx(50_000_000, rel=0.05) + + +def test_checksum_pickers_say_what_the_choice_costs(qapp, tmp_path): + """MD5 sits in the same list as XXHash3-64; without the cost attached they + read as equals, and the difference is 40x on the copy path.""" + from offloader.gui.preset_editor import PresetEditor + + panel = SimpleModePanel() + texts = [panel._algorithm.itemText(i) + for i in range(panel._algorithm.count())] + md5 = next(t for t in texts if t.startswith("MD5")) + assert "slower" in md5 + assert any(t.startswith("XXHash3-64") and "fastest" in t for t in texts) + + editor = PresetEditor(Preset(name="p")) + texts = [editor._algorithm.itemText(i) + for i in range(editor._algorithm.count())] + assert any("slower" in t for t in texts) + + # The stored key must stay the bare algorithm id, not the display text. + assert panel._algorithm.currentData() in {"xxh3-64"} + + +def test_start_offload_says_add_to_queue_while_the_queue_is_busy(qapp, tmp_path): + """Jobs run one at a time; while one runs, the button enqueues rather than + starts, and promising "Start offload" was a small lie.""" + panel = SimpleModePanel() + (tmp_path / "card").mkdir() + panel.set_source(tmp_path / "card") + panel.add_destination(tmp_path / "out") + + assert panel._start.text() == "Start offload" + panel.set_queue_busy(True) + assert panel._start.text() == "Add to queue" + assert "after the current job" in panel._hint.text() + panel.set_queue_busy(False) + assert panel._start.text() == "Start offload" + assert "after the current job" not in panel._hint.text() + + +def test_active_summary_promotes_stage_file_and_rate(monkeypatch): + from offloader.gui import queue_view + from offloader.gui import worker as worker_mod + + clock = {"now": 0.0} + monkeypatch.setattr(worker_mod.time, "monotonic", lambda: clock["now"]) + + item = _running_item() + item.stage = "copy" + item.current_file = "A003_C001.braw" + item.fraction = 0.30 + item.bytes_total = 1_000_000_000 + for _ in range(3): + clock["now"] += 1.0 + item.bytes_done += 100_000_000 + item.record_progress(item.bytes_done) + + text = queue_view._active_summary(item) + assert text.startswith("Copying A003_C001.braw") + assert "30%" in text and "/s" in text + + item.state = JobState.PAUSED + assert queue_view._active_summary(item).startswith("Paused") diff --git a/tests/test_gui_drives.py b/tests/test_gui_drives.py index 7a6660b..94b332d 100644 --- a/tests/test_gui_drives.py +++ b/tests/test_gui_drives.py @@ -38,8 +38,13 @@ def _pump(app, predicate, timeout_ms: int = 10_000) -> bool: return True +def _one_batch(*volumes): + """A scan_batches stand-in delivering a single, final batch.""" + return lambda: iter([(list(volumes), True)]) + + def test_watcher_reports_volumes_from_a_pool_thread(qapp, monkeypatch): - monkeypatch.setattr(drives, "list_volumes", lambda: [_volume("E:/")]) + monkeypatch.setattr(drives, "scan_batches", _one_batch(_volume("E:/"))) watcher = drives.VolumeWatcher() received: list[list] = [] watcher.volumesChanged.connect(received.append) @@ -56,7 +61,7 @@ def test_scan_survives_its_task_wrapper_being_collected(qapp, monkeypatch): a deleted object and the drive panel silently stops updating.""" import gc - monkeypatch.setattr(drives, "list_volumes", lambda: [_volume("E:/")]) + monkeypatch.setattr(drives, "scan_batches", _one_batch(_volume("E:/"))) watcher = drives.VolumeWatcher() received: list[list] = [] watcher.volumesChanged.connect(received.append) @@ -71,7 +76,7 @@ def test_a_failing_scan_is_reported_as_empty(qapp, monkeypatch): def boom(): raise OSError("drive not ready") - monkeypatch.setattr(drives, "list_volumes", boom) + monkeypatch.setattr(drives, "scan_batches", boom) watcher = drives.VolumeWatcher() received: list[list] = [] watcher.volumesChanged.connect(received.append) @@ -83,7 +88,7 @@ def boom(): def test_stop_suppresses_a_late_result(qapp, monkeypatch): - monkeypatch.setattr(drives, "list_volumes", lambda: [_volume("E:/")]) + monkeypatch.setattr(drives, "scan_batches", _one_batch(_volume("E:/"))) watcher = drives.VolumeWatcher() received: list[list] = [] watcher.volumesChanged.connect(received.append) @@ -97,8 +102,12 @@ def test_stop_suppresses_a_late_result(qapp, monkeypatch): def test_refresh_is_not_reentrant(qapp, monkeypatch): calls = [] - monkeypatch.setattr(drives, "list_volumes", - lambda: calls.append(1) or [_volume("E:/")]) + + def scan(): + calls.append(1) + yield [_volume("E:/")], True + + monkeypatch.setattr(drives, "scan_batches", scan) watcher = drives.VolumeWatcher() watcher.refresh() watcher.refresh() # ignored while the first is in flight @@ -161,3 +170,78 @@ def test_row_buttons_emit_the_volume_root(qapp): row.useAsDestination.emit(row.volume.root) assert sources == [Path("E:/")] assert destinations == [Path("E:/")] + + +# ------------------------------------------------------------------ batching + + +def _network(root: str, label: str = "NAS") -> Volume: + return Volume(root=Path(root), label=label, filesystem="NTFS", + total_bytes=100_000_000_000, free_bytes=50_000_000_000, + drive_type="network", is_camera_card=False) + + +def test_local_drives_are_delivered_before_network_shares(monkeypatch): + """The batch split is the fix for a panel that waited on the slowest SMB + share before showing the card reader plugged in next to the machine.""" + local_root = (Path("E:/"), "removable") + remote_root = (Path("H:/"), "network") + monkeypatch.setattr(drives, "list_roots", lambda: [local_root, remote_root]) + monkeypatch.setattr( + drives, "probe_many", + lambda roots: [_volume("E:/") if kind != "network" else _network("H:/") + for _, kind in roots]) + + batches = list(drives.scan_batches()) + assert len(batches) == 2 + first, final = batches + assert first[1] is False and final[1] is True + assert [v.drive_type for v in first[0]] == ["removable"] + assert sorted(v.drive_type for v in final[0]) == ["network", "removable"] + + +def test_no_network_shares_means_a_single_final_batch(monkeypatch): + monkeypatch.setattr(drives, "list_roots", + lambda: [(Path("E:/"), "removable")]) + monkeypatch.setattr(drives, "probe_many", lambda roots: [_volume("E:/")]) + batches = list(drives.scan_batches()) + assert len(batches) == 1 + assert batches[0][1] is True + + +def test_partial_batch_keeps_the_known_network_shares(qapp): + """While a poll's slow network probes are still in flight, the local-only + partial batch must not tear the share rows down for a few seconds.""" + watcher = drives.VolumeWatcher() + received: list[list] = [] + watcher.volumesChanged.connect(received.append) + + watcher._volumes = [_volume("E:/"), _network("H:/")] + watcher._on_batch([_volume("E:/")], False) + + assert received, "partial batch was not reported" + roots = [v.root for v in received[-1]] + assert Path("H:/") in roots and Path("E:/") in roots + + +def test_scanning_state_wraps_a_refresh(qapp, monkeypatch): + monkeypatch.setattr(drives, "scan_batches", _one_batch(_volume("E:/"))) + watcher = drives.VolumeWatcher() + states: list[bool] = [] + watcher.scanningChanged.connect(states.append) + + watcher.refresh() + assert states == [True] + assert _pump(qapp, lambda: len(states) == 2), "scan never finished" + assert states == [True, False] + watcher.stop() + + +def test_refresh_button_reports_the_scan(qapp): + panel = drives.DrivesPanel() + panel._on_scanning(True) + assert panel._refresh.text() == "Scanning…" + assert not panel._refresh.isEnabled() + panel._on_scanning(False) + assert panel._refresh.text() == "Refresh" + assert panel._refresh.isEnabled() diff --git a/tests/test_naming.py b/tests/test_naming.py index 0f96aae..f081603 100644 --- a/tests/test_naming.py +++ b/tests/test_naming.py @@ -1,8 +1,11 @@ from __future__ import annotations import datetime as _dt +import os from pathlib import Path +import pytest + from offloader import naming WHEN = _dt.datetime(2026, 8, 7, 16, 41, 26) @@ -52,3 +55,24 @@ def test_volume_label_falls_back_to_folder_name(): values = naming.context(Path("/Volumes/A001"), volume_label="CARD_A", when=WHEN) assert values["volume"] == "CARD_A" assert naming.context(Path("/Volumes/A001"), when=WHEN)["volume"] == "A001" + + +def test_card_token_prefers_the_volume_label_for_a_bare_root(): + """A card offloaded from its root has no folder name; what the operator + calls it is the volume label — A003, not a drive letter. + + Uses each platform's own root: "E:/" is only a root on Windows — on POSIX + it is a relative path whose *name* is "E:", which is exactly the case the + folder name should win.""" + root = Path("C:/") if os.name == "nt" else Path("/") + values = naming.context(root, volume_label="A003") + assert values["card"] == "A003" + # A real folder name still wins; the label describes the volume, the + # folder describes the selection. + values = naming.context(root / "DCIM", volume_label="A003") + assert values["card"] == "DCIM" + + +@pytest.mark.skipif(os.name != "nt", reason="drive letters are a Windows thing") +def test_card_token_falls_back_to_the_drive_letter_without_a_label(): + assert naming.context(Path("E:/"))["card"] == "E" diff --git a/tests/test_presets.py b/tests/test_presets.py index cc79c02..510946d 100644 --- a/tests/test_presets.py +++ b/tests/test_presets.py @@ -120,6 +120,7 @@ def test_to_options_carries_settings_through(tmp_path: Path): excludes=["*.tmp"], preserve_structure=False, skip_existing=True, + paranoid=True, ) options = preset.to_options(job_name="A001") @@ -130,6 +131,7 @@ def test_to_options_carries_settings_through(tmp_path: Path): assert options.job_name == "A001" assert options.preserve_structure is False assert options.skip_existing is True + assert options.paranoid is True assert "*.tmp" in options.excludes assert ".DS_Store" in options.excludes # defaults still applied @@ -184,7 +186,7 @@ def test_nonsense_types_fall_back_rather_than_raise(): assert preset.retry_attempts == 3 assert preset.excludes == [] assert preset.algorithm in ALGORITHMS - assert preset.verification is VerificationMode.SOURCE_ONLY + assert preset.verification is VerificationMode.FULL def test_an_explicitly_empty_report_list_is_respected(): @@ -192,3 +194,16 @@ def test_an_explicitly_empty_report_list_is_respected(): key should fall back to the default.""" assert Preset.from_dict({"reports": []}).reports == [] assert Preset.from_dict({}).reports == ["pdf"] + + +def test_paranoid_survives_a_save_and_reload(tmp_path: Path): + """It changes what "Verified" is worth, so it has to persist with the + preset rather than being re-chosen each time.""" + store = PresetStore(tmp_path / "presets.json") + store.presets = [Preset(name="Irreplaceable", destinations=[tmp_path / "d"], + paranoid=True)] + store.save() + + assert PresetStore(tmp_path / "presets.json").presets[0].paranoid is True + # And a preset written before the option existed still loads. + assert Preset.from_dict({"name": "old"}).paranoid is False diff --git a/tests/test_profile.py b/tests/test_profile.py index 816ade8..6792774 100644 --- a/tests/test_profile.py +++ b/tests/test_profile.py @@ -114,3 +114,31 @@ def test_preset_defaults_to_media_and_tolerates_missing_key(): assert Preset.from_dict({"name": "legacy"}).profile is Profile.MEDIA # A garbage value must never brick a load. assert Preset.from_dict({"name": "bad", "profile": "nonsense"}).profile is Profile.MEDIA + + +def test_the_data_profile_does_not_invent_companions(tmp_path: Path): + """Stem-matching says a `.sidecar` belongs to a clip. Under the data + profile nothing is a clip, so a dataset that happens to share a stem with + its metadata file would be linked on no evidence but the name.""" + card = tmp_path / "run_1440" + card.mkdir() + (card / "capture.h5").write_bytes(b"instrument data " * 200) + (card / "capture.xmp").write_bytes(b"") + + job = engine.run(card, _options(tmp_path, profile=Profile.DATA)) + + assert all(f.companion_of is None for f in job.files) + assert all(f.companions == [] for f in job.files) + + +def test_the_media_profile_still_groups_them(tmp_path: Path): + card = tmp_path / "A001" + card.mkdir() + (card / "A001_C001.braw").write_bytes(b"a clip " * 400) + (card / "A001_C001.sidecar").write_bytes(b"the grade") + + job = engine.run(card, _options(tmp_path, profile=Profile.MEDIA, + extra_probe=False)) + + sidecar = next(f for f in job.files if f.name == "A001_C001.sidecar") + assert sidecar.companion_of is not None diff --git a/tests/test_reports.py b/tests/test_reports.py index 540b42d..73982ed 100644 --- a/tests/test_reports.py +++ b/tests/test_reports.py @@ -265,3 +265,15 @@ def test_mhl_preserves_ordinary_unicode(sample_job: Job, tmp_path: Path): ) root = ET.parse(write_mhl(sample_job, tmp_path / "j.mhl")).getroot() assert any("café_日本" in (n.findtext("file") or "") for n in root.findall("hash")) + + +def test_pdf_document_title_carries_route_and_date(sample_job: Job, tmp_path: Path): + """A stack of reports is told apart by this title in a file manager or a + browser tab; "Offload Job Report" identified nothing.""" + path = write_pdf(sample_job, tmp_path / "JobReport.pdf") + with fitz.open(path) as document: + title = document.metadata["title"] + assert sample_job.name in title + assert str(sample_job.source_root) in title + assert str(sample_job.destination_roots[0]) in title + assert f"{sample_job.started:%Y-%m-%d}" in title diff --git a/tests/test_retry.py b/tests/test_retry.py index e169bf3..849ad7e 100644 --- a/tests/test_retry.py +++ b/tests/test_retry.py @@ -172,7 +172,12 @@ def _options(tmp_path: Path, **overrides) -> engine.OffloadOptions: class _FlakyReader: - """A reader that fails the first N whole-file attempts, then works.""" + """A reader that fails its first N reads, then works. + + Stands in for what `open` returns, so it has to carry the parts of a binary + file the engine actually uses — `seek` and `close` as well as `read`, since + recovering a bad chunk reopens the source and seeks back to it. + """ def __init__(self, handle, failures: dict, limit: int): self._handle = handle @@ -185,6 +190,12 @@ def read(self, size=-1): raise _os_error(errno.EIO, winerror=1117) return self._handle.read(size) + def seek(self, offset, whence=0): + return self._handle.seek(offset, whence) + + def close(self): + self._handle.close() + def __enter__(self): return self @@ -272,6 +283,12 @@ def read(self, size=-1): raise _os_error(errno.EIO, winerror=1117) return self._handle.read(size) + def seek(self, offset, whence=0): + return self._handle.seek(offset, whence) + + def close(self): + self._handle.close() + def __enter__(self): return self @@ -351,3 +368,254 @@ def test_retry_is_configurable_from_a_preset(tmp_path: Path): restored = Preset.from_dict(preset.to_dict()) assert restored.retry_attempts == 7 assert restored.retry_wait == pytest.approx(0.5) + + +# --------------------------------------------------------- chunk-level retry + + +class _BadSector: + """A reader that fails every read starting at one offset, `times` times. + + Records the offset of every read attempted, across reopens, which is what + lets a test tell a chunk-level retry from a restart of the whole file: a + restart reads offset 0 again, a chunk-level retry does not. + """ + + def __init__(self, handle, log: list, failures: dict, offset: int, times: int): + self._handle = handle + self._log = log + self._failures = failures + self._offset = offset + self._times = times + + def read(self, size=-1): + at = self._handle.tell() + self._log.append(at) + if at == self._offset and self._failures["n"] < self._times: + self._failures["n"] += 1 + raise _os_error(errno.EIO, winerror=1117) + return self._handle.read(size) + + def seek(self, offset, whence=0): + return self._handle.seek(offset, whence) + + def close(self): + self._handle.close() + + def __enter__(self): + return self + + def __exit__(self, *args): + self._handle.close() + + +def _patch_bad_sector(monkeypatch, card: Path, log: list, failures: dict, + offset: int, times: int) -> None: + real_open = builtins.open + + def flaky_open(path, mode="r", *args, **kwargs): + handle = real_open(path, mode, *args, **kwargs) + try: + inside = Path(path).resolve().is_relative_to(card.resolve()) + except (OSError, ValueError): + inside = False + if inside and "r" in str(mode) and "b" in str(mode): + return _BadSector(handle, log, failures, offset, times) + return handle + + monkeypatch.setattr(builtins, "open", flaky_open) + + +def _chunked_card(tmp_path: Path, chunks: int) -> tuple[Path, bytes]: + card = tmp_path / "card" + card.mkdir() + payload = bytes(range(256)) * (engine.CHUNK_SIZE * chunks // 256) + (card / "A001_C001.mov").write_bytes(payload) + return card, payload + + +def test_a_bad_sector_is_recovered_without_re_reading_the_file( + tmp_path: Path, monkeypatch +): + """The point of retrying per chunk. Restarting a 79 GB clip to recover a + few bytes near the end is most of an hour; re-reading the chunk is a + moment.""" + monkeypatch.setattr(engine, "CHUNK_SIZE", 4096) + card, payload = _chunked_card(tmp_path, 3) + log: list[int] = [] + _patch_bad_sector(monkeypatch, card, log, {"n": 0}, offset=4096, times=1) + + job = engine.run(card, _options(tmp_path)) + monkeypatch.undo() + + assert job.final_status == "Verified" + assert (tmp_path / "dest" / "A001_C001.mov").read_bytes() == payload + assert log.count(0) == 1, f"the file was restarted: {log}" + + +def test_a_recovered_chunk_is_reported_with_where_it_was(tmp_path: Path, + monkeypatch): + monkeypatch.setattr(engine, "CHUNK_SIZE", 4096) + card, _payload = _chunked_card(tmp_path, 3) + _patch_bad_sector(monkeypatch, card, [], {"n": 0}, offset=8192, times=1) + + job = engine.run(card, _options(tmp_path)) + monkeypatch.undo() + + assert any("byte 8192" in w and "may be failing" in w for w in job.warnings), \ + job.warnings + + +def test_a_sector_that_never_reads_does_not_restart_the_whole_file( + tmp_path: Path, monkeypatch +): + """Once the chunk has had every attempt the policy allows, running the same + attempts again from byte zero only repeats them against the same fault.""" + monkeypatch.setattr(engine, "CHUNK_SIZE", 4096) + card, _payload = _chunked_card(tmp_path, 3) + log: list[int] = [] + _patch_bad_sector(monkeypatch, card, log, {"n": 0}, offset=4096, times=99) + + job = engine.run(card, _options(tmp_path)) + monkeypatch.undo() + + assert job.final_status == "Failed" + assert log.count(0) == 1, f"the file was restarted: {log}" + assert log.count(4096) == 3, f"the chunk got {log.count(4096)} attempts: {log}" + + +def test_the_failure_still_names_the_offset_that_could_not_be_read( + tmp_path: Path, monkeypatch +): + monkeypatch.setattr(engine, "CHUNK_SIZE", 4096) + card, _payload = _chunked_card(tmp_path, 3) + _patch_bad_sector(monkeypatch, card, [], {"n": 0}, offset=4096, times=99) + + job = engine.run(card, _options(tmp_path)) + monkeypatch.undo() + + assert "offset 4096" in job.notes, job.notes + + +def test_writes_are_still_retried_at_the_whole_file(tmp_path: Path, monkeypatch): + """A read that fails produced nothing, so it can be resumed. A write that + fails part-way leaves the destination at a length the copy loop does not + know, so it starts over.""" + monkeypatch.setattr(engine, "CHUNK_SIZE", 4096) + card, payload = _chunked_card(tmp_path, 2) + real_open = builtins.open + state = {"failed": False} + + class FailingWrite: + def __init__(self, handle): + self._handle = handle + + def write(self, data): + if not state["failed"]: + state["failed"] = True + raise _os_error(errno.EIO, winerror=1117) + return self._handle.write(data) + + def flush(self): + return self._handle.flush() + + def fileno(self): + return self._handle.fileno() + + def close(self): + self._handle.close() + + def flaky_open(path, mode="r", *args, **kwargs): + handle = real_open(path, mode, *args, **kwargs) + if "w" in str(mode) and "b" in str(mode): + return FailingWrite(handle) + return handle + + monkeypatch.setattr(builtins, "open", flaky_open) + job = engine.run(card, _options(tmp_path)) + monkeypatch.undo() + + assert job.final_status == "Verified" + assert (tmp_path / "dest" / "A001_C001.mov").read_bytes() == payload + + +class _FlakySectors: + """Fails the first read at each of several offsets, then lets it through. + + A card failing over a stretch rather than at one sector, which is the case + that decides how the recovery is reported. + """ + + def __init__(self, handle, offsets: set[int], seen: set[int]): + self._handle = handle + self._offsets = offsets + self._seen = seen + + def read(self, size=-1): + at = self._handle.tell() + if at in self._offsets and at not in self._seen: + self._seen.add(at) + raise _os_error(errno.EIO, winerror=1117) + return self._handle.read(size) + + def seek(self, offset, whence=0): + return self._handle.seek(offset, whence) + + def close(self): + self._handle.close() + + def __enter__(self): + return self + + def __exit__(self, *args): + self._handle.close() + + +def test_a_run_of_recovered_sectors_is_one_warning_not_one_each( + tmp_path: Path, monkeypatch +): + """One warning per 8 MiB is how a dying card buries every other warning in + the job. The useful fact stops being which byte and becomes how much of the + file would not read first time.""" + monkeypatch.setattr(engine, "CHUNK_SIZE", 4096) + card, payload = _chunked_card(tmp_path, 6) + real_open = builtins.open + seen: set[int] = set() + + def flaky_open(path, mode="r", *args, **kwargs): + handle = real_open(path, mode, *args, **kwargs) + try: + inside = Path(path).resolve().is_relative_to(card.resolve()) + except (OSError, ValueError): + inside = False + if inside and "r" in str(mode) and "b" in str(mode): + return _FlakySectors(handle, {4096, 8192, 12288}, seen) + return handle + + monkeypatch.setattr(builtins, "open", flaky_open) + job = engine.run(card, _options(tmp_path)) + monkeypatch.undo() + + assert job.final_status == "Verified" + assert (tmp_path / "dest" / "A001_C001.mov").read_bytes() == payload + + recovered = [w for w in job.warnings if "recovered" in w] + assert len(recovered) == 1, recovered + assert "3 failed reads between byte 4096 and byte 12288" in recovered[0] + assert "may be failing" in recovered[0] + + +def test_a_single_recovered_sector_is_still_named_exactly(tmp_path: Path, + monkeypatch): + """Bounding a range is only worth it when there is a range. One bad sector + keeps the offset that found it.""" + monkeypatch.setattr(engine, "CHUNK_SIZE", 4096) + card, _payload = _chunked_card(tmp_path, 3) + _patch_bad_sector(monkeypatch, card, [], {"n": 0}, offset=8192, times=1) + + job = engine.run(card, _options(tmp_path)) + monkeypatch.undo() + + recovered = [w for w in job.warnings if "recovered" in w] + assert len(recovered) == 1, recovered + assert "at byte 8192 on attempt 2" in recovered[0] diff --git a/tools/screenshots.py b/tools/screenshots.py new file mode 100644 index 0000000..b9c9b9f --- /dev/null +++ b/tools/screenshots.py @@ -0,0 +1,185 @@ +"""Render the desktop app to the PNGs the README embeds. + + python tools/screenshots.py # writes docs/images/ + python tools/screenshots.py OUT_DIR + +Screenshots go stale the moment the interface moves, so they are generated +rather than captured by hand. What the app would otherwise read off the machine +running this is replaced: + +- **The config directory is a throwaway.** Real presets, settings and offload + history are neither read nor written, and the presets in the pictures are + seeded into the sandbox from `PRESETS` below. +- **The drive panel is fed invented volumes.** Whatever is actually mounted + would otherwise put its label and free space into a public README. + +Everything else is the real thing: the status bar reports on the ffmpeg that is +genuinely on `PATH`, and the throughput and ETA are computed by the app from +the queue state set up here. Nothing is copied — the queue items are built +directly rather than enqueued, so no job ever runs. +""" + +from __future__ import annotations + +import json +import os +import sys +import tempfile +import time +from pathlib import Path + +REPO = Path(__file__).resolve().parent.parent +DEFAULT_OUT = REPO / "docs" / "images" + +# This has to happen before the app reads any of it. `config_dir()` checks the +# environment on each call, but the preset store is built inside +# `MainWindow.__init__`, so the sandbox must be in place before the import +# below — hence the deliberate E402s. +_sandbox = Path(tempfile.mkdtemp(prefix="offloader-shots-")) +os.environ["APPDATA"] = str(_sandbox) +os.environ["XDG_CONFIG_HOME"] = str(_sandbox) + +from PySide6.QtCore import Qt # noqa: E402 +from PySide6.QtWidgets import QApplication, QSplitter # noqa: E402 + +from offloader.config import config_dir # noqa: E402 +from offloader.gui import drives, theme # noqa: E402 +from offloader.gui.main_window import MainWindow # noqa: E402 +from offloader.gui.preset_editor import PresetEditor # noqa: E402 +from offloader.gui.worker import JobState, QueueItem # noqa: E402 +from offloader.models import VerificationMode # noqa: E402 +from offloader.presets import PRESET_COLORS, Preset # noqa: E402 +from offloader.volumes import Volume # noqa: E402 + +GB = 1024 ** 3 + +PRESETS = [ + Preset(name="Dailies — single copy", + destinations=[Path(r"D:\Dailies")], + algorithm="xxh3-64", verification=VerificationMode.SOURCE_ONLY, + reports=["pdf"], color=PRESET_COLORS[0], + naming_template="{card}", use_count=34), + Preset(name="Archive — two copies, full verify", + destinations=[Path(r"D:\Archive\2026"), Path(r"N:\cold\2026")], + algorithm="xxh3-64", verification=VerificationMode.FULL, + reports=["pdf", "csv", "ascmhl"], color=PRESET_COLORS[1], + naming_template="{card}_{date}", use_count=12), + Preset(name="Irreplaceable — read twice", + destinations=[Path(r"D:\Masters"), Path(r"N:\cold\masters")], + algorithm="sha256", verification=VerificationMode.FULL, + reports=["pdf", "ascmhl", "html"], color=PRESET_COLORS[2], + naming_template="{card}_{date}", paranoid=True, use_count=3), +] + +#: Two cards and three fixed disks — enough for the panel to show the `CARD` +#: badge, the sort that puts cards first, and an amber bar past 80 %. +VOLUMES = [ + Volume(root=Path("E:\\"), label="A001 (PYXIS)", filesystem="exFAT", + total_bytes=512 * GB, free_bytes=61 * GB, + drive_type="removable", is_camera_card=True), + Volume(root=Path("F:\\"), label="B002 (KOMODO)", filesystem="exFAT", + total_bytes=256 * GB, free_bytes=203 * GB, + drive_type="removable", is_camera_card=True), + Volume(root=Path("C:\\"), label="System", filesystem="NTFS", + total_bytes=1024 * GB, free_bytes=402 * GB), + Volume(root=Path("D:\\"), label="Shuttle", filesystem="NTFS", + total_bytes=8 * 1024 * GB, free_bytes=5734 * GB), + Volume(root=Path("N:\\"), label="Cold storage", filesystem="NTFS", + total_bytes=48 * 1024 * GB, free_bytes=7100 * GB), +] + + +def seed_config() -> None: + (config_dir() / "presets.json").write_text( + json.dumps([p.to_dict() for p in PRESETS], indent=2), encoding="utf-8") + + +def fill_queue(window: MainWindow) -> None: + """One job running, one waiting, one done. + + Built by hand rather than enqueued: `enqueue` would start a real offload, + and there is nothing here to copy. + """ + controller = window.controller + controller._auto_start = False + now = time.monotonic() + + controller.items = [ + QueueItem(identifier=1, source=Path("E:\\"), name="A001", + preset=PRESETS[1], state=JobState.RUNNING, fraction=0.62, + stage="copy", current_file="A001_08041254_C007.braw", + bytes_done=int(283.4 * GB), bytes_total=int(457.0 * GB), + started_at=now - 512), + QueueItem(identifier=2, source=Path("F:\\"), name="B002_080426", + preset=PRESETS[2], state=JobState.QUEUED, + bytes_total=int(198.0 * GB)), + QueueItem(identifier=3, source=Path("E:\\"), name="A002", + preset=PRESETS[0], state=JobState.DONE, fraction=1.0, + stage="verify", bytes_done=int(129.7 * GB), + bytes_total=int(129.7 * GB), + started_at=now - 940, finished_at=now - 512), + ] + controller.itemsChanged.emit() + + +def settle(app: QApplication, rounds: int = 12) -> None: + for _ in range(rounds): + app.processEvents() + + +def shoot(widget, out: Path, name: str) -> None: + pixmap = widget.grab() + pixmap.save(str(out / name), "PNG") + print(f"{name} {pixmap.width()}x{pixmap.height()}") + + +def main(argv: list[str] | None = None) -> int: + argv = sys.argv[1:] if argv is None else argv + out = Path(argv[0]).resolve() if argv else DEFAULT_OUT + out.mkdir(parents=True, exist_ok=True) + seed_config() + + # The panel scans real volumes on a worker thread; give it ours instead. + drives.list_volumes = lambda: list(VOLUMES) + + app = QApplication([]) + theme.apply(app) + + window = MainWindow() + window.resize(1280, 900) + window.show() + settle(app) + + # The queue is the part a reader most needs to see, and the default split + # leaves it a row and a half tall. + for splitter in window.findChildren(QSplitter): + if splitter.orientation() == Qt.Vertical: + splitter.setSizes([500, 400]) + + window.drives._rebuild(list(VOLUMES)) + fill_queue(window) + settle(app) + + window._set_mode(0) + settle(app) + shoot(window, out, "app-preset-mode.png") + + window._set_mode(1) + # A named folder rather than the bare drive root: the drop zone shows the + # name above the full path, and for a root both lines read "E:\". + window.simple.set_source(Path(r"E:\A001_PYXIS")) + window.simple.destinations.set_paths([Path(r"D:\Archive\2026"), + Path(r"N:\cold\2026")]) + settle(app) + shoot(window, out, "app-simple-mode.png") + + editor = PresetEditor(PRESETS[2]) + editor.show() + settle(app) + shoot(editor, out, "app-preset-editor.png") + + return 0 + + +if __name__ == "__main__": + raise SystemExit(main())