Skip to content

Conversation

vsariola
Copy link
Contributor

@vsariola vsariola commented Oct 7, 2021

Previously, the sound registers were used to synthesize fixed amount of sound after every TIC() and this was pushed to SDL. The problem with this approach was crackling if TIC() takes too long to reach 60Hz. In this fix, the sound registers are instead pushed to a ring buffer and the audio is synthesized in SDL callback, to render audio. If the ring buffer does not have enough values, the sound synthesis uses the last good values from the ring buffer, so the music slows down yes, but at least it does not crackle.

The corollary to this is that calling tic_core_tick_end(tic) is not enough during the Studio sound export; one should also call tic_core_synthesize_sound which does the job of pulling the sound registers from the tail of the ring buffer and doing the sound synthesis.

The ring buffer is kept short on purpose, because if there slight desync between the TIC() frequency and sound frequency, the TIC() frequency might run faster and delay might build up. So, the ring buffer length sets a maximum on the delay that this technique can create.

Just to note that you might want to experiment with TIC_SOUND_RINGBUF_LEN and SDL_AudioSpec.samples to get good amount of buffering, without significant delays, and not too much overhead. SDL_AudioSpec.samples controls how many times a second the callback is called and presumably there's a small overhead with each call.

This fix also btw probably fixes the complaints about the sound getting desynchronized with the video; the short ring buffer ensures that the audio can never be too much out of sync with the video.

@vsariola
Copy link
Contributor Author

vsariola commented Oct 7, 2021

I'm getting reports that the studio might feel sluggish with this patch. I don't feel it but perhaps this is related to computer speed. One plausible explanation is that now with the audio synthesis done in the callback, maybe SDL only has one thread for doing event and audio handling? And if we are doing lot of stuff in the audio callback (the whole sound synthesis), then there might not be enough time to handle e.g. key events.

This is just a theory; please test and comment.

Previously, the sound registers were used to synthesize fixed amount of sound after every TIC() and this was pushed to SDL. The problem with this approach was crackling if TIC() takes too long to reach 60Hz. In this fix, the sound registers are instead pushed to a ring buffer and the audio is synthesized in SDL callback to render audio. If the ring buffer does not have enough values, the sound synthesis uses the last good values from the ring buffer, so the music slows down yes, but at least it does not crackle.

The corollary to this is that calling tic_core_tick_end(tic) is not enough during the Studio sound export; one should also call tic_core_synthesize_sound which does the job of pulling the sound registers from the tail of the ring buffer and doing the sound synthesis.

The ring buffer is kept short on purpose, because if there slight desync between the TIC() frequency and sound frequency, the TIC() frequency might run faster and delay might build up. So, the ring buffer length sets a maximum on the delay that this technique can create.
@vsariola
Copy link
Contributor Author

vsariola commented Oct 9, 2021

I reduced slightly the callback frequency; SDL_AudioSpec.samples is now 1024. I also made the ring buffer have a length of 6 items. Furthermore, I cleaned some unnecessary whitespace.

@nesbox
Copy link
Owner

nesbox commented Oct 9, 2021

Hi, thank you for the PR, but unfortunately I'm in a hospital now and can't review/merge it. Sorry.

@vsariola
Copy link
Contributor Author

vsariola commented Oct 9, 2021

No problem! Get well soon!

Copy link
Owner

@nesbox nesbox left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just tested your version and it works perfectly, thank you very much for fixing this annoying issue.
One thing, we also have to update the player.c and other implementations, like libretro/n3ds/sokol/...

@nesbox nesbox merged commit 1965b60 into nesbox:master Nov 1, 2021
nesbox added a commit that referenced this pull request Nov 1, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants