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

Soul Calibur: Broken Destiny, Tekken 6 and other games now missing sound fx since build v0.7.6-1250-gb7152c2-windows-x86 (updated) #2189

Closed
solarmystic opened this issue Jun 10, 2013 · 20 comments

Comments

@solarmystic
Copy link
Contributor

The issue is stated in the title.

(Update:- thanks to bisecting, we can now determine that issue first began with v0.7.6-1250-gb7152c2 b7152c2)

(Update 2:- Tekken 6 is also affected by the issue #2189 (comment))

(Update 3:- This seems to affect a larger amount of games than was previously thought. Dragon Ball Z Tenkaichi, and Dragon Ball Z Shin Budokai 1 and 2 are also affected. Thanks to Combone from the forums and @brujo5 for their own corroborating issue reports.)

Last build to have working sound fx for Soul Calibur is v0.7.6-1249-gdd41d4a-windows-x86.

There are many, many commits in between 1249 and 1258 that don't have a downloadable build from the buildbot repository and one of them could be responsible, which I could not test, unfortunately.

I suspect it is this @unknownbrackets commit b7152c2 that is responsible as it has something to do with the volume of the sound effects, although I could be wrong.

Youtube video comparisons highlighting the aural differences:-

v0.7.6-1249-gdd41d4a-windows-x86 (working sound fx):-

http://www.youtube.com/watch?v=K-goGAMNE7c

v0.7.6-1258-g3185594-windows-x86 (broken sound fx):-

http://www.youtube.com/watch?v=bRLCEZHuX-c

Notice the BGM playing in both videos, but the sound fx in the second video (the affected build) can't be heard at all, as I flick through the menu options.

Logfile from the affected build (v0.7.6-1258-g3185594):-

http://pastebin.com/Fk5Q6k5g

@unknownbrackets
Copy link
Collaborator

Hmm, darn. When did it start having sound effects?

It'd be really helpful if someone could bisect the individual commits.

-[Unknown]

@solarmystic
Copy link
Contributor Author

@unknownbrackets

As I've stated in my opening post, the last downloadable build with functional sound fx is v0.7.6-1249-gdd41d4a-windows-x86.

@unknownbrackets
Copy link
Collaborator

So in other words, each of these commits (which do not include each other) broke it individually?

v0.7.6-1250-g1c12938
v0.7.6-1250-g1d35cc4
v0.7.6-1250-gb7152c2

If so, the game is just trying to be broken. I don't think people will want v0.7.6-1250-g1c12938 reverted and FF4 broken again.

-[Unknown]

@solarmystic
Copy link
Contributor Author

@unknownbrackets

It could also easily be that VAG undead thingy that you reverted in one of your commits. unknownbrackets@3aaabfb

I don't know, I do wish we had individual builds in that list we could download to test. I wouldn't mind helping you narrow it down to the precise build that broke it.

Would reverting that sound effects commit b7152c2 break FF IV again?

I thought this seperate unrelated 1c12938 commit was the one that fixed FFIV?

It's kinda sad that this happened cause the game was practically fully playable with perfect video and audio thanks to @hrydgard save game magic number fixes up to v0.7.6-1249-gdd41d4a-windows-x86.

@hrydgard
Copy link
Owner

It's not "sad", it's just a regression, no big deal.

@unknownbrackets
Copy link
Collaborator

I queued up the builds, they'll take a while. I don't have this game so I can't guess which commit would fix it. That's why I was saying it'd really help if they were individually bisected, but you said that wasn't necessary since anything after 1249 broke it...

-[Unknown]

@solarmystic
Copy link
Contributor Author

@hrydgard

Just a figure of speech.

@unknownbrackets

Thanks for making your separate commits downloadable on the build bot.

I managed to grab those three individual builds the moment the buildbot made them and inexplicably removed them:-

ppsspp-v0.7.6-1250-g1c12938-windows-x86

ppsspp-v0.7.6-1250-g1d35cc4-windows-x86

ppsspp-v0.7.6-1250-gb7152c2-windows-x86

Further testing of those individual commits now clears commits in ppsspp-v0.7.6-1250-g1c12938-windows-x86 1c12938 and ppsspp-v0.7.6-1250-g1d35cc4-windows-x86 1d35cc4 as the suspects. They are innocent.

It also now definitively implicates v0.7.6-1250-gb7152c2-windows-x86 b7152c2 as the origin of the issue, as I suspected earlier on in my opening post.

It was the sound effects commit that was responsible after all.

Youtube video of v0.7.6-1250-gb7152c2-windows-x86:-

http://www.youtube.com/watch?v=pll6HpIm4V4

Logfile from v0.7.6-1250-gb7152c2-windows-x86:-

http://pastebin.com/uQYn3T7Q

@solarmystic
Copy link
Contributor Author

Done some further testing in other games.

Soul Calibur isn't the only game affected by this issue.

Tekken 6 is also affected, possibly because it uses the same sce function for its sound fx.

Youtube comparison between a working build and a broken build:-

v0.7.6-1248-g9e34513-windows-x86 (working)

http://www.youtube.com/watch?v=0gWkVUCczAA

v0.7.6-1250-gb7152c2-windows-x86 (broken)

https://www.youtube.com/watch?v=1dP7xJgHAwA

@unknownbrackets
Copy link
Collaborator

The old code was adding a random value to the envelope height, which is definitely wrong.

Could try (this will reduce it to 13 bits as before, much less than the comment said, and less than JPCSP does):
envelopeValue = (envelopeValue + (1 << 15)) >> 16;

This I'm sure is wrong:
sample = sample * (envelopeValue + 0x4000) >> 15;

It adds something to the envelope prior to multiplying. This makes a bunch of things that should be silent audible. If reverting that part fixes it, the problem is somewhere else (such as bad volume handling, or bad handling of syscalls, or a syscall not being called properly, etc.)

-[Unknown]

@oioitff
Copy link
Contributor

oioitff commented Jun 10, 2013

I wonder how should the envelop really work with following codes:

sceSasSetADSR(%08x, %i, %i, 0x7FFFFFFF, 0, 0, 0x7FFFFFFF)
sceSasSetADSRMode(%08x, %i, %i, 5, 5, 5, 5)

It seems those games just do that. In our current version, it just goes to STATE_RELEASE directly and return envelop height_ 0, so games don't get any volume.

@unknownbrackets
Copy link
Collaborator

Hmm, well, flag may matter but that should mean max attack, max release. 5 = PSP_SAS_ADSR_CURVE_MODE_DIRECT, so that means it's a fixed value.

Does it ever call sceSasSetSL?

I guess that means no ADSR? Assuming the max height value is right, the sane way to interpret that is having a fixed height all the time. We'd retain the too high value all the way into SUSTAIN, at which point we'd be at 0...

What happens if you change this:
return height_ > PSP_SAS_ENVELOPE_HEIGHT_MAX ? PSP_SAS_ENVELOPE_HEIGHT_MAX : height_;

To:
return height_ >= PSP_SAS_ENVELOPE_HEIGHT_MAX ? PSP_SAS_ENVELOPE_HEIGHT_MAX - 1 : height_;

If it's supposed to play sounds before release, I'm not sure. That means our curve handling stuff is probably wrong or something...

-[Unknown]

@oioitff
Copy link
Contributor

oioitff commented Jun 10, 2013

It has called sceSasSetSL, but it should not be a problem, because it set sustainLevel = PSP_SAS_ENVELOPE_HEIGHT_MAX .
So it would hit:

STATE_ATTACK height_ = 0x7FFFFFFF 
(height_ >= PSP_SAS_ENVELOPE_HEIGHT_MAX)==>
STATE_DECAY height_ = 0
(height_ <= 0) ==>
STATE_SUSTAIN height_ = 0

@unknownbrackets
Copy link
Collaborator

Hmm, that means that RELEASE should play something, since KeyOff will set to PSP_SAS_ENVELOPE_HEIGHT_MAX. So then RELEASE sets height to PSP_SAS_ENVELOPE_HEIGHT_MAX because of rate. That means RELEASE should definitely be audible...

-[Unknown]

@oioitff
Copy link
Contributor

oioitff commented Jun 10, 2013

Yep, but right now it just keeps at state STATE_SUSTAIN
Hmm, maybe your previous codes for STATE_SUSTAIN is necessary?

case STATE_SUSTAIN:
        WalkCurve(sustainRate, sustainType);
        if (height_ <= 0) {
            height_ = 0;
            SetState(STATE_RELEASE);
        }

With these codes, it works fine.

@solarmystic
Copy link
Contributor Author

@oioitff and @unknownbrackets

Great findings, both of you.

@unknownbrackets

Perhaps you could just reintroduce those STATE_SUSTAIN codes again to fix this issue?

Issue could also be affecting more than just Soul Calibur and Tekken 6 from these postings:-

#2200

http://forums.ppsspp.org/showthread.php?tid=384&pid=28082#pid28082

EDIT:- Ignore the Megaman one, apparently it never had sound fx at all in any previous build before v0.7.6-1250-gb7152c2

@unknownbrackets
Copy link
Collaborator

@solarmystic it's not something that was ever removed, we never had that code. I did notice JPCSP had it, but asking around people say it's wrong for ADSR. Still, I think it's probably fine. I'm formulating a plan to test how the PSP actually handles ADSR using PCM...

Someone can send a pull with that though. I don't think it'll hurt any games that I've tested.

-[Unknown]

@hrydgard
Copy link
Owner

I think it's probably fine too. If noone sends a pull I'll just commit it myself in a bit, somewhat busy atm.

One easy way to test ADSR should be to key-on a high frequency square wave, and simply look at the shape of the resulting wave after running sceSasCore a few times and concatenating the results.

@solarmystic
Copy link
Contributor Author

@hrydgard

Thanks for the commit 43af3d8

Those games (Tekken 6 and Soul Calibur) are now fixed and have proper sound fx again.

@brujo5
Copy link

brujo5 commented Jun 10, 2013

@hrydgard

dbz games still no sound effects

@hrydgard
Copy link
Owner

@brujo5 now they do.

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

No branches or pull requests

5 participants