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

Fix SampleChannelBass reporting wrong state #6401

Draft
wants to merge 4 commits into
base: master
Choose a base branch
from

Conversation

hwsmm
Copy link
Contributor

@hwsmm hwsmm commented Oct 25, 2024

This PR fixes SampleChannelBass reporting wrong state.

  • Calling Stop no longer makes IsAlive false.
  • Calling Play after disposal no longer makes enqueuedPlaybackStart true, so that Playing won't be true (this can be tested with TestDoesNotRestart because SampleChannel is disposed after the end of playback with Dispose SampleChannel in Sample if it is not disposed yet #6397).

Do not merge this yet. What should we do for looping samples? Stop no longer makes IsAlive false, but looping sample will be a zombie because it doesn't end. Should we make SampleChannels not resumable?

This commit also adds one for SampleChannelVirtual.
It is needed to let childern use IsAlive from parent of SampleChannel.
Before this commit, calling Stop makes playing false, which in turn makes IsAlive false.
@smoogipoo
Copy link
Contributor

smoogipoo commented Nov 20, 2024

Should we make SampleChannels not resumable?

How about we add a bool ManualFree { get; set; } to the interface? Opposite of BASS' auto-free because interfaces can't have default values (we really should be doing sample.GetChannel(new ChannelProperties { ... }) or similar, but...).

And then, every other case should auto-free when playback ends or Stop() is called.

I believe everywhere in osu! we operate under the assumption that channels are auto-freeing by default as I believe that's effectively how they work in BASS when they're HCHANNELs, so this could be considered a regression from when mixers were added (and they were made into HSTREAMs).

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

Successfully merging this pull request may close these issues.

2 participants