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

When seeking to atrac positions, force alignment and start from zero #6976

Merged
merged 4 commits into from
Mar 25, 2015

Conversation

unknownbrackets
Copy link
Collaborator

Starting from zero allows us to actually get the correct data. Otherwise, whether forwards or backwards, we get junk for a frame or two.

This fixes some sound artifacts when looping.

-[Unknown]

@daniel229
Copy link
Collaborator

Break BGM in GTA .
57:19:273 mix sound th I[ME]: HW\MediaEngine.cpp:85 FF: Invalid gain location: ch=0, sb=4, pos=1, val=33
57:19:273 mix sound th E[ME]: HLE\sceAtrac.cpp:408 avcodec_decode_audio4: Error decoding audio -1094995529 / bebbb1b7
57:19:273 mix sound th I[ME]: HW\MediaEngine.cpp:85 FF: Invalid gain location: ch=0, sb=4, pos=1, val=33
57:19:273 mix sound th E[ME]: HLE\sceAtrac.cpp:408 avcodec_decode_audio4: Error decoding audio -1094995529 / bebbb1b7
57:19:273 mix sound th I[ME]: HW\MediaEngine.cpp:85 FF: Invalid gain location: ch=0, sb=4, pos=1, val=33
57:19:273 mix sound th E[ME]: HLE\sceAtrac.cpp:408 avcodec_decode_audio4: Error decoding audio -1094995529 / bebbb1b7

@unknownbrackets
Copy link
Collaborator Author

When, is that after a seek? Does the same audio also have GHA phase shifting?

Maybe it needs that "forcing" because it's seeking by overwriting frames in the buffer (even though it seems to try to reconstruct the entire file.) But that means we'll never get the right data out of ffmpeg's demuxer.

-[Unknown]

@daniel229
Copy link
Collaborator

What is after a seek? But the BGM seems no GHA phase shifting.

@unknownbrackets
Copy link
Collaborator Author

Well, should I close this pull?

As long as I'm going to have to create one pull request that simultaneously fixes every bug in sceAtrac, I'm not going to waste my free time on it. This game already logs and reports this error with or without these changes merged, so it's clearly already broken.

And there's no test case or data showing anything except that my change is correct.

So, I'm not going to waste my time further on sceAtrac.

-[Unknown]

@hrydgard
Copy link
Owner

Sorry about that. I was hoping to find time to fix it or make a test case before merging, but I just haven't and then forgot. I'll give it another shot really soon and then probably merge either way...

I agree that it looks like the game is already broken but superficially it actually does appear to work correctly with the current code, which is why I've been dreading the resulting complains flood on a merge...

@sum2012
Copy link
Collaborator

sum2012 commented Nov 13, 2014

I think that we can merge after v1.0.
Then open a issue of it.

@daniel229
Copy link
Collaborator

Last changes help #7278 and #6820 break GTA,and break bgm looping in Radiant Mythology 2

@daniel229
Copy link
Collaborator

Break bgm in Kurohyou 2

@unknownbrackets
Copy link
Collaborator Author

Hmm, is that reproducible in the demo? It seems to be working, but I have some debugging code in.

In other news, I finally understood enough to beat the first fight in that demo.

-[Unknown]

@daniel229
Copy link
Collaborator

Seems not happen in demo,the bug happen in the savedata screen.

@daniel229
Copy link
Collaborator

Radiant Mythology 2 and Kurohyou 2 broken in unknownbrackets@ad3ab88
#7278 and #6820 fixed in unknownbrackets@70a3d0f

unknownbrackets@70a3d0f also break Radiant Mythology 2? or the previous?

@unknownbrackets
Copy link
Collaborator Author

What if you add before int got_frame = 0; (in the change that broke those two games):

if (packet->size < (int)atracBytesPerFrame) {
    return ATDECODE_FEEDME;
}

And add after:

memcpy(tempPacket.data + initialSize, packet->data, to_copy);

This:

NOTICE_LOG(HLE, "Pulling %d bytes into packet, need %d", to_copy, needed);

I assume some values of to_copy are less than needed, just wondering how they look.

-[Unknown]

@daniel229
Copy link
Collaborator

That may fixes the Radiant Mythology 2,but not Kurohyou 2,I don't see any NOTICE_LOG when running these games

@daniel229
Copy link
Collaborator

In Kurohyou 2 demo,hit a GHA phase shift,the bgm would stop,the master buid won't.
Go to the second option in the main menu,the bgm in battle would hit GHA phase shift.

@unknownbrackets
Copy link
Collaborator Author

I did some special move and the screen turned black while it asked me to press buttons like triangle and square. Pretty sure that's a bug but not directly related... probably more depal problems in this game.

Hmm, I let the fight last a pretty long time, still can't get a GHA phase shift. It's the first fight after selecting the second item, right? I don't have to go find anyone?

-[Unknown]

@daniel229
Copy link
Collaborator

Just the first battle,possiable different BGM?it happen when going to the battle.could make a savestate in the "VS" screen,after pressing circle, the BGM of the battle play a sound and then stop,it get that error.

@daniel229
Copy link
Collaborator

Surely cound have different bgm in that battle,try serveral times savestate could get it.

@daniel229
Copy link
Collaborator

A savestate for the GHA phase shift in Kurohyou 2 demo.
http://1drv.ms/1w9f5fl

@vnctdj
Copy link
Contributor

vnctdj commented Jan 12, 2015

This pull request makes no change to Monster Hunter Freedom Unite, there is still the same message in the log console as I stated in this comment : #7323 (comment)

@daniel229
Copy link
Collaborator

Fixes the crash after title BGM looping in Aliens Vs Predator Requiem.

@daniel229
Copy link
Collaborator

The Kurohyou 2 demo is good now with the GHA phase shift fix.The retail verson is still not good.

35:11:441 sgx-psp-at3- I[ME]: HW\MediaEngine.cpp:85 FF: Invalid gain location: ch=0, sb=0, pos=2, val=31
35:11:441 sgx-psp-at3- E[ME]: HLE\sceAtrac.cpp:415 avcodec_decode_audio4: Error decoding audio -1094995529 / bebbb1b7
35:11:441 sgx-psp-at3- I[ME]: HW\MediaEngine.cpp:85 FF: Invalid gain location: ch=0, sb=0, pos=2, val=31
35:11:441 sgx-psp-at3- E[ME]: HLE\sceAtrac.cpp:415 avcodec_decode_audio4: Error decoding audio -1094995529 / bebbb1b7
35:11:441 sgx-psp-at3- I[ME]: HW\MediaEngine.cpp:85 FF: Invalid gain location: ch=0, sb=0, pos=2, val=31
35:11:441 sgx-psp-at3- E[ME]: HLE\sceAtrac.cpp:415 avcodec_decode_audio4: Error decoding audio -1094995529 / bebbb1b7

@daniel229
Copy link
Collaborator

I found some errors happen in the Kurohyou 2 demo.It happen when you entrying or exiting the shop

49:31:413 sgx-psp-at3- I[ME]: HW\MediaEngine.cpp:85 FF: Frame data doesn't match channel configuration!
49:31:413 sgx-psp-at3- E[ME]: HLE\sceAtrac.cpp:415 avcodec_decode_audio4: Error decoding audio -1094995529 / bebbb1b7

Load the savestate and press circle button.
http://1drv.ms/1CiACut

@unknownbrackets unknownbrackets force-pushed the atrac-minor branch 2 times, most recently from a7764b3 to 8d40561 Compare February 1, 2015 20:51
It seems like we can only (safely) seek to sample boundaries, which makes
sense.  Also, we get wrong data unless we "warm" it by seeking from 0, it
seems like.

This makes looping produce correct data.  Fixes hrydgard#6970.
@sum2012
Copy link
Collaborator

sum2012 commented Feb 28, 2015

I use v1.0.1-31-g5ebfa57 + this pull request,
GTA voice still not solved

01:29:820 ATRAC3 play D[ME]: HLE\sceAtrac.cpp:998 00000000=sceAtracDecodeData(0, 08e29500, 0bfd4e40[00000800], 0bfd4e54[00000000], 0bfd4e44[694])
01:29:859 ATRAC3 play D[ME]: HLE\sceAtrac.cpp:1229 sceAtracGetRemainFrame(0, 0bfd4e44)
01:29:860 ATRAC3 play I[ME]: HW\MediaEngine.cpp:85 FF: Invalid gain location: ch=0, sb=4, pos=1, val=13
01:29:860 ATRAC3 play E[ME]: HLE\sceAtrac.cpp:415 avcodec_decode_audio4: Error decoding audio -1094995529 / bebbb1b7
01:29:860 ATRAC3 play D[ME]: HLE\sceAtrac.cpp:998 80630024=sceAtracDecodeData(0, 08e2b500, 0bfd4e40[00000000], 0bfd4e54[00000001], 0bfd4e44[0])
01:29:860 ATRAC3 play E[ME]: HLE\sceAtrac.cpp:1133 UNIMPL sceAtracGetInternalErrorInfo(0, 0bfd4e60)

full log:https://gist.github.com/sum2012/b9bf90aa6355dff03b31

@sum2012
Copy link
Collaborator

sum2012 commented Mar 9, 2015

@unknownbrackets
Not directly rated to this pull request.
Just for note.
For the JPCSP emulator for return less samples at the first
call,it also do a memory move the sample buffer to skip the needed samples
http://code.google.com/p/jpcsp/source/detail?r=3714
Maybe PPSSPP also need to do the same thing ?

@unknownbrackets
Copy link
Collaborator Author

I'm not going to worry specifically about how JPCSP has chosen to implement things. Instead, my concern is making the decoded atrac data match (with some fuzz, since the decode is not precisely the same) what the PSP firmware generates with the same syscalls.

We currently generate the correct first and second chunk of samples. So any addition of memory moving would probably make it incorrect.

It looks like JPCSP is decoding the entire chunk, and then moving that decoded data to skip the initial bytes. This is actually incorrect (fyi, @gid15), since that sounds like it would overwrite memory after the chunk that would not be overwritten on a real PSP. It may cause problems in games like Resistance: Retribution, iirc. Memory should not be modified beyond the returned sample count.

-[Unknown]

@hrydgard
Copy link
Owner

Right, I almost forgot ...... it's time to merge this and break stuff.

Will play with it a bit and merge soon.

@sum2012
Copy link
Collaborator

sum2012 commented Mar 20, 2015

Hope we can fix GTA before major release

@sum2012
Copy link
Collaborator

sum2012 commented Mar 22, 2015

@hrydgard
At least #7625 solved before merge

@hrydgard
Copy link
Owner

Let's take the plunge and hope we can make progress towards a proper solution. If we fail, might go back, but as this matches the tests better it is a step in the right direction, even if some stuff breaks for a little while.

hrydgard added a commit that referenced this pull request Mar 25, 2015
When seeking to atrac positions, force alignment and start from zero
@hrydgard hrydgard merged commit 62e621f into hrydgard:master Mar 25, 2015
@sum2012
Copy link
Collaborator

sum2012 commented Mar 25, 2015

Yes,we do not afraid merge this,if before major version do not solved Gta ,I pull request the commit again(hack?)

hrydgard added a commit that referenced this pull request Sep 27, 2015
…dio work in GTA again.

Hopefully we will find a better fix in the future.

Also see issue #7863.
hrydgard added a commit that referenced this pull request Sep 27, 2015
Add a compatibility flag to revert sceAtrac to before #6976. Makes audio work in GTA again.
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

Successfully merging this pull request may close these issues.

5 participants