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

AUDIO: Use SeekableSubReadStream instead of reallocating streams #2786

Open
wants to merge 3 commits into
base: master
from

Conversation

@SupSuper
Copy link
Contributor

@SupSuper SupSuper commented Feb 19, 2021

This cleans up audio code that mallocs raw audio data just to discard format headers, instead of just wrapping the stream. This reduces redundant reallocations since a lot of engines/decoders already do this anyways, and makes it easier to add WAV decoders since the extra headers are always discarded now.

One downside of this change is some engines have edge cases freeing sounds that are still being played (eg. closing the game mid-sound), which now crashes. I tried to fix the engines I ran across, but obviously I can't test every engine using raw WAVs, so I'd appreciate extra testing (and engine fixes) for this.

@mgerhardy
Copy link
Contributor

@mgerhardy mgerhardy commented Feb 19, 2021

It would maybe be a good idea to stop all sounds and music when the return-to-launcher-event is added to the queue - or is that too late already?

@SupSuper
Copy link
Contributor Author

@SupSuper SupSuper commented Feb 19, 2021

@mgerhardy The base Engine destructor already stops the mixer, but by then it's too late. You could change it to happen before the destruction instead, or just let the mixer free the resources.

Since this is a rare edge case (most audio decoders assume the resources won't be freed while in use), I think it's better to fix any problematic engines than try to cover it up in the common code.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

2 participants