diff --git a/crates/sylpheed-export/src/audio.rs b/crates/sylpheed-export/src/audio.rs index c7253115..383ab131 100644 --- a/crates/sylpheed-export/src/audio.rs +++ b/crates/sylpheed-export/src/audio.rs @@ -79,6 +79,20 @@ pub struct BgmSpec { pub why: String, #[serde(default)] pub loop_why: Option, + /// How a bank's two sub-waves become one file. **Only `"sum"` is + /// implemented**, and this field exists to say so when it is not. + /// + /// 🔴 It was `stems_why` alone until 2026-08-30 — the *reason* was + /// deserialised and the *value* was not, so `stems` sat in + /// `authored/audio.json` being ignored by serde. Changing it to anything at + /// all did nothing and warned nobody, which is the seventh instance in this + /// port of an authored value with no reader. + /// + /// It is ASSERTED rather than implemented: a weighted mix is not written, + /// and inventing one would be a level decision nobody measured (HANDOFF Q10 + /// settles that the two waves are summed, not what wave 1 *is*). + #[serde(default)] + pub stems: Option, #[serde(default)] pub stems_why: Option, } @@ -370,6 +384,18 @@ pub fn export_bgm( role: &str, spec: &BgmSpec, ) -> Result> { + // Assert the authored value this function was built for, rather than + // silently doing something else. Only `sum` is implemented. + if let Some(mode) = spec.stems.as_deref() { + if mode != "sum" { + bail!( + "authored/audio.json bgm.{role}.stems is {mode:?}; \ + export_bgm implements only \"sum\" (HANDOFF Q10 settles that a \ + bank's two waves are summed; a weighting would be an unmeasured \ + level decision)" + ); + } + } let riffs = match media::sound_bank_riffs(source, &spec.bank) { Ok(r) if !r.is_empty() => r, Ok(_) => return Ok(None), diff --git a/docs/port/BLOCKED.md b/docs/port/BLOCKED.md index 25743535..e8395808 100644 --- a/docs/port/BLOCKED.md +++ b/docs/port/BLOCKED.md @@ -575,3 +575,30 @@ is a second, independent phase measurement and neither is wrong. ⚠️ The port cannot separate a phase error from a systematic error in how it draws the sweeps — a geometry mistake could be absorbed by shifting the phase. The sharpness of the basin argues against that, but one capture cannot settle it. + +--- + +## A failed export leaves a partial tree that reads as "not an export tree" + +*Derived from HANDOFF `9ca1eb5`. Raised 2026-08-30 by the port. **Not blocked on +anybody** — filed because the fix is a judgement about failure semantics, not a +bug.* + +When `sylpheed-export export` fails part-way — for instance on one of the new +authored-value assertions — it leaves `export/` **without a `manifest.json`**. +Every tool then reports *"has no manifest.json — is that an export tree?"*, which +reads as a broken harness rather than as the aftermath of a deliberate abort. + +It cost a wrong reading within minutes of being introduced: an audit of the +validator reported every case as "no manifest" and nearly concluded the validator +was checking nothing. + +Writing the manifest **last** is correct — a manifest is a claim about a finished +tree. So the candidate fixes are about the *message*, not the order: + +* leave a marker on failure that the next tool can name (`export/.failed`), or +* have `check` say *"no manifest — the last export did not finish"* when the tree + has screens but no manifest, which is exactly the distinguishable case. + +⚠️ Deliberately not chosen here: both change failure semantics across every tool, +and neither is measured against anything. It goes to whoever owns that call. diff --git a/docs/port/DECISIONS.md b/docs/port/DECISIONS.md index fefc82e2..8fb4d58e 100644 --- a/docs/port/DECISIONS.md +++ b/docs/port/DECISIONS.md @@ -6284,3 +6284,57 @@ instances of exactly that. Verified: clean boot with no invariant errors and unchanged timings; setting `left_right: "move"` produces the error; all five MODDING rules still pass; the oracle rows are unmoved. + +## `FORMAT.md` declared the port's own export invalid, and a failed export is not atomic + +Continuing the audit that has now found seven unexercised rules: `FORMAT.md` is +the **open format spec** — written for a stranger reading the tree with no access +to the disc or this exporter. So the question is whether what it promises is what +`sylpheed-export check` enforces. + +Five documented requirements, each broken in a copy of the tree: + +| broken | caught | +|---|---| +| `unresolved` removed from a screen | ✅ | +| `peak_dbfs` removed | ✅ | +| `peak_dbfs` = −120 (silence) | ✅ | +| `duration_s` removed | ✅ | +| `peak_dbfs` = 0.0 on an `se` | **passes** | + +### The last row is the doc's error, not the code's + +`FORMAT.md` said flatly that check *"refuses a tree whose peak is ≤ −90 dBFS or +**≥ 0 dBFS**"*. The implementation is kind-dependent and deliberate: a `bgm` is a +sum **we** produced, so a peak at or above full scale is our arithmetic and is +refused outright; an `se` or `voice` is a single wave off the disc, mastered near +full scale, whose lossy decode overshoots by a fraction of a dB, and those are +allowed to +1.0. + +🔴 **And the doc was wrong about the port's own export.** It ships `confirm` at +**+0.18 dBFS** and the `ADV` voice at **+0.31** — both above 0. A consumer +implementing a validator from `FORMAT.md` would have rejected a valid tree, and +the file that exists to let someone check our work without trusting us would have +been the thing that misled them. Corrected, with the +1.0 marked as the judgement +it is. + +✅ Verified both directions: a `bgm` forced to 0.0 is refused with *"a SUM we +produced clips"*; an `se` at 0.0 passes. + +### 🔴 A failed export leaves a tree that is not an export tree + +Found by accident, and worth more than the way it was found. Testing the new +`stems` assertion, the exporter `bail!`ed part-way — and left `export/` **with no +`manifest.json` at all**. Every subsequent tool then reported *"has no +manifest.json — is that an export tree?"*, which reads as a broken harness rather +than as the aftermath of a deliberate failure. + +⚠️ It cost me a wrong reading immediately: the first run of the requirement audit +above reported every case as "no manifest", and I nearly recorded that the +validator was checking nothing. It was checking a tree that had been half-written. + +The exporter writes the manifest last, which is the right order — a manifest is a +claim about a tree, and a manifest for a tree that was never finished would be +worse. So this is **filed rather than fixed**: the behaviour is defensible and the +message is not, since "is that an export tree?" describes the symptom and hides +the cause. What a stranger needs to be told is *the last export failed; re-run it*. diff --git a/docs/port/FORMAT.md b/docs/port/FORMAT.md index 0bf69f26..a990d025 100644 --- a/docs/port/FORMAT.md +++ b/docs/port/FORMAT.md @@ -356,7 +356,7 @@ not done its job. |---|---| | `kind` | `se`, `bgm` or `voice`. The runtime dispatches on it, so it is a field rather than a prefix on `name` that a consumer would have to parse | | `name` | the **role**, not the disc asset: `move`, `confirm`, `back`, `main_menu`. Which bank plays a role is authored and expected to change; a rename on the disc side must not be a change to the Godot project. ⚠️ **`voice` is the exception and keys by MOVIE NAME** (`ADV`, `S00A`), because there is no role to name: the binding of recording to picture came off the disc's own movie manifest, so unlike a music bed nothing about it was chosen | -| `peak_dbfs` | measured off the finished file. **Required.** Silence is the audio failure that looks like success — right duration, right channel count, right size, full of zeroes — and clipping is the other one, which the BGM can produce because it is a sum of two stems at unity gain. `sylpheed-export check` refuses a tree whose peak is ≤ −90 dBFS or ≥ 0 dBFS | +| `peak_dbfs` | measured off the finished file. **Required.** Silence is the audio failure that looks like success — right duration, right channel count, right size, full of zeroes — and clipping is the other one, which the BGM can produce because it is a sum of two stems at unity gain. `sylpheed-export check` refuses a tree whose peak is ≤ −90 dBFS, and applies a **kind-dependent** upper bound. 🔴 This paragraph used to state a flat *≥ 0 dBFS* and was wrong about the port's own export: `confirm` ships at **+0.18** and the `ADV` voice at **+0.31**, so a consumer implementing a validator from this file would have rejected a valid tree. The rule is: a **`bgm`** is a sum *we* produced, so a peak at or above full scale is our arithmetic and is refused outright; an **`se`** or **`voice`** is a single wave off the disc, mastered near full scale, and a lossy decode of it overshoots by a fraction of a dB — those are allowed to **+1.0 dB**. ⚠️ The +1.0 is a judgement, not a measurement: a few tenths is reconstruction overshoot and a whole dB is not, and if a cue ever trips it the right response is to measure the overshoot distribution, not to loosen the bound | | `duration_s` | measured off the finished file, so that a claim about a cue's length can be checked against the finding that produced it | | `name_match` | the game's own cue identifier **guessed by name**. Absent means nobody claimed one — never that the binding is unknown. The binding is the measured part; the name is not | | `loop_mode` | what the runtime does at the end of the file, where that was authored. Absent on a cue: a cue ends |