Remove global state

[jcmoyer]

Hello, first of all thank you for releasing this project. The SC-55 is legendary and it is great to see such an important piece of history preserved.

The main goal of this PR is to remove global state from the emulator so that multiple instances can be created in a single process. This PR splits the emulator into a frontend (the SDL application) and a backend (the SC-55 emulator). A frontend can create and manage as many backends as it sees fit. As a side effect this also turns the backend into a library that can be linked into new frontends.

Global state is blocking a couple of issues:

• #11 - Polyphony limits on prototype midi-wav renderer
• #12 - Proposal for a frontend/backend split with linkable backend
• #29 - Polyphony limits; closed by this PR

In an effort to make this PR as mergeable as possible, I tried to make a minimum number of changes to existing code. New code attempts to follow existing patterns.

• Most variable names were preserved
  • One exception: mcu_timer had some variable shadowing (global mcu_timer_t timer + local frt_t* timer) - this was resolved by renaming the local frt_t* timer to ftimer
• I tried to stick to using C features since the project seems to be doing that already. Two exceptions: references for parameters (done to avoid changing . to -> everywhere, but these can be changed to pointers on request), and two math function templates (these can also be changed, but I'd like to avoid it).
• All globals that were previously initialized at file scope are now initialized in their respective <module>_Init function. For example, sw_pos is initialized to 3 in MCU_Init. They are not initialized inline because doing so would cause the compiler to generate a default constructor, which makes the type non-trivial so it cannot be memset.

That said, the changes weren't all simple and some problems took a bit more effort to solve. Other contributors might like to be aware of these points:

• All global variables were moved into their respective component structures. For example, lcd_width is now a field in lcd_t.
• Some structures now contain RAM/ROM inline and pointers to other components. They cannot be memset to reset their state.
• TIMER_Reset wasn't used anywhere. I renamed it TIMER_Init and use it to initialize a timer and link it to its MCU.
• LCD size is now only set in LCD_Init instead of two places.
• LCD m_back_path was removed because moving a std::string into lcd_t would make it non-trivial. LCD_SetBackPath was replaced with LCD_LoadBack and should be called after LCD_Init.
• Global lcd_init was removed. If LCD_Init succeeded you have an LCD.
• LCD event handling was extracted from LCD_Update into LCD_HandleEvent. Since multiple LCDs are supported, each LCD needs to handle only events targeted at it.
• The current frontend SDL application was extracted from the MCU module. main is now located in main_frontend.cpp.
• The MCU now posts sample data to the frontend as it is produced via callback. This approach was chosen because it allows the frontend to decide how to handle sample data in a zero-copy manner.
• A new emu_t type was introduced that is conceptually a single, complete backend instance. It contains all of the SC-55 emulator state and links the individual modules together. Romset detection and loading has been moved here since romsets need to be loaded into multiple emulator components.
• The ringbuffer in the current MCU module was extracted into a new type ringbuffer_t. The frontend creates a ringbuffer for each emu instance and fills it as the frontend receives sample data. All instance buffers are then read and mixed in the SDL audio callback. Right now ringbuffer_t is specialized for signed 16-bit sample data but I would like for it to eventually become a template type to handle different types of sample data (useful for #59 for instance).

As an example of what this PR enables I have modified the frontend SDL application so that you can pass -instances:<n> to open n instances of the emulator. Non-sysex MIDI events are routed to their midi_channel_id % n (so if n=2, even channels go to the first emulator, and odd channels go to the second one). Sysex events are broadcast to all emulators. This is a quick and easy way to solve polyphony problems at the expense of twice the computing power.

Performance impact

Performance seems to be unchanged or marginally better. I benchmarked running the emulator as fast as possible without locking or waiting for the ringbuffer to drain for 100_000_000 iterations. These are the time measurements for 5 consecutive runs:

	master	multi-instance -instances:1	multi-instance -instances:2
1	30411 ms	29287 ms	29872 ms
2	30009 ms	29393 ms	29709 ms
3	30972 ms	28519 ms	28786 ms
4	30259 ms	29483 ms	29867 ms
5	30463 ms	29288 ms	29466 ms
average	30423 ms	29194 ms	29540 ms
%change		-4.04%	-2.90%

[nikitalita]

This is really cool; I've tested this with -instances:2 on midi files that had polyphony >28 such that parts would cut out or go flat on a real sc-55 mk2, and this plays those beautifully.

[pachuco]

Yes please, make it easier to use as lib.

[johnnovak]

Very nice @jcmoyer, this makes it a lot easier to turn this into a plugin that can be instantiated multiple times. Then clear separation and boundaries between the processing engine and the frontend is just good design. You should be able to run the processing engine completely "headless", without any UI, just from the command line.

[Axis4s]

Hello, first of all thank you for releasing this project. The SC-55 is legendary and it is great to see such an important piece of history preserved.

The main goal of this PR is to remove global state from the emulator so that multiple instances can be created in a single process. This PR splits the emulator into a frontend (the SDL application) and a backend (the SC-55 emulator). A frontend can create and manage as many backends as it sees fit. As a side effect this also turns the backend into a library that can be linked into new frontends.

Global state is blocking a couple of issues:

* [Outputting MIDIs to WAV/MP3 possible? #11](https://github.com/nukeykt/Nuked-SC55/issues/11) - Polyphony limits on prototype midi-wav renderer

* [proposal to make standalone library and add multi-instance capability #12](https://github.com/nukeykt/Nuked-SC55/issues/12) - Proposal for a frontend/backend split with linkable backend

* [combine 2 emus to increase polyphony #29](https://github.com/nukeykt/Nuked-SC55/issues/29) - Polyphony limits; closed by this PR

In an effort to make this PR as mergeable as possible, I tried to make a minimum number of changes to existing code. New code attempts to follow existing patterns.

* Most variable names were preserved
  
  * One exception: `mcu_timer` had some variable shadowing (global `mcu_timer_t timer` + local `frt_t* timer`) - this was resolved by renaming the local `frt_t* timer` to `ftimer`

* I tried to stick to using C features since the project seems to be doing that already. Two exceptions: references for parameters (done to avoid changing . to -> everywhere, but these can be changed to pointers on request), and two math function templates (these can also be changed, but I'd like to avoid it).

* All globals that were previously initialized at file scope are now initialized in their respective `<module>_Init` function. For example, `sw_pos` is initialized to 3 in `MCU_Init`. They are not initialized inline because doing so would cause the compiler to generate a default constructor, which makes the type non-trivial so it cannot be `memset`.

That said, the changes weren't all simple and some problems took a bit more effort to solve. Other contributors might like to be aware of these points:

* All global variables were moved into their respective component structures. For example, `lcd_width` is now a field in `lcd_t`.

* Some structures now contain RAM/ROM _inline_ and pointers to other components. They cannot be `memset` to reset their state.

* `TIMER_Reset` wasn't used anywhere. I renamed it `TIMER_Init` and use it to initialize a timer and link it to its MCU.

* LCD size is now only set in `LCD_Init` instead of two places.

* LCD `m_back_path` was removed because moving a `std::string` into `lcd_t` would make it non-trivial. `LCD_SetBackPath` was replaced with `LCD_LoadBack` and should be called after `LCD_Init`.

* Global `lcd_init` was removed. If `LCD_Init` succeeded you have an LCD.

* LCD event handling was extracted from `LCD_Update` into `LCD_HandleEvent`. Since multiple LCDs are supported, each LCD needs to handle only events targeted at it.

* The current frontend SDL application was extracted from the MCU module. `main` is now located in `main_frontend.cpp`.

* The MCU now posts sample data to the frontend as it is produced via callback. This approach was chosen because it allows the frontend to decide how to handle sample data in a zero-copy manner.

* A new `emu_t` type was introduced that is conceptually a single, complete backend instance. It contains all of the SC-55 emulator state and links the individual modules together. Romset detection and loading has been moved here since romsets need to be loaded into multiple emulator components.

* The ringbuffer in the current MCU module was extracted into a new type `ringbuffer_t`. The frontend creates a ringbuffer for each emu instance and fills it as the frontend receives sample data. All instance buffers are then read and mixed in the SDL audio callback. Right now `ringbuffer_t` is specialized for signed 16-bit sample data but I would like for it to eventually become a template type to handle different types of sample data (useful for [Output 32-bit float samples to preserve dynamic range and prevent clipping #59](https://github.com/nukeykt/Nuked-SC55/pull/59) for instance).

As an example of what this PR enables I have modified the frontend SDL application so that you can pass -instances:<n> to open n instances of the emulator. Non-sysex MIDI events are routed to their midi_channel_id % n (so if n=2, even channels go to the first emulator, and odd channels go to the second one). Sysex events are broadcast to all emulators. This is a quick and easy way to solve polyphony problems at the expense of twice the computing power.

Performance impact

Performance seems to be unchanged or marginally better. I benchmarked running the emulator as fast as possible without locking or waiting for the ringbuffer to drain for 100_000_000 iterations. These are the time measurements for 5 consecutive runs: master multi-instance -instances:1 multi-instance -instances:2 1 30411 ms 29287 ms 29872 ms 2 30009 ms 29393 ms 29709 ms 3 30972 ms 28519 ms 28786 ms 4 30259 ms 29483 ms 29867 ms 5 30463 ms 29288 ms 29466 ms average 30423 ms 29194 ms 29540 ms %change -4.04% -2.90%

As a side effect this also turns the backend into a library that can be linked into new frontends.

Does it mean it can be more easily integrated as a VST form?

[johnnovak]

Does it mean it can be more easily integrated as a VST form?

Yes.

[Falcosoft]

Hi, I have tried with the latest updates using VS 2019 and I have noticed 2 problems:

1. It does not work with SC-55 MK1 roms (and supposedly neither with any rom sets where the 2nd rom is not 512k). The problem seems to be EMU_ReadStreamUpTo() and the error is "FATAL ERROR: Failed to read the mcu ROM2". It works this way: std::streamsize rom2_read = EMU_ReadStreamUpTo(s_rf[1], m_mcu->rom2, m_mcu->mcu_mk1 ? ROM2_SIZE / 2 : ROM2_SIZE);
2. If enable_lcd = false in EMU_Options then LCD_Update() throws a null pointer exception (not with the wave renderer).
   It works with this check: if (!lcd.mcu) return;

[jcmoyer]

Hi @Falcosoft I think you are building the modernize branch which isn't related to this PR, but I'll take a look.

EDIT:

1 is a bug, thanks for reporting it.

As for 2, enable_lcd isn't complete yet. Right now the idea is that when a frontend sets enable_lcd it either commits to calling LCD_* functions (true), or never calling LCD_* functions (false). Eventually, I would like to have a version of the backend that doesn't depend on SDL at all, and it's not clear yet how enable_lcd will fit in. That said I think it's desirable to be able to run the main frontend headless today, so I implemented this feature keeping the above design in mind. You can simply pass --no-lcd on the command line.

[Falcosoft]

Hi @Falcosoft I think you are building the modernize branch which isn't related to this PR, but I'll take a look.

EDIT:

1 is a bug, thanks for reporting it.

As for 2, enable_lcd isn't complete yet. Right now the idea is that when a frontend sets enable_lcd it either commits to calling LCD_* functions (true), or never calling LCD_* functions (false). Eventually, I would like to have a version of the backend that doesn't depend on SDL at all, and it's not clear yet how enable_lcd will fit in. That said I think it's desirable to be able to run the main frontend headless today, so I implemented this feature keeping the above design in mind. You can simply pass --no-lcd on the command line.

OK, thanks!

[johnnovak]

@jcmoyer Please read your email 😏 (the one linked to your GitHub account)