For discussion: es550[56] devices generate 20bit samples, not 16-bit ones.

[cbrunschen]

This is intended as much for discussion as for potentially merging into MAME.

The Ensoniq ES5506 (OTTO) sound synthesis chip generates 20-bit samples.

See "ENSONIQ OTTO Specification Rev. 2.3" page 14, section "6 THE CHANNEL REGISTERS - page 40hex":

All the Channel register are 23 bits right justified. The upper three bits are three overflow guard bits.
The lower 20 data bits are transferred to the serial output register.

See also Figure 16.6 on page 48, "SERIAL INTERFACE TIMING", which shows that the data shifted out on the serial audio data lines are the most significant 16 or 18 of the 20 bits in the channel register

The es5506_device code in es5506.cpp correctly generates 20-bit samples, but they are currently added to the audio stream as if they were 16-bit samples:

stream.put_int(c, sampindex, cursample[c], 32768);

This results in samples in a range of ±16 rather than ±1.

While the Ensoniq ES5505 (OTIS) chip does generate 16-bit samples, the es5505_device uses the same code as es5506_device, and thus also generates 20-bit samples, again added to the audio stream as if they were 16-bit ones.

The esq_5505_5510_pump_device in esqpump.cpp has always compensated for this by shifting the incoming 20-bit samples from the es5505_device right by SAMPLE_SHIFT = 4 bits, ever since this was added back when samples were still integers.

I have verified, by logging the incoming samples to the esq_5505_5501_pump_device while playing music on an sd132 (in esq5505.cpp), that the samples it receives from the es5505_device are generally in the ±16 range, so this is not just theoretical.

None of this prevents anything from working, but it does mean that the es550[56]_device generated samples are outside of the documented tech specs:

Samples in streams are encoded as sample_t. In the current implementation, this is a float. Nominal values are between -1 and 1 [...]

Anyone who reads the documentation and then tries to use an es550[56] device would likely be surprised by the sample range they would actually encounter. To me, it seems better if the es550[56]_device classes follow the principle of least astonishment and add their samples to their output streams in the documented ±1 range.

Another issue is that the current code even generates samples outside the ±16 range - samples which both ES550[56] chips would have clamped, see the ES5505 (OTIS) datasheet" page 7, under "Serial Mode Register (SERMODE) -8",

MSB[4:0]
The channel registers are 16 bits with 5 guard bits to prevent overflow during accumulation.
Since the bus interface is 16 bits, these 5 bits are prevented and are written to the 5 MSB's when a
channel register is written. If these bits are not either all 0's or all 1's, then overflow has occurred
and the output will be saturated at either most positive or most negative depending on the state of MSB[4].

and the "ENSONIQ OTTO Specification Rev. 2.3" on page 14, section "6 THE CHANNEL REGISTERS - page 40hex"

If an overflow or underflow condition is present at the time of transfer to the output, the data is
saturated to the appropriate rail so that data "wrapping" does not occur.

While the tech specs say that

clamping at the device level is not recommended (unless that's what happens in hardware of course)

the clamping here does happen in the real hardware, so should be simulated.

That actually also fixes some real observed audio glitches when playing back a demo sequence ($SD-PALETTE from the SD-1 Sequencer OS 4.10 disk).

Without this PR, so without clamping: without_clamping.wav

With this PR, so including clamping: with_clamping.wav

This PR proposes to address both of these by updating es5506.cpp to add generated samples to the output stream by

stream.put_int_clamp(c, sampindex, cursample[c], (1 << 19));

which not only brings the generated samples from 20-bit integers into the ±1 float range, but also clamps them.

Once the samples are generated in the correct ±1 range, the SAMPLE_SHIFT in esqpump.cpp can be removed, which this PR also does.

This PR also updates all routes that I was able to find in the codebase of audio from an es550[56]_device to anything that is not an esq_5505_5510_pump_device (that one is also being updated to handle the change in sample range), multiplying the route gain by 16 to compensate for the fact that the generated samples are now a factor of 16 smaller.

This should leave all audio sounding as before.

I have verified that the VFX family of keyboards (vfx, vfxsd, sd1 and sd132) do in fact sound the same after as before, which makes sense, as they use the esq_5505_5510_pump_device and the reduction in the incoming sample range from ±16 is exactly compensated for by removing the 4-bit SAMPLE_SHIFT.

However, as I am not familiar with any of the other devices, they should probably be tested by someone who is, to verify that they still behave as expected. I may also have missed something, so more and more knowledgeable pairs of eyes would be a great help.

So I hope that this PR can be a starting point for discussion and finding the correct solution, which may of course be something completely different than this - or simply accepting the status quo, since it does after all work.