mattgodbolt / jsbeeb

Javascript BBC micro emulator
GNU General Public License v3.0
351 stars 68 forks source link

Sound error in Repton 2 Speech demo #96

Closed simon--taylor closed 5 years ago

simon--taylor commented 8 years ago

There's some background noise during the speaking of the word "Superior" at the start of the Speech demo when loading Repton 2. B-em has the same problem but it sounds ok in BeebEm.

mattgodbolt commented 8 years ago

Confirmed. Seems like Speech is setting the pitch registers to [1010, 1, 1008, 0], which is weird to me (ch3 and ch4 are muted). The "1" makes sense as that is effectively "always on".

If I break again when the sound clears up the pitch registers are: [1, 1, 1008, 0].

I suspect something shonky is going on when the registers are written to. Will dig more. Thanks for reporting this!

mattgodbolt commented 8 years ago

Ok this is as far as I got; here are the writes to pitch registers around the time it goes wrong:

3934381 eb2f : Write to 0 of 82 giving 00e2
3934481 eb2f : Write to 0 of 3f giving 03f2
3934993 eb2f : Write to 1 of a1 giving 03f1
3935093 eb2f : Write to 1 of 3f giving 03f1
3935605 eb2f : Write to 2 of c0 giving 03f0
3935705 eb2f : Write to 2 of 3f giving 03f0
3936095 eb2f : Write to 3 of e0 giving 0000
3939579 415a : Write to 3 of e0 giving 0000
XXX
3999065 2cb6 : Write to 3 of 00 giving 0000

The routine at 2caa is the "poke A to the sound chip" routine. The XXX marks a point where something funny happens...

Interestingly it seems like one of the updates is being swallowed up. There's an init routine at 2b35:

2b35 a9 81..LDA #$81
2b37 20 aa 2c .,JSR $2caa
2b3a a9 00..LDA #$00
2b3c 20 aa 2c .,JSR $2caa
2b3f a9 a1..LDA #$a1
2b41 20 aa 2c .,JSR $2caa
2b44 a9 00..LDA #$00
2b46 20 aa 2c .,JSR $2caa
2b49 4c bf 2cL.,JMP $2cbf

and the call at 2b37 doesn't appear to "take". I can only assume something in an IRQ routine somewhere has left part of the via in a weird state. Suspiciously there's a write at 415a just beforehand. I'll have to investigate what that's doing.

mattgodbolt commented 8 years ago

Indeed...when the sound "fixes" itself, the init routine has been called again a second time...and this time the call at 2b37 takes effect.

Now puzzling over why the first call doesn't work.

mattgodbolt commented 8 years ago

I instrumented the first write and it's not being sent to the soundchip ( https://github.com/mattgodbolt/jsbeeb/blob/404b9b0ede38333c5f6ccbe90f3bc01746a234d7/via.js#L499 ) because bit 0 of the IC32 isn't being cleared on the first write.

I'll have to dig some more: this might explain why jsbeeb and b-em suffer from this problem as the jsbeeb code borrowed heavily from b-em in the via emulation...

richtw1 commented 8 years ago

I recommend trying the tape image to see if it sounds OK. I've heard this on a real beeb too, I think it's possibly a bad hacked disc.

richtw1 commented 8 years ago

I think b-em has more accurate emulation than beebem in this respect - it emulates the need to hold the line low for 16us for the sound chip write to work... I think.

richtw1 commented 8 years ago

Just tried tape version, still seems to happen there too, so guessing this is an emulation issue.

richtw1 commented 8 years ago

The way this should work is as follows: the sound chip is clocked at 1/16th of the CPU. Every 16 cycles (8us) it takes the value off the slow data bus if the /enable line is low (IC32 bit 0). This is why there's a requirement to hold it low for at least 8us, otherwise writes to the SDB may not get picked up by the sound chip. I can't see any code in B-Em, nor in jsbeeb, which does this (although I was sure Tom had said he'd implemented it).

This 1/16th frequency corresponds to the sound chip tick rate (unsurprisingly). The output level will be toggled at multiples of this interval (according to the contents of the pitch registers for each channel).

mattgodbolt commented 8 years ago

So, seems that fixes it., Would appreciate testing on other games and giving the code a look over too!

mattgodbolt commented 8 years ago

Things aren't quite as good as I'd've hoped. Reopening until we either revert the change or find the proper fix

mattgodbolt commented 8 years ago

(with reference to discussion on b07ad0f01df6e5dddc43c2de8d787f6c214cf0b8)

richtw1 commented 8 years ago

OK I think I get it. The sound chip enable falling edge (i.e. disabled to enabled) is important - it is this which gets the data put into the internal register. The sound chip only checks this falling edge every 32 internal cycles (16 cycles from the 6502 point of view, 8us). Hence after the data is put on the data bus, the sound chip enable needs to be pulled low for at least this much time so it can be acted upon. Then it has to be pulled high again, otherwise it wouldn't be able to see a falling edge after the next data write.

This also explains how sound chip writes can get lost by not waiting enough time. If the enable line is pulled low and then high again before the sound chip has had a chance to notice the edge, nothing will happen.

This is pretty much what the old code was doing (acting on the falling edge) apart from there's nothing there to only check for it every 16 cpu cycles.

mattgodbolt commented 8 years ago

More info after discussion and investigation:

There's approx 60k cycles between the spurious write at 415a and the volume set at 2bc3. Maybe OS IRQ would sometimes rescue the WE bit? Maybe there's something else at play where the sound chip re-samples the databus after "a while" if WE is left low for "too long"? Need a real Beeb to investigate.

To debug:

processor.debugInstruction.add(function(x){return x== 0x2cb3 || x === 0x415a;});
mattgodbolt commented 8 years ago

Have asked on the STH forums; hopefully will get some ideas.

mattgodbolt commented 8 years ago

More info on the STH thread but we can reproduce something very similar on a real BBC: it seems to actually be a bug in Speech!

Toggling caps lock off "fixes" it in jsbeeb (and breaks it on a real beeb), so we're on the right track