Skip to content

S86 volume envelope fix#13

Open
grgowski wants to merge 1 commit intosuperctr:c60from
grgowski:c60
Open

S86 volume envelope fix#13
grgowski wants to merge 1 commit intosuperctr:c60from
grgowski:c60

Conversation

@grgowski
Copy link

S2X_S86WSGTrackUpdate calls S2X_S86WSGEnvelopeUpdate before S2X_S86WSGChannelUpdate which causes an issue at the very first track update. I think that the envelope flag is set and no update is being performed resulting in zero volumes. I have looked at the S1 equivalent and saw that S2X_S1WSGEnvelopeUpdate is called within the S2X_S1WSGChannelUpdate which is the proposed fix for the S86 driver. I have checked that solution against the register dumps from Mame and it seems that it fixes the issue but I am not sure if that does not have any other side effects. Can you please have a look and see if that makes sense?

@superctr
Copy link
Owner

superctr commented Feb 28, 2021 via email

@grgowski
Copy link
Author

grgowski commented Mar 1, 2021

The problem is causing a single frame delay for all volume envelopes starting in frame 0. The first time S2X_S86WSGEnvelopeUpdate is called it does not update volumes because the Env[0].Flag is set. The first time that flag is cleared is in S2X_S86WSGChannelUpdate in the if(!C->WSG.SeqWait) part which comes after the envelope update was called.

If you check volume values for all channels after the first iteration you will see that all of them are zero and in the consecutive frames the envelopes are delayed. Here is a register dump for the first channel in Thunder Ceptor. The corresponding volume envelope is xC and the volume should be set to 8 in the first frame.

frame 0: 0x00 0xE1 0x24 0x00
frame 1: 0x08 0xE1 0x24 0x00  
frame 2: 0x06 0xE1 0x24 0x00 

I also see that it does not make sense to move this envelope update to the wrong place so this PR can be closed. Perhaps a better workaround would be to not set the Flag in S2X_S86WSGChannelStart immediately after the initial volume envelope (from the header) is assigned. Let me know if that makes sense or suggest alternative and I will issue another PR.

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

Successfully merging this pull request may close these issues.

3 participants