GUS: Fix the ADC sample rate divisor missing the +2 bias - #7941
Merged
Merged
Conversation
Register 48h holds FREQ, and the SDK gives the rate as 9878400 / (16 * (FREQ + 2)). The divisor used 617400 / FREQ, so writing 12 for 44100 Hz armed the record sample timer at 19.44 us instead of 22.68 us and paced the record DMA about 16.7% faster than the guest asked for. The quotient was also stored into a uint16_t before it was range checked, so a write of 4 wrapped 154350 down to 23278, and inputlatch was taken from the unclamped value, so the clamp only ever reached the log string. Compute the rate in a uint32_t that cannot wrap, keep the 44.1 kHz ceiling the SDK documents, and derive inputlatch from that value. The + 2 removes the division by zero on a write of 0 without a separate guard. The 4000 Hz floor is dropped rather than applied to the timer: the SDK states no minimum, and FREQ = 255 is a real 2402 Hz that the floor would have forced up to 4000 Hz.
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
2 participants
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.
Summary
Writing the GF1 recording sample rate register (index 48h) programs the wrong timer interval, and one value divides by zero.
src/sound/snd_gus.c:760computed the rate as617400 / gus->adc_srate. The Ultrasound SDK gives it asrate = 9878400 / (16 * (FREQ + 2))in section 2.6.1.6, and the comment on that same line carries the inverse form,9878400 / (freq * 16) - 2, so the same bias was already documented and only the code dropped it. Writing 12, the documented value for 44100 Hz, therefore armed the record sample timer at 19.44 us instead of 22.68 us, sogus_input_poll()paced the recording DMA about 16.7% faster than the guest asked for and raised the terminal count IRQ atsrc/sound/snd_gus.c:387about 14.3% early in wall time. 22050 Hz was 7.7% fast and 11025 Hz 3.7% fast.Two smaller problems came out of the same line. The quotient was stored into
uint16_t gus->adc_freq(src/sound/snd_gus.c:247) before it was range checked, so a write of 4 wrapped 154350 down to 23278. Andtempatsrc/sound/snd_gus.c:761was taken from that value before the 4000/44100 clamp ran, whiletempalone fedgus->inputlatch, so the clamp only ever reached the debug log string and never the timer. A write of 0 divided by zero.The change computes the rate the way the SDK documents it, in a
uint32_tthat cannot wrap, and derivesinputlatchfrom it. The+ 2removes the division by zero on its own, so no extra guard is needed.The 44.1 kHz ceiling stays, since that is the maximum the SDK documents and it stops a write of 0, which asks for 308700 Hz, from arming a 3.2 us timer. The 4000 Hz floor is dropped. Now that the clamped value actually reaches the timer, keeping that floor would be a real behavior change rather than dead code, and the wrong one: FREQ = 255 is a genuine 2402 Hz on hardware, and the SDK states no minimum anywhere. For every register value that asks for a rate at or below the ceiling, the new interval is within 0.04% of the hardware formula.
One thing this does not fix: the GUS ADC is still a stub, since
gus_input_poll()atsrc/sound/snd_gus.c:364DMAs constant filler bytes rather than sampling anything. So this corrects the pacing of the recording DMA and the IRQ that ends it, not the content. The program that path exists for is MegaEM 3.x, named in the commit that added this handler.I checked this by rebuilding the arithmetic of both versions in a standalone program. Value 12 gives a 19.44 us latch before and 22.68 us after, and value 26 gives 23746 Hz before and 22050 Hz after. No new warnings under the project's own flags; with
-Wextraadded, the same nine pre-existing-Wmissing-field-initializersin the device config tables appear before and after. I have not run this inside a built emulator, so the numbers are the arithmetic only.Checklist
References
Ultrasound Software Development Kit 2.10, section 2.6.1.6 "Sampling Frequency - (48)", which states
rate = 9878400/(16*(FREQ+2)), and the feature list stating "Playback and recording rates up to 44.1 kHz": http://archives.oldskool.org/pub/drivers/Gravis/UltraSound/ULT/programming/sdk2.10/ULTRADOC.TXTThe commit that added this register handler, for context on the MegaEM 3.x case: c6f67b979