Skip to content

Improvement: versioned Octatrack source-machine START callbacks - #1029

Merged
repeat98 merged 21 commits into
repeat98:mainfrom
ex1010:octatrack-machine-start
Oct 31, 2026
Merged

repeat98 merged 21 commits into
repeat98:mainfrom
ex1010:octatrack-machine-start

Conversation

@ex1010

@ex1010 ex1010 commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

Problem and change: Source machines with custom controls need an independent sample-START callback. This PR adds ot_machine_starts: a separate versioned 16-byte record naming its same-package ot_machines descriptor and a standard three-argument callback. It supplies the shared SDK prerequisite for FRACT's machine migration in #926 without replacing the shared FLEX handler-table cell.

The machine descriptor remains ABI 1/80 bytes (88 with SETUP_PAGE). Selection happens before stock pushes the arguments, preserving Mute Modes' trig gate and the selected stock/FM/legacy fallback. Missing, duplicate, malformed or DSP-unpaid START ownership refuses and suppresses that owner's previous voice until an accepted START or removal of that descriptor. Both machine and START tables admit eight records. The new START table uses hook 37. Admission checks extents, SETUP_PAGE overlap, same-package ownership, alignment and reconstructed-image relocation before publication; Part/UI and paused-context retention remain.

Maintainer review follow-ups:

  • Removing a descriptor now clears its cached START refusal on every affected track before its address can be reused. An unrelated table update keeps a still-live owner's refusal. The regression failed on the earlier implementation, then passed with eight refused tracks, partial/full removal and a replacement at the exact descriptor address rendering before any accepted START.
  • The linked SDK builder now requires a START callback to point to the entry of a sized, defined ELF STT_FUNC wholly within .text, and requires its pointer word to self-relocate. C supplies that metadata; assembly callbacks must use .type and .size. Real linked assembly and C-builder tests reject read-only/writable/BSS data, object data inside .text, function-typed data outside .text, interior/odd/out-of-image targets, a missing relocation and a missing ELF map; a valid function at image offset zero is accepted.
  • The developer chose build-time checking with the existing format. The package ABI, 16-byte START record and machine descriptor are unchanged. Installation still receives a flat image with no ELF code/data section map, so independently constructed packages retain the existing initialized-image bounds checks. ELF provenance checks packaging mistakes; it is not a proof that authored code is safe to execute.
  • Nine active reconstruction declarations update only the builder source SHA. Their original image hashes, stock spans and all other input hashes remain unchanged. Frozen historical Play Modes stays untouched. The original image checks and byte-for-byte published-package rebuilds pass with the new builder.

Current integration: 425685975eb3e1e32ac64eb043e174b0b65591bc includes main 886098f5de8bd7e187d0b80910114ac38b7eecd5. It retains boot/bank guards, assignment/live-parameter reload, LFO naming, base-update trial/write safety and #1091's stale FLEX voice/retrig stop before a machine's own render (including START refusal) and an absent machine's silent render. #1091 fixes #1051 independently of this START hook; the earlier proposed Analog BD follow-up has been withdrawn. No module's audio implementation is changed here.

Validation on 4256859:

  • npm run pr:ready reports Ready against main 886098f5. SDK: 498 tests, 12 skipped; recipe tests: 39 passed; app tests: 2763 passed, one skipped, 340 files. Lint/typecheck/bundle, catalogue, compatibility/pairs, licences, changelog and board checks pass. Node 24.22.0, Python 3.12.2 and pinned DSP tools.
  • All 12 machine-event tests pass, including sanitized dispatch/admission and ColdFire compilation/assembly. All 19 runtime-loader tests pass (one opt-in emulator test skipped), including both new callback tests and the byte-for-byte published-package rebuild. Generated event declarations are consistent. No gate or timeout limits were changed.
  • The supported beta generator rebuilt all seven USB layouts with native/browser equality, stock-free scans, identical controller links and declared-site/bootloader checks. Default identity: 57c82d5e92bebe8786dc94c42a711a8ec347e7423a43b44b7e31e2ad9dd9f79c; private .bin SHA-256: 90f1d774bfcffd5b102c6e9a64ea64c4085d1caf693285c0bb56fecf23f134bd. All five base-bound packages passed stock-byte scans and were regenerated against the current base links. All five actual payload files retain their previous SHA-256 values; the package manifest differs from 809c06e6 only in five required-base identities. Controller-policy source bindings were regenerated by the supported tool.
  • Fresh clean committed-source builds on 4256859, using immutable Docker image sha256:3b40aa253afa03508735107cdaefac0d4116824da50059a6a2ba61e5d11ef56d, pass both export verifiers: Octatrack artifacts, complete source inventory and version pins match; all 28 current source-only release exports reproduce. One fresh build per family, with tracked-source-only, read-only, no-network, resource-limited mounts; no firmware or private history mounted.
  • Incremental Jev advisory: the authored hardening delta from 809c06e6, before generated-base refresh, was examined in three bounded pages: 19 distinct AST fragments / 19 fresh requests, no service errors, all three configured Stage 0 checks pass. Zero flags / 19 uncertainties, all investigated, including omitted records. No defect was confirmed among the investigated signals. Coverage remains partial: one production-registry C parser blocker, checked manually with its red/green lifecycle regressions, and five static-only exclusions. Generated-base/binding updates followed this pass and are not API-reviewed; their separate seven-layout and source-export proofs are above. The earlier full-PR advisory remains a separate partial review of 809c06e6. No source/context fix followed the successful advisory pages, so no unchanged paid recheck was run.

Reported hardware evidence: repeat98's MKII check used a --dev twin of 9d8748c, beta flags, no controller rows, base prefix a384a952, RAM-booted from flashed d15d3e2d. He reported successful boot/playback and Part 1 → 2 → 1 while playing, no new DSP stall or skipped frames, and frame max 65%. He confirmed the selector jump and reviewed the replayed lookup, flags and Mute Modes gate. No installed module opted into START, so this is evidence for the existing fallback path on that named earlier build. Custom START execution remains host-tested; the report does not qualify this later rebuilt base or a callback timing limit. This contributor performed no hardware installation or flashing during the refresh. No firmware, extracted stock code/tables, dumps or complete OS images are committed.

Guides: docs/module-guides/machines.md documents refusal cleanup, typed/relocated callback requirements and the build-time/runtime distinction while preserving interrupt/ABI obligations, Mute Modes and eight-record capacity. sdk/runtime/loader/README.md explains ELF provenance, assembly declarations and preserving recipe spans only after unchanged image/payload checks. The verification record records the hardening cases and their limits. The changelog describes both fixes. Hook 37's agent-board claim is current; the inbox and open Octatrack questions/findings are empty.

GitHub status: Head 42568597 is mergeable against main 886098f5; GitHub reports no conflicts. The current Module PR checks run is action_required, awaiting maintainer approval. CI is not green and required module-pr-ok has not run. Local Ready and both clean-source builds/export checks above pass. The PR remains open for the owner to merge; no merge or auto-merge was requested.

# Conflicts:
#	docs/module-guides/machines.md
#	sdk/machines/octatrack/base-binding.json
#	src/engine/assets/octatrack-base-variants.json
#	src/engine/assets/octatrack-base.elemod
#	src/engine/assets/octatrack-module-packages.json
repeat98 added a commit that referenced this pull request Oct 29, 2026
Hook index 37 is held for ot_machine_starts (#1029) by a reserved entry
(events.json "kind": "reserved", room 0): the loader's room check and the
site's package check refuse any word there, and #1029's entry replaces it
when it merges, so neither hook renumbers whichever merges first.

A PERSONALIZE row now names the stock row it goes after (after, 1-15;
MODWERK_PERSONALIZE_AFTER_* in modwerk_events.h): MUTE MODE after PREVIEW
WITHOUT FX, as in the card build and octabam. Rows on the same anchor keep
the table's order, unanchored rows go after stock's fifteenth, LED
BRIGHTNESS stays last, and an anchor past 15 refuses its contribution.
Checked against 1.40C (privately, addresses only): the three tables and the
window's list state are read only by the window's renderer, row count and
key handler (0x40068e00-0x40069074), so moving stock rows down is safe.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
@repeat98

Copy link
Copy Markdown
Owner

Hey @ex1010 please pull the latest changes to make sure your agent uses the inter-agent message board.

@npp1993

npp1993 commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator

This hook would also fix #1051, an Octatrack DSP stop when a playing track leaves Analog BD for FLEX. The root cause, traced in ot_emu, is in #1051.

The cause. A module machine that keeps stock's START gets stock's FLEX voice started on every trig, with the machine's bytes read as FLEX settings. A bank or Part change then makes the track FLEX without mw_assign, and stock renders that stale voice. At Analog BD's rate (about 3.0), the record gets more sections than its header can name. The stock DSP source then overruns X memory, and core 1 jumps to P:0.

The consequence for this PR. Machines without a START record keep that behaviour after this lands (the PR notes FM Synth's fallback stays). Once this merges, Analog BD, and probably FM Synth, should contribute a START record so stock never starts that voice. I'll prepare the Analog BD side locally against this branch and offer it to its author after this merges.

@ex1010

ex1010 commented Oct 30, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @npp1993 for tracing this and linking #1051! A separate Analog BD follow-up once the shared START hook lands makes sense. Thanks for preparing it against this branch.

@repeat98

Copy link
Copy Markdown
Owner

Thanks @ex1010, I reviewed the code and it looks right to me. The shim replays moveal %a1@(0,%d4:l:4),%a0 (register-indexed, so moving the stack is safe) and the tst.l %a0 the beq at 0x4000d484 needs, and Mute Modes' gate at 0x4000d498 still decides first.

Unit run on my MKII, a --dev twin of 9d8748c (beta flags, no controller rows; base a384a952…) RAM-booted from the flashed d15d3e2d: it booted, played, and switched Part 1 → 2 → 1 while playing, with no new DSP stall and no skipped frames (frame max 65 %). mem:0x4000d47e:6 read 4ef940ad0b68, the jump to mw_source_start, so every trig went through the new selector. No installed module sets MODWERK_MACHINE_START yet, so this only shows the existing path is unchanged; the START path itself is covered by your host tests. Feel free to add this to the verification record.

One thing before I can merge: #1077 (the boot guard) just merged and regenerated the base, so this conflicts now. Could you merge main, regenerate through the supported generators and run npm run pr:ready until it says Ready? Once the checks are green I'll merge it.

Two small things, neither blocking:

  • start_refused[track] isn't cleared when a module is removed, so a new machine loaded at the same descriptor address would stay silent until its first accepted START.
  • The callback check only requires an address inside the package's image, so it could point into data rather than code.

@npp1993

npp1993 commented Oct 30, 2026

Copy link
Copy Markdown
Collaborator

Update on my earlier note: this hook won't fix #1051 after all. The builder arms the retrig after the START handler runs, so the handler fires too early to prevent it. #1091 fixed it in the base instead and is merged, so no Analog BD follow-up is coming from me. Nothing here needs to change for that.

@ex1010

ex1010 commented Oct 31, 2026

Copy link
Copy Markdown
Contributor Author

Thanks @repeat98 for the review and MKII check! Following up on your two notes, I've pushed 42568597.

start_refused[track] isn't cleared when a module is removed

Fixed: table updates now clear refusals for departed descriptors while retaining them for live owners. The regression covers all eight tracks, partial/full removal and descriptor-address reuse before the replacement's first accepted START.

it could point into data rather than code.

The SDK now rejects START callbacks unless they are relocated, typed ELF function entries within .text. Tests cover data and interior-function targets, including the real C builder. The package and START record formats remain unchanged. This is a build-time check; installation retains initialized-image bounds checks because the flat format carries no code/data map.

Main, including #1091, is merged; all seven base layouts were regenerated through the supported generators. npm run pr:ready reports Ready, and both clean-source build/export checks pass. Your MKII report is recorded with its original build identity and fallback-only scope.

The current Module PR checks run is awaiting maintainer approval.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Octatrack base: switching a playing track from Analog BD to FLEX can stop the DSP

3 participants