From 4f5e3674e1bba91a5a04284a8581249047c9cbff Mon Sep 17 00:00:00 2001 From: Tillmann Karras Date: Sun, 14 Jan 2024 02:21:53 +0000 Subject: [PATCH 1/2] DSPHLE/Zelda: use MixingBuffer type (NFC) --- Source/Core/Core/HW/DSPHLE/UCodes/Zelda.cpp | 22 ++++++++++----------- Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h | 10 +++++----- 2 files changed, 16 insertions(+), 16 deletions(-) diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.cpp b/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.cpp index 996db94991..bd9e15ae3e 100644 --- a/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.cpp +++ b/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.cpp @@ -1233,24 +1233,24 @@ void ZeldaAudioRenderer::AddVoice(u16 voice_id) struct { - MixingBuffer* buffer; + MixingBuffer& buffer; s16 volume; s16 volume_delta; } buffers[8] = { - {&m_buf_front_left, quadrant_volumes[0], volume_deltas[0]}, - {&m_buf_back_left, quadrant_volumes[1], volume_deltas[1]}, - {&m_buf_front_right, quadrant_volumes[2], volume_deltas[2]}, - {&m_buf_back_right, quadrant_volumes[3], volume_deltas[3]}, + {m_buf_front_left, quadrant_volumes[0], volume_deltas[0]}, + {m_buf_back_left, quadrant_volumes[1], volume_deltas[1]}, + {m_buf_front_right, quadrant_volumes[2], volume_deltas[2]}, + {m_buf_back_right, quadrant_volumes[3], volume_deltas[3]}, - {&m_buf_front_left_reverb, reverb_volumes[0], reverb_volume_deltas[0]}, - {&m_buf_back_left_reverb, reverb_volumes[1], reverb_volume_deltas[1]}, - {&m_buf_front_right_reverb, reverb_volumes[2], reverb_volume_deltas[2]}, - {&m_buf_back_right_reverb, reverb_volumes[3], reverb_volume_deltas[3]}, + {m_buf_front_left_reverb, reverb_volumes[0], reverb_volume_deltas[0]}, + {m_buf_back_left_reverb, reverb_volumes[1], reverb_volume_deltas[1]}, + {m_buf_front_right_reverb, reverb_volumes[2], reverb_volume_deltas[2]}, + {m_buf_back_right_reverb, reverb_volumes[3], reverb_volume_deltas[3]}, }; for (const auto& buffer : buffers) { AddBuffersWithVolumeRamp(buffer.buffer, input_samples, buffer.volume << 16, - (buffer.volume_delta << 16) / (s32)buffer.buffer->size()); + (buffer.volume_delta << 16) / (s32)buffer.buffer.size()); } vpb.dolby_volume_current = vpb.dolby_volume_target; @@ -1303,7 +1303,7 @@ void ZeldaAudioRenderer::AddVoice(u16 voice_id) continue; } - s32 new_volume = AddBuffersWithVolumeRamp(dst_buffer, input_samples, + s32 new_volume = AddBuffersWithVolumeRamp(*dst_buffer, input_samples, vpb.channels[i].current_volume << 16, volume_step); vpb.channels[i].current_volume = new_volume >> 16; } diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h b/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h index 509c983a72..22689eae88 100644 --- a/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h +++ b/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h @@ -51,6 +51,8 @@ private: // See Zelda.cpp for the list of possible flags. u32 m_flags; + typedef std::array MixingBuffer; + // Utility functions for audio operations. // Apply volume to a buffer. The volume is a fixed point integer, usually @@ -82,16 +84,15 @@ private: // // Note: On a real GC, the stepping happens in 32 steps instead. But hey, // we can do better here with very low risk. Why not? :) - template - s32 AddBuffersWithVolumeRamp(std::array* dst, const std::array& src, s32 vol, + s32 AddBuffersWithVolumeRamp(MixingBuffer& dst, const MixingBuffer& src, s32 vol, s32 step) { if (!vol && !step) return vol; - for (size_t i = 0; i < N; ++i) + for (size_t i = 0; i < 0x50; ++i) { - (*dst)[i] += ((vol >> 16) * src[i]) >> 16; + dst[i] += ((vol >> 16) * src[i]) >> 16; vol += step; } @@ -120,7 +121,6 @@ private: u16 m_output_volume = 0; // Mixing buffers. - typedef std::array MixingBuffer; MixingBuffer m_buf_front_left{}; MixingBuffer m_buf_front_right{}; MixingBuffer m_buf_back_left{}; From 0233286729e3257d7bcac1b42b79d5d57eea79d8 Mon Sep 17 00:00:00 2001 From: Tillmann Karras Date: Sun, 14 Jan 2024 02:23:01 +0000 Subject: [PATCH 2/2] DSPHLE/Zelda: use precise 32-step volume ramping Whilst the 80-step approach theoretically improves ramping smoothness, in practice it causes accumulating rounding errors because the delta value is not always a multiple of the buffer size. Also pull the step computation into AddBuffersWithVolumeRamp() so that all ramping related math is in the same place. --- Source/Core/Core/HW/DSPHLE/UCodes/Zelda.cpp | 13 ++++----- Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h | 31 +++++++++++++-------- 2 files changed, 25 insertions(+), 19 deletions(-) diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.cpp b/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.cpp index bd9e15ae3e..011d00ddea 100644 --- a/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.cpp +++ b/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.cpp @@ -1249,8 +1249,7 @@ void ZeldaAudioRenderer::AddVoice(u16 voice_id) }; for (const auto& buffer : buffers) { - AddBuffersWithVolumeRamp(buffer.buffer, input_samples, buffer.volume << 16, - (buffer.volume_delta << 16) / (s32)buffer.buffer.size()); + AddBuffersWithVolumeRamp(buffer.buffer, input_samples, buffer.volume, buffer.volume_delta); } vpb.dolby_volume_current = vpb.dolby_volume_target; @@ -1285,13 +1284,11 @@ void ZeldaAudioRenderer::AddVoice(u16 voice_id) else volume_delta = vpb.channels[i].target_volume - vpb.channels[i].current_volume; - s32 volume_step = (volume_delta << 16) / (s32)input_samples.size(); // In 1.31 format. - // TODO: The last value of each channel structure is used to // determine whether a channel should be skipped or not. Not // implemented yet. - if (!vpb.channels[i].current_volume && !volume_step) + if (!vpb.channels[i].current_volume && !volume_delta) continue; MixingBuffer* dst_buffer = BufferForID(vpb.channels[i].id); @@ -1303,9 +1300,9 @@ void ZeldaAudioRenderer::AddVoice(u16 voice_id) continue; } - s32 new_volume = AddBuffersWithVolumeRamp(*dst_buffer, input_samples, - vpb.channels[i].current_volume << 16, volume_step); - vpb.channels[i].current_volume = new_volume >> 16; + s16 new_volume = AddBuffersWithVolumeRamp(*dst_buffer, input_samples, + vpb.channels[i].current_volume, volume_delta); + vpb.channels[i].current_volume = new_volume; } } diff --git a/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h b/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h index 22689eae88..d1ceecb8c1 100644 --- a/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h +++ b/Source/Core/Core/HW/DSPHLE/UCodes/Zelda.h @@ -79,24 +79,33 @@ private: ApplyVolumeInPlace(buf, vol); } - // Mixes two buffers together while applying a volume to one of them. The - // volume ramps up/down in N steps using the provided step delta value. - // - // Note: On a real GC, the stepping happens in 32 steps instead. But hey, - // we can do better here with very low risk. Why not? :) - s32 AddBuffersWithVolumeRamp(MixingBuffer& dst, const MixingBuffer& src, s32 vol, - s32 step) + // Mixes two buffers together while applying a volume to one of them. + // We try to match LLE, hence the slightly odd ramping. + s16 AddBuffersWithVolumeRamp(MixingBuffer& dst, const MixingBuffer& src, s16 start_volume, + s16 delta) { - if (!vol && !step) - return vol; + if (!start_volume && !delta) + return start_volume; - for (size_t i = 0; i < 0x50; ++i) + // The delta is applied in 32 steps over the first 64 samples. + s32 vol = start_volume << 16; + s32 step = delta << (16 - 5); + for (size_t i = 0; i < 0x40;) { dst[i] += ((vol >> 16) * src[i]) >> 16; + ++i; + dst[i] += ((vol >> 16) * src[i]) >> 16; + ++i; vol += step; } - return vol; + // The last 16 samples are mixed at the target volume. + for (size_t i = 0x40; i < 0x50; ++i) + { + dst[i] += ((vol >> 16) * src[i]) >> 16; + } + + return vol >> 16; } // Does not use std::array because it needs to be able to process partial