Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[libretro] Audio is distorted when using Runahead single-instance mode #31

Open
msheehan79 opened this issue Jul 12, 2023 · 5 comments
Open
Labels

Comments

@msheehan79
Copy link
Contributor

When the runahead feature is enabled in RetroArch for this core, the audio becomes distorted and requires you to enable the second instance mode to render audio properly.

Ideally, if the audio information can be serialized fully in the save state, this should allow runahead to work in single-instance mode without distortion.

I found during some testing my side that if I added m_pApu and m_pBuffer to the save state in Audio.cpp, and removed the reset + clear functions from the load state that audio was clear with runahead enabled. But I'm not sure if there is any negative impacts to this, apart from the obvious impact to compatibility of existing states.

@drhelius drhelius added the bug label Jul 20, 2023
@msheehan79
Copy link
Contributor Author

@drhelius I've been testing the audio save state adjustment I made on https://github.com/msheehan79/Gearcoleco/commits/test-serialization and this seems to fix audio distortion with runahead enabled on all the games I have tried. The only part I'm not 100% sure on is how big the serialized data really needs to be; I set it to 255 on both and that seems to work, but not sure if there is more accurate way to calculate the exact size needed.

If there is a desire to preserve the existing Save State format, one solution is the libretro API has a callback (RETRO_ENVIRONMENT_GET_AUDIO_VIDEO_ENABLE) that allows you to identify if the save state is a runahead-type save state (in which case the additional audio values need to be serialized) or a normal save state (in which case we could omit these audio flags and retain the original size and format to retain compatibility). We could pass a parameter down to the core to indicate which type of save state to create and return the correct type accordingly.

If you think the above approach is worth implementing, I could take a crack at it when time allows.

@drhelius
Copy link
Owner

Sorry for the late reply. Let me check your code and understand what is going on. Never tested run ahead before.

I like the idea of generating a different save state for the run ahead only, yes that's ok.

I'll get back to you.

@drhelius drhelius changed the title Libretro: Audio is distorted when using Runahead single-instance mode [libretro] Audio is distorted when using Runahead single-instance mode Dec 28, 2023
@drhelius
Copy link
Owner

drhelius commented Jan 2, 2024

Sorry for the delay. Saving the state of the APU is a bit more complex that what you have in your code. I may be able to add it but is not that simple. Your method can trash the memory.

But it looks like is working fine if you enable the second instance in RA. Just for me to understand, the benefit of running it single-instanced is just performance right?

@msheehan79
Copy link
Contributor Author

Thanks for your inputs on this, and honestly not surprised my method did not work - it was definitely just a shot in the dark approach that seemed to work on the surface but I had my doubts.

In my experience its not just performance for runahead second-instance. I've run into weird issues before with the 2 cores not syncing up, where for example the core is in one state and the audio is out of sync. I don't remember specifics as it was several months ago but I know that the general view is that second instance mode is basically a workaround for the audio issues but it had its drawbacks, in addition to the additional overhead of having to run 2 instances of the core.

I've tried to use single-instance mode in as many cores as I can for that reason. But I totally understand if its a non-trivial implementation for a feature you don't use.

@drhelius
Copy link
Owner

drhelius commented Jan 3, 2024

Understood. Thanks for the clarification.
I'll keep it as a feature to implement when time allows.

@drhelius drhelius added feature and removed bug labels Jan 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: No status
Development

No branches or pull requests

2 participants