libretro / mame2003-plus-libretro

Updated 2018 version of MAME (0.78) for libretro. with added game support plus many fixes and improvements
Other
196 stars 110 forks source link

Title fight - right screen missing some sounds #1876

Closed mahoneyt944 closed 3 weeks ago

mahoneyt944 commented 3 weeks ago

While testing I noticed there's missing sounds for the right screen but not the left, such as:

arcadez2003 commented 3 weeks ago

One or two bug reports regarding missing voices and or sounds i dunno if you've read em... https://mametesters.org/view_all_bug_page.php?filter=132399

mahoneyt944 commented 3 weeks ago

Yeah I've seen those. Idk if it's related to the missing engine sounds of scross or not but it could also be our MultiPCM / mem or port map

arcadez2003 commented 3 weeks ago

@mahoneyt944 Try this.zip

mahoneyt944 commented 3 weeks ago

@mahoneyt944 Try this.zip

No luck

grant2258 commented 3 weeks ago

The count starts with countdown 1 2 3 .. here im not it could be android specific is fine on the pc and raspberry pi5. For the right screen its an easy one to over look you only have one set of ym and multipcm you need two of each if you want the second side to have sound and hook it up of course. There is also a problem with a+b screen select its just black. I don't really use these settings anyway just to let you know.

grant2258 commented 3 weeks ago

On closer inspection of the mame code system32 is using 2 ym but multi32 is just using one. It looks like the port map is missing some interrupt handling.

https://github.com/grant2258/mame/blob/4da96a0c4fbc9ee692082e889b3a5a913fc0659e/src/mame/sega/segas32.cpp#L1243

mahoneyt944 commented 3 weeks ago

There is also a problem with a+b screen select its just black.

Are you using the latest commit? If so what's the setup to recreate this?

mahoneyt944 commented 3 weeks ago

On closer inspection of the mame code system32 is using 2 ym but multi32 is just using one. It looks like the port map is missing some interrupt handling.

https://github.com/grant2258/mame/blob/4da96a0c4fbc9ee692082e889b3a5a913fc0659e/src/mame/sega/segas32.cpp#L1243

We are using that hookup. What's missing? https://github.com/libretro/mame2003-plus-libretro/blob/9965d14d88f0e04b3cdbe8433093e4152fc42f11/src/drivers/segas32.c#L1119

grant2258 commented 3 weeks ago

@mahoneyt944 there are some mirrors in that map sorry if that wasnt clear what I was saying. You can check you logs for and unmapped port read writes if there are none it wont be a issue. I could well be the the re trigger bug but its worth checking any unmapped read writes. I am at the latest commit btw.


commit 9965d14d88f0e04b3cdbe8433093e4152fc42f11 (HEAD -> master, origin/master, origin/HEAD)
Author: mahoneyt944 <49591133+mahoneyt944@users.noreply.github.com>
Date:   Fri Nov 8 13:50:41 2024 -0500

    Simplify
111
mahoneyt944 commented 3 weeks ago

I am at the latest commit btw.

And you're getting black outs on the A+B selection? I can't replicate that. I toggle it like crazy and it always displays correctly for me.

grant2258 commented 3 weeks ago

Yes its happening on the pc and raspberry pi5 with titlef because I wanted both screens on. ill test with another game orunners, hard drunk and scross are doing the same thing. The A B alone works fine. Try this set the dipswitch to ab and restart the game. The display is so far off where it should be you cant even bring up the gui.

mahoneyt944 commented 3 weeks ago

Try this set the dipswitch to ab and restart the game. The display is so far off where it should be you cant even bring up the gui.

Works fine for me. Are you loading ra by command line?

grant2258 commented 3 weeks ago

yea i am https://github.com/libretro/mame2003-plus-libretro/commit/6009a8ed48837b5e14db10acdbc5b28d3c0f20e2 is where it broke I just reverted back till it worked. Works with launching from RA itself rather than the command line itself without the revert

mahoneyt944 commented 3 weeks ago

I don't see how that would effect anything, the visible area is updated on every video update based on the dip selection? I'm launching from the gui on the current commit without any issue no matter if I toggle the dip or reset.

grant2258 commented 3 weeks ago

not sure whats going to be honest your setting up the screen bitmap as MDRV_SCREEN_SIZE(5282, 288) which is fine size wise. And the clips set to MDRV_VISIBLE_AREA(08, 528-1, 08, 28*8-1)

the difference is the max size is 416x416 which is correct for 52*8 the mystery really is why the RA max size is 832 when launched from retroarch and 416 when launched from command line which is the information mame is giving

mahoneyt944 commented 3 weeks ago

It seems like launching from command line is riddled with odd bugs. Likely devs have not tested this against the behavior from Ra directly

grant2258 commented 3 weeks ago

Its not an odd bug its an odd bug it works MDRV_VISIBLE_AREA(08, 528-1, 08, 288-1) = 416 * 224 video code reads the visible area here. https://github.com/libretro/mame2003-plus-libretro/blob/9965d14d88f0e04b3cdbe8433093e4152fc42f11/src/mame2003/video.c#L68 unless you at some point are updating the visible area which I havent checked its a miracle its getting changed at all.

mahoneyt944 commented 3 weeks ago

unless you at some point are updating the visible area which I havent checked its a miracle its getting changed at all.

Yes every video update https://github.com/libretro/mame2003-plus-libretro/blob/9965d14d88f0e04b3cdbe8433093e4152fc42f11/src/vidhrdw/segas32.c#L2743-L2772

grant2258 commented 3 weeks ago

this is strange the max setting in the geometry is 416 from the command or retroarch 832 will need to hunt down where its tripping

Screenshot from 2024-11-09 18-18-23 Screenshot from 2024-11-09 18-17-15

mahoneyt944 commented 3 weeks ago

this is strange the max setting in the geometry is 416 from the command or retroarch 832 will need to hunt down where its tripping

That is odd. Maybe command skips some function call from Ra? Or skips the video update on init

mahoneyt944 commented 3 weeks ago

We could add *2 back into the driver start if that solves the command issue?

grant2258 commented 3 weeks ago

looked this up checked the latest libretro.h its description is changed from ours. The initial config sticking to 416 makes sense no idea why its acting differently from command line though. It should be exhibiting this behavior on both.

https://github.com/libretro/RetroArch/blob/876cc19d49e241dfaa2eb794f2544e7ae70d9858/libretro-common/include/libretro.h#L1323-L1359

Seems to make sense might be a change in behavior since the video code was done years ago.

mahoneyt944 commented 3 weeks ago

We default the dips to monitor A, which is why the visible area is set to one screen in the driver, but regardless it should behave the same way since it updates it in the video update every call. Is this a core bug or Ra bug?

grant2258 commented 3 weeks ago

its not a bug read the description again we use RETRO_ENVIRONMENT_SET_GEOMETRY.

The very first setting with the visible area is max 416 x 224 now. So anything bigger would require a RETRO_ENVIRONMENT_SET_SYSTEM_AV_INFO change due to the visible area change being bigger than what it was initialized with. That change just lowered the max area it probably should be fixed properly for any edge cases.

mahoneyt944 commented 3 weeks ago

I just assumed it would init that based on MDRV_SCREEN_SIZE, I pushed the *2 back in though. Did that fix any odd behavior to you saw?

mahoneyt944 commented 3 weeks ago

@grant2258 hey, see if this fixes loading from command line. https://github.com/libretro/mame2003-plus-libretro/commit/8daf4bcd9b9d9d86ad36d81c3f88a77a87bcceac geom->max_width and geom->max_height should always account for the largest size, https://github.com/libretro/mame2003-plus-libretro/blob/44062887a6b29587b5ab5a0af7de12826fd11e2d/src/mame2003/video.c#L56

video_config.width is set here: https://github.com/libretro/mame2003-plus-libretro/blob/44062887a6b29587b5ab5a0af7de12826fd11e2d/src/mame.c#L608-L618

grant2258 commented 3 weeks ago

@grant2258 hey, see if this fixes loading from command line. already answered here https://github.com/libretro/mame2003-plus-libretro/issues/1876#issuecomment-2466275257

https://github.com/libretro/mame2003-plus-libretro/issues/1876#issuecomment-2466421952 explains the issue the video code is fine and does what it should be doing. Its not setting it wrong its RA that is not handling the resolution changes properly due to the geometry call limitation when the resolution is bigger than the original RA max set, RA has a mechanism if the resolution gets larger though that would be the optimal solution for lower end hardware.

The solution will work the only problem is the screen size is often bigger than the visual display its not ideal for low end hardware using extra memory when not needed. If your happy using the extra memory always it will work. I would personally use a solution that used as little memory as possible due to some of the platforms people use this core on. I don't personally use the core as such but its down to yourself what way you want to do it all roads lead to Rome.

Just for some extra info the call should be done in retrorun not the way it is called right now could explain the differences in behavior from the command line. Its also worth noting that the screen size effects scan line timing I found out the hard way working on a driver thinking ill just make the screen bigger.

mahoneyt944 commented 3 weeks ago

Well I'm not sure what the best solution is, it seems safe to set the max to the actual max, as it would likely crash if it goes outside that.... but idk if there's any major hits by doing that. I didn't look close enough to see if video_config.width is ever recalculated higher than Machine->drv->screen_width, I wouldn't think so but that commit could be simplified if it doesn't.

Also probably don't want to rely on ra fallback to fix this overreach, we should provide a proper max to ra one way or another.

grant2258 commented 3 weeks ago

The solution is like it should be you set it to what is required not what its possible to change too. Both work at the end of the day. You should really change the geometry in retro_run that the api spec mentions this it might play nice in other front ends even in RA there is difference from the command line and launching from ra itself.

mahoneyt944 commented 3 weeks ago

Well I can revert that last commit if you think it's better that way. As far as calling it in retro_run(), we do technically, its just buried deep within mame_frame().....It would be called before retro_run() completes when there's a geometry change. I can ask if that meets the API's requirements if that's in question.

grant2258 commented 3 weeks ago

@mahoneyt944 its the libretto api that says this not me. I can tell you using RETRO_ENVIRONMENT_SET_SYSTEM_AV_INFO in the main code in the past (which we dont use) caused crashes sometimes unless you called it at in retro_run itself. You have to member mame is rendering the next frame not the current one its draws the one made in the last frame.

You can implement it anyway you see fit. mameframe does many things it runs the iteration required if you can guarantee the order and the time of main loop, who am I to debate the common sense approach libretro takes by says call it in retro run so there isint any issues in any front ends and its guaranteed to be called in a timely fashion. Im only discussing this not asking you to change anything. It was only the inconsistent launch from the command line and from RA itself that I thought needed to be looked into and at least the reason was found in this case. This isint the first time we came across this the other time its was something else cant even remember what it was that didn't launch properly from the command line I never looked into it then.

If you want to patch the max up like you did go for it. I just like keeping memory and things as low as possible in an ideal world.

mahoneyt944 commented 3 weeks ago

As long as the *2 fixes the command line issue, I'm indifferent about it. I'll leave it be.

Hopefully we can figure out the sound issue in titlef as it plays great otherwise. I can't really read a debug log on my phone due to how big the file gets in just a second or two. So I can't check for unmapped writes. But I suspect it maybe something else since the other multi32 games seem to work ok

grant2258 commented 3 weeks ago

It would probably be best to check in mame you can turn the sound up and down to see what chip makes what sounds. If its the multipcm it could well be the re trigger. I would defo check for any unmapped mirrors in the port reads I think there was 2 mirrors in there.

mahoneyt944 commented 3 weeks ago

Did the pre-segas32 driver have the same issue? That might tell us something too.

grant2258 commented 3 weeks ago

Tried it on my test core seems to have the second player sounds missing not sure about this core though. I would imagine the drivers where around the same state since its only mame095. The logging in this core does need addressed its to say the least somewhat overly verbose.

mahoneyt944 commented 3 weeks ago

Hmm. It's probably the same bug as the missing sounds in scross. Not sure how difficult it would be to implement that

arcadez2003 commented 3 weeks ago

Edit never mind @mahoneyt944 have you tried switching titlef to use the Stadium Cross sound banking style.?? im firm believer in trying shit to see what happens :)

GAMEX(1992, titlef, 0, scross, titlef, titlef, ROT0, "Sega", "Title Fight (World)", GAME_IMPERFECT_GRAPHICS )

The reason im mentioning the banking as this was changed when the multi and segas 32 drivers were merged to be a one size fits all rather than by type eg Model 1, Multi32 and one for scross

Title Fight had only been playable for 10 builds when 97.u5 rolled along so maybe no one noticed the sound issues.?? which might need the same mono hookup as scross it's gotta be worth a try and it'll only take ya 2 secs.

grant2258 commented 3 weeks ago

@arcadez2003 it works with the other banking just tried it. The funny thing is mame current isint doing this maybe we need that simple bank change all in only 2 sega drivers are touched I think.

grant2258 commented 3 weeks ago

ive added the 097u5 updates to my test core in case there is any other gremlins that can arise, without the video changes I would like to keep it from low end for now.

mahoneyt944 commented 3 weeks ago

@arcadez2003 you're the man! Nice find. Sound working now. Curious is the sound broken in current mame?

mahoneyt944 commented 3 weeks ago

@grant2258 @arcadez2003 so I looked into the missing engine sounds in scross, using dinks commit https://github.com/finalburnneo/FBNeo/commit/a565a5d6dc362398362f363091abd5e12cbf980e I was able to get it working in this core https://github.com/libretro/mame2003-plus-libretro/commit/c86232158c7ccdf0152ff1b42dc160841d37ea6c

Look correct to you guys? Seems like this will only effect model1 vr, the multi games seem correct as far as I can tell

arcadez2003 commented 3 weeks ago

Code seems correct it's chalk and cheese isn't it FBN vs MAME2003+ it's good there's not that many games affected as we can test em a bit a time in short order, as for current MAME i dont know if Title Fight has sound issues you'd need to test it.

But they'd never accept a fix from us anyhow as we're the bad guys :)

arcadez2003 commented 3 weeks ago

@arcadez2003 you're the man! Nice find. Sound working now. Curious is the sound broken in current mame?

Well im the man that likes to guess :) i thought two screens problem with the sound on the second and then spotted scross has different handling as it's mono for both screens then thought maybe Title Fight is the same.

Anyway another happy ending what with the gfx and the sound sorted nice game this one i like it so happy to see this.

mahoneyt944 commented 3 weeks ago

Yeah the oversight is in current mame and fbneo, I let dink know about it in case they want to sort it. But at least the sound and gfx seem to be correct for us and I'm happy with that 👍

I'm going to try to get the ring floor to draw for a final hurrah but that will take a little more work.

arcadez2003 commented 3 weeks ago

If they dont accept it then that's the way it goes... https://github.com/mamedev/mame/pull/12967

mahoneyt944 commented 3 weeks ago

If they dont accept it then that's the way it goes... mamedev/mame#12967

😂

grant2258 commented 3 weeks ago

If they dont accept it then that's the way it goes... mamedev/mame#12967

It works fine in mame, did any of you test it?

mahoneyt944 commented 3 weeks ago

I tried fbneo and noticed "Sega presents the main event" is cut off. Current mame seems to work though on the multi32 banking. Curiously, current mame uses a similar solution as dinks to fix missing engine sounds in scross but has the retrigger twice instead. Maybe this allows the original banking to work? @grant2258 can you test if removing the the alternate banking for scross still allows scross to have sound in current mame? Maybe the double retrigger eliminates the need to change the banking at all?

@arcadez2003 maybe close that until we see why it's different between all three emulators

grant2258 commented 3 weeks ago

@mahoneyt944 I dont have mame compiled right now only do it on released but ill check it on the next release. I could be updates to the sound driver itself that fixed it though. There was some over sized roms on scross you might want to fix them. Forgot to mention it done it when I ported to my test core. https://github.com/mamedev/mame/commit/f5ab78727a35e96374292a6e1fdf62b94ac7ee8e

grant2258 commented 3 weeks ago

Both sides work fine on my port with them rom changes posted (scross still needs the alt banking) . Actually the other banking is needed for scross in my test core. As far as titlef I dont need the scross banking in my test core and its using the same sound core without the patch you put in plus