From 5197b9d78a93ae13e058fdc4489a48e6853aa0ec Mon Sep 17 00:00:00 2001 From: Aidan Mueller Date: Sun, 30 May 2021 11:59:46 -0500 Subject: [PATCH 1/4] Fixed issue #5734 (FreeBoy Division by zero). Added comments and used more descriptive variable names for noise channel initialization block. Also indented the nested for loop to improve code clarity. The reasons for doing this can be found in this answer: https://softwareengineering.stackexchange.com/a/362796 --- plugins/FreeBoy/FreeBoy.cpp | 34 +++++++++++++++++++++------------- 1 file changed, 21 insertions(+), 13 deletions(-) diff --git a/plugins/FreeBoy/FreeBoy.cpp b/plugins/FreeBoy/FreeBoy.cpp index 95ba5a215b5..cce1c8c5525 100644 --- a/plugins/FreeBoy/FreeBoy.cpp +++ b/plugins/FreeBoy/FreeBoy.cpp @@ -364,25 +364,33 @@ void FreeBoyInstrument::playNote( NotePlayHandle * _n, if( tfp == 0 ) { + // Initialize noise channel... //PRNG Frequency = (1048576 Hz / (ratio + 1)) / 2 ^ (shiftclockfreq + 1) - char sopt=0; - char ropt=1; - float fopt = 524288.0 / ( ropt * pow( 2.0, sopt + 1.0 ) ); - float f; - for ( char s=0; s<16; s++ ) - for ( char r=0; r<8; r++ ) { - f = 524288.0 / ( r * pow( 2.0, s + 1.0 ) ); - if( fabs( freq-fopt ) > fabs( freq-f ) ) { - fopt = f; - ropt = r; - sopt = s; + char clock_freq = 0; + char div_ratio = 1; + float closest_freq = 524288.0 / ( div_ratio * pow( 2.0, clock_freq + 1.0 ) ); + // This nested for loop iterates over all possible combinations of clock frequency and dividing + // ratio and chooses the combination whose resulting frequency is closest to the note frequency + for ( char s = 0; s < 16; s++ ) + { + for ( char r = 0 ; r < 8; r++ ) + { + float r_val = (r == 0 ? 0.5 : r); + float f = 524288.0 / ( r_val * pow( 2.0, s + 1.0 ) ); + if( fabs( freq - closest_freq ) > fabs( freq - f ) ) + { + closest_freq = f; + div_ratio = r; + clock_freq = s; + } } } - data = sopt; + + data = clock_freq; data = data << 1; data += m_ch4ShiftRegWidthModel.value(); data = data << 3; - data += ropt; + data += div_ratio; papu->write_register( fakeClock(), 0xff22, data ); //channel 4 init From 32e98b7bf8338479b0530d63927a4bcd0cff645f Mon Sep 17 00:00:00 2001 From: Spekular Date: Fri, 8 Jul 2022 15:03:02 -0400 Subject: [PATCH 2/4] Better initial div_ratio guess Allows us to skip r = 0 and a conditional in the loop below. --- plugins/FreeBoy/FreeBoy.cpp | 13 +++++++------ 1 file changed, 7 insertions(+), 6 deletions(-) diff --git a/plugins/FreeBoy/FreeBoy.cpp b/plugins/FreeBoy/FreeBoy.cpp index cce1c8c5525..e95b5064d06 100644 --- a/plugins/FreeBoy/FreeBoy.cpp +++ b/plugins/FreeBoy/FreeBoy.cpp @@ -365,18 +365,19 @@ void FreeBoyInstrument::playNote( NotePlayHandle * _n, if( tfp == 0 ) { // Initialize noise channel... - //PRNG Frequency = (1048576 Hz / (ratio + 1)) / 2 ^ (shiftclockfreq + 1) + // PRNG Frequency = (1048576 Hz / (ratio + 1)) / 2 ^ (shiftclockfreq + 1) + // When div_ratio = 0 the ratio should be 0.5. Since s = 0 is the only case where r = 0 gives + // a unique frequency, we can start by guessing s = r = 0 here and then skip r = 0 in the loop. char clock_freq = 0; - char div_ratio = 1; - float closest_freq = 524288.0 / ( div_ratio * pow( 2.0, clock_freq + 1.0 ) ); + char div_ratio = 0; + float closest_freq = 524288.0 / ( 0.5 * pow( 2.0, clock_freq + 1.0 ) ); // This nested for loop iterates over all possible combinations of clock frequency and dividing // ratio and chooses the combination whose resulting frequency is closest to the note frequency for ( char s = 0; s < 16; s++ ) { - for ( char r = 0 ; r < 8; r++ ) + for ( char r = 1 ; r < 8; r++ ) { - float r_val = (r == 0 ? 0.5 : r); - float f = 524288.0 / ( r_val * pow( 2.0, s + 1.0 ) ); + float f = 524288.0 / ( r * pow( 2.0, s + 1.0 ) ); if( fabs( freq - closest_freq ) > fabs( freq - f ) ) { closest_freq = f; From aea255257f0d5de6561bd21a9f013ed43844eeaf Mon Sep 17 00:00:00 2001 From: Hyunjin Song Date: Sun, 2 Jul 2023 12:02:20 +0900 Subject: [PATCH 3/4] Fix formatting a bit --- plugins/FreeBoy/FreeBoy.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/plugins/FreeBoy/FreeBoy.cpp b/plugins/FreeBoy/FreeBoy.cpp index d4c4944e0ed..b528ac2f00a 100644 --- a/plugins/FreeBoy/FreeBoy.cpp +++ b/plugins/FreeBoy/FreeBoy.cpp @@ -369,7 +369,7 @@ void FreeBoyInstrument::playNote(NotePlayHandle* nph, sampleFrame* workingBuffer // ratio and chooses the combination whose resulting frequency is closest to the note frequency for (char s = 0; s < 16; ++s) { - for (char r = 1 ; r < 8; ++r) + for (char r = 1; r < 8; ++r) { float f = 524288.0 / (r * std::pow(2.0, s + 1.0)); if (std::fabs(freq - closest_freq) > std::fabs(freq - f)) @@ -380,7 +380,7 @@ void FreeBoyInstrument::playNote(NotePlayHandle* nph, sampleFrame* workingBuffer } } } - + data = clock_freq; data = data << 1; data += m_ch4ShiftRegWidthModel.value(); From df2d535120829ba2c5dd37bba532715af71e6982 Mon Sep 17 00:00:00 2001 From: Hyunjin Song Date: Sun, 2 Jul 2023 12:03:09 +0900 Subject: [PATCH 4/4] Fix a merge mistake --- plugins/FreeBoy/FreeBoy.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/plugins/FreeBoy/FreeBoy.cpp b/plugins/FreeBoy/FreeBoy.cpp index b528ac2f00a..1fd3f2513cc 100644 --- a/plugins/FreeBoy/FreeBoy.cpp +++ b/plugins/FreeBoy/FreeBoy.cpp @@ -386,7 +386,7 @@ void FreeBoyInstrument::playNote(NotePlayHandle* nph, sampleFrame* workingBuffer data += m_ch4ShiftRegWidthModel.value(); data = data << 3; data += div_ratio; - papu->write_register(0xff22, data); + papu->writeRegister(0xff22, data); //channel 4 init papu->writeRegister(0xff23, 128);