search

libretro-mirrors / beetle-saturn-libretro

Standalone port of Mednafen Saturn to the libretro API.
GNU General Public License v2.0
61 stars 40 forks source link

[fails to run] mednafen/ss/scu.inc:1741: void DSP_Init(): Assertion (...) failed. #134

Closed 5schatten closed 5 years ago

5schatten commented 5 years ago

Commit: https://github.com/libretro/beetle-saturn-libretro/commit/c726c623b9df052ae457ff75bbb231d46b97dfc7

Retroarch log: https://pastebin.com/tkGhwHpB

Package: https://github.com/5schatten/LibreELEC.tv/blob/libreelec-9.x-rr/packages/5schatten/emulation-lr-cores/lr-beetle-saturn/package.mk

hiddenasbestos commented 5 years ago

This appears to be a crash report for "Virtua Fighter 2 (Europe) (Rev A) (Made in EU)"

Can you test the same game in Mednafen 1.21.2. Does it crash in that specific version as well ?

5schatten commented 5 years ago

Well I haven't tested it with mednafen 1.21.2 or else but the game runs fine with commit 564a1463c082d9f7cdf9c95a1746b4077c5e4ffb

If you check out https://forum.fobby.net/index.php?t=msg&goto=4414& there are some reports about Virtua Fighter 2 but nothing special.

hiddenasbestos commented 5 years ago

Well, it could either be a real error in the upstream emulator (especially with it being the PAL version of the game) and this has maybe even been fixed in a later version, so if/when we get to the later versions this might go away.

Alternatively even though the SCU code is now very similar between the LibRetro core and Mednafen, I absolutely could have missed something during the merge. Definitely would be helpful to have my work peer reviewed.

5schatten commented 5 years ago

I see if I can run further tests so any known good stuff that runs fine with the latest commits?

hiddenasbestos commented 5 years ago

I mostly tested things were working with US version of Daytona USA - Championship Circuit Edition

5schatten commented 5 years ago

I tested several europe game versions and none of them worked. https://pastebin.com/3LWt9ahw

I'll try your US version later but if eu games won't work it's quite a problem ;-)

EDIT: Daytona USA - US version also fails for me https://pastebin.com/gwienNkd

hiddenasbestos commented 5 years ago

I have no idea then. This code is nearly identical to the Mednafen code in that file. Maybe it's a Linux thing ?

5schatten commented 5 years ago

Well did nobody test this changes on Linux before they were merged?

jdgleaver commented 5 years ago

I just encountered the same issue on Linux x86_64 (literally 10 minutes ago! Strange coincidence...)

There's a weird typedef issue going on. Take a look at this:

https://github.com/libretro/beetle-saturn-libretro/blob/c726c623b9df052ae457ff75bbb231d46b97dfc7/mednafen/ss/scu_dsp_common.inc#L22-L28

The recent commits have changed nothing here, but it now seems that DSP_INSTR_BASE_UIPT and DSP_INSTR_RECOVER_TCAST are set incorrectly. If you manually force the following:

//#if (defined(__x86_64__) && defined(__code_model_small__) && !defined(__PIC__) && !defined(__pic__)) || SIZEOF_VOID_P <= 4
// #define DSP_INSTR_BASE_UIPT 0
// #define DSP_INSTR_RECOVER_TCAST uint32
//#else
 #define DSP_INSTR_BASE_UIPT ((uintptr_t)DSP_Init)
 #define DSP_INSTR_RECOVER_TCAST int32
//#endif

...then the core builds and seems to run without issues.

I do not understand the exact problem here (I certainly can't see why DSP_INSTR_RECOVER_TCAST should be signed), nor do I have enough time to go through all the code, but perhaps this will help with debugging...?

hiddenasbestos commented 5 years ago

Thanks @jdgleaver "SIZEOF_VOID_P" triggered a memory - it was code I removed during the merge but seems to have been added downstream to fix this issue without any comment to indicate it's importance. In real mednafen it's part of configure script - so if this fix works I'll probably put that in our makefile.

jdgleaver commented 5 years ago

Ah, nice - thanks for working out a fix so quickly! Certainly does look like something that should go in the makefile.

Uncommented legacy code is a misery to deal with... The fact you managed to merge all these changes with only one tiny bug is a pretty fantastic achievement!

hiddenasbestos commented 5 years ago

Hi @5schatten - can you build #136 and see if it works?

5schatten commented 5 years ago

@hiddenasbestos I've tested the latest commit + your patch/PR and it works for me. https://pastebin.com/0bAWEUD3

hiddenasbestos commented 5 years ago

Great. I've updated the patch PR to apply to the makefile, rather than that one file - should avoid future problems merging that file, and also instances of SIZEOF_VOID_P used elsewhere in future.

5schatten commented 5 years ago

@hiddenasbestos patch v2 also works fine :-)