-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Need for Speed games crash after a while since 1.13. version #15788
Comments
Btw, are you using texture upscaling / texture replacement / shader? |
No, I don't. Upscale level and Texture shader are turned off. |
Ugh. This might be related to #15783 .. will investigate. |
Can confirm this issue also happen on my phone, playing NFS Carbon Own The City sometimes crashing. |
Could you get some adb logs? As always, here's my recommended command line:
|
|
Hm, that is really strange, don't know how that assert could be hit on that path, but apparently it can... Maybe indeed running out of memory, causing strange issues. |
Tried and failed to reproduce this on the latest build, I played two races in NFS Most Wanted with OpenGL and one with Vulkan just because, with no issues. Is it still happening? |
Cannot reproduce the crash on my Redmi 10c Snapdragon 680 Adreno 610 |
Carbon doesn't crash but it has a graphics issue when you use N2O and go fast like the #15940 issue with Most Wanted |
This is the log from the crash 09-01 19:44:14.995 11982 11982 F DEBUG : *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** *** |
got any more lines after "backtrace" at the end? that's the interesting part.. that it's ffmpeg-related is interesting though, weird.. Which mode are you playing? Just quick race? |
Unfortunately, there's no more lines after that. I played the "career" mode, a lap knockout race |
I tried to get another log but again, only one line after the backtrace one. It was a circuit race now. The crash happens around two and a half minutes every time in a race (middle/end of second lap depends on the track). |
Hm. is it the same point in the music? same song? |
Actually yes, it's the end of the same song: Mudvayne - Determined |
Cool, now we're getting somewhere tracking this down :) To clarify, that's in Underground Rivals I guess? |
Yes |
Does the same song not crash in an older version? Or was it just that this particular song wasn't playing before? -[Unknown] |
It doesn't crash in 1.12.3. Since 1.13 the song crashes the game. Should I check the dev builds between the two versions? |
please do, this is very strange. Maybe some memory corruption, or changed error code leading to a different call sequence, or something... |
Ok, I won't have time today but I hope I'll be able to do it tomorrow. |
I had a free hour now, so I tried to find where the issue started: In the v1.12.3-8-gc75784351 build the song works fine, the app doesn't crash. However in the next build (v1.12.3-10-g1cd520ae3), it does crash. I think the target Android change from 11 to 12 did something that causes the problem. (I have Android 12 on my phone) |
Wow, that is surprising! No idea what that could be. Possibly some linking/ABI thing? But thanks so much for narrowing it down! |
Could #15001 have affected the NDK / system libraries used? Looking around that function, we might consider backporting this (not sure if it'd help): -[Unknown] |
That looks like a good find, and something we should definitely backport whether it helps or not. Argh, and rebuild all the binaries... Maybe just for Android to start with. |
Just a note that I have a repro of this crash now, only on Android 12 phones indeed. If #15969 turns out to be too much work or not fruitful, I'll just rebuild ffmpeg once again, with the patch... Let's see. |
We don't know if that patch even helps yet, though, right? I'd guess it's something with memory allocation behavior that the crash is Android 12 specific then? Or maybe the memory tagging thing? Presumably whatever bad memory access is happening elsewhere, just not crashing for some reason. -[Unknown] |
We don't yet, but I'll try to confirm today. |
Managed to rebuild FFMPEG with the patch, it still crashes. However, I was able to increase the amount of debug info, so I managed to get a better stack trace. The fault address is not close to sp so it's likely to be an out-of-bounds on the heap. The function is pretty huge after inlining so I'm gonna have to see if I can get addr2line or similar working (or first just build without optimization...)
|
OK, some results (from another run, slightly different addrs):
Looks like there's inlining going on, even though I built ffmpeg with -O0, maybe LTO? Anyway, it seems that we're just running out of bound of the get_bits buffer. No idea why this only would happen on Android. It happens at the very end of playing the song, which makes sense. |
Hm, there's a macro, The crash is at the end of decode_spectrum, line 907:
|
Think I might have to add some extra |
Yeah, confirmed that I can fix the crash with:
in atrac3p_decode_frame. Now, this is of course horrible, but shows clearly that it's running out of bits unchecked. I added some manual checks for bits_remaining but doesn't seem to do the trick :/ |
After messing around a bit with trying to limit the bits read in various ways, I've decided that it's not worth the time for now, I'm just gonna do a hack like the one above for the time being, and rebuild the ARM64 binaries only. This will fix the crash. Can be revisited at any time later. I'll definitely do the cmake setup at some point though. |
@unknownbrackets pointed out that we're in control of the buffer, so made a PPSSPP-side fix instead, avoiding all the ffmpeg recompilation mess: #15990 |
Thanks for your work! |
So it turns out that this was enough for Need for Speed, but I have a similar crash in Ratchet & Clank: Size Matters that still persists. Will dig into it. It's using atrac3, not atrac3plus. Still I would have though the fix would be the same.... Actually I think I got it... |
Yup that did it. |
Btw, could the out of bound/bits issue also related to the |
Could possibly be related in some way, hard to tell |
I don't think that's likely. The real problem is what this TODO says: if (atrac->bufferState_ == ATRAC_STATUS_ALL_DATA_LOADED || atrac->bufferState_ == ATRAC_STATUS_HALFWAY_BUFFER) {
// This says, don't use the dataBuf_ array, use the PSP RAM.
// This way, games can load data async into the buffer, and it still works.
// TODO: Support this always, even for streaming.
atrac->ignoreDataBuf_ = true;
} In short, from the original implementation of sceAtrac in PPSSPP (I think from JPCSP even?), PPSSPP tries to "reconstruct" the buffer in RAM. It's actually all about the buffer this fix made longer by a bit. That entire buffer is really a bad idea to begin with. It caused this issue, and it also causes that #7601 issue. What happens is, the game performs these steps:
Seems simple, right? The problem is, you can actually switch steps 2 and 3 above on a PSP, and games sometimes (by accident, I think) do. As long as you do step 2 before the new data is decoded, it really doesn't matter on a PSP. Everything works out. However, PPSSPP's code makes the assumption that the moment it's notified about the file data, it's time to copy that data from RAM into its reconstructed version of the entire .at3 file (the "dataBuf" buffer.) So that means, if the game accidentally swaps steps 2 and 3 - it loads corrupt data into the dataBuf. Even after the game loads correct data into RAM, PPSSPP ignores it - it only cares about that corrupt dataBuf data at this point. That might even be the actual cause of this issue: it could be the data at the end of the buffer was simply corrupt, and that's why FFmpeg ran over the end of the file trying to decode it. Maybe the game had not quite finished reading the file, but notified early because it was at the end. No one noticed the bug on a PSP, because it didn't matter - the data was loaded by the time that part of the song played, anyway. For now, we've simply added more space at the end of the file, so that FFmpeg won't crash. This doesn't fix the underlying issue of trying to decode corrupt data, which can only really be solved by decoding packets at the right time directly from PSP RAM. -[Unknown] |
Right, that buffering is an abomination aging from the time when we only had a full-file atrac3 decoder, IIRC... It definitely needs a rewrite to decode properly on the fly directly from PSP RAM indeed, which wouldn't have had this problem. Only problem is finding motivation to work on it, because it's not easy... |
Btw, is this notification is using a callback? I remembered something similar happened on networking callback, where the game calling a syscall that can trigger a callback but the callback handler is checking a boolean var, and skips processing the callback if that boolean wasn't initialized yet. I guess the game dev had a bad habit of initializing stuff later after using a syscall that can use that stuff, which isn't fail-safe. |
No, just a call to In some cases, like if you setup the network init with thread priority set to worse than the current thread, and if the processing ought to happen on the thread, it's totally deterministic that it won't run until you yield, on the PSP. -[Unknown] |
Game or games this happens in
Need for Speed Underground Rivals, Need for Speed Carbon: Own the City
What area of the game / PPSSPP
I can't finish races longer than 2.5-3 minutes because the app crashes on Android. The games ran fine in 1.12.3 but since the 1.13 update I have this problem. I've tried the latest git versions and the issue is still there. When I reinstall 1.12.3 again the games work fine again.
What should happen
It shouldn't crash.
Logs
No response
Platform
Android
Mobile phone model or graphics card
Samsung Galax A52s 5G
PPSSPP version affected
v1.13.1
Last working version
v1.12.3
Graphics backend (3D API)
OpenGL / GLES
Checklist
The text was updated successfully, but these errors were encountered: