ADPCM: fixed end/limit address checking and reading external data

[hyano]

This patch fixes checking of end and limit addresses in ADPCM.

In the original implementation, loop playback of ADPCM sound was sometimes incorrect because the limit address checking was done first and it changed the data address to 0.

[aaronsgiles]

I like that this gets rid of the dummy reads. Can you point me to some examples I can look at to see this behavior?

[hyano]

@aaronsgiles Thenks for your reply.

Sorry, I cannot show good examples immediately. And sorry this PR contains multiple fixes.

• end/limit address cheking.
• read ADPCM regs and memory.
• dummy read implementation.

The idea for "dummy read" implementation using read buffer is taken from "fmgen". Fmgen is old&good FM sound engine for M88 which is an old NEC PC-88 emulator. Module version of fmgen is distributed on http://retropc.net/cisc/sound/.

I don't know the actual circuit in the chip, but I feel fmgen's implementation which has 2 bytes internal read buffer is reasonable based on YM2608's application manual (Japanese version). RAM/ROM-READ (外部メモリ(external memory) -> OPNA -> CPU):

• 「最初の2回はダミーリード(== The first 2 reads should be "dummy read")」
• 「最後の2データ分はフラグが立たないので、(== Because the flag is not active for the last 2 data,)」 (I expect "flag" means EOS bit.)

YM2608 was used for Japanese PC (PC88/98) and users transfered data between CPU and OPNA/RAM. M88 team researched the behavior of real PC and implemented it when M88 development was active. That's another reason why I think the implementation for this area is good for many sowfwares.

I wish I still had real HW, I could confirm...

[aaronsgiles]

Can you please split these fixes into separate PRs? I would like to consider them independently.

Are you using ymfm in a project? I don't know of good examples for the dummy read case, but it seems that you must have some reason why you think it should be changed. :)

[hyano]

Can you please split these fixes into separate PRs? I would like to consider them independently.

I have small dependency concerns. These are the reason why this PR contains 3 changes.

• 18dc04e: small code change but contains off by one error. Fixed in ae4dde9 -- last commit for dummy read.
  • I can prepare new branch for new separated PR. Only remove '-1' from adpcm_b_channel::at_end()/at_limit().
• 6339a8a and ae4dde9: depend on previous commits... and all 3 commits have to be serialized...
  • Is it OK for you that PRs depend on the other PR? These shoul be almost same as 3 commits in ths PR.

Anyway I'll try to split and reorder.

Are you using ymfm in a project? I don't know of good examples for the dummy read case, but it seems that you must have some reason why you think it should be changed. :)

Yes, I am using ymfm in my private emulation project (non-commercial) and found current ADPCM implementation doesn't work well for some titles.

• Sound effect with ADPCM doens't play with looping.
• ADPCM memory verification after transfer doesn't work because of data mismatch. (register read and gap by dummy read)
• Investigated the register accesses of emulated code and the differences from MAME/fmgen implementations' behavior.

Ymfm is excellent. Very thanks for your work!

[hyano]

I've split this PR into new 3 PRs.

• 33 https://github.com/aaronsgiles/ymfm/pull/31/commits/18dc04e21a92c90d44d4e5b9c5096f84ed51d3d0 + small fix.

• 34 a part of https://github.com/aaronsgiles/ymfm/pull/31/commits/6339a8ae8b6edea35fb22dde6603cd4ae18adf9c

• 35 a part of https://github.com/aaronsgiles/ymfm/pull/31/commits/6339a8ae8b6edea35fb22dde6603cd4ae18adf9c

But creating PR for dummy read is pending, because:

• Change for dummy read depends on other commits above.
• I would like to investigate another behavior of dummy read and status register.
• I'll open new PR for dummy read when I'll have some conclusion.

It's OK to reject this PR and continue on #33, #34, #35.

[aaronsgiles]

Thanks, I'll consider the others individually now.