Sound cards end to end (iXML slate), proxies-first ordering, and run.py - #5
Open
owenpkent wants to merge 6 commits into
Open
Sound cards end to end (iXML slate), proxies-first ordering, and run.py#5owenpkent wants to merge 6 commits into
owenpkent wants to merge 6 commits into
Conversation
`python run.py` opens the desktop app; `python run.py <anything>` forwards to the CLI untouched, exit codes included. The point of it is the import path. It prepends `src/` rather than appending, so a fresh clone, or a branch checked out on a machine that also has a regular `pip install offloader`, runs the code sitting next to the script instead of resolving `import offloader` to site-packages and quietly testing a different version. With an editable install both paths are the same tree and this changes nothing. `prefer_local_source` removes any existing entry before inserting, so re-entry or an inherited PYTHONPATH cannot leave `src/` sitting behind site-packages, which is the exact failure it exists to prevent.
Camera proxies are a rounding error next to the originals: a 27-clip BRAW card is roughly 110 GB of original against 0.4 GB of H.264. Moving them first costs well under a percent of the job's runtime and hands the edit something to cut with minutes in, rather than after the last original has landed. That is why it is the default rather than an opt-in. It also improves the contact sheet. Thumbnails for an original ffmpeg cannot decode are borrowed from the matching proxy, and `companions.thumbnail_source` looks at the destination before the source, so with the proxies already down that read comes off the destination disk instead of competing with the copy for the card. Transfer order and report order are now separate. Walking the reordered list would also have reordered `job.files`, so the contact sheet would have opened with the proxy folder and buried the clips behind it: an I/O optimisation paying for itself with a worse report. `run` sorts the rows back into tree order before returning, and a test asserts the rows and checksums come out identical whichever order the job ran in. `is_proxy` reuses `companions.PROXY_DIRECTORIES` rather than starting a second list of the names cameras use, and matches on directories only, so a clip that happens to be called `proxy` is still a clip. The partition is stable, and a card with no proxy directory is returned untouched. `rescan` is deliberately untouched: `report` copies nothing, so ordering there is purely report order. Selectable as `--proxies-first` / `--originals-first`, and as a checkbox in Simple mode and the preset editor. Presets saved before the option existed inherit the new default rather than silently keeping the old order.
The media profile already probed .wav/.aif/.bwf, so a sound card copied and verified fine. What it could not do was describe itself: the summary read "(0 video)", the report was a picture report with the interesting fields blank, and the one field a sound report is actually read for was dropped on the floor. Counting. `MediaInfo.is_audio` and `Job.audio_files` sit beside the video count, defined as "no picture" rather than "has sound" so the two stay disjoint. A clip with dialogue is a video file and is counted once; the numbers still add up to something a reader can check. The CLI reports "(48 audio)" on a sound card and "(54 video, 12 audio)" on a card carrying both. The header cell. The reference grid is exactly four rows deep and its zips are strict, so a sound card borrows the "Video Files" cell instead of growing a fifth row: on a card with no picture, "Video Files: 0" is the one number on the page that tells the reader nothing. Every coordinate is unchanged. Timecode. A broadcast WAV stores its origin as `time_reference`, the sample count since midnight, which divided by the sample rate gives the start clock. It renders as 10:00:00.000, not 10:00:00:00, and that is deliberate: the frame rate needed to turn the remainder into frames lives in iXML, which ffprobe does not read. Printing frames would mean picking a rate at random and showing a guess in the field the report exists for, so the milliseconds are exact and visibly not frames. Format. Bit depth is taken from `bits_per_raw_sample`, falling back past the zero several compressed codecs report in `bits_per_sample`. An audio-only file renders "LINEAR PCM 48 kHz 24-bit"; a clip's own audio line is left exactly as the reference renders it, so picture reports still match ShotPut digit for digit and the two documents agree clip for clip. The CSV gains Audio Codec, Audio Channels, Sample Rate (Hz) and Bit Depth, blank for files with no track. Thumbnails needed no change: the engine already gates them on `is_video`, and the layout already swaps the contact sheet for a filmstrip glyph on a file with no picture. Verified end to end against real broadcast WAVs written by ffmpeg with a bext chunk, not only against captured-shape JSON.
ffprobe does not surface iXML at all, so every field a sound report is actually read for was invisible: scene, take, sound roll, the mixer's note, the circled-take flag, and what each track was. A sound card described itself as a duration and a sample rate. `offloader.ixml` walks the RIFF chunks directly, the way `braw` walks the moov atom, for a few seeks and a few KB. It is only entered for a WAV with no video stream, so a card of camera originals never opens a file for it. It also settles the timecode question the previous commit had to leave open. Printing frames needs three things and no one chunk has all three: the origin (bext, and again in iXML), the sample rate (fmt), and the frame rate -- which only iXML carries, in SPEED/TIMECODE_RATE. With it a take renders as 10:00:00:00 NDF. Without it the millisecond clock stays, for the same reason it was chosen: a frame count with no rate behind it is a guess printed in the one field the report exists for. Rendering. The PDF reads "Roll SR082226 / Scene 12A / Take 3 CIRCLED" over "Boom, Lav 1 LINEAR PCM 48 kHz 24-bit" -- naming the channels beats describing their shape, and it costs no extra line. The metadata block has seven baselines and a fully-slated take fills all seven exactly; the docs said five, which was stale, and now says seven. The CSV gains Recorder, Project, Track Names and Note, and fills the existing Reel, Scene, Take and Good Take columns from whichever department wrote the slate, so one column keeps meaning one thing whichever cart the card came off. A card is untrusted input, and a WAV truncated by a battery dying mid-write must not take the offload down with it. Every step of the walk advances, so a zero-size chunk cannot pin it; the payload is capped, so a corrupt size field is not an allocation; RF64's ds64 sizes are honoured, so an iXML chunk filed after 4 GB of audio is still reachable; Wave64 is declined rather than misread, because its GUID chunk identifiers would parse as garbage here; and a doctype or entity declaration is refused outright, which closes billion-laughs and XXE without taking on defusedxml. iXML text is escaped on its way into the HTML report. Each of those has a reproduction in tests/test_ixml.py. Fixtures are built by tests/bwf.py rather than shelled out to ffmpeg, which cannot write an iXML chunk at all -- so the parser is tested without needing ffmpeg on PATH.
Three features landed across four commits and left claims scattered in files that were never opened. This is the pass that reconciles them. The contradiction worth naming: the CHANGELOG's sound-card entry stated that timecode renders as 10:00:00.000 "because the frame rate lives in iXML, which ffprobe cannot read". True when written, and flatly contradicted two entries later by the iXML work that reads exactly that rate. Both sat in the same Unreleased block. The earlier entry now says the millisecond clock is what a file gets when the sample count is all it gives up, and points at the entry that turns it into frame timecode. README gains a "Proxies first" section, which the desktop-app section was already linking to before it existed. The intro said the media profile was for camera cards; it covers the sound cart too. The CSV bullet said "camera metadata" when the slate now comes from whichever department wrote it. Both GUI modes' new checkbox is documented where the modes are described. report-layout.md claimed five metadata baselines. The code has had seven for some time and a fully-slated sound take fills all seven, so the stale number would have read as a bug in the renderer rather than in the prose. The header grid's four rows are unchanged and stated separately, because the two grids are easy to confuse. braw.md's "thumbnails come from the proxy" section predates proxies-first: the proxy is now normally already at the destination when its clip is probed, so that read comes off the destination disk. Noted, with the point that nothing depends on the ordering. data-safety.md gains the three hazards the WAV parser introduced, since that file is the index of what is guarded. CONTRIBUTING's fixture table gains the broadcast-WAV builders, and its heading no longer says "camera card" now that a sound card is equally testable without hardware. ROADMAP gains a Sound section for what iXML leaves unread, so the honest list stays honest. Checked mechanically as well as by eye: every local link and anchor resolves, every flag named in prose exists in the CLI (bar --paranoid, which the roadmap correctly frames as unbuilt), and every source and test file named in the docs is on disk.
The four iXML tests added with the sound work drove `probe.probe` against a real file, so they only exercised anything on a runner with ffprobe: without it `_probe` returns an empty MediaInfo, `is_audio` is False, and the slate is never read. Three failed everywhere but the with-ffmpeg job; the fourth passed vacuously, asserting nothing. Stub the stream report the way the rest of the module already does, keeping the WAV on disk real for the iXML walk to read. That fourth test then bit, on a genuine fault: a raising `read_sound_info` was left to the blanket catch in `probe`, which throws away the whole document. A card pulled mid-read cost us the container, the tracks and the clock ffprobe had already returned. Catch it at the optional read instead.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Four changes that came out of a live 110 GB offload of a Blackmagic card.
Stacked on #4, so it targets
fuzz-edge-cases. Review that one first.Sound recorder cards are a first-class offload
The
mediaprofile already probed.wav/.aif/.bwf, so a sound card copied andverified fine. What it could not do was describe itself: the summary read
(0 video), the report was a picture report with the interesting fields blank,and the one field a sound report is actually read for was dropped on the floor.
is_audiois "no picture", not "has sound", so aclip with dialogue is counted once as video and the two numbers still add up
to something a reader can check.
(48 audio), or(54 video, 12 audio).four rows deep with strict zips, so a sound card borrows the
Video Filescell. Every coordinate is unchanged, and
Video Files: 0stops being the onenumber on the page that tells the reader nothing.
time_referenceis the sample count sincemidnight; divided by the sample rate it gives the start clock. It renders
10:00:00.000, not10:00:00:00, and that is deliberate: the frame rateneeded to convert the remainder lives in iXML, which ffprobe cannot read.
Printing frames would mean picking a rate at random and showing a guess in
the field the report exists for.
bits_per_raw_sample, falling backpast the zero several compressed codecs report. A clip's own audio line is
left exactly as the reference renders it, so picture reports still match
ShotPut digit for digit.
Thumbnails needed no change: the engine already gates them on
is_videoandthe layout already swaps the contact sheet for a filmstrip glyph.
Verified end to end against real broadcast WAVs written with a
bextchunk,not only against captured-shape JSON.
Copy proxies before the originals
Camera proxies are a rounding error next to the originals: the card that
prompted this is ~110 GB of BRAW against ~0.4 GB of H.264. Moving them first
costs well under a percent of the job's runtime and hands the edit something to
cut with minutes in. It also improves the contact sheet, which I did not expect
going in: BRAW thumbnails are borrowed from the matching proxy, and
companions.thumbnail_sourcechecks the destination before the source, sothat read now comes off the destination disk instead of competing with the copy
for the card.
Transfer order and report order are now separate concerns. Simply walking
the reordered list would also have reordered
job.files, so the contact sheetwould have opened with the proxy folder and buried the clips: an I/O
optimisation paying for itself with a worse report.
runsorts the rows backinto tree order, and a test asserts rows and checksums come out identical
whichever order the job ran in.
rescanis untouched:reportcopies nothing, so ordering there is purelyreport order.
--originals-firstrestores the old order, and presets savedbefore the option existed inherit the new default.
run.pypython run.pyopens the desktop app;python run.py <anything>forwards tothe CLI untouched, exit codes included. It prepends
src/to the import path,so a checkout beside an older
pip install offloaderruns the code you arelooking at rather than site-packages.
The sound slate, from iXML
ffprobe does not surface the
iXMLchunk at all, so every field a sound reportis read for was invisible.
offloader.ixmlwalks the RIFF chunks directly, theway
brawwalks the moov atom, for a few seeks and a few KB. It is onlyentered for a WAV with no video stream, so a card of camera originals never
opens a file for it.
It settles the timecode question the commit above had to leave open.
Printing frames needs three things and no one chunk has all three: the origin
(
bext, and again in iXML), the sample rate (fmt), and the frame rate,which only iXML carries in
SPEED/TIMECODE_RATE. With it, a take renders10:00:00:00 NDF. Without it the millisecond clock stays, for the reason itwas chosen.
The PDF reads
Roll SR082226 · Scene 12A · Take 3 CIRCLEDoverBoom, Lav 1 LINEAR PCM 48 kHz 24-bit— naming the channels beatsdescribing their shape, and costs no extra line. The metadata block has seven
baselines and a fully-slated take fills all seven;
report-layout.mdclaimedfive, which was stale.
The CSV gains
Recorder,Project,Track NamesandNote, and fills theexisting
Reel/Scene/Take/Good Takefrom whichever department wrote theslate, so one column keeps meaning one thing.
A card is untrusted input
A WAV truncated by a battery dying mid-write must not take the offload down
with it. Each of these has a reproduction in
tests/test_ixml.py:ds64sizes are honoured, so an iXML chunk filed after 4 GB of audiois still reachable
parse as garbage here
billion-laughs and XXE without taking on
defusedxmlas a dependencyFixtures are built by
tests/bwf.pyrather than shelled out to ffmpeg, whichcannot write an iXML chunk at all — so the parser is tested without ffmpeg on
PATH.
Updated totals: 597 passing (+115 over the base), ruff clean.