---
description: "Audit DPCM/drum sampling — drum mapping, sample conversion, DMC constraints"
argument-hint: "[--focus <dims>]"
---

# DPCM / Drum-Sampling Audit

Audit the DPCM/drum subsystem — how MIDI General-MIDI percussion notes become DPCM
samples, how `.wav` is converted/packed into NES 1-bit delta data, and how the DMC
channel is driven. This subsystem owns everything under `dpcm_sampler/` plus the
DMC-facing edges of the channel pipeline and the CA65 exporter.

Shared protocol (layout, dedup, finding format): `.claude/commands/_audit-common.md`.
Severity rubric: `.claude/commands/_audit-severity.md`. Do not restate them.

Hardware claims **must** cite `docs/APU_DMC_REFERENCE.md` (register map, the
`$C000 + A*64` address formula, the `(L*16)+1` length formula, 64-byte address
alignment, the 0–127 output-level clamp, the `$FFFF`→`$8000` address-wrap quirk) and
`docs/NES_DMA_REFERENCE.md` (DMC DMA steals 3–4 CPU cycles per byte and is the source
of the controller/`$2007`/`$4015` extra-read glitch). Prefer those docs over
re-deriving APU behavior from source.

A dropped or out-of-range drum sample removes audible content, so per
`_audit-severity.md` it is at least MEDIUM, and HIGH when it silently strips a hit
that the MIDI clearly intended.

> **Sprint note**: a large bug-fixing pass (commits `be4d2bd`…`8225696`) closed most
> of the issues this audit used to lead with (#64–#74, #140). The dimensions below
> describe the **current** (fixed) behavior and ask you to verify the fix holds up
> under edge cases, rather than re-discovering the original bugs. #76 remains
> genuinely open (Dimension 6). #75 (the `length_reg` rounding) was closed on GitHub,
> then regressed, and was re-fixed on this tree as #295 (commit `d392ef6`) —
> `_place_sample` now ceils, so Dimension 4's item is fixed (#295); verify it holds.

## Parameters (from $ARGUMENTS)
- `--focus <dims>` — comma-separated dimension numbers (e.g. `--focus 1,4`). Default: all.

## Extra Per-Finding Field
- **Hardware ref**: the `docs/APU_DMC_REFERENCE.md` / `docs/NES_DMA_REFERENCE.md`
  section backing any hardware claim (omit for pure-software findings).

## Dimensions

### Dimension 1: Drum-note → sample mapping & coverage
The GM-note maps live in `dpcm_sampler/drum_engine.py`
(`DEFAULT_MIDI_DRUM_MAPPING` lines 8-56, `ADVANCED_MIDI_DRUM_MAPPING` lines 58-78).
Mapping resolution is in `EnhancedDrumMapper._resolve_dpcm_sample_name` and
`map_drums` (`dpcm_sampler/enhanced_drum_mapper.py`):
- **Fixed (#73/D-10, verify)**: `DEFAULT_MIDI_DRUM_MAPPING` now covers the full GM
  percussion range 35–81 with generic role names (kick/snare/tom/cymbal/etc, not
  just 7 notes). `ADVANCED_MIDI_DRUM_MAPPING` still only hand-tunes velocity splits
  for notes 36 and 38; every other note relies on the `DEFAULT_MIDI_DRUM_MAPPING`
  fallback by design (see the trailing comment at
  `dpcm_sampler/drum_engine.py:75-77`). Confirm a mid-range note (e.g. 47, mid tom)
  still resolves to a real sample name and not `None`.
- **Fixed (verify)**: `_resolve_dpcm_sample_name`
  (`dpcm_sampler/enhanced_drum_mapper.py:390-418`) no longer stops at the first
  candidate — it tries the velocity-split name, then the advanced `"primary"` name,
  then the `DEFAULT_MIDI_DRUM_MAPPING` role name, in that order, and only returns
  `None` (→ noise fallback) if none of the three exist in `self.sample_index`.
  Verify this cascade actually reaches the index for a name like `kick_soft`
  that legitimately isn't present (falls through to `"kick"` then to the default
  role name) rather than silently dropping.
- Velocity-0 note-offs are correctly skipped
  (`if e.get('velocity', 0) == 0: continue`, `enhanced_drum_mapper.py:254-255`).
- `_get_advanced_sample` (`enhanced_drum_mapper.py:374-388`) selects by
  `velocity_ranges`; its result is now just one candidate in the fallback chain
  above rather than a hard commit — confirm a nonexistent velocity-split name no
  longer kills the whole event, only that one candidate.

### Dimension 2: `dpcm_index.json` schema integrity
The index is loaded in the same three places, still against two different shapes:
- `EnhancedDrumMapper._load_sample_index` (`dpcm_sampler/enhanced_drum_mapper.py:211-224`)
  passes each entry as `sample_data` to `DPCMSampleManager.allocate_sample`
  (`dpcm_sampler/dpcm_sample_manager.py:15-65`), which reads
  `sample_data.get('length', 1024)` (line 34), `sample_data.get('data', [])`
  (line 55), and `sample_data.get('frequency', 33144)` (line 58).
- **Still true** (this is not a bug that was "fixed", just the current shape of the
  data): the real `dpcm_index.json` entries only contain **`id`** and **`filename`**
  (verify: `python -c "import json;
  print(list(json.load(open('dpcm_index.json')).values())[0])"`). So `length`,
  `data`, and `frequency` still always fall back to defaults on real input. What
  *did* change (#70/#71, see Dimension 7) is that the sample manager now uses one
  consistent accounting formula for those defaults instead of two divergent ones,
  and the now-dead similarity/dedup code that also depended on `data` was removed
  outright rather than left silently inert. Assess whether relying on constant
  placeholder size/frequency for every sample is still an acceptable simplification
  or worth back-filling from the real `.dmc` files at index-generation time.
- The packer path moved: `dpcm_sampler/generate_dpcm_index.py:load_dpcm_index_into_packer`
  (lines 38-79) is now the single shared call site (used from both
  `main.py:run_export` ~lines 317-345 and `main.py:run_full_pipeline` ~lines
  625-670) and reads `sample.get('pitch', 15)` (line 75) and `sample['filename']`
  (line 66) — `pitch` is still absent from the shipped index. Confirm `id`/`filename`
  remain the only keys any consumer can rely on, and that `generate_dpcm_index`
  (`dpcm_sampler/generate_dpcm_index.py:82-102`) is still the sole writer (it emits
  exactly `id` + `filename`).

### Dimension 3: DPCM conversion correctness (1-bit delta)
`dpcm_sampler/dpcm_converter.py` does WAV→PCM→delta→packed-bits. This file has not
changed in the recent fix sprint — the following are still open, unverified claims:
- `delta_encode` (lines 34-42) walks a ±1 step counter, but `dpcm_compress`
  (lines 45-66) then re-derives the bit purely from `encoded[i] > encoded[i-1]`.
  Check that the two stages agree and that the bit polarity matches hardware: per
  `docs/APU_DMC_REFERENCE.md`, a `1` bit **adds 2** to the output level and `0`
  **subtracts 2** (the engine never sets a level, only nudges ±2). A constant-input
  run encodes all-zero bits ⇒ the level ramps *down* on playback — verify the
  start-level assumption (`prev = 0x40`, line 36).
- Bit packing order (line 63): `byte |= (bits[i+j] << j)` packs LSB-first. Confirm
  against the DMC shifter order in `docs/APU_DMC_REFERENCE.md` ("Reader → Buffer →
  Shifter") — wrong bit order plays the sample bit-reversed (audible garbage).
- Resampling: `convert_wav_to_unsigned_pcm` (line 7) uses `np.interp` linear
  resampling to a fixed `sample_rate=8000`, independent of the DMC rate index
  written elsewhere (`pitch`/`$4010`). Flag any mismatch between the conversion
  rate and the playback rate index that would pitch-shift samples.

### Dimension 4: Sample size / address / DMC range constraints
`dpcm_sampler/dpcm_packer.py` computes the `$4012`/`$4013` register values:
- **Fixed (#75 → regressed → #295/DP-01) — verify it holds**: `_place_sample`
  (lines 77-98) computes `dpcm_address_val = (start_address - 0xC000) // 64`
  (line 78) and `dpcm_length_val = max(0, (sample['size'] + 14) // 16)` (line 88).
  Verify against `docs/APU_DMC_REFERENCE.md`: address = `$C000 + A*64`, length =
  `(L*16)+1` bytes. `length_reg` now uses **ceiling** division (`(size + 14) // 16`
  = ceil((size-1)/16)) so a `size` not of the form `16k+1` still gets its full tail
  read: the engine reads `(length_reg*16)+1` bytes and the `.align 64` gap after
  each sample makes the few extra bytes safe zero-pad, not neighbouring data. This
  is the flooring bug originally closed as #75 that regressed and was re-fixed here
  as #295 (commit `d392ef6`); confirm the ceiling still holds and that `size` stays
  bounded to 4081 so the 8-bit register can't overflow.
- **Fixed (verify)**: the 4081-byte oversized-sample path no longer aborts the pack.
  `DpcmPacker.add_sample` (lines 13-47) now truncates to 4081 bytes when
  `truncate=True` (lines 31-36) instead of always raising `ValueError` — and the
  shared call site `load_dpcm_index_into_packer`
  (`dpcm_sampler/generate_dpcm_index.py:72-77`) always passes `truncate=True`, so
  in practice a too-long sample is clamped, not fatal (#68). `add_sample` still
  raises if a caller explicitly passes `truncate=False`; confirm no current call
  site does that. `dpcm_converter.dpcm_compress` (line 66) independently truncates
  with `dmc_bytes[:4081]` at conversion time — the two truncation points are
  consistent (both clamp to 4081), not contradictory.
- `START_ADDR = 0xC000`, `BANK_SIZE = 8192` (lines 5-6), and the 60-bank ceiling
  (`OverflowError` at line 70). Check `.align 64` in `generate_assembly` (line 100)
  keeps every sample 64-byte aligned, and whether anything guards the
  `$FFFF`→`$8000` address-wrap quirk documented in `docs/APU_DMC_REFERENCE.md`
  (a sample bleeding past `$FFFF` plays garbage from `$8000`).
- **Fixed, verify no regression (#140)**: the packer used to receive the *entire*
  1923-sample catalog regardless of what a song used, overflowing the 60-bank
  budget and silencing percussion on every drummed song. It's now filtered via
  `get_dpcm_sample_ids_from_frames` (`dpcm_sampler/generate_dpcm_index.py:105-117`,
  reads frame `note = sample_id + 1`) passed as `sample_ids=` into
  `load_dpcm_index_into_packer`, so only samples the exported song actually
  references get packed. `generate_assembly` (lines 91-147) now emits sparse
  lookup tables sized to `max_id + 1` with `$00` placeholders for unpacked ids
  (lines 123-146) — verify those placeholder slots are provably unreachable (no
  frame indexes an id that wasn't packed) rather than merely "usually" unreachable.

### Dimension 5: DMC level handling & DMA-timing implications
- **Fixed, verify no regression (#72/D-09)**: the DMC output level (`$4011`)
  `CMD_DMC_LEVEL` ($87) emitter path was removed entirely (commit `5c032d2`) — no
  stage ever produced `dmc_level`, so the branch was dead. Confirm it hasn't been
  re-added; if it is re-added for the `$4011` non-linear-mixer trick
  (`docs/APU_DMC_REFERENCE.md` §6), re-check the level is clamped to the 7-bit
  0–127 range `$4011` accepts before emission.
- Silence init: `docs/APU_DMC_REFERENCE.md` says init should write `$00` to `$4011`
  so the DMC counter doesn't muffle Triangle/Noise via the non-linear mixer.
  Live on the bytecode path: `nes/audio_engine.asm:128` writes `LDA #$00` / `STA $4011`
  at `audio_init`. (The direct-export `init_music`/`reset` omits it — open gap
  #348/NH-HW-1; nes/mmc3_init.asm, which also had this write, was deleted as dead
  code, #203.) The live DPCM trigger is `@write_dpcm` (`nes/audio_engine.asm`, ~line
  512), which writes `$4010` → `$4012` → `$4013` then toggles `$4015`
  (disable-then-enable-with-DMC), matching `docs/APU_DMC_REFERENCE.md`; re-verify this
  order is still correct if the trigger routine changes. (The old
  `seq_cmd_dpcm_play`/`seq_cmd_instrument` copies in `nes/project_builder.py` were
  removed as dead code — #314/EXP-12; only the live `fetch_sequence_byte` remains.)
- DMA cost: `docs/NES_DMA_REFERENCE.md` notes each DMC DMA steals 3–4 CPU cycles and
  a heavy drum catalog fires constantly, delaying OAM DMA and corrupting
  side-effect reads (`$4016`/`$2007`/`$4015`). This subsystem can't avoid the cost,
  but flag if the generated engine/docs omit the mandatory DPCM-safe controller-read
  warning, or if rapid back-to-back triggers are emitted with no awareness of the
  cycle budget.

### Dimension 6: Config robustness (`DrumMapperConfig`)
`dpcm_sampler/enhanced_drum_mapper.py` defines `DrumPatternConfig` (line 12),
`SampleManagerConfig` (line 53), `DrumMapperConfig` (line 95) with `validate()` /
`to_file` / `from_file` (line 163):
- **Still open (#76/D-13)**: `from_file` (lines 163-191) does
  `DrumPatternConfig(**config_data.get('pattern_detection', {}))` (line 170) and the
  equivalent for `SampleManagerConfig` (line 172) — an unexpected/renamed key in the
  JSON raises `TypeError` (not caught; only `FileNotFoundError` and
  `json.JSONDecodeError` are handled, lines 188-191). No commit in the recent sprint
  touched this. Note the current blast radius: the CLI `--config` flag that used to
  feed `main.py:load_config` (`main.py:462-466`) into this path was intentionally
  removed (`main.py:772-774`, #13) because nothing wired it to
  `assign_tracks_to_nes_channels` — so today `from_file` is reachable only via
  direct API use (and is exercised by `tests/test_drum_mapper_config.py`), not the
  CLI. Still worth fixing/hardening since it's public API surface.
- `validate()` (`DrumMapperConfig.validate`, line 110) enforces weight sums ≈ 1 and
  ranges. `EnhancedDrumMapper.__init__` (line 196) calls `self.config.validate()`
  unconditionally on whatever config object it holds, so a config passed in after
  `DrumMapperConfig.from_file(...)` **is** validated before use, so long as it's
  routed through `EnhancedDrumMapper.__init__` — confirm there's no path that uses
  a `from_file`-loaded config directly without going through that constructor.
- `SampleManagerConfig.memory_limit` is bounded 1KB–16KB and `max_samples` 1–64
  (lines 77-80), but `DPCMSampleManager.__init__` defaults
  (`max_samples=16, memory_limit=4096`) are declared independently
  (`dpcm_sampler/dpcm_sample_manager.py:5`). In the current codebase the only
  production instantiation site is `EnhancedDrumMapper.__init__`
  (`enhanced_drum_mapper.py:203-206`), which always passes the validated config
  values through — direct unvalidated construction only happens in
  `tests/test_dpcm_sample_manager.py`. Low residual risk; flag only if a new
  production call site constructs `DPCMSampleManager` directly.

### Dimension 7: Sample-manager dedup & lifecycle
`dpcm_sampler/dpcm_sample_manager.py` (130 lines):
- **Fixed, verify (#69/D-06)**: `allocate_sample` (lines 15-65) now assigns ids from
  a monotonic `self._next_id` counter (line 13, incremented at line 51) instead of
  `len(self.active_samples)`, so an evicted id is never handed out again to a later
  allocation. Confirm `dpcm_events` emitted before an eviction still resolve to the
  sample that was live when they were created (i.e. nothing re-keys already-emitted
  events).
- **Fixed, verify (#70/D-07)**: memory accounting is now unified — `allocate_sample`
  checks `self._get_total_memory() + sample_size > self.memory_limit` up front
  (line 42, accounting for the pending sample before it's inserted) and
  `_get_total_memory` (lines 120-130) sums `s['metadata']['size']`, the same field
  `allocate_sample` populates (line 57) — previously these used two different
  formulas (one of which, `len(data)//8`, was always 0 for real index data).
  `_optimize_sample_bank` (lines 67-111) now triggers on memory pressure alone, not
  just the sample-count limit (condition at lines 79-81). Verify a
  few-but-large-samples scenario (small `max_samples`, large `metadata.size`) still
  evicts correctly.
- **Removed as dead code, not "fixed to use real data" (#71/D-08)**:
  `_find_similar_sample` / `_calculate_sample_similarity` no longer exist in this
  file — they were deleted (commit `5c032d2`) rather than repaired, since the
  underlying `data` arrays are always empty for real index entries (Dimension 2)
  and the comparison was permanently inert. If similarity-based dedup is
  reintroduced, it needs real waveform data from the index to do anything useful.

### Dimension 8: Channel-pipeline integration (noise vs DMC)
`tracker/track_mapper.py:assign_tracks_to_nes_channels(midi_events, dpcm_index_path)`
(line 179; called from `main.py:run_map` and the full pipeline):
- It calls `map_drums_to_dpcm` (line 244) and routes results:
  `nes_tracks['dpcm'] = dpcm_events` (line 246-247, still overwrites any prior dpcm
  assignment) and `noise_events` only land on `noise` if it's still empty
  (lines 249-251). **Fixed, but only partially (#74/D-11)**: when `noise` is
  already occupied, the drum noise-fallback events are still discarded (this is
  physically unavoidable — NES has one Noise channel, per
  `docs/APU_NOISE_REFERENCE.md`) — but this is no longer silent: a `print(...)`
  warning now reports how many events were dropped and why
  (`tracker/track_mapper.py:253-259`). Verify the count in the warning matches
  `len(noise_events)` exactly and that this is the only discard path for these
  events.
- The hardcoded default `'dpcm_index.json'` path means a missing file still raises
  inside `_load_sample_index` (`enhanced_drum_mapper.py:216-219`,
  `FileNotFoundError`). This is **not caught** in the standalone `map` subcommand
  (`main.py:run_map`, lines 104-112; the mapper call is line 110) — that path still crashes with a raw traceback (#256),
  unlike every other step-by-step guard in `main.py` (`load_json_stage`, #120).
  `run_full_pipeline` fares better only because its whole body is wrapped in one
  outer `try/except Exception` (`main.py:~992-997`) that turns any such crash into
  a clean `[ERROR] Pipeline failed: ...` message — but the pipeline still aborts
  entirely rather than degrading to a drumless build, in contrast to the DPCM
  *packer* path (`main.py:run_export`/`run_full_pipeline`), which explicitly
  handles a missing index file gracefully ("No dpcm_index.json found, skipping").
  Assess whether `run_map` should get the same guard as the packer path.
- **Fixed, verify (#9, #66, #67, #200/D-14, #254)**: `dpcm_events` carry `{frame,
  sample_id, velocity}`; the frame-generation stage (`nes/emulator_core.py:188-235`)
  remaps the raw catalog `sample_id`s a song actually references to a dense,
  song-local `0..N-1` range (`dense_id_of`, line 218) and encodes
  `note = min(255, dense_id + 1)` (line 227; 0 stays the rest sentinel), emitting a
  `dpcm_sample_map` (dense_id → catalog_id) side table (lines 232-235) so the pack
  stage can resolve the real sample files. This replaced both the old 0-95 tone-note
  clamp (#67) and the raw `min(255, sample_id + 1)` that aliased every catalog id
  ≥ 255 onto note 255 (#200/D-14); the `MAX_SAFE_SAMPLE_ID = 254` guard that used to
  pre-empt the remap and route all shipped-catalog drums to noise is also gone (#254).
  The exporter (`exporter/exporter_ca65.py`) applies the same byte-ceiling for the
  `'dpcm'` channel and the generated trigger routine (`nes/project_builder.py`)
  recovers the sample via `note - 1`. Confirm the round-trip holds when a song
  references near 254 *distinct* drums (`dense_id + 1` must stay ≤ 255), not merely
  when a raw catalog id is near 254.

## Skeptical checklist
- [ ] Does an unmapped/rare GM drum note (e.g. 47, mid tom) still resolve through
      `DEFAULT_MIDI_DRUM_MAPPING`, or does it now silently fail some other way?
- [ ] Does the velocity → primary → default fallback chain in
      `_resolve_dpcm_sample_name` ever return a name that isn't actually in
      `self.sample_index` (a logic gap in the final `for name in candidates` loop)?
- [ ] Do `length`/`data`/`frequency` ever come from the real index, or always
      defaults — and does that still matter now that the dead similarity/dedup
      code that depended on `data` has been removed?
- [ ] Does `length_reg = max(0, (size+14)//16)` (ceiling) read the full sample tail
      for a sample not sized `16k+1` (#75 regressed, re-fixed as #295 — verify)?
- [ ] Does `DrumMapperConfig.from_file` still raise an uncaught `TypeError` on a
      stray config key (#76, unfixed)?
- [ ] Can an evicted sample id still be reused now that `_next_id` is monotonic —
      or is there an edge case (overflow, reset) that reintroduces reuse?
- [ ] Is the memory limit actually enforced end-to-end now that both call sites use
      `metadata['size']`, including the up-front `pending_size` check?
- [ ] When a real noise track exists, are drum noise-fallback hits still discarded
      — and is the new warning message accurate?
- [ ] Does the `dpcm` channel's dense-remapped id survive correctly into `music.asm`
      when a song references near 254 *distinct* drums (`dense_id + 1` must stay ≤ 255)?
- [ ] Does packing only referenced samples (#140) ever leave a frame pointing at an
      id that wasn't packed (a `$00` placeholder misread as a real sample)?

For every hardware claim, re-open the cited `docs/APU_DMC_REFERENCE.md` /
`docs/NES_DMA_REFERENCE.md` line and confirm before reporting. Attempt to disprove
each finding (per `_audit-common.md`) before including it.

## Output
Write the report to: **`docs/audits/AUDIT_DPCM_<TODAY>.md`** (YYYY-MM-DD).

Structure:
1. **Summary** — finding counts by severity, the highest-risk drop/round-trip issues.
2. **Findings** — base format from `_audit-common.md` plus the `Hardware ref` field.

Then suggest:
```
/audit-publish docs/audits/AUDIT_DPCM_<TODAY>.md
```
</content>