Released libsidplayfp-2.15.2
Released sidplayfp-2.15.1
Released libsidplayfp-2.15.1
resid: fix 6581 DC offset
Released libsidplayfp-2.15.0
Released sidplayfp-2.14.1
Released libsidplayfp-2.14.0
Released libsidplayfp-2.13.1
Released libsidplayfp-2.13.0
Well, some aliasing is still there but it's less than before. Amplified recordings attached for reference.
resid: fix aliasing
In fact the aliasing noise was there since before that but the change amplified it, more so for the 8580. The problem lies in the filter when the input voices are scaled, adding some rough dithering noise saves the day.
It likely is, https://sourceforge.net/p/vice-emu/code/40418/ Does it only affect the 8580?
If you can find the exact revision where things got worse it would help narrowing down the issue.
There was quantization noise due to float to integer conversion, as mentioned here: https://github.com/libsidplayfp/libsidplayfp/issues/156#issuecomment-2408899548 But the issue in resid must be somewhere else, the code is quite different. The attached test by OP should show it, if you crank the volume very loud.
Yes, please. Right in time for Xmas, thanks!
There are still four bits available, can you try this patch? I hear a faint sound now and it doesn't break other tunes. diff --git a/src/resid/filter8580new.cc b/src/resid/filter8580new.cc index 88560e78aa..bc880d7f54 100644 --- a/src/resid/filter8580new.cc +++ b/src/resid/filter8580new.cc @@ -760,7 +760,7 @@ void Filter::set_w0() { // MOS 8580 cutoff: 0 - 12.5kHz. model_filter_t& f = model_filter[1]; - n_dac = (n_param * f.f0_dac[fc]) >> 15; + n_dac = (n_param * f.f0_dac[fc]) >> 11; } } diff --git...
That's because there aren't enough bits to represent lower cutoffs correctly: tmp_n_param[1] = denorm*(1 << 13)*((fi.uCox/2.)*1.0e-6/fi.C) = 141.684364 n_param = (int)(tmp_n_param[1] * 32 + 0.5) = 4534 f0_dac[0]: 3 f0_dac[1]: 6 f0_dac[2]: 12 [...] n_dac = (n_param * f.f0_dac[0]) >> 15 = 13602 >> 15 = 0 n_dac = (n_param * f.f0_dac[1]) >> 15 = 27204 >> 15 = 0 n_dac = (n_param * f.f0_dac[2]) >> 15 = 54408 >> 15 = 1 [...] Maybe it can be fixed with some shifting magic...
Released libsidplayfp-2.12.0
Released libsidplayfp-2.12.0
Adjust 8580 Vref voltage
Released libsidplayfp-2.11.0
Released libsidplayfp-2.10.1
Released libsidplayfp-2.10.0
resid: uninitialized var in old filter
resid: scale down filtered voices on 6581
resid: add more asserts
Yes, one to define NDEBUG in resid for non-debug builds and the other for the assert that silence the warning
And here comes the build part, adding an --enable-debug option to resid. To be applied with the above v2 patch.
Released libsidplayfp-2.9.0
As suggested the proper way to silence the warning is adding an assert. This however would require some machinery to define NDEBUG for standard builds in resid (and while at it maybe rename the configure.in into a more modern configure.ac)
Bug filed: https://gcc.gnu.org/bugzilla/show_bug.cgi?id=116315
BTW, I see VICE now requires c++11, is that true for resid too? It would be even better replacing that enum values with constexpr definitions maybe.
Hmmm, this trivial patch silence the warning: --- a/src/resid/filter8580new.cc +++ b/src/resid/filter8580new.cc @@ -300,6 +300,7 @@ Filter::Filter() // double_point scaled_voltage[fi.opamp_voltage_size]; double_point scaled_voltage[50]; + const int opamp_voltage_last = fi.opamp_voltage_size - 1; for (int i = 0; i < fi.opamp_voltage_size; i++) { // The target output range is 16 bits, in order to fit in an unsigned // short. @@ -329,7 +330,7 @@ Filter::Filter() scaled_voltage[fi.opamp_voltage_size...
resid: fix deprecated-enum-float-conversion warnings
Just for the record I can reproduce this with -O3 but not with -O2 (gcc-14.2)
Released libsidplayfp-2.8.0
resid: dac leakage tuning
Released libsidplayfp-2.7.1
Seems like a false positive to me, maybe the compiler is getting confused by the fact we only use part of the scaled_voltage array. This could be silenced with a memset call. Will give it a more thorough check once I have gcc-14 installed.
resid: amplify output
I don't think changing the default based on a single recording is a good idea. And it would be off for NTSC anyway.
Released libsidplayfp-2.7.0
resid dac leakage
Released sidplayfp-2.6.2
Released sidplayfp-2.6.1
Released libsidplayfp-2.6.0
Released libsidplayfp-2.5.1
The change only affects the 6581, tested with /MUSICIANS/G/Galway_Martin/Wizball.sid and /MUSICIANS/T/Tel_Jeroen/Myth.sid
resid: allow negative values for kVg-Vx
Still any interest in reviving the project? I have my own clone on github with a bunch of fixes, but it would be good to have an official release.
Found the actual problem. It was there to fix a few tests of the noise writeback suite but it looks like the problem is elsewhere. --- a/src/resid/wave.cc +++ b/src/resid/wave.cc @@ -174,9 +174,11 @@ bool do_pre_writeback(reg8 waveform_prev, reg8 waveform, bool is6581) // no writeback without combined waveforms if (likely(waveform_prev <= 0x8)) return false; +#if 0 // This need more investigation if (waveform == 8) return false; +#endif if (waveform_prev == 0xc) { if (is6581) return false; Too bad...
Found the actual problem. It was there to fix a few tests of the noise writeback suite but it looks like the problem is elsewhere. --- a/src/resid/wave.cc +++ b/src/resid/wave.cc @@ -174,9 +174,11 @@ bool do_pre_writeback(reg8 waveform_prev, reg8 waveform, bool is6581) // no writeback without combined waveforms if (likely(waveform_prev <= 0x8)) return false; +#if 0 // This need more investigation if (waveform == 8) return false; +#endif if (waveform_prev == 0xc) { if (is6581) return false;
They're not random values, just different from what expected. From the schematics I'd say that when the test bit is set the writeback actually happens but only affects the latched value which becomes active once the test bit falls. So the code is still wrong as it was before the mentioned change. Unfortunately all my attempts to fix the shift register writeback and improve the testsuite results have failed miserably up to now.
BTW, shouldn't this be done for ST and PST too? Every combination that include sawtooth, on 6581, is affected by the problem.
Released libsidplayfp-2.5.0
If there's no objection the one I've attached (filter8580-bug1696.patch) is good to go.
If there's no objection the one I've attached is good to go.
The last patch posted above (also attached here) increases the volume and reduces clipping on problematic tunes. It looks to me an overall improvement for the sids I've tested. Anyway it seems there's quite some variation for the 8580 too with regard to filter distortion and digi samples volume.
Updated patch attached. One final thing I noticed when reading through noise_writeback_check.asm is that 9 -> 9 is marked as unstable for the 8580. So perhaps it's better to change the new reset routine to use noise + sawtooth + triangle instead of just noise + triangle? Right, on the chips I've tested it seems that $b or $f are more reliable. Changed it to N+S+T
Nice!
Uh right, missed that! BTW, any plan moving to git? ;)
Thanks again for reviewing! Here's the third round.
patch updated, thanks!
Thanks! I've just exctracted the reset routine part from the source to fit it into the existing tests.
Here's a first patch. It still uses the old method by default but switching to the new one seems to work pretty well. There are still a few tests that can be converted: * SID/noisewriteback/noise_writeback_test1.asm * SID/noisewriteback/noise_writeback_test2.asm * SID/waveforms/waveforms.asm One thing to keep in mind is that the new method messes with the frequency registers.
Use the mingw-w64-xa65 package for msys2 build
Use the new fast noise LFSR reset by Dag Lem to improve the tests
This is recorded from an 8580R5 0591
I do (more than one even) - what exactly do you want me to do? :) play /MUSICIANS/L/Linus/64_Forever.sid and see if you experience the same distortion as in the sample bozz64 posted. As always, if you think it's an improvement, sure. I know close to nothing about this code :) The tunes I've tested sounds louder and with less clipping so it LGTM. Credits to @bozz64 for the suggestion.
@gpz guess this can be closed now ;)
Yeah, it looks like analog saturation. It doesn't happen for me, but I'm using exSID so it might be the different clock frequency or the voltage, can't say for sure. Will post a recording later. @gpz do you have an 8580 equipped C64 to check out? BTW, any objection applying the above patch? It sounds like a good overall improvement to me.
Yep! The tune confirms the problem, the patch solves both bugs.
Interesting finding! It's related to [bugs:#1436] This patch fixes both the test and the tune: diff --git a/src/resid/wave.cc b/src/resid/wave.cc index 7cee96d15e..e5dda1e0be 100644 --- a/src/resid/wave.cc +++ b/src/resid/wave.cc @@ -27,7 +27,7 @@ namespace reSID // Number of cycles after which the shift register is reset // when the test bit is set. -const cycle_count SHIFT_REGISTER_RESET_START_6581 = 9768; // 0x8000 +const cycle_count SHIFT_REGISTER_RESET_START_6581 = 35000; // 0x8000 const cycle_count...
I can confirm, no clipping nor distortion on the real chip. BTW here's the revised patch, was missing a line: --- a/src/resid/filter8580new.cc +++ b/src/resid/filter8580new.cc @@ -204,9 +204,9 @@ static model_filter_init_t model_filter_init[2] = { opamp_voltage_8580, sizeof(opamp_voltage_8580)/sizeof(*opamp_voltage_8580), // FIXME: Measure for the 8580. - 0.25, - // The 4.75V voltage for the virtual ground is generated by a PolySi resistor divider - 4.80, // FIXME + 0.30, + // FIXME: Measure for...
ok, then I can conclude that you didn't test the values I provided (without the patches that were done later). The tune you mentioned doesn't clip with the values I provided and doesn't have an annoying distortion. You're right, but I see no point in trying out unrealistic values
Released libsidplayfp-2.5.0 alpha2
Vdd is the supply voltage and it can't be anything other than 9 Volts and the ringing fix has been proved both theoretically and empirically. Given that, a DC_voltage of 4.84 indeed seems to produce a better result (you also have to change Vg as in the patch below) but a voltage range greater than 0.3 still causes an annoying distortion in /MUSICIANS/L/Linus/64_Forever.sid --- a/src/resid/filter8580new.cc +++ b/src/resid/filter8580new.cc @@ -204,9 +204,9 @@ static model_filter_init_t model_filter_init[2]...
hmmm, by increasing the voice_DC_voltage and leaving Vdd untouched I can get a louder volume with little or no distortion, but then the digis become too strong :/ --- a/src/resid/filter8580new.cc +++ b/src/resid/filter8580new.cc @@ -204,9 +204,9 @@ static model_filter_init_t model_filter_init[2] = { opamp_voltage_8580, sizeof(opamp_voltage_8580)/sizeof(*opamp_voltage_8580), // FIXME: Measure for the 8580. - 0.25, + 0.35, // The 4.75V voltage for the virtual ground is generated by a PolySi resistor...
CIA TOD minor rework
Released libsidplayfp-2.5.0 alpha
Released sidplayfp-2.4.1
Released sidplayfp-2.4.1
Revert r36405
Resid filter comment fixes
Yes, the original problem is fixed, I'll post a patch for the comments in the upcoming days.
Just for the record, I've finally been able to make sense of this. We have the root function of the variable vx: f(vx) = (n + 1)*(Vddt - vx)^2 - n*(Vddt - vi)^2 - (Vddt - (vx + x))^2 where vx + x = vo We can expand the function to: f(vx) = a*(b - vx)^2 - c - (b - (vx + x))^2 = a*(b^2 + vx^2 - 2*b*vx) - c - (b^2 + (vx + x)^2 - 2*b*(vx + x)) = a*b^2 + a*vx^2 - 2*a*b*vx - c - b^2 - (vx + x)^2 + 2*b*(vx + x) = a*b^2 + a*vx^2 - 2*a*b*vx - c - b^2 - vx^2 - x^2 - 2*x*vx + 2*b*vx + 2*b*x Then we calculate...
Released libsidplayfp-2.4.2
Can the patch above (resid-ringing_fix_5.patch) be applied to trunk, so there's plenty of time to test it before 3.8?
Yep! He and ttlworks, who also did the SID reverse-engineering with me. Hopefully the 6526 and VIC-II will come up too in the near future.
Yep! Him and ttlworks, who also did the SID reverse-engineering with me. Hopefully the 6526 and VIC-II will come up too in the near future.
If anyone want to have a look some nice guys have made an 8521 dissection available at http://forum.6502.org/viewtopic.php?f=4&t=7418
Nice catch! There's indeed an off by one error, the attached patch should fix it.
Released libsidplayfp-2.4.1
Released libsidplayfp-2.4.0
Released libsidplayfp-2.4.0
Links
SID internals
Home
Ignoring the warnings looks more like an hack than a fix. I'd better null terminate the string after strncpy to prevent possible buffer overflows: --- a/lib/porg-log/log.c +++ b/lib/porg-log/log.c @@ -102,12 +102,14 @@ static void porg_get_absolute_path(int fd, const char* path, char* abs_path) /* relative to CWD */ else if (fd < 0) { strncpy(abs_path, cwd, PORG_BUFSIZE - 1); + abs_path[PORG_BUFSIZE - 1] = 0; strncat(abs_path, "/", PORG_BUFSIZE - strlen(abs_path) - 1); strncat(abs_path, path, PORG_BUFSIZE...