vsu: output_left/right assignment in sample_clock() looks suspect

[jwestfall69]

'''
fn sample_clock(&mut self, audio_driver: &mut AudioDriver) {
...
let mut acc_left = 0;
let mut acc_right = 0;

        mix_sample(&mut acc_left, &mut acc_right, &self.voice1, self.voice1.output(&self.wave_tables)); 
        mix_sample(&mut acc_left, &mut acc_right, &self.voice2, self.voice2.output(&self.wave_tables));

...
let output_left = ((((acc_left & 0xfff8) << 3) as isize) - 32768) as i16;
let output_right = ((((acc_right & 0xfff8) << 3) as isize) - 32768) as i16;
'''

The assignment of output_left/right looks fishy.

The values in acc_left/right get truncated down to being 16bits wide, then you make it 19bits wide with the shift, subtract 32k (I assume to try and make it signed?), then truncate it down to a i16?

I seem to recall the << 3 being added as a hack to increase the volume. Seems like it would make more sense to do the shift before the 0xfff8 truncation?

[yupferris]

Thanks for double-checking this as well btw! I'm not sure I'd call this a hack though :) . The logic here was that the value coming out of the summed voices would always fit into 13 bits, unsigned (all of the DSP is done unsigned). The bitmasks mask out the lower 3 bits of this sum as per the documentation. This should mean it's safe to shift the unsigned value to the left 3 bits (making it an unsigned 16-bit value). But before we output to a signed 16-bit value we need to bias the result (otherwise it would overflow, as the unsigned value could be out of signed range at this point).

Unless I misread something, all of this should be ok, however I'm in the middle of switching out rodio with cpal (which works great atm; just working on the resampling now) and I adjusted some of this in 809fbdc because the DC offset made audible clicks/pops if the buffer wasn't full (swapping the hand-rolled ring buffer with a VecDeque), which unfortunately is a common case right now with the emulator still being a unoptimized (it's really slow on my mac, for example).

[jwestfall69]

sounds good thanks.

[yupferris]

No worries! Gonna close this one for now.