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

Added Sinistar (Cockpit version) #11459

Closed
wants to merge 5 commits into from
Closed

Added Sinistar (Cockpit version) #11459

wants to merge 5 commits into from

Conversation

synamaxmusic
Copy link

Hi there, this is my first time contributing code to Mame. I used the driver code for Blaster's stereo sound to implement the rear sound board found in the cockpit version of Sinistar. The difference between Blaster's audio set up is that both sound boards in Sinistar receive the exact same input triggers as they share the same ribbon cable (according to the drawing set for the game).

The rear sound board also doesn't have a speech daughter board attached so only the front speaker plays speech.

As a side note, I was also able to reconstruct the source code to Video Sound Rom 9 and 10. If you are interested, you can look at the repository here:
https://github.com/synamaxmusic/Sinistar-Sound-ROM/tree/main

Thanks in advance!

@einstein95
Copy link
Contributor

einstein95 commented Aug 2, 2023

You may want to double check the diff for mame.lst, as GitHub is saying it has >40,000 changed lines

@synamaxmusic
Copy link
Author

You may want to double check the diff for mame.lst, as GitHub is saying it has >40,000 changed lines

Oh wow, I have no idea how that happened. What's the best/easiest way to fix that?

@synamaxmusic
Copy link
Author

That's weird, Github's editor is messing up mame.lst after I edit it. I made another branch to copy the file from the master branch and after adding "sinistarc" at line 27500 it modifies the entire file when I commit.

Comment on lines +716 to +723
/* Same as above, but for second sound board */
void sinistar_state::sound2_map(address_map &map)
{
map(0x0000, 0x007f).ram(); // internal RAM
map(0x0080, 0x00ff).ram(); // MC6810 RAM
map(0x0400, 0x0403).mirror(0x8000).rw(m_pia[3], FUNC(pia6821_device::read), FUNC(pia6821_device::write));
map(0xb000, 0xffff).rom();
}
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this is identical to the one for blaster, you should be able to lift it to a common base class.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Gotcha. So should I do something like williams_state instead?

src/mame/midway/williams.cpp Show resolved Hide resolved
Comment on lines +1746 to +1749
// sound hardware
config.device_remove("speaker");
config.device_remove("dac");
config.device_remove("cvsd");
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It’s better to move common stuff to a separate protected function so you can build stuff additively. Also FWIW, you can reset sound routes, you don’t need to replace a device just to change sound routing.

Comment on lines +1751 to +1752
SPEAKER(config, "lspeaker").front_left();
SPEAKER(config, "rspeaker").front_right();
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these are front and rear, they should be marked appropriately. I know MAME currently sucks for routing sound outputs, but it’s going to be more trouble to fix things later if they’re marked incorrectly.

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's okay if I do front_left() and rear_right()? That way headphone users can still experience the stereo effect.

Comment on lines -3133 to +3218


Sinistar's cockpit cabinet features two sound boards, one for the front speakers and another for the rear. The rear
sound board uses a different ROM, Video Sound ROM 10. This is nearly identicial to Video Sound ROM 9, except for
a routine that adds a slight delay to some of the sound effects which produces the stereo separation effect.
It also disables the extra ship and bounce sound effects. As seen in the drawing set for Sinistar, there are no
speech ROMs connected to the rear board, so only the front speakers play speech.

If you disconnect the speech ROMs from the upright sound board, Video Sound ROM 9 will play two replacement sound
effects for the Sinistar's missing audio. Any line of dialogue will be replaced by a generic alarm noise,
while the Sinistar roar is replaced by a loud square wave synth noise that attempts to emulate the "sini-scream".
Video Sound Rom 10 disables this functionality so that it doesn't play any error sounds during speech calls.
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There’s stray whitespace on lines here. Please use the srcclean tool to normalise spacing in the file.

void sinistar(machine_config &config);

private:
virtual void vram_select_w(u8 data) override;
virtual void main_map(address_map &map) override;

optional_device<cpu_device> m_soundcpu_b;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is never used outside the machine configuration function. You don’t need a member for it.

@cuavas
Copy link
Member

cuavas commented Aug 2, 2023

You definitely need to fix mame.lst – the GitHub editor destroys large files.

@cuavas
Copy link
Member

cuavas commented Aug 17, 2023

For expediency, I cribbed this and cleaned it up as 1c09558 and credited you for the added variants. While I was looking, I also noticed you’d used the revision 2 main program rather than revision 3.

I added commented code to send the front/rear speaker output to left/right. MAME’s audio output modules need a major rethink to push the mixdown up a layer.

Thanks for looking at this.

@cuavas cuavas closed this Aug 17, 2023
@synamaxmusic
Copy link
Author

synamaxmusic commented Aug 18, 2023

Hi Vas, thank you so much for taking the time to rewrite the code and approving it for the next version.

Unfortunately, I do have some issues with the current audio implementation. After doing extensive research on this game, it is apparent that the sound designer Mike Metz only used regular stereo audio, but placed the speakers in an unusual way (front = left, rear = right) to basically simulate a surround sound set up in the cockpit. Mike calls it stereo sound in his 1983 interview, multiple contemporary magazine articles and even the Sinistar manual itself say the same. It was never a true surround sound setup, therefore it shouldn't be treated in MAME as such.

That's also why I originally wrote "stereo separation" and not "reverberation" in the comments for the code, because that's what the second sound rom does; it introduces a delay to produce a stereo widening effect. There is absolutely no reverberation or any post-processing effect done to the audio. My reverse engineered source code of the sound rom breaks down how this works in further detail:
https://github.com/synamaxmusic/Sinistar-Sound-ROM/blob/6f7f19a74e167e9c85a92952118930ac8118c0ab/VSNDRM9.SRC#L656

https://youtu.be/-JXjx6J9Iog

Here's a video that I made a while back where I simulated the stereo sound before writing this code. You can hear just how much more impressive the explosions are with this. In the current code, they're notably distorted.

With trying to keep with this front and rear speaker setup, the original intention that the developers had with the audio is lost and sounds even worst. I feel that this is counter to MAME's mission in preserving video games as accurately as possible.

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