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

SasAudio: Always reinitialize the VAG decoder on sceSasSetVoice, even if already playing #18076

Merged
merged 4 commits into from
Sep 7, 2023

Conversation

hrydgard
Copy link
Owner

@hrydgard hrydgard commented Sep 5, 2023

Fixes terrible noises in After Burner.

EDIT: Also breaks the engine noise in Ridge Racer, so this needs more work.

Not sure why the logic was like it was, blame history wasn't much clue either.

Will of course test a large number of other games before merging.

@hrydgard hrydgard added the Audio label Sep 5, 2023
@hrydgard hrydgard added this to the v1.16.0 milestone Sep 5, 2023
@hrydgard hrydgard marked this pull request as draft September 5, 2023 11:40
@hrydgard
Copy link
Owner Author

hrydgard commented Sep 5, 2023

Oops, breaks the engine noise in Ridge Racer. There is something to this check.. What seems to happen in After Burner is that the VagDecoder and the Voice address parameters get out of sync.

@unknownbrackets
Copy link
Collaborator

Should it still set playing = on if it's not VAG, like noise or etc.? Maybe the engine noise uses that?

And should it really restart VAG on every pitch change? That seems wrong.

-[Unknown]

@hrydgard
Copy link
Owner Author

hrydgard commented Sep 6, 2023

It is indeed wrong to reset the VAG on just a pitch change, but, the fact that After Burner sounds right with this change kinda confirms that some detail of this is likely the cause of its problem, at least. I think this will just need a small extension of the old check to work correctly.

@unknownbrackets
Copy link
Collaborator

Is it enough to ignore v.ChangedParams(vagAddr == prevVagAddr); this condition? I wonder what that was added for, but it's been there a long time and I'm not sure if it's verified in tests or not:

4deaec8

Maybe it's just that every set voice restarts it? That would make more sense than resetting on pitch.

And that pitch thing looks pretty suspect too:

7346c4a

I guess this is really asking for more VAG tests...

-[Unknown]

@hrydgard
Copy link
Owner Author

hrydgard commented Sep 6, 2023

Oh I didn't even notice that ChangedParams was called from SetPitch as well. That's likely part of why Ridge Racer broke - reinitializing the vag decoder on solely a pitch change is just wrong indeed. It seems that the part of ChangedParams that is useful from there is basically if (!playing && on) playing = true. Which may or may not itself be accurate..

And there are indeed lots of possibilites in the current code for things to go wrong when changing voice type.

Yeah, tests would indeed be good, heh.

@hrydgard
Copy link
Owner Author

hrydgard commented Sep 6, 2023

I simply got rid of the confusing ChangedParams thing. Now both Ridge Racer and After Burner work great. Will test some more games.

@hrydgard hrydgard marked this pull request as ready for review September 6, 2023 13:28
v.loop = loop ? true : false;
v.ChangedParams(vagAddr == prevVagAddr);
v.loop = loop != 0;
v.playing = true;
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think we should still care about on? You can key on or off a voice before or after setting the VAG, I'm pretty sure tests do show that already...

-[Unknown]

Copy link
Owner Author

Choose a reason for hiding this comment

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

Oh yeah, I suppose so. I'll add that in.

@hrydgard
Copy link
Owner Author

hrydgard commented Sep 6, 2023

Added the check. The difference from before here is that on sceSasSetVoice, VAG parameters are now always updated - while still obeying the .on flag with regards to playing.

So far all seems fine. I'm hoping that this might also fix the occasionally reported stuck sounds in GTA, but who knows.

@hrydgard hrydgard merged commit c08a5d3 into master Sep 7, 2023
@hrydgard hrydgard deleted the after-burner-fix branch September 7, 2023 07:23
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