Skip to content

CD-i: Add Audio Mixing #14481

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

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
+78 −39
Open

CD-i: Add Audio Mixing #14481

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
3ba7f27
CD-i: Add Audio Mixing
Vincent-Halver Nov 5, 2025
7470928
Allow Compilation
Vincent-Halver Nov 5, 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
4 changes: 2 additions & 2 deletions src/mame/philips/cdi.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -21,14 +21,14 @@ class cdi_state : public driver_device
public:
cdi_state(const machine_config &mconfig, device_type type, const char *tag)
: driver_device(mconfig, type, tag)
, m_cdic(*this, "cdic")
, m_maincpu(*this, "maincpu")
, m_main_rom(*this, "maincpu")
, m_lcd(*this, "lcd")
, m_slave_hle(*this, "slave_hle")
, m_plane_ram(*this, "plane%u", 0U)
, m_servo(*this, "servo")
, m_slave(*this, "slave")
, m_cdic(*this, "cdic")
, m_cdrom(*this, "cdrom")
, m_mcd212(*this, "mcd212")
, m_dmadac(*this, "dac%u", 1U)
Expand All @@ -39,6 +39,7 @@ class cdi_state : public driver_device
void cdimono2(machine_config &config);
void cdi910(machine_config &config);

optional_device<cdicdic_device> m_cdic;
protected:
enum servo_portc_bit_t
{
Expand All @@ -54,7 +55,6 @@ class cdi_state : public driver_device
required_shared_ptr_array<uint16_t, 2> m_plane_ram;
optional_device<m68hc05c8_device> m_servo;
optional_device<m68hc05c8_device> m_slave;
optional_device<cdicdic_device> m_cdic;
required_device<cdrom_image_device> m_cdrom;
required_device<mcd212_device> m_mcd212;

Expand Down
93 changes: 58 additions & 35 deletions src/mame/philips/cdicdic.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -318,50 +318,33 @@ void cdicdic_device::play_raw_group(const uint8_t *data)
m_dmadac[1]->transfer(0, 1, 1, 28, samples);
}

void cdicdic_device::play_xa_group(const uint8_t coding, const uint8_t *data)
void cdicdic_device::play_xa_group(const uint8_t coding, const uint8_t *data, const uint16_t idx)
{
static const uint16_t s_4bit_header_offsets[8] = { 4, 5, 6, 7, 12, 13, 14, 15 };
Copy link
Contributor

@MooglyGuy MooglyGuy Nov 5, 2025

Choose a reason for hiding this comment

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

Unsure if I originally added these or someone else did, but the s_ prefix is unnecessary, and SHOUTY_CAPS is prefered for constants; something like HEADER_OFFSETS_4BIT et al would be nicer.

static const uint16_t s_8bit_header_offsets[4] = { 4, 5, 6, 7 };
static const uint16_t s_4bit_data_offsets[8] = { 16, 16, 17, 17, 18, 18, 19, 19 };
static const uint16_t s_8bit_data_offsets[4] = { 16, 17, 18, 19 };

int16_t samples[28];

switch (coding & (CODING_BPS_MASK | CODING_CHAN_MASK))
uint8_t num_samples = coding&CODING_8BPS ? 4 : 8;
Copy link
Contributor

@MooglyGuy MooglyGuy Nov 5, 2025

Choose a reason for hiding this comment

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

Missing spaces.

for (uint8_t i = 0; i < num_samples; i++)
{
switch (coding & (CODING_BPS_MASK | CODING_CHAN_MASK))
{
case CODING_4BPS | CODING_MONO:
for (uint8_t i = 0; i < 8; i++)
{
decode_4bit_xa_unit(0, data[s_4bit_header_offsets[i]], data + s_4bit_data_offsets[i], (i & 1) ? 4 : 0, samples);
m_dmadac[0]->transfer(0, 1, 1, 28, samples);
m_dmadac[1]->transfer(0, 1, 1, 28, samples);
}
return;
decode_4bit_xa_unit(0, data[s_4bit_header_offsets[i]], data + s_4bit_data_offsets[i], (i & 1) ? 4 : 0, &m_samples[0][idx + i * 28]);
break;

case CODING_4BPS | CODING_STEREO:
for (uint8_t i = 0; i < 8; i++)
{
decode_4bit_xa_unit(i & 1, data[s_4bit_header_offsets[i]], data + s_4bit_data_offsets[i], (i & 1) ? 4 : 0, samples);
m_dmadac[i & 1]->transfer(0, 1, 1, 28, samples);
}
return;
decode_4bit_xa_unit(i & 1, data[s_4bit_header_offsets[i]], data + s_4bit_data_offsets[i], (i & 1) ? 4 : 0, &m_samples[i & 1][idx + (i >> 1) * 28]);
break;

case CODING_8BPS | CODING_MONO:
for (uint8_t i = 0; i < 4; i++)
{
decode_8bit_xa_unit(0, data[s_8bit_header_offsets[i]], data + s_8bit_data_offsets[i], samples);
m_dmadac[0]->transfer(0, 1, 1, 28, samples);
m_dmadac[1]->transfer(0, 1, 1, 28, samples);
}
return;
decode_8bit_xa_unit(0, data[s_8bit_header_offsets[i]], data + s_8bit_data_offsets[i], &m_samples[0][idx + i * 28]);
break;

case CODING_8BPS | CODING_STEREO:
for (uint8_t i = 0; i < 4; i++)
{
decode_8bit_xa_unit(i & 1, data[s_8bit_header_offsets[i]], data + s_8bit_data_offsets[i], samples);
m_dmadac[i & 1]->transfer(0, 1, 1, 28, samples);
}
return;
decode_8bit_xa_unit(i & 1, data[s_8bit_header_offsets[i]], data + s_8bit_data_offsets[i], &m_samples[i & 1][idx + (i >> 1) * 28]);
break;
}
}
}

Expand All @@ -377,10 +360,12 @@ void cdicdic_device::play_cdda_sector(const uint8_t *data)
{
samples[0][i] = (int16_t)((data[(i * 4) + 1] << 8) | data[(i * 4) + 0]);
samples[1][i] = (int16_t)((data[(i * 4) + 3] << 8) | data[(i * 4) + 2]);
m_samples[0][i] = samples[0][i];
m_samples[1][i] = samples[1][i];
}

m_dmadac[0]->transfer(0, 1, 1, SECTOR_SIZE/4, samples[0]);
m_dmadac[1]->transfer(0, 1, 1, SECTOR_SIZE/4, samples[1]);
m_dmadac[0]->transfer(0, 1, 1, SECTOR_SIZE/4, &m_samples[0][0]);
Copy link
Contributor

@MooglyGuy MooglyGuy Nov 5, 2025

Choose a reason for hiding this comment

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

More missing spaces. If the line is being changed anyway, might as well fix the spacing along the way. The whole file could use a sweep for operator-spacing normalization.

m_dmadac[1]->transfer(0, 1, 1, SECTOR_SIZE/4, &m_samples[1][0]);
}

void cdicdic_device::play_audio_sector(const uint8_t coding, const uint8_t *data)
Expand Down Expand Up @@ -439,6 +424,19 @@ void cdicdic_device::play_audio_sector(const uint8_t coding, const uint8_t *data
m_dmadac[0]->set_volume(0x100);
m_dmadac[1]->set_volume(0x100);

uint8_t num_samples = 4;

switch (coding & (CODING_BPS_MASK | CODING_CHAN_MASK))
{
case CODING_4BPS | CODING_MONO:
num_samples = 8;
break;
case CODING_8BPS | CODING_STEREO:
num_samples = 2;
break;
}


if (bits == 16 && channels == 2)
{
for (uint16_t i = 0; i < SECTOR_AUDIO_SIZE; i += 112, data += 112)
Expand All @@ -448,13 +446,36 @@ void cdicdic_device::play_audio_sector(const uint8_t coding, const uint8_t *data
}
else
{
uint16_t offset = 0;
for (uint16_t i = 0; i < SECTOR_AUDIO_SIZE; i += 128, data += 128)
{
play_xa_group(coding, data);
play_xa_group(coding, data, offset);
offset += 28 * num_samples;
}
int16_t sampleL = 0, sampleR = 0, outL = 0, outR = 0;
for (uint16_t i = 0; i < 18 * 28 * num_samples; i++) {
Copy link
Member

@angelosa angelosa Nov 5, 2025

Choose a reason for hiding this comment

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

File uses Allman style.

sampleL = m_samples[0][i];
sampleR = m_samples[coding & CODING_STEREO][i];

float scaleLL = powf(10.0f, -m_atten[0] / 20.0f);
float scaleLR = powf(10.0f, -m_atten[1] / 20.0f);
float scaleRR = powf(10.0f, -m_atten[2] / 20.0f);
float scaleRL = powf(10.0f, -m_atten[3] / 20.0f);
outL = (sampleL * scaleLL + sampleR * scaleRL) * 0.5;
outR = (sampleL * scaleLR + sampleR * scaleRR) * 0.5;
m_dmadac[0]->transfer(0, 1, 1, 1, &outL);
m_dmadac[1]->transfer(0, 1, 1, 1, &outR);
}
}
}

void cdicdic_device::atten_w(uint8_t* args) {
m_atten[0] = ((args[0] & 0x8) << 7) | args[1];
m_atten[1] = ((args[0] & 0x4) << 7) | args[2];
m_atten[2] = ((args[0] & 0x2) << 7) | args[3];
m_atten[3] = ((args[0] & 0x1) << 7) | args[4];
Copy link
Contributor

@MooglyGuy MooglyGuy Nov 5, 2025 •
edited
Loading

Choose a reason for hiding this comment

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

Following on from Kale's comment, it's pretty unlikely that the real hardware connection between the SLAVE and CDIC devices is 20 individual traces for the 20 bits of state represented by args here.

The correct way to do this would be to have atten_w take a single uint8_t, have a private member that increments on each call from 0 to 4 and wrapping around, in order to determine how to apply the incoming byte to the m_atten array. You would then have a devcb_write8 with a public callback-registration function in cdislavehle_device - there are plenty of examples throughout the codebase on how to do this - and then in the machine configuration function for cdi_state::cdimono1_base, do -

m_slave_hle->atten_callback().set(m_cdic, FUNC(cdicdic_device::atten_w));

rather than grabbing into a protected class member in a way that doesn't compile.

Copy link
Contributor Author

@Vincent-Halver Vincent-Halver Nov 5, 2025

Choose a reason for hiding this comment

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

Added a temporary fix to expose the cdic for dynamic casting just so that it compiles so it can be validated it worked. Thanks for the feedback, I'll look into the callback change.

}

TIMER_CALLBACK_MEMBER( cdicdic_device::audio_tick )
{
if (m_audio_sector_counter > 0)
Expand Down Expand Up @@ -1318,6 +1339,7 @@ void cdicdic_device::device_start()
save_item(NAME(m_decode_addr));

save_item(NAME(m_xa_last));
save_item(NAME(m_atten));

m_audio_timer = timer_alloc(FUNC(cdicdic_device::audio_tick), this);
m_audio_timer->adjust(attotime::never);
Expand Down Expand Up @@ -1364,7 +1386,8 @@ void cdicdic_device::device_reset()
m_dmadac[0]->enable(1);
m_dmadac[1]->enable(1);

std::fill_n(&m_xa_last[0], 4, 0);
std::fill_n(m_xa_last, 4, 0);
std::fill_n(m_atten, 4, 0);
}

void cdicdic_device::ram_w(offs_t offset, uint16_t data, uint16_t mem_mask)
Expand Down
6 changes: 4 additions & 2 deletions src/mame/philips/cdicdic.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ class cdicdic_device : public device_t
void ram_w(offs_t offset, uint16_t data, uint16_t mem_mask = ~0);

uint8_t intack_r();
void atten_w(uint8_t* args);

protected:
// device_t implementation
Expand Down Expand Up @@ -189,12 +190,13 @@ class cdicdic_device : public device_t
int16_t m_xa_last[4];
std::unique_ptr<uint8_t[]> m_ram;
std::unique_ptr<int16_t[]> m_samples[2];
int16_t m_atten[4];

void decode_xa_unit(const uint8_t param, int16_t sample, int16_t &sample0, int16_t &sample1, int16_t &out_buffer);
void decode_xa_unit(const uint8_t param, int16_t sample, int16_t& sample0, int16_t& sample1, int16_t& out_buffer);
void decode_8bit_xa_unit(int channel, uint8_t param, const uint8_t *data, int16_t *out_buffer);
void decode_4bit_xa_unit(int channel, uint8_t param, const uint8_t *data, uint8_t shift, int16_t *out_buffer);
void play_raw_group(const uint8_t *data);
void play_xa_group(const uint8_t coding, const uint8_t *data);
void play_xa_group(const uint8_t coding, const uint8_t *data, const uint16_t idx);
void play_audio_sector(const uint8_t coding, const uint8_t *data);
void play_cdda_sector(const uint8_t *data);
void process_audio_map();
Expand Down
14 changes: 14 additions & 0 deletions src/mame/philips/cdislavehle.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@

#define VERBOSE (0)
#include "logmacro.h"
#include "cdi.h"

// device type definition
DEFINE_DEVICE_TYPE(CDI_SLAVE_HLE, cdislave_hle_device, "cdislavehle", "CD-i Mono-I Slave HLE")
Expand Down Expand Up @@ -227,6 +228,12 @@ void cdislave_hle_device::slave_w(offs_t offset, uint16_t data)
{
switch (m_in_buf[0])
{
case 0xc0: case 0xc1: case 0xc2: case 0xc3: case 0xc4: case 0xc5: case 0xc6: case 0xc7:
case 0xc8: case 0xc9: case 0xca: case 0xcb: case 0xcc: case 0xcd: case 0xce: case 0xcf:
dynamic_cast<cdi_state*>(m_owner)->m_cdic->atten_w(m_in_buf);
Copy link
Member

@angelosa angelosa Nov 5, 2025

Choose a reason for hiding this comment

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

This is unportable, you should rather provide this communication thru devcb_write8::array<4>.

m_in_index = 0;
m_in_count = 0;
break;
case 0xf0: // Set Front Panel LCD
memset(m_in_buf + 1, 0, 16);
m_in_count = 17;
Expand Down Expand Up @@ -264,6 +271,13 @@ void cdislave_hle_device::slave_w(offs_t offset, uint16_t data)
m_in_count = 0;
break;
}
case 0xc0: case 0xc1: case 0xc2: case 0xc3: case 0xc4: case 0xc5: case 0xc6: case 0xc7:
case 0xc8: case 0xc9: case 0xca: case 0xcb: case 0xcc: case 0xcd: case 0xce: case 0xcf:
{
LOGMASKED(LOG_COMMANDS, "slave_w: Channel %d: Set Attenuation Audio\n", offset);
m_in_count = 5;
break;
}
case 0xf0: // Set Front Panel LCD
LOGMASKED(LOG_COMMANDS, "slave_w: Channel %d: Set Front Panel LCD (0xf0)\n", offset);
m_in_count = 17;
Expand Down
Loading