Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

overclock question #51

Open
negativeExponent opened this issue Feb 13, 2022 · 5 comments
Open

overclock question #51

negativeExponent opened this issue Feb 13, 2022 · 5 comments

Comments

@negativeExponent
Copy link

the overclock logic is incorrect.

NST_SINGLE_CALL void Apu::Dmc::WriteReg2(const uint data)
{
regs.address = 0xC000 | (data << 6);
if (regs.address != 0)
{
overclockingIsSafe = true;
}
}

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

NST_SINGLE_CALL void Apu::Dmc::WriteReg3(const uint data)
{
regs.lengthCounter = (data << 4) + 1;
if (regs.lengthCounter != 0)
{
overclockingIsSafe = true;
}
}

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

@hizzlekizzle
Copy link
Contributor

What does "safe" actually mean in this context?

@negativeExponent
Copy link
Author

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
Copy link
Contributor

keithbowes commented Feb 18, 2022

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
Copy link
Author

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
Copy link
Contributor

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants