Fix envelope shape decoding for hold shapes 11, 13 and 15#14
Open
lxpollitt wants to merge 1 commit into
Open
Conversation
The envelope shape register (R13) write handler contained an extra assignment for the continue-plus-hold shapes: it overwrote the attack state with the alternate bit's raw value before the envelope cycle started. The attack state determines both the envelope's ramp direction and, via the end-of-cycle handling, the level a holding envelope holds at, so these shapes played wrong ramps and held at wrong levels. Shape 13 (attack then hold at maximum) instead started at maximum and faded to silence; shape 11 (decay then hold at maximum) played an erratic scrambled ramp and held 6 dB low; shape 15 (attack then hold at silence) held at a loud level instead. Shape 9 survived only by coincidence, as the wrong assignment happens to write the correct value for that shape. The per-sample envelope code already implements the data sheet's hold behaviour correctly when a cycle completes: it holds the last count, or flips to the initial count first when both the Hold and Alternate bits are set. The fix is simply to remove the bad assignment from the write handler. These shapes are reachable from Oric BASIC: PLAY envelope modes 5 and 7 map to shapes 11 and 13 via the ROM's envelope pattern table.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Context
Third in the series of AY-3-8912 sound emulation improvements, web version first as before. The WIP deployment with the whole series applied is available here if you want to hear the overall impact: https://joric-wip.pages.dev
Problem
Three of the envelope shapes - 11, 13 and 15, the "continue and hold" shapes that ramp once and then hold at a fixed level - decode incorrectly, so they play the wrong ramp and hold at the wrong volume.
Two of these are reachable from Oric BASIC via the PLAY command's envelope parameter, which maps to the shape register through a ROM table: PLAY envelope mode 7 selects shape 13, and mode 5 selects shape 11. (I don't think shape 15 is reachable from BASIC - see Testing.)
Two examples to reproduce to hear:
PLAY 0,0,0,0:WAIT 100:SOUND 1,100,0 : PLAY 1,0,7,2000- shape 13, which should ramp up and hold at maximum. On the current web version it instead starts at maximum and fades to silence.PLAY 0,0,0,0:WAIT 100:SOUND 1,100,0 : PLAY 1,0,5,2000- shape 11, which should decay once and then hold at maximum. On the current web version it plays an erratic ramp and holds below maximum (at level 13 instead of 15).The cause is in the envelope shape register (R13) write handler. For the hold shapes it stores the Alternate bit as its raw value (0 or 2) and then also assigns that into the attack state, which both controls the ramp direction and feeds the held-level calculation - so the held volume comes out corrupted.
Fix
The per-sample envelope logic already implemented the correct hold behaviour (hold the last count, or flip to the initial count when the Alternate bit is set), so we just needed to remove the erroneous assignment to the attack state from the R13 handler. The shapes then decode correctly: shape 13 ramps up and holds at maximum, shape 11 decays once and holds at maximum, and shape 15 ramps up and holds at silence.
Scope
Web platform only - one file (GwtAYPSG), a three-line deletion. No changes to core or the other platforms. Only shapes 11, 13 and 15 change behaviour; all other envelope shapes are unaffected.
Testing
Tested on macOS in Chrome. Confirmed
PLAY 1,0,7,2000(shape 13) now ramps up and holds at maximum instead of fading to silence, andPLAY 1,0,5,2000(shape 11) now decays cleanly and holds at maximum instead of the erratic, below-maximum behaviour. I don't think shape 15 is reachable from Oric BASIC (the ROM's PLAY table has no entry that maps to it), so I could not exercise it from BASIC, but it goes through the identical corrected code path as shapes 11 and 13, so I would expect any games that make use of it to benefit from this fix.