libretro / nestopia

Nestopia emulator with libretro interface
GNU General Public License v2.0
52 stars 46 forks source link

overclock question #51

Open negativeExponent opened 2 years ago

negativeExponent commented 2 years ago

the overclock logic is incorrect.

https://github.com/libretro/nestopia/blob/c24ffe8ef9c8d6b89f5a3d8b64105168a55e03d2/source/core/NstApu.cpp#L2382-L2390

overclockingIsSafe will always be true because regs.address is guaranteed to be at least have a value of 0xC000

https://github.com/libretro/nestopia/blob/c24ffe8ef9c8d6b89f5a3d8b64105168a55e03d2/source/core/NstApu.cpp#L2392-L2400

same here. regs.lengthCounter will always be at least 1.

hizzlekizzle commented 2 years ago

What does "safe" actually mean in this context?

negativeExponent commented 2 years ago

when DMC is latched, thats considered unsafe. It causes PCM audio channel to run faster than it should when overclocking. If this was following FCEUx ppu overclock logic, Reg2 and Reg3 writes should only check value of incoming data

https://github.com/libretro/libretro-fceumm/blob/59771792b0709f10c9bc7b408e41ae6df55d6dab/src/sound.c#L289-L302

keithbowes commented 2 years ago

Well, you should probably forward these concerns to the upstream code. I don't think anyone here does anything for the Nestopia backend but only for the libretro interface, so if this were fixed upstream, it would get merged into this core later.

negativeExponent commented 2 years ago

Upstream (if it's still even active) does not have overclock option, not has the libretro port. either case the condition is still incorrect and will always be true.

keithbowes commented 2 years ago

You're right. I didn't think the core had anything that the upstream backend didn't. I guess I was wrong.

Judging from the git logs, it seems that upstream did once have overclocking but it was considered buggy and was removed, but the project's originator readded the code specifically for the libretro core.