From ef997d0d822f4c75721313e0e5952838896817c6 Mon Sep 17 00:00:00 2001 From: JosJuice Date: Sat, 4 Apr 2026 12:20:41 +0200 Subject: [PATCH] Improve Game Boy Player audio conversion to PWM In the PWM audio we output, instead of each group of 512 bits consisting of a run of ones followed by a run of zeroes, now each group of 32 bits consists of a run of ones followed by a run of zeroes. This gets rid of noise that was previously present. Doing this for every group of 8 bits instead makes the Game Boy Player Start-Up Disc not start correctly for some reason. Game Boy Interface works fine, though. I also made us not discard the bottom 7 bits of each PCM sample. According to Extrems, a real GBA doesn't actually output that many bits, but doing it in this way makes the code simpler anyway. --- .../Core/Core/HW/HSP/HSP_DeviceGBPlayer.cpp | 47 +++++++++++++------ Source/Core/Core/State.cpp | 2 +- 2 files changed, 34 insertions(+), 15 deletions(-) diff --git a/Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.cpp b/Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.cpp index 68af359028..08c1c54739 100644 --- a/Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.cpp +++ b/Source/Core/Core/HW/HSP/HSP_DeviceGBPlayer.cpp @@ -133,6 +133,9 @@ private: u16 m_current_scanline_index = 0; u16 m_keys = 0; + u16 m_audio_l_remainder = 0; + u16 m_audio_r_remainder = 0; + // Used to trigger a video IRQ during the next audio IRQ in UpdateAudio. // Separately timed video IRQs cause the AV stream to enter a bad state. // It's very fragile for some reason. This might be a CPU timing issue ? @@ -194,6 +197,10 @@ void CGBPlayer_mGBA::Stop() m_current_scanline_index = 0; m_keys = 0; + + m_audio_l_remainder = 0; + m_audio_r_remainder = 0; + m_generate_video_irq = false; m_gba_core.Stop(); @@ -246,12 +253,16 @@ void CGBPlayer_mGBA::ReadScanlines(std::span scanlines) m_generate_video_irq = true; } -// TODO: Explain the meaning of this audio sample processing. -static constexpr u8 SampleToBits(u16* value) +// Takes 5 bits from a 16-bit PCM sample and converts them into 32 bits of PWM. +// +// The 11 bits that don't get converted are fed into the remainder, which should be used as an +// input to the next invocation of this function to average out quantization errors over time. +static constexpr u32 SampleToPWM(u16 value, u16* remainder) { - const auto x = std::min(*value, 8); - *value -= x; - return u8(0xff00u >> x); + const u16 x = value + *remainder; + const u16 y = x >> 11; + *remainder = x - (y << 11); + return u32(0xffff'ffff'0000'0000ull >> y); } void CGBPlayer_mGBA::ReadAudio(std::span audio) @@ -259,23 +270,27 @@ void CGBPlayer_mGBA::ReadAudio(std::span audio) std::array, AUDIO_READ_SIZE> buffer; const auto read_count = mAudioBufferRead(&m_audio_buffer, buffer.data()->data(), buffer.size()); - // Each s16 sample is converted to a u9 which is then converted to 64 u8 values. - // This must have something to do with interpretation by the DSP ? - // TODO: Explain things better. - constexpr u32 out_bytes_per_sample = AV_REGION_SIZE / AUDIO_READ_SIZE / AUDIO_CHANNEL_COUNT; static_assert(out_bytes_per_sample == 64); auto out_it = audio.begin(); for (auto [l, r] : std::span{buffer}.first(read_count)) { - auto l_9bit = u16(u32(l + 0x8000) >> 7u); - auto r_9bit = u16(u32(r + 0x8000) >> 7u); + const u16 l_unsigned = l + 0x8000; + const u16 r_unsigned = r + 0x8000; - for (u32 i = 0; i != out_bytes_per_sample; ++i) + for (u32 i = 0; i != out_bytes_per_sample / 4; ++i) { - *(out_it++) = SampleToBits(&l_9bit); - *(out_it++) = SampleToBits(&r_9bit); + u32 l_pwm = SampleToPWM(l_unsigned, &m_audio_l_remainder); + u32 r_pwm = SampleToPWM(r_unsigned, &m_audio_r_remainder); + + for (u32 j = 0; j != 4; ++j) + { + *(out_it++) = l_pwm >> 24; + *(out_it++) = r_pwm >> 24; + l_pwm <<= 8; + r_pwm <<= 8; + } } } } @@ -294,6 +309,10 @@ void CGBPlayer_mGBA::DoState(PointerWrap& p) p.Do(m_current_scanline_index); p.Do(m_keys); + + p.Do(m_audio_l_remainder); + p.Do(m_audio_r_remainder); + p.Do(m_generate_video_irq); // Resampled audio buffer. diff --git a/Source/Core/Core/State.cpp b/Source/Core/Core/State.cpp index a234b4f919..71b9350d40 100644 --- a/Source/Core/Core/State.cpp +++ b/Source/Core/Core/State.cpp @@ -95,7 +95,7 @@ struct CompressAndDumpStateArgs static Common::WorkQueueThreadSP s_compress_and_dump_thread; // Don't forget to increase this after doing changes on the savestate system -constexpr u32 STATE_VERSION = 186; // Last changed in PR 14535 +constexpr u32 STATE_VERSION = 187; // Last changed in PR 14552 // Increase this if the StateExtendedHeader definition changes constexpr u32 EXTENDED_HEADER_VERSION = 1; // Last changed in PR 12217