#6012 closed defect (fixed)
PSP: BASS-FR Intro Skip Crash with Music Enhancement
| Reported by: | SF/koima57 | Owned by: | fuzzie |
|---|---|---|---|
| Priority: | normal | Component: | Engine: Sky |
| Version: | Keywords: | ||
| Cc: | Game: | Beneath a Steel Sky |
Description
Hi, i report a bug preventing to bypass the long introduction of the game (using the side L button) without freezing the PSP console. It happens with langage set on French, doesn't happen with default.
Ticket imported from: #3512069. Ticket imported from: bugs/6012.
Change History (27)
comment:1 by , 13 years ago
comment:2 by , 13 years ago
| Owner: | set to |
|---|
comment:3 by , 13 years ago
koima57: Also, can you confirm which PSP firmware version you are running and which method of loading/running ScummVM you are using?: http://wiki.scummvm.org/index.php/PlayStation_Portable
comment:4 by , 13 years ago
| Summary: | PSP latest 1.4.1 - BASS French with Mr Woodcock music → PSP: BASS-FR Intro Skip Crash with Music Enhancement |
|---|
comment:5 by , 13 years ago
Hello thank you for the fast support as usual. I am using latest official Sony firmware 6.60 and custom firmware 6.60 PRO B-10 as method to run ScummVM.
About the proposed steps :
1: My model is PSP 3004 (sold as pack God of War) 2: Will update soon as i don't have my PSP for now to try latetst build. 3. It didn't happen with language set to defaulkt, will try with it set to english and update soon. 4. Other games soundtracks from Mr James Woodcock use the same OGG format and all run properly beside this crash with french BASS, though i could try and convert to mp3 if i get my hands on a fast working program to do so.
Will update soon for step 2 and 3, Thanks for top support. :)
comment:6 by , 13 years ago
Update on point 3, no problem without the remade soundtrack files and french language.
comment:7 by , 13 years ago
koima57: Thank you for this information. I should be clear that I'm just doing bug triage, though the PSP maintainer is assigned so he should be CC'ed these messages.
Since I don't have a PSP, I can't investigate this in detail unless it is an obvious engine bug, but I needed to ensure that when the PSP maintainer gets some time to look at this, all the relevant information to investigate/replicate is already attached to the bug and any obvious mistakes are eliminated as the cause.
The PSP3004 has 64MB RAM... so even with the overheads, the game should not be starved of memory... Though this could be an out of memory issue triggered by RAM fragmentation or similar. Please be patient and standby...
comment:8 by , 13 years ago
Hi, yes i am aware this is just to pinpoint where and how the issue happens. Glad to be a part of the improvement of the program. :)
The game remains fully playable waiting past the introduction. This weekend I could test further and here are my results :
Pressing L1 (same as ESC for PC version) on my PSP makes the game freeze on intro screen until PSP power shut off. Could hear first tune sound starting to play.
Tried with Music soundtrack 1.0 and 1.1, with language set on default, english and french. With latest official 1.4.1 build and latest daily build. Lastest would no let me add game on the menu at all i guess bad build on April 1st. Too much shutting off and on psp, reloading the custom firmware for another 5 minutes to try a lot more sorry! Good luck with the infos i could gather about this issue.
Report i could load a game save past intro screen so it really is minor issue. Thank you.
comment:9 by , 12 years ago
This issue does not occur on a desktop build of ScummVM - tried it under Windows 7 (64-bit). I set the language to French in both CD and floppy versions, skipped the introduction with ESC and no crash occurred
comment:10 by , 12 years ago
This bug is nice to get fixed before the release. Raising priority for keeping the track.
comment:11 by , 12 years ago
| Priority: | normal → high |
|---|
comment:12 by , 11 years ago
I've been able to identify the cause of crash within backends/platform/psp/audio.cpp . I tried compiling with uncommented #define __PSP_DEBUG_PRINT__, and the compile fails, because of line 104: PSP_DEBUG_PRINT("remaining samples[%d]\n", _remainingSamples);
The reason is that _remainingSamples is undefined. If #define __PSP_DEBUG_PRINT__ is disabled (like it is by default), nothing needs to get printed in SCUMMTRACE.TXT, so the program compiles, but the Debug call is left available to be called upon entering the function, only without having to print output to the trace file.
I've been able to solve this by either commenting out the Debug call, or add a: uint32 _remainingSamples = 0; at the beginning of the class. (which I don't think is a good solution, since remaining samples aren't being calculated this way).
I've tested it both ways (on multiple languages), the game didn't crash and allowed me to skip past the intro safely.
Either way, that being a DEBUG Call, I think it's safe just to comment it, or even remove it.
comment:13 by , 11 years ago
Nitrus: Checked this. The variable _remainingSamples does not exist and this is a left over from earlier code. It should resolve via the preprocessor to a NOP/ no statement passed to the compiler, and should not really cause the observed issue. However, it _could_ be exposing an obsure bug in the toolchain, but this is unlikely.
Anyway, this is irrelevant as the code is dead, so removed anyway in commit: 718db9f3dfd9779195c94ea07a85bfebc38397fa
This is more likely a race hazard according to wjp, so if you can do some more testing and see if you can replicate/trace down that would be optimal. Thanks.
comment:14 by , 11 years ago
When skipping the intro, the game calls loadSection(1) twice in a quick succession, and restarts the music for the first screen very quickly. Seems like the PSP mutex code has issues with that fast reloading.
The culprit is the following snippet inside sky/sky.cpp:190: if (!shouldQuit()) { _skyLogic->initScreen0(); if (introSkipped) _skyControl->restartGame(); }
initScreen0() starts screen 0 together with the animation where Foster hides upstairs. restartGame() starts screen 0 without that animation. So the initScreen0() call is superfluous and should not be called when restartGame() is called.
Changing the above code like this: if (!shouldQuit()) { if (introSkipped) _skyControl->restartGame(); else _skyLogic->initScreen0(); }
fixes the issue without causing any regressions.
We've tested the aforementioned change with Nitrus, and this fixes the freezes in the intro.
Unless someone can spot an edge case with the above change, I believe that it can be safely added without causing further issues
comment:15 by , 11 years ago
thebluegr: Sounds good. Kudos to you and Nitrus for working on this. If we can get this engine fix in, hopefully someone can look at the PSP threading and audio backend code and see if this can be made more "safe".
comment:17 by , 11 years ago
My theory from IRC last night, for reference: "for example it looks like you can deadlock in loadSection(), which locks _mutex, then calls stopMusicInternal which needs the mixer lock, if the audio thread happens to call pollMusic() in the middle, which needs to lock _mutex? same for calling stopMusic() I guess"
comment:18 by , 11 years ago
| Owner: | changed from to |
|---|
comment:19 by , 11 years ago
I can reproduce my theorised deadlock on my desktop, with enough cheating. Pushed b16f2d8405bab54a891325f0c6dec156f6006b1a which solves it. Helpful?
comment:20 by , 11 years ago
With the latest commit by fuzzie, the deadlock has been resolved, and I can no longer reproduce the freeze.
Also, I am able to hear less than half a second of music immediately after skipping the intro (by pressing L), and then the game continues on as normal. With the previous changes suggested by [md5], this had been resolved as well.
comment:21 by , 11 years ago
fuzzie: So do we intend to commit [md5]'s fix for sky.cpp as well? And then revert if we find any regressions during testing.... or do we close this bug as-is as fixed?
comment:22 by , 11 years ago
The current code for skipping the intro loads the first scene twice, which is why nitrus hears that half second of music.
The proposed code loads the scene only once. There shouldn't be any regressions (at least not in theory), as the restart code is reinitializing everything and puts the player in the first screen, whereas the normal scene 0 entry code starts the scene from the animation where Foster is being chased.
I have done preliminary testing on all of the use cases I can think of, so IMHO the proposed code change for intro skipping is safe, however I would like a second opinion before applying it, just in case I missed something. Joostp? fuzzie? Any opinion?
comment:23 by , 11 years ago
I've committed the fix described for the intro with commit c48a7ee - it works correctly for me and it looks to be the right thing (TM), without having any side effects.
Lowering the priority to normal
comment:24 by , 11 years ago
| Priority: | high → normal |
|---|
comment:25 by , 11 years ago
thebluegr: Thanks for the fix... This bug is now fully fixed and should be closed?
comment:27 by , 11 years ago
| Resolution: | → fixed |
|---|---|
| Status: | new → closed |
Have tested using the latest Git master on Linux x86_32, along with BASS CD v1.2 and James Woodcock's Digital Music Enhancement v1.1 files. Set the language from default (English) to French, but can't replicate the crash when skipping the intro. No valgrind issues evident.
koima57: Could you try the following to help investigate this?: 1. Please indicate the exact model of PSP you are running this on, as this is probably an Out of Memory crash? My desktop profiling shows that BASS is using around 8 MB at this point... 2. Please download the latest development PSP build and see if you can replicate with that: http://buildbot.scummvm.org/builds.html 3. Please confirm whether this happens with Default/English language with the Music enhancement.. If not, does it happen with just French language i.e. with the Music enhancement files removed. 4. The music enhancement files are Ogg format digital audio files. Could you try compressing the audio files of another game that you own with the ScummVM Tools to Ogg format and see if this also provokes a crash when you skip over a spoken line? i.e. I am trying to work out if the issue here is in the PSP Ogg library port, rather than ScummVM itself.