libretro / opera-libretro

Port of 4DO/libfreedo to libretro.
63 stars 36 forks source link

[BUG] Alone in the Dark - Audio Looping Issues #182

Closed oblivioncth closed 9 months ago

oblivioncth commented 1 year ago

Once getting past the character introduction after starting a new game, portions of the music tracks get stuck looping continuously in segments that last about 3 seconds.

Seems to only affect some of the audio channels.

If the music is then disabled at any point, instead of actually stopping the entirety of the music then proceeds to loop the past second or so of the track until the music is re-enabled.

Some kind of buffer issue?

https://user-images.githubusercontent.com/24661585/211134358-da15c3d0-5c84-40b5-903b-a798a178a159.mp4

There is also a loud buzzing noise that occurs right after the 3DO logo fades (not shown in the video).

System:

All settings are default except that the Alone in the Dark timing hack is enabled (which does not affect the presence of this issue).

Disk:

Alone in the Dark (US).cue ``` REM Ripping Tool: Trurip CATALOG 0000000000000 FILE "Alone in the Dark (US).img" BINARY TRACK 01 MODE1/2352 INDEX 01 00:00:00 REM DISC HASHES REM CRC32 : 2c03e3af REM MD5 : 192de93ff970cc04caa49d4a723a390f REM SHA1 : d643c74677b283ec01431c6457d21f888747c77b REM Gap Append Method: Prev [crc32] REM Trk 01: 2c03e3af REM Gap Append Method: Prev [md5] REM Trk 01: 192de93ff970cc04caa49d4a723a390f REM Gap Append Method: Prev [sha1] REM Trk 01: d643c74677b283ec01431c6457d21f888747c77b REM Gap Append Method: None [crc32] REM Trk 01: 2c03e3af REM Gap Append Method: None [md5] REM Trk 01: 192de93ff970cc04caa49d4a723a390f REM Gap Append Method: None [sha1] REM Trk 01: d643c74677b283ec01431c6457d21f888747c77b REM Gap Append Method: Next [crc32] REM Trk 01: 2c03e3af REM Gap Append Method: Next [md5] REM Trk 01: 192de93ff970cc04caa49d4a723a390f REM Gap Append Method: Next [sha1] REM Trk 01: d643c74677b283ec01431c6457d21f888747c77b ```
oblivioncth commented 1 year ago

Should be a known issue, but figured I'd at least formally add this.

trapexit commented 1 year ago

Yeah, it's a known issue. A need to emulate a rarely used feature of the DSP.

rtomasa commented 11 months ago

Any plan on developing this missing feature? AITD is one of the most unique games in the system.

trapexit commented 11 months ago

Yes, when myself or someone has the cycles to do so.

Andrew-Twins commented 9 months ago

Yes, when myself or someone has the cycles to do so.

Well, whenever the cycles come, know that there are a lot of people that would be deeply grateful. Me, for example. I really hope you will make us dream again fixing this issue with Alone in the Dark, Trapexit!

ghost commented 9 months ago

A youtuber named Patzawa has discovered how to fix this issue. It involves changing a line of code in the file clio.cpp. Here is a link to his YouTube video describing the fix as well as a screen shot of the code change that is required. I have no idea if this actually fixes the problem, just passing this along to people way more knowledgeable than me! I hope this helps! I feel the 3DO version is superior to the MS-DOS version of the game.

image https://www.youtube.com/watch?v=xmfcGUEmyqw&ab_channel=PATZAWA

trapexit commented 9 months ago

Thanks. I'll take a look.

Andrew-Twins commented 9 months ago

Thanks. I'll take a look.

Thank you very, very much!

oblivioncth commented 9 months ago

I only did a brief test like in the shared video, but I can confirm the change does appear to fix the issue with no immediately discernable side-effects.

--- a/libopera/opera_clio.c
+++ b/libopera/opera_clio.c
@@ -724,7 +724,7 @@ opera_clio_fifo_ei(uint16_t channel_)
           opera_clio_fiq_generate(1<<(channel_+16),0);

           /* reload enabled see patent WO09410641A1, 49.16 */
-          if(CLIO.fifo_i[channel_].next.addr != 0)
+          if(CLIO.fifo_i[channel_].next.addr != 0 && (CLIO.regs[0x304] & (0x1 << channel_)))
             {
               CLIO.fifo_i[channel_].start.addr = CLIO.fifo_i[channel_].next.addr;
               CLIO.fifo_i[channel_].start.len  = CLIO.fifo_i[channel_].next.len;

This might also be pertinent for: https://github.com/libretro/opera-libretro/blob/100ae1e7decefe1f17d98cfcb9f2af4ff8452691/libopera/opera_clio.c#L772-L777

I have little bearing on of this is actually proper for emulation of the co-processor, nor do I have a 3DO to test with, but at the very least it could be added as a hack if it can't be applied generally; however, it does seem sensible as this register is partially cleared (assumedly at the appropriate bit corresponding to the channel at play) due to a DMA stop request, which is how I imagine Patzawa decided to test this.

https://github.com/libretro/opera-libretro/blob/100ae1e7decefe1f17d98cfcb9f2af4ff8452691/libopera/opera_clio.c#L406-L410

With the obligatory "download random binaries at your own risk", if anyone else wants to try it out on Windows here is the retroarch core with the tweak applied: opera_libretro_dma_tweak.zip

Andrew-Twins commented 9 months ago

I don't really understand shit about coding. But if this hack or mod could be added to the Retroarch core, that would be awesome!

ghost commented 9 months ago

Thank you! This game will be getting a lot of renewed interest when the remake drops early next year. This will be very helpful to many!

rtomasa commented 9 months ago

I confirm that it fixes Alone in the Dark sound issue. Tested in RPi5. I was testing more games with no apparent issues. Even more, I found out that Cyberia is now also playable. I was hanging to me in the first intro sequence on the sea. Now it is working fine.

oblivioncth commented 9 months ago

I confirm that it fixes Alone in the Dark sound issue. Tested in RPi5. I was testing more games with no apparent issues. Even more, I found out that Cyberia is now also playable. I was hanging to me in the first intro sequence on the sea. Now it is working fine.

In reference to your original comment, as I'm sure you realized I just referenced the DMA stop section to speculate on why this change might be correct generally. The only actual change is on the line shown in the diff.

Otherwise, that's fascinating. When I initially went to add the change (which I tweaked the syntax of slightly due to personal preference) there was a typo present which caused Alone in the Dark to freeze a fraction of a second after the sound starts in the startup company FMVs, so games are clearly sensitive to DMA loops/reloads being stopped under the correct conditions.

rtomasa commented 9 months ago

Yes probably this is also why Cyberia was hanging too. I've tested it several times with original and patched core and only worked with the patch. And it is true that FMVs in general seem to transition faster than before.

I faced a weird sound issue in AITD when testing the patch but I realized that I had also the threaded sound enabled so not sure if it was caused because of that. After disabling the same I was not able to replicate the problem anymore.

ghost commented 9 months ago

I was not able to replicate the hanging issue you are having in Cyberia. Game works fine for me with both versions of the core. I did notice this game breaks if you overclock the CPU. So I don't think the AITD fix affected this game at all. I wasn't able to find any previously known issue with Cyberia either.

trapexit commented 9 months ago

Thank you all for testing this. I'll do so later today and if I / we don't find any negatives I'll commit the change.

rtomasa commented 9 months ago

I was not able to replicate the hanging issue you are having in Cyberia. Game works fine for me with both versions of the core. I did notice this game breaks if you overclock the CPU. So I don't think the AITD fix affected this game at all. I wasn't able to find any previously known issue with Cyberia either.

Maybe it is something specific on the platform and audio driver. I'm on linux ARM using ALSA driver. Anyhow, since the "issue" is that it now works, I won't comply on the same 😂

ghost commented 9 months ago

AITD fix also works on Lemmings!!! Issue #109 fixed!!!

oblivioncth commented 9 months ago

I was curious so I added log entries when register 0x304 has bits cleared due to a poke at 0x308 (DMA stop), and when a input FIFO is cleared due to the current channel's bit in said register being low (i.e. the patch). In both AITD and Lemmings any time the FIFO was cleared for this reason, a little bit higher up in the logs you can reliably find at least one (though often more) DMA stop being triggered for that same channel.

Basically, log snippets that more-or-less look like:

[libretro INFO] [Opera]: DMA stopped with mask : 0x00000001 ... [libretro INFO] [Opera]: DMA aborted due to channel stop: 0 (mask 0x00000001)

So hopefully this is the right move in general, though I get the impression that there's a little more to perfectly emulating this behavior than what's currently here. Of course, this is potentially a major improvement nonetheless. I am curious on if trapexit has any hunches in regards to if this should also be applied to the output queue/FIFO.

I had a good chuckle at the irony of the lemmings issue being fixed by this:

There is a continuous annoying static noise in the in game musics (It starts just after the initial FMV intro, just when the DMA design logo appears)

Very fitting.

trapexit commented 9 months ago

I just did a quick test and as reported by others both the demo and retail release of AitD music appear to be fixed by this. To be thorough I'm testing some other software to be sure it doesn't obviously screw anything up. I'm also going over Portofolio OS source to see how the OS used the DMA registers to see if anything stands out. In retrospect this is a semi-obvious fix but numerous people, including an original Freedo author, suggested the issue was likely missing DSP opcodes which was were I spent my time in the past investigating this. Thank you to everyone who have looked at this. Will commit something later today unless something crops up.

ghost commented 9 months ago

@trapexit Can anything be learned from the Phoenix Emulator? I bring it up because it has 100% compatibility. I'm not sure if the code for that project open source or not. I just kinda get the feeling that somebody figured all this 3DO stuff out a long time.

rtomasa commented 9 months ago

I just did a quick test and as reported by others both the demo and retail release of AitD music appear to be fixed by this. To be thorough I'm testing some other software to be sure it doesn't obviously screw anything up. I'm also going over Portofolio OS source to see how the OS used the DMA registers to see if anything stands out. In retrospect this is a semi-obvious fix but numerous people, including an original Freedo author, suggested the issue was likely missing DSP opcodes which was were I spent my time in the past investigating this. Thank you to everyone who have looked at this. Will commit something later today unless something crops up.

Hope everything goes well in your tests because this is an easy workaround until the DSP opcodes are introduced.

trapexit commented 9 months ago

Phoenix is closed source... so not really. Also, Phoenix may loosely have "100% compatibility" but it is not 100% accurate to the hardware.

I just kinda get the feeling that somebody figured all this 3DO stuff out a long time.

Kinda... but not really. Phoenix was written by one of the people who worked on Freedo. It was only that small group of people who did the original reverse engineering. I am in close contact with one of those individuals but the source to later releases of Freedo (which later was used for Phoenix) and of Phoenix are not going to be opened. Freedo was never written to be very accurate and Phoenix is in fact many ways a high level emulator. The fact there is an OS abstracting the hardware both makes emulation easier for general compatibility but harder for accuracy. Phoenix does a lot of high level emulation. Which is cool for seeing what software is doing but not for trying to ensure you have a truly accurate emulation. For instance: neither emulator emulates the MMU.

trapexit commented 9 months ago

Hope everything goes well in your tests because this is an easy workaround until the DSP opcodes are introduced.

I'm not sure the issue with AitD is opcodes now that this has been brought up. The current code doesn't look correct to me. That said I'm also not positive the solution suggested is quite right either. It would seem to me that the check should happen before the next_addr check / reload. I'm testing both to see if I can tell a difference.

oblivioncth commented 9 months ago

I just did a quick test and as reported by others both the demo and retail release of AitD music appear to be fixed by this. To be thorough I'm testing some other software to be sure it doesn't obviously screw anything up. I'm also going over Portofolio OS source to see how the OS used the DMA registers to see if anything stands out. In retrospect this is a semi-obvious fix but numerous people, including an original Freedo author, suggested the issue was likely missing DSP opcodes which was were I spent my time in the past investigating this. Thank you to everyone who have looked at this. Will commit something later today unless something crops up.

I was trying to look through WO09410641A1 for anything referencing the DMA stop function at 0x308 but nothings quite lined up so far as it's mostly just mentions DMA completion through the length hitting zero or the next/current address explicitly being set to zero by the CPU. So, if you do find anything explicity covering this in the OS source it would be cool to see the reference.

I'm not sure the issue with AitD is opcodes now that this has been brought up. The current code doesn't look correct to me. That said I'm also not positive the solution suggested is quite right either. It would seem to me that the check should happen before the next_addr check / reload. I'm testing both to see if I can tell a difference.

I was thinking the same thing, more or less what I was alluding to earlier. The check seems kind of late. If the channel is outright disabled/stopped then perhaps the function should return sooner before doing much/anything with that channel at all.

trapexit commented 9 months ago

I tried moving the channel enabled check to the beginning of the function and got a lot more audio crackling. Unless there is something else funny it would seem that disabling the DMA doesn't stop it immediately but it runs till length == 0 either due to transfer or because it was set as such.

The patent mentions that neither addr or length can be zero but really don't say how zero is used. I put in an additional check for len==0 and exit just like addr==0 (with logging to see if it is triggered but I didn't see it in a few tests.)

Another thing that isn't clear is the +4 being added to the length in the code. I removed that +4 and heard no difference but without a digital capture I can't be positive.

Regardless, while I still hear some pops occasionally at the end of samples and videos this clearly is better. I will do a bit more tidying up and testing and submit something soon.

trapexit commented 9 months ago

BTW... running Woody Woodpecker video I notice "DisableDMAChannel: DMA not stopped!" in the logs. The version of Portfolio source seems later than the version used and explicitly mention that log message being removed. I need to dig in a bit more to see if I can figure out what the condition was when that log would be printed.

ghost commented 9 months ago

Seal of the Pharaoh is a good example of audio crackling and popping. Happens every time you take a step in the game. Psychic Detective has some crazy audio crackling in it as well in the first few FMV sequences. Maybe these two games can help you figure out this weird audio issue.

trapexit commented 9 months ago

Still some crackles and pops and even a loop at the end of the char select screen dialog but this is clearly a lot better than before. I'll close this and #109 now and will spend some time looking over the DMA code later to see if those other issues can be resolved.

oblivioncth commented 9 months ago

BTW... running Woody Woodpecker video I notice "DisableDMAChannel: DMA not stopped!" in the logs. The version of Portfolio source seems later than the version used and explicitly mention that log message being removed. I need to dig in a bit more to see if I can figure out what the condition was when that log would be printed.

That function doesn't seem to be present in your repo of the sources, other than a log message stating it was added. Did the function itself predate the version of the OS snapshot in the original source dump?

trapexit commented 9 months ago

That function doesn't seem to be present in your repo of the sources, other than a log message stating it was added. Did the function itself predate the version of the OS snapshot in the original source dump?

Yes, that's what I was saying above. The changelog indicates it was removed in a previous release to what we have. Regardless, similar errors are found elsewhere and my guess is that the code was the same. It is simply looping reading start address register to catch when it stops changing. Apparently it is just a notification. Not an error.

Andrew-Twins commented 9 months ago

Thank you, Trapexit, and all of you for the testing as well. It is indeed way better than before.