Skip to content

Commit

Permalink
Clean up some confusing identifiers around detune parameters
Browse files Browse the repository at this point in the history 
There was no consistency with what was called detune,
detune1, dt1, dt2. I believe the root of this issue comes
from the fact that we have a unified operator
implementation that is used by every different channel
implementation. As such, some terms become harder
to distinguish.

I tried to give more unique names where that made
sense, based on the existing code comments. And to
ensure some consistency with names like detune1 and
detune2.

These changes are internal, as none of the related
parameters have been exposed yet (partially, because
of this confusion being present).
  • Loading branch information
@YuriSizov
YuriSizov committed Nov 13, 2024
1 parent 5cabcdf commit e143fdc
Show file tree
Hide file tree
Showing 14 changed files with 91 additions and 95 deletions.
24 changes: 12 additions & 12 deletions src/chip/channels/siopm_channel_fm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -155,7 +155,7 @@ void SiOPMChannelFM::set_channel_params(const Ref<SiOPMChannelParams> &p_params,
}
}

void SiOPMChannelFM::set_params_by_value(int p_ar, int p_dr, int p_sr, int p_rr, int p_sl, int p_tl, int p_ksr, int p_ksl, int p_mul, int p_dt1, int p_detune, int p_ams, int p_phase, int p_fix_note) {
void SiOPMChannelFM::set_params_by_value(int p_ar, int p_dr, int p_sr, int p_rr, int p_sl, int p_tl, int p_ksr, int p_ksl, int p_mul, int p_dt1, int p_dt2, int p_ams, int p_phase, int p_fix_note) {
#define SET_OP_PARAM(m_setter, m_value) \
if (m_value != INT32_MIN) { \
_active_operator->m_setter(m_value); \
Expand All @@ -170,8 +170,8 @@ void SiOPMChannelFM::set_params_by_value(int p_ar, int p_dr, int p_sr, int p_rr,
SET_OP_PARAM(set_key_scaling_rate, p_ksr);
SET_OP_PARAM(set_key_scaling_level, p_ksl);
SET_OP_PARAM(set_multiple, p_mul);
SET_OP_PARAM(set_dt1, p_dt1);
SET_OP_PARAM(set_detune, p_detune);
SET_OP_PARAM(set_detune1, p_dt1);
SET_OP_PARAM(set_ptss_detune, p_dt2);
SET_OP_PARAM(set_amplitude_modulation_shift, p_ams);
SET_OP_PARAM(set_key_on_phase, p_phase);

Expand Down Expand Up @@ -293,7 +293,7 @@ void SiOPMChannelFM::_set_by_opm_register(int p_address, int p_data) {

switch ((p_address - 0x40) >> 5) {
case 0: { // DT1:6-4 MUL:3-0
op->set_dt1((p_data >> 4) & 7);
op->set_detune1((p_data >> 4) & 7);
op->set_multiple(p_data & 15);
} break;
case 1: { // TL:6-0
Expand All @@ -309,7 +309,7 @@ void SiOPMChannelFM::_set_by_opm_register(int p_address, int p_data) {
} break;
case 4: { // DT2:76 SR:4-0
int options[4] = { 0, 384, 500, 608 };
op->set_detune(options[(p_data >> 6) & 3]);
op->set_ptss_detune(options[(p_data >> 6) & 3]);
op->set_sustain_rate((p_data & 31) << 1);
} break;
case 5: { // SL:7-4 RR:3-0
Expand Down Expand Up @@ -655,7 +655,7 @@ void SiOPMChannelFM::set_phase(int p_value) {
}

void SiOPMChannelFM::set_detune(int p_value) {
_active_operator->set_detune(p_value);
_active_operator->set_ptss_detune(p_value);
}

void SiOPMChannelFM::set_fixed_pitch(int p_value) {
Expand Down Expand Up @@ -725,7 +725,7 @@ void SiOPMChannelFM::initialize_lfo(int p_waveform, Vector<int> p_custom_wave_ta

for (SiOPMOperator *op : _operators) {
if (op) {
op->set_detune2(0);
op->set_pm_detune(0);
}
}
}
Expand All @@ -746,7 +746,7 @@ void SiOPMChannelFM::set_pitch_modulation(int p_depth) {
if (_pitch_modulation_depth == 0) {
for (SiOPMOperator *op : _operators) {
if (op) {
op->set_detune2(0);
op->set_pm_detune(0);
}
}
}
Expand All @@ -767,16 +767,16 @@ void SiOPMChannelFM::_update_lfo(int p_op_count) {
_pitch_modulation_output_level = (((value_base << 1) - 255) * _pitch_modulation_depth) >> 8;

if (p_op_count > 0 && _operators[0]) {
_operators[0]->set_detune2(_pitch_modulation_output_level);
_operators[0]->set_pm_detune(_pitch_modulation_output_level);
}
if (p_op_count > 1 && _operators[1]) {
_operators[1]->set_detune2(_pitch_modulation_output_level);
_operators[1]->set_pm_detune(_pitch_modulation_output_level);
}
if (p_op_count > 2 && _operators[2]) {
_operators[2]->set_detune2(_pitch_modulation_output_level);
_operators[2]->set_pm_detune(_pitch_modulation_output_level);
}
if (p_op_count > 3 && _operators[3]) {
_operators[3]->set_detune2(_pitch_modulation_output_level);
_operators[3]->set_pm_detune(_pitch_modulation_output_level);
}

_lfo_timer += _lfo_timer_initial;
Expand Down
2 changes: 1 addition & 1 deletion src/chip/channels/siopm_channel_fm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ class SiOPMChannelFM : public SiOPMChannelBase {

virtual void get_channel_params(const Ref<SiOPMChannelParams> &p_params) const override;
virtual void set_channel_params(const Ref<SiOPMChannelParams> &p_params, bool p_with_volume, bool p_with_modulation = true) override;
void set_params_by_value(int p_ar, int p_dr, int p_sr, int p_rr, int p_sl, int p_tl, int p_ksr, int p_ksl, int p_mul, int p_dt1, int p_detune, int p_ams, int p_phase, int p_fix_note);
void set_params_by_value(int p_ar, int p_dr, int p_sr, int p_rr, int p_sl, int p_tl, int p_ksr, int p_ksl, int p_mul, int p_dt1, int p_dt2, int p_ams, int p_phase, int p_fix_note);

virtual void set_wave_data(const Ref<SiOPMWaveBase> &p_wave_data) override;
virtual void set_channel_number(int p_value) override;
Expand Down
4 changes: 2 additions & 2 deletions src/chip/channels/siopm_channel_ks.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -135,7 +135,7 @@ void SiOPMChannelKS::_apply_karplus_strong(SinglyLinkedList<int>::Element *p_buf
SinglyLinkedList<int>::Element *target = p_buffer_start;
const int pitch_idx_max = SiOPMRefTable::PITCH_TABLE_SIZE - 1;

int pitch_idx = _ks_pitch_index + _operators[0]->get_detune() + _pitch_modulation_output_level;
int pitch_idx = _ks_pitch_index + _operators[0]->get_ptss_detune() + _pitch_modulation_output_level;
pitch_idx = CLAMP(pitch_idx, 0, pitch_idx_max);
double wave_length_max = _table->pitch_wave_length[pitch_idx];

Expand All @@ -148,7 +148,7 @@ void SiOPMChannelKS::_apply_karplus_strong(SinglyLinkedList<int>::Element *p_buf
int value_base = _lfo_wave_table[_lfo_phase];
_pitch_modulation_output_level = (((value_base << 1) - 255) * _pitch_modulation_depth) >> 8;

pitch_idx = _ks_pitch_index + _operators[0]->get_detune() + _pitch_modulation_output_level;
pitch_idx = _ks_pitch_index + _operators[0]->get_ptss_detune() + _pitch_modulation_output_level;
pitch_idx = CLAMP(pitch_idx, 0, pitch_idx_max);
wave_length_max = _table->pitch_wave_length[pitch_idx];

Expand Down
14 changes: 7 additions & 7 deletions src/chip/channels/siopm_channel_pcm.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ void SiOPMChannelPCM::set_channel_params(const Ref<SiOPMChannelParams> &p_params
_operator->set_operator_params(p_params->get_operator_params(0));
}

void SiOPMChannelPCM::set_params_by_value(int p_ar, int p_dr, int p_sr, int p_rr, int p_sl, int p_tl, int p_ksr, int p_ksl, int p_mul, int p_dt1, int p_detune, int p_ams, int p_phase, int p_fix_note) {
void SiOPMChannelPCM::set_params_by_value(int p_ar, int p_dr, int p_sr, int p_rr, int p_sl, int p_tl, int p_ksr, int p_ksl, int p_mul, int p_dt1, int p_dt2, int p_ams, int p_phase, int p_fix_note) {
#define SET_OP_PARAM(m_setter, m_value) \
if (m_value != INT32_MIN) { \
_operator->m_setter(m_value); \
Expand All @@ -103,8 +103,8 @@ void SiOPMChannelPCM::set_params_by_value(int p_ar, int p_dr, int p_sr, int p_rr
SET_OP_PARAM(set_key_scaling_rate, p_ksr);
SET_OP_PARAM(set_key_scaling_level, p_ksl);
SET_OP_PARAM(set_multiple, p_mul);
SET_OP_PARAM(set_dt1, p_dt1);
SET_OP_PARAM(set_detune, p_detune);
SET_OP_PARAM(set_detune1, p_dt1);
SET_OP_PARAM(set_ptss_detune, p_dt2);
SET_OP_PARAM(set_amplitude_modulation_shift, p_ams);
SET_OP_PARAM(set_key_on_phase, p_phase);

Expand Down Expand Up @@ -193,7 +193,7 @@ void SiOPMChannelPCM::set_phase(int p_value) {
}

void SiOPMChannelPCM::set_detune(int p_value) {
_operator->set_detune(p_value);
_operator->set_ptss_detune(p_value);
}

void SiOPMChannelPCM::set_fixed_pitch(int p_value) {
Expand Down Expand Up @@ -249,7 +249,7 @@ void SiOPMChannelPCM::initialize_lfo(int p_waveform, Vector<int> p_custom_wave_t
_pitch_modulation_output_level = 0;

_pcm_table = Ref<SiOPMWavePCMTable>();
_operator->set_detune2(0);
_operator->set_pm_detune(0);
}

void SiOPMChannelPCM::set_amplitude_modulation(int p_depth) {
Expand All @@ -266,7 +266,7 @@ void SiOPMChannelPCM::set_pitch_modulation(int p_depth) {
_set_lfo_state(_pitch_modulation_depth != 0 || _amplitude_modulation_depth > 0);

if (_pitch_modulation_depth == 0) {
_operator->set_detune2(0);
_operator->set_pm_detune(0);
}
}

Expand All @@ -291,7 +291,7 @@ void SiOPMChannelPCM::_update_lfo() {
_amplitude_modulation_output_level = (value_base * _amplitude_modulation_depth) >> 7 << 3;
_pitch_modulation_output_level = (((value_base << 1) - 255) * _pitch_modulation_depth) >> 8;

_operator->set_detune2(_pitch_modulation_output_level);
_operator->set_pm_detune(_pitch_modulation_output_level);

_lfo_timer += _lfo_timer_initial;
}
Expand Down
2 changes: 1 addition & 1 deletion src/chip/channels/siopm_channel_pcm.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,7 @@ class SiOPMChannelPCM : public SiOPMChannelBase {
public:
virtual void get_channel_params(const Ref<SiOPMChannelParams> &p_params) const override;
virtual void set_channel_params(const Ref<SiOPMChannelParams> &p_params, bool p_with_volume, bool p_with_modulation = true) override;
void set_params_by_value(int p_ar, int p_dr, int p_sr, int p_rr, int p_sl, int p_tl, int p_ksr, int p_ksl, int p_mul, int p_dt1, int p_detune, int p_ams, int p_phase, int p_fix_note);
void set_params_by_value(int p_ar, int p_dr, int p_sr, int p_rr, int p_sl, int p_tl, int p_ksr, int p_ksl, int p_mul, int p_dt1, int p_dt2, int p_ams, int p_phase, int p_fix_note);

virtual void set_wave_data(const Ref<SiOPMWaveBase> &p_wave_data) override;

Expand Down
40 changes: 20 additions & 20 deletions src/chip/channels/siopm_operator.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -90,23 +90,23 @@ void SiOPMOperator::set_key_scaling_level(int p_value, bool p_silent) {
}

int SiOPMOperator::get_multiple() const {
return (_multiple >> 7);
return (_fine_multiple >> 7);
}

void SiOPMOperator::set_multiple(int p_value) {
int multiple = p_value & 15;
_multiple = (multiple != 0) ? (multiple << 7) : 64;
_fine_multiple = (multiple != 0) ? (multiple << 7) : 64;
_update_pitch();
}

void SiOPMOperator::set_dt1(int p_value) {
_dt1 = p_value & 7;
void SiOPMOperator::set_detune1(int p_value) {
_detune1 = p_value & 7;
_update_pitch();
}

void SiOPMOperator::set_dt2(int p_value) {
_dt2 = p_value & 3;
_pitch_index_shift = _table->dt2_table[_dt2];
void SiOPMOperator::set_detune2(int p_value) {
_detune2 = p_value & 3;
_pitch_index_shift = _table->dt2_table[_detune2];
_update_pitch();
}

Expand Down Expand Up @@ -173,8 +173,8 @@ void SiOPMOperator::_update_pitch() {

void SiOPMOperator::_update_phase_step(int p_step) {
_phase_step = p_step;
_phase_step += _table->dt1_table[_dt1][_key_code];
_phase_step *= _multiple;
_phase_step += _table->dt1_table[_detune1][_key_code];
_phase_step *= _fine_multiple;
_phase_step >>= (7 - _table->sample_rate_pitch_shift); // 44kHz:1/128, 22kHz:1/256
}

Expand Down Expand Up @@ -222,19 +222,19 @@ void SiOPMOperator::set_pitch_index(int p_value)
_update_pitch();
}

void SiOPMOperator::set_detune(int p_value) {
_dt2 = 0;
void SiOPMOperator::set_ptss_detune(int p_value) {
_detune2 = 0;
_pitch_index_shift = p_value;
_update_pitch();
}

void SiOPMOperator::set_detune2(int p_value) {
void SiOPMOperator::set_pm_detune(int p_value) {
_pitch_index_shift2 = p_value;
_update_pitch();
}

void SiOPMOperator::set_fine_multiple(int p_value) {
_multiple = p_value;
_fine_multiple = p_value;
_update_pitch();
}

Expand Down Expand Up @@ -276,7 +276,7 @@ void SiOPMOperator::set_key_fraction(int p_value) {
void SiOPMOperator::set_fnumber(int p_value) {
// Naive implementation.
_update_key_code((p_value >> 7) & 127);
_dt2 = 0;
_detune2 = 0;
_pitch_index = 0;
_pitch_index_shift = 0;
_update_phase_step((p_value & 2047) << ((p_value >> 11) & 7));
Expand Down Expand Up @@ -456,10 +456,10 @@ void SiOPMOperator::set_operator_params(const Ref<SiOPMOperatorParams> &p_params

set_amplitude_modulation_shift(p_params->get_amplitude_modulation_shift());

_multiple = p_params->get_fine_multiple();
_fine_multiple = p_params->get_fine_multiple();
_fm_shift = (p_params->get_frequency_modulation_level() & 7) + 10;
_dt1 = p_params->get_detune1() & 7;
_pitch_index_shift = p_params->get_detune();
_detune1 = p_params->get_detune1() & 7;
_pitch_index_shift = p_params->get_detune2();

_mute = p_params->is_mute() ? SiOPMRefTable::ENV_BOTTOM : 0;
_ssg_type_envelope_control = p_params->get_ssg_type_envelope_control();
Expand Down Expand Up @@ -494,8 +494,8 @@ void SiOPMOperator::get_operator_params(const Ref<SiOPMOperatorParams> &r_params
r_params->set_key_scaling_rate(get_key_scaling_rate());
r_params->set_key_scaling_level(_key_scaling_level);
r_params->set_fine_multiple(get_fine_multiple());
r_params->set_detune1(_dt1);
r_params->set_detune(get_detune());
r_params->set_detune1(_detune1);
r_params->set_detune2(get_ptss_detune());
r_params->set_amplitude_modulation_shift(get_amplitude_modulation_shift());

r_params->set_ssg_type_envelope_control(_ssg_type_envelope_control);
Expand Down Expand Up @@ -605,7 +605,7 @@ String SiOPMOperator::_to_string() const {

params += "keyscale=(" + itos(get_key_scaling_rate()) + ", " + itos(get_key_scaling_level()) + "), ";
params += "fmul=" + itos(get_fine_multiple()) + ", ";
params += "detune=(" + itos(get_dt1()) + ", " + itos(get_detune()) + "), ";
params += "detune=(" + itos(get_detune1()) + ", " + itos(get_ptss_detune()) + "), ";

params += "amp=" + itos(get_amplitude_modulation_shift()) + ", ";
params += "phase=" + itos(get_key_on_phase()) + ", ";
Expand Down
36 changes: 17 additions & 19 deletions src/chip/channels/siopm_operator.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,12 +65,12 @@ class SiOPMOperator : public Object {
int _key_scaling_rate = 0;
// Key scaling level [0,3].
int _key_scaling_level = 0;
// _multiple = (mul ? (mul << 7) : 64); [64,128,256,384,512...].
int _multiple = 0;
// dt1 [0,7].
int _dt1 = 0;
// dt2 [0,3]. This value is linked with _pitch_index_shift.
int _dt2 = 0;
// Fine multiple [64,128,256,384,512...].
int _fine_multiple = 0;
// Raw detune1 [0,7].
int _detune1 = 0;
// Raw detune2 [0,3].
int _detune2 = 0;
// Amp modulation shift [16,0].
int _amplitude_modulation_shift = 0;
// Key code = oct << 4 + note [0,127].
Expand Down Expand Up @@ -104,9 +104,9 @@ class SiOPMOperator : public Object {

// Pitch index = note * 64 + key fraction.
int _pitch_index = 0;
// This value is linked with dt2 and detune.
// Detune for pTSS. 1 halftone divides into 64 steps.
int _pitch_index_shift = 0;
// Pitch index shift by pitch modulation. This value is linked with dt2.
// Detune for pitch modulation.
int _pitch_index_shift2 = 0;
// Frequency modulation left-shift. 15 for FM, fb+6 for feedback.
int _fm_shift = 0;
Expand Down Expand Up @@ -202,14 +202,14 @@ class SiOPMOperator : public Object {
void set_multiple(int p_value);

// Fine multiple for pTSS. 128=x1.
int get_fine_multiple() const { return _multiple; }
int get_fine_multiple() const { return _fine_multiple; }
void set_fine_multiple(int p_value);

int get_dt1() const { return _dt1; }
void set_dt1(int p_value);
int get_detune1() const { return _detune1; }
void set_detune1(int p_value);

int get_dt2() const { return _dt2; }
void set_dt2(int p_value);
int get_detune2() const { return _detune2; }
void set_detune2(int p_value);

bool is_amplitude_modulation_enabled() const;
void set_amplitude_modulation_enabled(bool p_enabled);
Expand Down Expand Up @@ -251,12 +251,10 @@ class SiOPMOperator : public Object {
// Setting to 0 disables the pitch fixed flag.
void set_fixed_pitch_index(int p_value);

// Detune for pTSS. 1 halftone divides into 64 steps.
int get_detune() const { return _pitch_index_shift; }
void set_detune(int p_value);
// Detune for pitch modulation.
int get_detune2() const { return _pitch_index_shift2; }
void set_detune2(int p_value);
int get_ptss_detune() const { return _pitch_index_shift; }
void set_ptss_detune(int p_value);
int get_pm_detune() const { return _pitch_index_shift2; }
void set_pm_detune(int p_value);

int get_fm_shift() const { return _fm_shift; }

Expand Down
4 changes: 2 additions & 2 deletions src/chip/siopm_channel_params.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -129,8 +129,8 @@ void SiOPMChannelParams::set_by_opm_register(int p_channel, int p_address, int p
} break;
case 4: { // DT2:76 SR:4-0
int options[4] = { 0, 384, 500, 608 };
op_params->detune = options[(p_data >> 6) & 3];
op_params->sustain_rate = (p_data & 31) << 1;
op_params->detune2 = options[(p_data >> 6) & 3];
op_params->sustain_rate = (p_data & 31) << 1;
} break;
case 5: { // SL:7-4 RR:3-0
op_params->sustain_level = (p_data >> 4) & 15;
Expand Down
6 changes: 3 additions & 3 deletions src/chip/siopm_operator_params.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ void SiOPMOperatorParams::initialize() {

fine_multiple = 128;
detune1 = 0;
detune = 0;
detune2 = 0;

amplitude_modulation_shift = 0;
initial_phase = 0;
Expand Down Expand Up @@ -66,7 +66,7 @@ void SiOPMOperatorParams::copy_from(const Ref<SiOPMOperatorParams> &p_params) {

fine_multiple = p_params->fine_multiple;
detune1 = p_params->detune1;
detune = p_params->detune;
detune2 = p_params->detune2;

amplitude_modulation_shift = p_params->amplitude_modulation_shift;
initial_phase = p_params->initial_phase;
Expand All @@ -93,7 +93,7 @@ String SiOPMOperatorParams::_to_string() const {

params += "keyscale=(" + itos(key_scaling_rate) + ", " + itos(key_scaling_level) + "), ";
params += "fmul=" + itos(fine_multiple) + ", ";
params += "detune=(" + itos(detune1) + ", " + itos(detune) + "), ";
params += "detune=(" + itos(detune1) + ", " + itos(detune2) + "), ";

params += "amp=" + itos(amplitude_modulation_shift) + ", ";
params += "phase=" + itos(initial_phase) + ", ";
Expand Down
Loading

0 comments on commit e143fdc

Please sign in to comment.