mirror of
https://github.com/got-feedBack/feedBack.git
synced 2026-07-19 19:31:26 +00:00
329cc86315
* fix(sloppak): the full mix is a stem — drop the invented `original_audio` key (#933) Core read, served, and depended on `original_audio:` — a top-level manifest key this repo invented in #583 that the feedpak spec never defined. The format already had a home for the pre-separation mixdown: it is a stem. feedpak 1.15.0 (feedpak-spec#53) RESERVES the id `full` for it, so read it from there. The key existed to work around a bug in our own reader. The packer's comment said so plainly: "we must NOT list the full mix as a playable stem — the player sums every entry in `stems` and does not gate playback on `default`, so a listed full mix plays on top of the stems". Faced with a reader that would double the song, the packer put the mixdown outside `stems` and invented a key to point at it. The fix belongs in the reader, and that is what this is. load_song() now partitions the stem list: `full` comes out as LoadedSloppak.full_mix, the instruments stay in .stems. Nothing that sums stems or draws one fader per stem can see the mixdown, so retaining it is safe — which is what lets the packer put it where the format says it goes. - ws_highway: `song_info` gains full_mix_url / has_full_mix. The old original_audio_url / has_original_audio remain as deprecated aliases for one release so an older stems plugin keeps working (#945). - `stems` on the wire, and stem_ids / stem_count in the library index, are now INSTRUMENT stems only — a separated pack that retains its mixdown no longer advertises a bogus "full" chip or an inflated stem count. - enrichment: fingerprint against the mixdown wherever it lives. This widens coverage — _song_audio_file() previously returned None for any pack without the invented key, so fingerprinting silently did nothing for nearly every pack. - sloppak: `original_audio:` is still READ as a deprecated fallback, because every pack in the wild carries it and would otherwise lose its pristine mix. tools/migrate_full_mix_stem.py rewrites those packs into the spec shape (original/full.ogg -> stems/full.ogg, add the `full` stem at default:off, drop the key); the fallback and the aliases die with #945. The spec gate keeps the debt honest: the grandfather entry now tracks #945, and the gate fails if it goes stale. Verified: spec gate OK (4/4, incl. ingesting the spec's new example pack that retains `full`); 2493 python tests, 995 js tests; migrator round-tripped over real packs from the library and the results pass the spec's reference validator. * fix(migrate): discover directory-form packs instead of silently skipping them iter_packs() searched only files, so a directory-form pack (`song.sloppak/`, the authoring shape) was walked INTO and never yielded — silently missed by a run that's meant to be exhaustive. Discover suffix-named directories too (yielded whole, not descended into), and route packs through migrate_pack/verify_pack. Directory packs are REPORTED as `dir-form-unsupported`, not rewritten in place: a single-file pack is replaced atomically (a fully-built temp archive swapped in with one os.replace), but a populated directory can't be swapped that way, so an interrupted in-place rewrite could leave an authoring pack half-migrated. The status is a problem status, so it counts against the run's exit code and shows in the summary — the operator re-packs or migrates it as a `.feedpak` instead of it vanishing from the report. Addresses a CodeRabbit review finding. Signed-off-by: Kris Anderson <topkoa@gmail.com> * fix(migrate): verify requires an explicit `off` on a retained full mix verify_zip accepted any non-truthy `default` on a multi-stem `full` (missing, empty, boolean, `false`/`no`/`0`, malformed) as "ok". But core defaults an ABSENT `default` to True — ON (lib/sloppak.py: `s.get("default", True)`) — and treats an empty/unrecognized string as ON too, so a migrated-shape pack whose `full` stem has a missing or blank default beside instrument stems would actually play the mixdown on open and double the song. verify was certifying that as safe. Require an explicit normalized `off` beside instrument stems: `on`-ish values are reported `full-stem-default-on` (actively plays), everything that is not a normalized `off` is reported `full-stem-default-not-off`. The migrator already writes the literal `off`, so its own output is unaffected; this also certifies the pack is in the tool's canonical, most-portable shape. The len>1 gate is kept, so a sole `full` stem (which IS the audio) is not policed. Adds parametrized coverage for missing / empty / boolean / off-ish / malformed defaults, and a sole-full-stem case. Addresses a CodeRabbit review finding. Signed-off-by: Kris Anderson <topkoa@gmail.com> --------- Signed-off-by: Kris Anderson <topkoa@gmail.com> Co-authored-by: Kris Anderson <topkoa@gmail.com>
57 lines
3.1 KiB
YAML
57 lines
3.1 KiB
YAML
# CLOSED grandfather list — manifest keys core reads or writes that predate the
|
|
# spec-conformance gate and that the feedpak spec does not define.
|
|
#
|
|
# Please don't add entries here — CI will flag any PR that grows this list, so
|
|
# it can only shrink over time. That's by design, not distrust: the moment the
|
|
# app touches a key the spec doesn't define, every teammate's PR starts failing
|
|
# the conformance gate too, and whoever added the key is the only person who
|
|
# can fix it. The FEP process below avoids putting anyone in that spot. The
|
|
# feedpak spec's own governance is explicit:
|
|
#
|
|
# "This repository defines the format only. Applications that read or write
|
|
# feedpak ... track this spec as a dependency; they do not drive it.
|
|
# A change is not part of the format until it lands here."
|
|
# — got-feedback/feedpak-spec, GOVERNANCE.md
|
|
#
|
|
# So a new manifest key goes through the feedpak Enhancement Proposal (FEP)
|
|
# process — see feedpak-spec/CONTRIBUTING.md:
|
|
#
|
|
# 1. Open a FEP issue on got-feedback/feedpak-spec describing the problem, the
|
|
# on-disk shape, backward compatibility, and the version bump implied.
|
|
# 2. Land one PR there updating the normative spec, the JSON Schemas, an
|
|
# example that exercises it, and the changelog — together.
|
|
# 3. Back here, re-run this PR's checks. The gate verifies against the spec's
|
|
# HEAD, so once your key is in the spec, the gate goes green.
|
|
#
|
|
# That's the supported route — and usually a quick one for additive keys. If
|
|
# your PR is blocked by this gate, a FEP will get you unblocked properly; an
|
|
# entry here won't (CI rejects it).
|
|
#
|
|
# Entries below exist ONLY because they predate the gate. Each is debt with a
|
|
# tracking issue, and each disappears when its issue is fixed. The gate also
|
|
# fails if an entry goes stale — the spec caught up, or core no longer reads or
|
|
# writes the key — so this file cannot quietly become a place drift hides.
|
|
|
|
exceptions:
|
|
- key: original_audio
|
|
issue: https://github.com/got-feedback/feedback/issues/945
|
|
reason: >-
|
|
Added by #583 (the full mix played while every stem fader sits at unity,
|
|
since demucs recombination is lossy). It never went through a FEP and the
|
|
spec does not define it — the drift this gate exists to prevent.
|
|
|
|
#933 fixed the drift: feedpak 1.15.0 RESERVES the stem id `full` for the
|
|
complete mixdown (feedpak-spec#53), and core now reads the full mix from
|
|
that stem. Nothing depends on this key any more — not the loader, not
|
|
lib/enrichment.py, not the stems plugin, and the packer no longer writes it.
|
|
|
|
What remains is a READ-ONLY deprecated fallback in lib/sloppak.py
|
|
(_legacy_full_mix), kept for one release because every pack produced before
|
|
the spec caught up carries `original_audio: original/full.ogg` and would
|
|
otherwise silently lose its pristine mix. tools/migrate_full_mix_stem.py
|
|
rewrites those packs into the spec shape.
|
|
|
|
This entry disappears with that fallback — tracked by #945, which cannot be
|
|
forgotten: the gate fails if the entry goes stale, and deleting the read is
|
|
what makes it stale.
|