Skip to content

Hooking up MIDI output from soundboard #226

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.

Sign up for GitHub

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

Draft
gm-matthew wants to merge 2 commits into
base: master
Choose a base branch
from
Draft

Hooking up MIDI output from soundboard #226

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
62561a0
Hooking up MIDI output from soundboard
gm-matthew Feb 17, 2025
e55e466
Hooking up MIDI output from soundboard
gm-matthew Feb 17, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions Config/Games.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1217,7 +1217,7 @@
<!-- Correct dump of Magical Truck Adventure, redumped September 2022. -->
<identity>
<title>Magical Truck Adventure</title>
<version>Japan</version>
<version>Export</version>
<manufacturer>Sega</manufacturer>
<year>1998</year>
</identity>
Expand All @@ -1233,8 +1233,8 @@
</hardware>
<roms>
<patches>
<!-- Enable Region change (remove the patch to return to Japan region) -->
<patch region="crom" bits="32" offset="0x14B25C" value="0x38600001" />
<!-- Force region to Japan -->
<!--<patch region="crom" bits="32" offset="0x001794" value="0x38000000" />-->
</patches>
<region name="crom" stride="8" chunk_size="2" byte_swap="true">
<file offset="0" name="epr-21434.20" crc32="0x18F8626A" />
Expand Down Expand Up @@ -1288,7 +1288,7 @@
-->
<identity>
<title>Magical Truck Adventure</title>
<version>Japan</version>
<version>Export</version>
<manufacturer>Sega</manufacturer>
<year>1998</year>
</identity>
Expand All @@ -1306,8 +1306,8 @@
<patches>
<!-- Fixing bad ROM dump which causes attract mode to stop rendering -->
<patch region="crom" bits="32" offset="0x091A34" value="0x917F0068" />
<!-- Enable Region change (remove the patch to return to Japan region) -->
<patch region="crom" bits="32" offset="0x14B25C" value="0x38600001" />
<!-- Force region to Japan -->
<!--<patch region="crom" bits="32" offset="0x001794" value="0x38000000" />-->
</patches>
<region name="crom" stride="8" chunk_size="2" byte_swap="true">
<file offset="0" name="epr-21434.20" crc32="0xE028D7CA" />
Expand Down
32 changes: 14 additions & 18 deletions Src/Model3/Model3.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -978,9 +978,9 @@ UINT8 CModel3::Read8(UINT32 addr)
switch (addr & 0xf)
{
case 0x0: // MIDI data port
return 0x00; // Something to do with region locked in magtruck (0=locked, 1=unlocked). /!\ no effect if rom patch is activated!
return SoundBoard.ReadMIDIPort();
case 0x4: // MIDI control port
return 0x83; // magtruck country check
return SoundBoard.CheckMIDIStatus();
default:
return 0;
}
Expand Down Expand Up @@ -1428,7 +1428,7 @@ void CModel3::Write8(UINT32 addr, UINT8 data)
else if ((addr & 0xF) == 4) // MIDI control port
{
midiCtrlPort = data;
if ((data & 0x20) == 0)
if ((data & 0x01) == 0)
IRQ.Deassert(0x40);
}
break;
Expand Down Expand Up @@ -2124,17 +2124,19 @@ void CModel3::RunMainBoardFrame(void)
/*
* Sound:
*
* Bit 0x20 of the MIDI control port appears to enable periodic interrupts,
* which are used to send MIDI commands. Often games will write 0x27, send
* a series of commands, and write 0x06 to stop. Other games, like Star
* Wars Trilogy and Sega Rally 2, will enable interrupts at the beginning
* by writing 0x37 and will disable/enable interrupts to control command
* output.
* The PPC communicates with the soundboard via an Intel 8251 compatible
* USART device, which transmits/receives data to/from the master SCSP MIDI
* input/output lines. Setting bit 0 (0x01) when writing to the control
* port enables data transmission; when it is enabled, a sound IRQ (0x40)
* is triggered when the USART device is ready to transmit data.
*
* Most games disable sound IRQs once they have finished sending commands
* by writing 0x06 to the control port, clearing bit 0 to disable data
* transmission. Sega Rally 2 and Star Wars Trilogy instead disable sound
* IRQs by masking them via the IRQ controller.
*/
//printf("\t-- BEGIN (Ctrl=%02X, IRQEn=%02X, IRQPend=%02X) --\n", midiCtrlPort, IRQ.ReadIRQEnable()&0x40, IRQ.ReadIRQState());
int irqCount = 0;
while ((midiCtrlPort & 0x20))
//while (midiCtrlPort == 0x27) // 27 triggers IRQ sequence, 06 stops it
while (midiCtrlPort & 0x01 && SoundBoard.CheckMIDIStatus() & 0x01) // don't send any more commands if the FIFO buffer is full
{
// Don't waste time firing MIDI interrupts if game has disabled them
if ((IRQ.ReadIRQEnable() & 0x40) == 0)
Expand All @@ -2144,12 +2146,6 @@ void CModel3::RunMainBoardFrame(void)
IRQ.Assert(0x40);
ppc_execute(1000); // give PowerPC time to acknowledge IR
dispCycles -= 1000;

++irqCount;
if (irqCount > 128)
{
break;
}
}

IRQ.Assert(0x0D);
Expand Down
17 changes: 17 additions & 0 deletions Src/Model3/SoundBoard.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,13 +330,30 @@ int SCSP68KRunCallback(int numCycles)
Sound Board Interface
******************************************************************************/

UINT8 CSoundBoard::ReadMIDIPort(void)
{
UINT8 result = SCSP_MidiOutR();
return result;
}

void CSoundBoard::WriteMIDIPort(UINT8 data)
{
SCSP_MidiIn(data);
if (NULL != DSB) // DSB receives all commands as well
DSB->SendCommand(data);
}

UINT8 CSoundBoard::CheckMIDIStatus(void)
{
UINT8 status = 0x80;
if (SCSP_MidiInFill() < 256) // MIDI input buffer not full
Copy link
Owner

@trzy trzy Feb 18, 2025

Choose a reason for hiding this comment

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

How come 256 is used here but it appears that inside of SCSP.cpp, the FIFOs are only of size 4? It may indeed make sense to have either a function or a static constexpr var that specifies the buffer length.

Copy link
Contributor Author

@gm-matthew gm-matthew Feb 19, 2025

Choose a reason for hiding this comment

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

The real MIDI input FIFO buffer is only 4 bytes, but it is increased to 256 bytes in Supermodel simply because the soundboard doesn't synchronize with the mainboard in the emulator as often as it does on real hardware. Some games (Sega Rally 2 in particular) write several sound commands in a single frame so the increased FIFO size is a way of making sure they are all received by the soundboard.

As a side note, the MIDI input FIFO buffer was actually originally 128 bytes but I increased it to 256 because Sega Rally 2 was sending so many commands it was overflowing the FIFO buffer (this commit adds a check to prevent overflows).

In SCSP.cpp there is MIDI_STACK_SIZE which is defined as 0x100 (256) but SoundBoard.cpp doesn't have access to this. If I were to create a function within SCSP.cpp such as SCSP_MidiInFull() I could use the existing MIDI_STACK_SIZE; probably a good idea to turn it into a constexpr as well.

Copy link
Owner

@trzy trzy Feb 19, 2025

Choose a reason for hiding this comment

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

Ok, I understand, that makes sense. Ignore what I said in the comment I just posted, then. I think the functions you propose make sense and would be good to include this note about the buffer being larger due to less frequent synchronization.

Copy link
Collaborator

@dukeeeey dukeeeey Apr 18, 2025

Choose a reason for hiding this comment

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

What's the status on this? Was there a reason it didn't get merged?

Copy link
Contributor Author

@gm-matthew gm-matthew Apr 19, 2025

Choose a reason for hiding this comment

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

Still haven't finished refining the code, in particular I wanted to remove the hardcoded FIFO sizes. I will get around to it at some point

status |= 1;
if (SCSP_MidiOutFill() > 0) // MIDI output buffer not empty
status |= 2;

return status;
}

#ifdef SUPERMODEL_LOG_AUDIO
static INT16 ClampINT16(float x)
{
Expand Down
20 changes: 20 additions & 0 deletions Src/Model3/SoundBoard.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -74,6 +74,16 @@ class CSoundBoard: public IBus
void Write16(UINT32 addr, UINT16 data);
void Write32(UINT32 addr, UINT32 data);

/*
* ReadMIDIPort(void):
*
* Reads from the sound board MIDI port.
*
* Returns:
* Data read from MIDI port.
*/
UINT8 ReadMIDIPort(void);

/*
* WriteMIDIPort(data):
*
Expand All @@ -84,6 +94,16 @@ class CSoundBoard: public IBus
*/
void WriteMIDIPort(UINT8 data);

/*
* CheckMIDIStatus(void):
*
* Checks status of MIDI input/output buffers.
*
* Returns:
* Status of MIDI input/output buffers.
*/
UINT8 CheckMIDIStatus(void);

/*
* SaveState(SaveState):
*
Expand Down
8 changes: 4 additions & 4 deletions Src/Sound/SCSP.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1757,8 +1757,8 @@ void SCSP_MidiOutW(BYTE val)

//printf("68K: MIDI out\n");
//DebugLog("Midi Out Buffer push %02X",val);
MidiStack[MidiOutW++]=val;
MidiOutW&=31;
MidiOutStack[MidiOutW++]=val;
MidiOutW &= 3;
Copy link
Owner

@trzy trzy Feb 18, 2025

Choose a reason for hiding this comment

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

The buffer is only 4 bytes?

Copy link
Contributor Author

@gm-matthew gm-matthew Feb 19, 2025

Choose a reason for hiding this comment

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

Yes, according to the official SCSP documentation.

Copy link
Owner

@trzy trzy Feb 19, 2025

Choose a reason for hiding this comment

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

I see. In that case, shouldn't SoundBoard.cpp use 4 rather than 256? Definitely makes sense to define that constant in SCSP.cpp or SCSP.h. I think your original suggestion of functions that indicate full vs. empty makes the most sense because it doesn't make sense for external components like CSoundBoard to be worrying about FIFO size explicitly.

++MidiOutFill;

if (s_multiThreaded)
Expand All @@ -1779,9 +1779,9 @@ unsigned char SCSP_MidiOutR()
if (s_multiThreaded)
MIDILock->Lock();

val=MidiStack[MidiOutR++];
val=MidiOutStack[MidiOutR++];
//DebugLog("Midi Out Buffer pop %02X",val);
MidiOutR&=31;
MidiOutR&=3;
--MidiOutFill;

if (s_multiThreaded)
Expand Down