Skip to content

Fixes for nsstool, furtool and the nullsound driver #166

Open

Description

@sasja-san

This is actually a reply to this discussion: #164
But since this this touches so much more it deserves its own issue.

I've updated the devkit version I run on my machine so I'm running the latest now.

A friend told me to try out Claude Code, so I did. Damn, this thing is pretty powerful. I had it make a version of furnace called furnace-cli. And to it I can now feed MML files/strings. This opened up a new possibility of testing, paired with the MAME feature of sound recordings and comparisons.

So it created a bunch of tests. One or two of these may be hallucinations or bullshit that will never happened either way, but I don't know which is which. And the lists aren't too big. Maybe there are some good points in here? I'm too dumb to understand what much of this means 😅

Here's the report it made for me:

furtool.py, nsstool.py and nullsound: proposed changes

These are against ngdevkit 0.5+202610171524, upstream master 5106cbd.

Changes to implement

furtool.py / nsstool.py

  • Accept songs without samples. A Furnace song (v240+) with no samples has no
    sample element (type 6). furtool.py and nsstool.py then fail with KeyError: 6,
    which affects every FM- or SSG-only song. Treat the missing element as an empty list.
  • Use the FM defaults when an instrument has no FM block. Furnace leaves out an
    instrument's FM feature block when it equals the defaults, for example a new,
    untouched FM instrument. furtool.py then fails on None (AttributeError: ... 'name').
    Fill in Furnace's default FM values.
  • Handle the phase-reset macro on SSG instruments. Furnace offers it (macro code
    14), and furtool.py fails with IndexError because its macro-name list stops at 8.
    Either support it or give a clear "unsupported macro" error.

nullsound

  • Mask or bounds-check the command byte. snd_process_pending_commands uses the
    byte from the 68k to index cmd_jmptable, which has 128 entries, without masking it.
    A command from 0x80 to 0xFF jumps into whatever code follows the table, instead of
    being ignored like other unused commands. A one-instruction fix (and #0x7f, or a
    compare that routes to snd_command_unused).
    Found by reading the code; not yet reproduced in MAME.
  • Check the RAM layout in the game's own link, not only the library's. The
    page alignment of the state blocks is fixed padding (align.inc), which assumes
    nullsound's DATA starts at 0xF800.
    • If a game's Z80 code with DATA is linked before nullsound.lib, the blocks
      shift. 16 bytes moves _state_fm2 to 0xFB15–0xFC0F, across a page, and the 8-bit
      pointer arithmetic on it then reads and writes the wrong bytes, silently.
    • check-bounds.py runs only on the library's own link (linkcheck.map).
    • Possible fixes: document that game DATA must come after the library, ship
      check-bounds.py for games to run on their own link map, or place the aligned
      blocks in an absolute area so their position doesn't depend on link order.

Low priority, or probably fine as it is

  • Stack space. The driver's variables end at 0xFF96 and the stack starts at
    0xFFFD, which leaves about 103 bytes. A game's variables linked after nullsound go
    into that space, and nothing checks it. This is probably fine, but the real
    worst-case depth hasn't been measured. Worth a look only if a measurement shows it's
    tight.
  • Padding uses RAM. 821 of the 1942 bytes of DATA are alignment padding. This
    is a known trade-off for fast pointer arithmetic, and matters only if a game runs out
    of Z80 RAM.
  • The command queue doesn't check for overflow. The 64-entry ring buffer in
    snd_push_pending_command would lose commands if 64 arrived before the main loop
    ran. That's unlikely in practice: the 68k waits for an acknowledgement, and the
    main loop empties the queue quickly.
  • Interrupts after a driver reset. Commands 2 and 3 restart the driver with
    interrupts off, and they come back on only with a song's timer_tempo opcode. This
    is probably by design (see the old open PR Enable interrupts before jumping to PLAYER_START #112).
  • Open PR Fix stream volume commands ignored at boot #160 ("Fix stream volume commands ignored at boot") is still unmerged,
    so the bug it fixes is in the released driver.
  • The SSG macro names in furtool.py are off by one place. It calls code 5
    "phaseReset", but code 5 is ex1 (phase reset is 14). Code 5 never appears in a
    YM2610 SSG instrument, so it's cosmetic.
  • The MVS/AES clock is taken from the system name ("MVS" or "AES" in it), not from
    the chip flag clockSel. Furnace's presets set both together, so this only matters
    if someone renames the system.
  • Messages and wording.
    • Operator macros (O1–O4) only give an "unexpected feature" warning.
    • CSM songs (system 0xc2) fail with "invalid chip".
    • nsstool.py rejects a first note without an instrument; that's intended, but
      could be documented.
  • Tempo changes. Furnace's F0xx (set BPM) isn't supported by nsstool.py. This is
    a feature request.
  • Small cleanups in nullsound.
    • snd_command_unused returns with retn but is called from the main loop
      (harmless, since retn acts as ret there).
    • There is a dead retn after jp snd_push_pending_command in the NMI handler.
    • The nullsound_id string still says v0.2.0.

Activity

  1. sasja-san commented on Oct 19, 2026

    @sasja-san
    Author

    I had all these Claude credits burning a hole in my pocket. So I thought

    I'm not smart enough to understand all the z80 asm stuff that's going on with this driver. But the maintainer is. And Claude is.

    So here are the changes it proposes. As usual, clanker code can be a bit unreliable. But I'm runnig my AI the second most expensive level (without shoveling in a bunch of extra money).

    Here I'm assuming anyone maintaining this project is like me: They want it to be really good, but they know that generated code can get a bit hairy at times. And you lose your familiarity, because the changes didn't pass through oneself. So instead of just blindly making commits, here's a list of proposed changes. For a caring human to look at 😉


    Proposed changes to furtool.py and nullsound

    These changes are against ngdevkit master 5106cbd ("Fix various parsing errors for
    Furnace v240+", 2026-10-17). They come from testing Furnace v255 (dev255) modules
    through furtool.py/nsstool.py, and the driver in MAME 0.285 with SNK's BIOS. Each
    change was verified on its own; the results are given with each one.

    # Area Problem Effect
    1 furtool.py Missing module elements KeyError: 6 for every song without samples
    2 furtool.py Chip without flags SSG volume adjustment not applied (from 5106cbd)
    3 furtool.py FM instrument equal to the defaults crash ('NoneType' object has no attribute 'name')
    4 furtool.py SSG macros nullsound doesn't play crash (IndexError, KeyError: 'pitch')
    5 nullsound Commands 0x80–0xFF jump past cmd_jmptable, run RAM as code
    6 nullsound RAM layout in a game's own link silent corruption or stack overlap, never checked

    1. furtool.py: songs without samples, instruments or patterns

    A Furnace module (v240+) only lists the element types it has. A song without
    samples, for example any FM- or SSG-only song, has no element 0x06. read_module()
    then raises KeyError: 6 in furtool.py and nsstool.py. Instruments (0x04) and
    patterns (0x07) are read the same way.

        # INS2 elements: pointers to instruments get parsed later
        mod.instruments = elements[0x04]
        ...
        mod.samples = elements[0x06]
        ...
        mod.patterns = elements[0x07]
        # Elements absent from the module (e.g. no samples) are empty lists
    
        # INS2 elements: pointers to instruments get parsed later
        mod.instruments = elements.get(0x04, [])
        ...
        mod.samples = elements.get(0x06, [])
        ...
        mod.patterns = elements.get(0x07, [])

    Tested: an FM-only song and SSG-only songs, all without samples. Before the
    change, furtool.py and nsstool.py fail with KeyError: 6; after it, both convert
    the songs.

    2. furtool.py: no SSG volume adjustment when the chip has no flags

    5106cbd handles a FLAG element that points to offset 0, which is how Furnace saves
    a chip without flags. But in that case neither branch runs, so HALF_SSG_VOL isn't
    set. A chip without flags uses Furnace's defaults, including the half SSG volume,
    the same as when there is no element 0x02 at all.

        if 0x02 in elements:
            el_flag_pos = elements[0x02][0]
            # Element 2 might be present but point to a null offset,
            # which isn't a real element and should be ignored
            if el_flag_pos:
                check_chip_flags(el_flag_pos, bs)
        else:
            HALF_SSG_VOL = True
        # Element 2 might be present but point to a null offset: the chip
        # has no flags, so it uses Furnace defaults, as with no element 2
        el_flag_pos = elements.get(0x02, [0])[0]
        if el_flag_pos:
            check_chip_flags(el_flag_pos, bs)
        else:
            HALF_SSG_VOL = True

    Tested: an SSG instrument with volume macro 15 12 9.

    • Without chip flags it now gives 0x0e 0x0b 0x08, with the usual "Volume of SSG
      channel is halved" message.
    • With ssgVol=256 it still gives 0x0f 0x0c 0x09.

    3. furtool.py: FM instruments that equal Furnace's default

    Furnace writes an instrument's FM feature only when the FM data differs from its
    default (DivInstrument::putInsData2). A new, untouched FM instrument therefore has
    no FM feature. ins stays None and read_instrument() fails with
    AttributeError: 'NoneType' object has no attribute 'name'.

    The fix uses Furnace's default values, from DivInstrumentFM() in
    src/engine/instrument.h. It also moves the detune conversion into a function that
    both paths use:

            tmpdetune, op.multiply = ubits(bs.u1(), [6, 4], [3, 0])
            # convert furnace detune format into ym2610 format
            tmpdetune-=3
            if tmpdetune<0:
                tmpdetune=abs(tmpdetune)+0b100
            op.detune=tmpdetune
    def ym2610_detune(furnace_dt):
        """Furnace's detune (0..7, 3 is none) as the YM2610 register's (4..7 negative)"""
        dt = furnace_dt - 3
        return abs(dt) + 0b100 if dt < 0 else dt
    
    
    def default_fm_instrument():
        """Furnace's default FM instrument. Furnace leaves the FM feature out of an
        instrument whose FM data equals this, e.g. a new, untouched FM instrument"""
        ifm = fm_instrument(algorithm=0, feedback=4)
        # (tl, ar, dr, sl, rr, mult, dt) of op[0..3], as in Furnace's DivInstrumentFM()
        for tl, ar, dr, sl, rr, mult, dt in [(42, 31, 8, 15, 3, 5, 5), (48, 31, 4, 11, 1, 1, 5),
                                             (18, 31, 10, 15, 4, 1, 0), (2, 31, 9, 15, 9, 1, 0)]:
            ifm.ops.append(fm_operator(detune=ym2610_detune(dt), multiply=mult, total_level=tl,
                                       attack_rate=ar, decay_rate=dr, kvs=2,
                                       sustain_level=sl, release_rate=rr))
        return ifm
    ...
            tmpdetune, op.multiply = ubits(bs.u1(), [6, 4], [3, 0])
            op.detune = ym2610_detune(tmpdetune)
    ...
        # an FM instrument equal to Furnace's default has no FM feature
        if itype == 1 and ins is None:
            ins = default_fm_instrument()

    Tested: an untouched FM instrument now converts. Its output is byte-for-byte the
    same as for that instrument with the FM feature present (forced by changing only a
    field furtool.py doesn't read, block).

    4. furtool.py: SSG macros that nullsound doesn't play

    read_ssg_macro() names macro codes 0–8 only, and the name for code 5 is wrong.
    Furnace's codes (DivMacroType) are:

    • 0 vol, 1 arp, 2 duty (noise frequency), 3 wave, 4 pitch;
    • 5 ex1, 6 ex2 (envelope), 7 ex3 (auto-envelope numerator), 8 alg (auto-envelope
      denominator);
    • 9 fb, 10 fms, 11 ams, 12/13 pan left/right, 14 phase reset, 15–21 ex4–ex10.

    That causes two crashes on macros that Furnace's SSG instrument editor offers:

    • Phase reset (code 14): IndexError: list index out of range.
    • Pitch (code 4), and any other code without a load bit: KeyError: 'pitch',
      because only codes in code_load_bit can be played.

    The change lists all of Furnace's codes, and skips the ones nullsound doesn't play
    with a warning:

        # TODO -1 are unsupported in nullsound
        # name taken from Furnace's newIns.md and UI entries
        code_name = ["vol",
                     "arp",
                     "noiseFreq",
                     "wave",
                     "pitch",
                     "phaseReset",
                     "env",
                     "num",
                     "den"
                     ]
    ...
        for code in macros:
            data = macros[code]
            if HALF_SSG_VOL and code_name[code] == "vol":
        # Furnace's macro codes (DivMacroType); names from the SSG instrument UI.
        # Only the codes in code_load_bit, plus num and den, are played by nullsound
        code_name = ["vol", "arp", "noiseFreq", "wave", "pitch",
                     "ex1", "env", "num", "den", "fb", "fms", "ams",
                     "panLeft", "panRight", "phaseReset",
                     "ex4", "ex5", "ex6", "ex7", "ex8", "ex9", "ex10"]
    ...
        for code in macros:
            data = macros[code]
            name = code_name[code] if code < len(code_name) else "%02x"%code
            if name not in code_load_bit and name not in ["num", "den"]:
                warning("SSG macro not supported by nullsound, ignored: %s"%name)
                continue
            if HALF_SSG_VOL and code_name[code] == "vol":

    The existing warning for keys without an NSS offset printed a stale loop variable
    instead of the key it was about:

                warning("macro element not supported yet: %02x"%code)
                warning("macro element not supported yet: %s"%k)

    Tested:

    • SSG instruments with a phase-reset macro and with a pitch macro now convert, and
      -v shows "SSG macro not supported by nullsound, ignored: …".
    • An instrument with only supported macros (vol, arp) converts as before. The only
      difference in its output is change 2's volume adjustment, since the test module
      had no chip flags.

    5. nullsound: commands 0x80–0xFF jump past the command table

    snd_process_pending_commands (entrypoint.s) uses the command byte from the 68k
    directly as an index into cmd_jmptable, which init_unused_cmd_jmptable fills to
    128 entries. A command from 0x80 to 0xFF jumps to whatever follows the table.

            ld      a, (de)
            ;; retrieve command address in the jump table
            ;;   hl <- &cmd_jmptable[a]
            ld      a, (de)
            ;; the jump table has 128 entries: ignore commands past it
            cp      #128
            jr      nc, _ret_from_cmd
            ;; retrieve command address in the jump table
            ;;   hl <- &cmd_jmptable[a]

    Unknown commands are now ignored, as unused entries below 0x80 already are.

    Reproduced in MAME 0.285 with a game whose cmd_jmptable is the last thing in its
    code (at 0x2BB4).
    A Lua autoboot script wrote command 0x80 to the sound latch at
    0x320000 after the eyecatcher, and the Z80 was traced (-debug -debugger none,
    trace):

    • It jumps to 0x2D34, just past the table.
    • It runs through about 52,000 NOPs (the ROM's zero padding and the banked windows).
    • It then executes 191 instructions of the driver's own RAM as code, starting at
      0xF800 (command_fifo), where the command byte itself becomes add a,b. These
      include di and add hl,hl.
    • It gets back only through a stray rst $30, which lands on a ret.

    The di leaves interrupts off, which stops the timer that drives music. What
    happens depends on what's in RAM at that moment.

    With the change: commands 0x80, 0x81, 0x90, 0xC0, 0xFE and 0xFF were each sent,
    then a valid command. Execution never left the driver code, and the queue
    (command_fifo_current == command_fifo_pending) kept going. Commands 0x05, 0x06 and
    0x7F behave as before. A 16.2 s recording of the boot jingle (-wavwrite) is
    identical, sample for sample.

    6. nullsound: check the RAM layout in a game's own link

    nullsound's state blocks must each stay within one 256-byte page, because they're
    indexed with 8-bit arithmetic. The alignment is fixed padding (align.inc). It's
    right when nullsound's DATA starts at 0xF800, which check-bounds.py verifies, but
    only for the library's own link (linkcheck.map).

    A game's link can break it in two ways, and nothing reports either:

    • Game variables before nullsound.lib (area DATA, linked first) move the
      blocks. With just 16 bytes in front, _state_fm2 spans 0xFB15–0xFC0F, across a
      page, and the driver silently reads and writes the wrong bytes.
    • Game variables after nullsound.lib go into the stack's space. The library
      alone leaves 103 bytes between the end of DATA (0xFF96) and the top of the
      stack (0xFFFD). In MAME I measured 42 bytes of stack used during the jingle with
      a burst of commands, by filling the free RAM with a pattern.

    There's also a trap in the current script: on a map made without -w, the narrow
    format cuts the symbol names, nothing matches, and the check passes having checked
    nothing.

    The proposal makes the script useful for games:

    • it fails when it finds no ranges (and says to link with -m -w);
    • the page error says how to fix it;
    • it checks that at least N bytes (default 64, --stack N) stay free between the
      end of DATA and the stack;
    • it's installed as nullsound-check-bounds.py, so a game can run it on its own
      link map.

    check-bounds.py:

    import sys
    import re
    
    ram={}
    with open(sys.argv[1],"r") as f:
        for l in f.readlines():
            m = re.match(r' *0000(F...) *(_state[^ ]*_(begin|end)) *nullsound', l)
            if m:
                ram[m.group(2)] = m.group(1)
    range_keys=list(dict.fromkeys([re.sub(r"_(begin|end)", "", x) for x in ram]))
    for k in range_keys:
        start, end = ram[k+"_begin"], ram[k+"_end"]
        # print(k, start, end)
        if start[0:2] != end[0:2]:
            sys.exit("range \"%s\" crosses 256 bytes boundaries: [%s, %s]"%(k, start, end))
    #!/usr/bin/env python3
    """Check nullsound's RAM layout in a linker map (sdld -m -w).
    
        python3 check-bounds.py [--stack N] LINK.map
    
    nullsound's state blocks must each stay within one 256-byte page, because
    the driver indexes them with 8-bit arithmetic. Their alignment is fixed
    padding, which is right when nullsound's DATA starts at 0xf800: a game
    that links its own Z80 variables (area DATA) before nullsound.lib moves
    the blocks, and one may then cross a page.
    
    The stack grows down from 0xfffd towards the end of DATA. Variables linked
    after nullsound.lib eat into it, so at least N bytes (default 64) must stay
    free between the end of DATA and the top of the stack.
    
    Run it on the game's own map as well as on the library's.
    """
    
    import re
    import sys
    
    START_OF_STACK = 0xfffd     # entrypoint.s
    DEFAULT_STACK_ROOM = 64     # bytes: the jingle measured 42 in MAME, plus margin
    
    
    def main():
        args = sys.argv[1:]
        room = DEFAULT_STACK_ROOM
        if len(args) == 3 and args[0] == "--stack":
            room = int(args[1], 0)
            args = args[2:]
        if len(args) != 1:
            sys.exit(__doc__)
        ram = {}
        data_start = data_len = None
        with open(args[0], "r") as f:
            for l in f.readlines():
                m = re.match(r' *0000(F...) *(_state[^ ]*_(begin|end)) *nullsound', l)
                if m:
                    ram[m.group(2)] = m.group(1)
                m = re.search(r'0000([0-9A-F]{4}) +s_DATA\b', l)
                if m:
                    data_start = int(m.group(1), 16)
                m = re.search(r'0000([0-9A-F]{4}) +l_DATA\b', l)
                if m:
                    data_len = int(m.group(1), 16)
        if not ram:
            sys.exit("%s: no nullsound state ranges found; link with sdld -m -w "
                     "(the narrow map format cuts symbol names)" % args[0])
        range_keys = list(dict.fromkeys([re.sub(r"_(begin|end)", "", x) for x in ram]))
        for k in range_keys:
            start, end = ram[k+"_begin"], ram[k+"_end"]
            if start[0:2] != end[0:2]:
                sys.exit("range \"%s\" crosses 256 bytes boundaries: [%s, %s]; "
                         "link nullsound.lib before any object with area DATA" % (k, start, end))
        if data_start is None or data_len is None:
            sys.exit("%s: no s_DATA/l_DATA in the map" % args[0])
        free = START_OF_STACK - (data_start + data_len)
        if free < room:
            sys.exit("only %d bytes between the end of DATA (0x%04x) and the stack (0x%04x); "
                     "nullsound needs %d" % (free, data_start + data_len, START_OF_STACK, room))
    
    
    main()

    Makefile.in:

    install-nullsound: $(LIB_MVS) $(LIB_AES)
    	$(INSTALL) -d $(DESTDIR)$(prefix)/z80-neogeo-ihx/lib && \
    	$(INSTALL) $^ $(DESTDIR)$(prefix)/z80-neogeo-ihx/lib
    install-nullsound: $(LIB_MVS) $(LIB_AES)
    	$(INSTALL) -d $(DESTDIR)$(prefix)/z80-neogeo-ihx/lib && \
    	$(INSTALL) $^ $(DESTDIR)$(prefix)/z80-neogeo-ihx/lib && \
    	$(INSTALL) -d $(DESTDIR)$(prefix)/bin && \
    	$(INSTALL) -m 755 check-bounds.py $(DESTDIR)$(prefix)/bin/nullsound-check-bounds.py

    Tested on these link maps:

    Map Result
    linkcheck.map (library build) passes
    a game's driver link (nullsound.lib, then the game's objects without DATA) passes
    16 bytes of game DATA linked after nullsound.lib passes
    16 bytes of game DATA linked before nullsound.lib range "_state_fm2" crosses 256 bytes boundaries: [FB15, FC0F]; link nullsound.lib before any object with area DATA
    48 bytes of game DATA linked after nullsound.lib only 55 bytes between the end of DATA (0xffc6) and the stack (0xfffd); nullsound needs 64
    the same, with --stack 40 passes
    a map made without -w no nullsound state ranges found; link with sdld -m -w …

    A longer-term option would be to make the layout independent of link order, for
    example by placing the aligned blocks in an area of their own at a fixed address.
    That would also free most of the 821 bytes of padding in DATA (out of 1942).

  2. dciabrin commented on Oct 19, 2026

    @dciabrin
    Owner

    There's a lot to unpack here, half of which is misleadingingly characterized as bugs...

    Generally speaking as a maintainer, I do not like having a single bucket of fixes because it's unbounded work and it's hard to have a sense of progress with such reports.

    That plus the fact that AI-generated reviews even out of good will generally flood me as a maintainer, so please that the time to understand the issue if it impacts you before filing a bug, and be prepared to answer questions in case I challenge the code that is proposed.

    For now on, I'm keeping it as low prio because I need that a few issues are effectively valid.

  3. sasja-san commented on Oct 19, 2026

    @sasja-san
    Author

    Very reasonable stance.

    I won't bother you with any more reports like this, then. I was suspecting Claude was venturing off into hallucination land with some of this 😉

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

Metadata

Metadata

Assignees

No one assigned

    Labels

    Projects

    No projects

      Milestone

      No milestone

      Relationships

      None yet

      Development

      No branches or pull requests

      Issue actions