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

Add an option to change behavior of IsAlive in SampleChannel #6401

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

hwsmm
Copy link
Contributor

@hwsmm hwsmm commented Oct 25, 2024

This PR provides an option to change behavior of IsAlive in SampleChannel. To make auto-free work, #6397 is required.

If ManualFree is false (default), it works the same as before: IsAlive will be false after Stop or the end of playback.
If ManualFree is true, IsAlive will never be true until the user explicitly calls Dispose.

It also contains a fix for an issue where calling Play after disposal makes Playing true.

@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).

@hwsmm
Copy link
Contributor Author

hwsmm commented Nov 23, 2024

I think adding ManualFree flag is unnecessary as it's only relevant when Looping is true. Adding a comment about this may be helpful?

When Looping is false, SampleChannel is automatically disposed once playback ends, but when Looping is true, you're responsible for disposing this channel manually, as the framework can't detect its end.

@smoogipoo
Copy link
Contributor

So what happens when the channel is played, manually stopped, and the ISample is just left to live (as we never dispose samples in game)? It sounds like in this case samples would never be disposed, no?

@hwsmm
Copy link
Contributor Author

hwsmm commented Nov 26, 2024

That's right. I'll live until it gets disposed somehow.

I think there is a misunderstanding about ManualFree. Do you want channels to be unable to resume playback once it is stopped when ManualFree is false?

I first assumed that you don't want such behavior, but I guess I misread your comment.

I'm okay with that if that's what you mean.

@smoogipoo
Copy link
Contributor

Yeah, my idea is to consider Stop() the same as playback end. That way everything is safe except when you set ManualFree = true, which is the only case you need to make a conscious effort to handle it.

@hwsmm hwsmm changed the title Fix SampleChannelBass reporting wrong state Add an option to change behavior of IsAlive in SampleChannel Nov 29, 2024
@pull-request-size pull-request-size bot added size/S and removed size/M labels Nov 29, 2024
@hwsmm hwsmm marked this pull request as ready for review November 29, 2024 16:07
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