Fix noise period 0 running at twice the speed of period 1#16
Open
lxpollitt wants to merge 2 commits into
Open
Conversation
…unters The handlers for the tone (R0-R5), noise (R6) and envelope (R11/R12) period registers adjust the running countdown to the next flip-flop toggle when the period changes. The adjustment's sign was inverted: it subtracted the period change from the remaining count instead of adding it. The counters count down to the next toggle, so preserving the time already elapsed since the last toggle - which is what the real chip does, as a period write only changes the value the counter is compared against - requires moving the remaining count by the same amount as the period. With the inverted sign, every period write displaced the next toggle in the wrong direction by twice the period change: period increases could force an immediate spurious toggle (an audible click), and period decreases stalled the next toggle by up to nearly a full old period. The audible effect was extra transient noise during repeated period writes, e.g. pitch slides, vibrato and alternating tones, giving them a buzzy texture. Steady tones were unaffected, since the adjustment only happens at write time and the counter refills correctly from the new period at each toggle.
The noise period register (R6) handler mapped a written value of 0 to half the period 1 value, making the noise generator's shift rate twice as fast as period 1. The data sheet notes that, as with the tone period, the lowest noise period value is 1 (divide by 1), so a written value of 0 behaves the same as 1. This has also been verified on original Oric-1 hardware: noise periods 0 and 1 sound identical there (while 1 and 2 are clearly distinguishable), whereas the emulation produced noticeably brighter noise for period 0. The zero mapping predates the code's doubling of the noise period value and was not updated when the doubling was added, which is how "0 behaves as 1" silently became "0 behaves as a half". The fix applies the zero mapping before the doubling.
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
1 participant
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.
Note: This PR is stacked on #15 (the period register writes fix) because the two touch adjacent lines in the noise period handler. I've based it on #15 rather than directly on master so that the two apply cleanly in the order I'd expect them to be merged (#15 first, then this), with no merge conflicts needing to be resolve. The trade-off is that this PR shows two commits - please review only the second ("Fix noise period 0 running at twice the speed of period 1"); the first is #15 and will drop out of this diff automatically once #15 is merged.
Context
Fifth 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
A noise period of 0 plays noise that doesn't match the original Oric hardware - the noise generator runs at twice the rate it should. On the real chip a noise period of 0 behaves the same as a period of 1 (the data sheet notes that, as with the tone period, the lowest noise period value is 1).
To hear it: enable noise on channel A with
PLAY 0,1,0,0, then compareSOUND 4,0,15(noise period 0) withSOUND 4,1,15(noise period 1). On the current web version period 0 is audibly different from period 1; on a real Oric-1 the two are identical.The cause: the noise period handler maps a written value of 0 to "period 1", but it does so after the code has already doubled the period value, so 0 ends up mapped to half of period 1 - i.e. twice the rate.
Fix
Apply the zero-to-one mapping before the doubling rather than after, so a noise period of 0 correctly behaves as period 1.
No equivalent change is needed in the tone period path: that handler clamps to a minimum period (one sample per half-cycle) as an anti-aliasing guard, which already covers a written value of 0.
Scope
Web platform only - one file (GwtAYPSG), a small change to the noise period handler. No changes to core or the other platforms. Only a noise period of 0 is affected; all other noise periods are unchanged.
Testing
Tested on macOS in Chrome. Confirmed
SOUND 4,0,15andSOUND 4,1,15(afterPLAY 0,1,0,0) now sound identical, instead of period 0 being noticeably different. I also verified this against a real Oric-1: on real hardware noise periods 0 and 1 are indistinguishable, while 1 and 2 are clearly different - matching the fixed behaviour.