Fix desktop audio rate misalignment and add simple pre-fill & audio timing drift management#10
Open
lxpollitt wants to merge 1 commit into
Open
Conversation
…g drift correction management for desktop
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.
Problem
The desktop emulator sound output was suffering from very frequent glitches (at least on macOS) - stuttering, clicks and pops.
The desktop emulator currently clocks each frame at exactly 50 FPS (
NANOS_PER_FRAME = 20_000_000ns, derived implicitly fromMachineType.PAL.getFramesPerSecond()rounding to50). But I think a real PAL Oric ran 312 scanlines × 64 cycles per scanline at 1 MHz = 19,968 cycles per frame (~50.08 FPS). On its own that's only a ~0.16% accuracy issue, but it makes the emulator's audio sample-production rate (driven by the system clock andNANOS_PER_FRAME) run slower than the audio hardware's consumption rate (driven by the desktop host's audio hardware clock). The result is that the audio buffer frequently runs dry.In addition, the current code doesn't pre-fill the audio buffer at startup, which means jitter in the audio consumer pipeline or in the emulator audio sample production can easily lead to the audio buffer becoming dry for fractions of a second.
On macOS, when combined with the jitter of Java Sound's integration with the Core Audio pipeline, these issues produce audible glitches several times per second during gameplay. (It is possible that other OS audio pipelines are more forgiving - I don't currently have access to Windows or Linux desktops, so don't know for sure. But the underlying rate mismatch still applies so there will still be a shortfall of a small number of audio samples per frame.)
Finally, on some desktop environments the system clock and the audio clock are not tightly coupled and depend on the specific audio hardware being used. So while adjusting
NANOS_PER_FRAMEcan make them theoretically the same, in reality they can still drift very slowly relative to each other. Even on a system with zero jitter, this slow drift can lead to theaudioLinebuffer running dry or overrunning if the emulator is running for a long time, again leading to audio glitches.Fix
Two changes:
JOricRunner: replaced the implicitNANOS_PER_FRAMEderivation withprotected static final int NANOS_PER_FRAME = 19_968_000;, derived directly from the Oric PAL frame structure (312 scanlines × 64 cycles per scanline × 1000 ns).lwjgl3/AY38912PSG: switched to using an explicitaudioLinebuffer size, with simple pre-fill and drift management that roughly targets maintaining ~120ms of audio in theaudioLinebuffer. The audio latency should be comparable to the web version, and be enough to eliminate jitter issues during normal operation. If the buffer fullness is significantly below target then silence is written to top up the buffer. If the buffer is significantly over target then the next chunk of samples is dropped. With theNANOS_PER_FRAMEcycle-rate fix in place these paths fire essentially only at startup (one top-up does the initial pre-fill) and then sit idle - they act as a safety net for any residual drift of real audio hardware sample rate timing not being perfectly aligned with the system clock.Scope
NANOS_PER_FRAMEcycle-rate fix lives in shared code (JOricRunner), so it's inherited by the web and Android platforms too. For the web path, the constant doesn't appear to be used by the active emulation loop inJOricWebWorker(which instead drives cycle execution based on the audio queue consumption rate when sound is on, and based on animation-frame timestamps when sound is off), so there should be no observable change. For Android, the frame rate moves from 50 FPS to ~50.08 FPS, which results in a ~0.16% increase in audio sample production rate. But from what I can see, this is small compared to the existing producer/consumer rate mismatch on Android (which appears to be absorbed today byAudioTrack.write()blocking when the buffer is full), and I'd expect no new audio issues from it. But see notes on testing below.lwjgl3/AY38912PSGso the web and Android audio paths are unaffected.DIAGNOSTIC_LOGGINGstatic-final boolean (defaultfalse). The JIT compiler should eliminate the dead branches when disabled, so there's no runtime cost. Flip totrueif investigating audio behaviour on a specific OS or audio backend.Testing
I've confirmed the fix behaves as expected for me on macOS. The drift with the new
NANOS_PER_FRAMEis low enough that an hour-long emulator session doesn't hit the drift management paths (apart from the initial pre-fill at start). I've also tested with artificially small and largeNANOS_PER_FRAMEin order to force the drift management paths to be exercised regularly and they behave as expected.I've also verified the
NANOS_PER_FRAMEchange did not have any perceivable impact on web.But I don't have access to Android, so have not been able to confirm the
NANOS_PER_FRAMEchange has no perceivable impact on Android. (See code reading analysis above which makes me think it should be ok.)I also don't have easy access to Windows or Linux desktops, so wasn't able to verify the fix works as well on those as it does on macOS (or verify whether the old
NANOS_PER_FRAMEvalue resulted in audible artefacts on those platforms or whether their audio pipelines are in some way inherently less prone than macOS to the audio glitches). It would be good to verify the PR has no negative impact on at least one other OS before merging.