From 5ec9c8a397afc0db1a3968de21a6d51cfcc40be4 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Tue, 23 Jul 2024 21:50:00 +1000 Subject: [PATCH] CDROM: Fix handling of 8-bit ADPCM and decoder overruns Fixes pops in menu music in Simple 1500 Series Vol. 92 - The Tozan RPG - Ginrei no Hasha. I haven't seen anything that _properly_ uses 8-bit ADPCM yet. The above game does, but only in the inaudible portion of the start of the track. --- src/core/cdrom.cpp | 185 ++++++++++++++++++++++++++++++---- src/util/CMakeLists.txt | 2 - src/util/cd_xa.cpp | 102 ------------------- src/util/cd_xa.h | 73 -------------- src/util/util.vcxproj | 2 - src/util/util.vcxproj.filters | 2 - 6 files changed, 165 insertions(+), 201 deletions(-) delete mode 100644 src/util/cd_xa.cpp delete mode 100644 src/util/cd_xa.h diff --git a/src/core/cdrom.cpp b/src/core/cdrom.cpp index 71bf0f592..0f96c1037 100644 --- a/src/core/cdrom.cpp +++ b/src/core/cdrom.cpp @@ -12,7 +12,6 @@ #include "system.h" #include "util/cd_image.h" -#include "util/cd_xa.h" #include "util/imgui_manager.h" #include "util/iso_reader.h" #include "util/state_wrapper.h" @@ -42,6 +41,10 @@ enum : u32 DATA_SECTOR_OUTPUT_SIZE = CDImage::DATA_SECTOR_SIZE, SECTOR_SYNC_SIZE = CDImage::SECTOR_SYNC_SIZE, SECTOR_HEADER_SIZE = CDImage::SECTOR_HEADER_SIZE, + + XA_SUBHEADER_SIZE = 4, + XA_ADPCM_SAMPLES_PER_SECTOR_4BIT = 4032, // 28 words * 8 nibbles per word * 18 chunks + XA_ADPCM_SAMPLES_PER_SECTOR_8BIT = 2016, // 28 words * 4 bytes per word * 18 chunks XA_RESAMPLE_RING_BUFFER_SIZE = 32, XA_RESAMPLE_ZIGZAG_TABLE_SIZE = 29, XA_RESAMPLE_NUM_ZIGZAG_TABLES = 7, @@ -214,6 +217,61 @@ union RequestRegister BitField BFRD; }; +struct XASubHeader +{ + u8 file_number; + u8 channel_number; + + union Submode + { + u8 bits; + BitField eor; + BitField video; + BitField audio; + BitField data; + BitField trigger; + BitField form2; + BitField realtime; + BitField eof; + } submode; + + union Codinginfo + { + u8 bits; + + BitField mono_stereo; + BitField sample_rate; + BitField bits_per_sample; + BitField emphasis; + + ALWAYS_INLINE bool IsStereo() const { return mono_stereo; } + ALWAYS_INLINE bool IsHalfSampleRate() const { return sample_rate; } + ALWAYS_INLINE bool Is8BitADPCM() const { return bits_per_sample; } + u32 GetSamplesPerSector() const + { + return bits_per_sample ? XA_ADPCM_SAMPLES_PER_SECTOR_8BIT : XA_ADPCM_SAMPLES_PER_SECTOR_4BIT; + } + } codinginfo; +}; + +union XA_ADPCMBlockHeader +{ + u8 bits; + + BitField shift; + BitField filter; + + // For both 4bit and 8bit ADPCM, reserved shift values 13..15 will act same as shift=9). + u8 GetShift() const + { + const u8 shift_value = shift; + return (shift_value > 12) ? 9 : shift_value; + } + + u8 GetFilter() const { return filter; } +}; +static_assert(sizeof(XA_ADPCMBlockHeader) == 1, "XA-ADPCM block header is one byte"); + } // namespace static void SoftReset(TickCount ticks_late); @@ -290,6 +348,9 @@ static void ResetAudioDecoder(); static void ClearSectorBuffers(); static void CheckForSectorBufferReadComplete(); +// Decodes XA-ADPCM samples in an audio sector. Stereo samples are interleaved with left first. +template +static void DecodeXAADPCMChunks(const u8* chunk_ptr, s16* samples); template static void ResampleXAADPCM(const s16* frames_in, u32 num_frames_in); @@ -343,7 +404,7 @@ static u8 s_xa_current_channel_number = 0; static u8 s_xa_current_set = false; static CDImage::SectorHeader s_last_sector_header{}; -static CDXA::XASubHeader s_last_sector_subheader{}; +static XASubHeader s_last_sector_subheader{}; static bool s_last_sector_header_valid = false; // TODO: Rename to "logical pause" or something. static CDImage::SubChannelQ s_last_subq{}; static u8 s_last_cdda_report_frame_nibble = 0xFF; @@ -3047,6 +3108,12 @@ ALWAYS_INLINE_RELEASE void CDROM::ProcessDataSector(const u8* raw_sector, const SetAsyncInterrupt(Interrupt::DataReady); } +static constexpr std::array s_xa_adpcm_filter_table_pos = { + {0, 60, 115, 98, 122, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + +static constexpr std::array s_xa_adpcm_filter_table_neg = { + {0, 0, -52, -55, -60, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0, 0}}; + static std::array, 7> s_zigzag_table = { {{0, 0x0, 0x0, 0x0, 0x0, -0x0002, 0x000A, -0x0022, 0x0041, -0x0054, 0x0034, 0x0009, -0x010A, 0x0400, -0x0A78, 0x234C, 0x6794, -0x1780, 0x0BCD, -0x0623, @@ -3106,17 +3173,67 @@ s16 CDROM::SaturateVolume(s32 volume) return static_cast((volume < -0x8000) ? -0x8000 : ((volume > 0x7FFF) ? 0x7FFF : volume)); } +template +void CDROM::DecodeXAADPCMChunks(const u8* chunk_ptr, s16* samples) +{ + // The data layout is annoying here. Each word of data is interleaved with the other blocks, requiring multiple + // passes to decode the whole chunk. + constexpr u32 NUM_CHUNKS = 18; + constexpr u32 CHUNK_SIZE_IN_BYTES = 128; + constexpr u32 WORDS_PER_CHUNK = 28; + constexpr u32 SAMPLES_PER_CHUNK = WORDS_PER_CHUNK * (IS_8BIT ? 4 : 8); + constexpr u32 NUM_BLOCKS = IS_8BIT ? 4 : 8; + constexpr u32 WORDS_PER_BLOCK = 28; + + for (u32 i = 0; i < NUM_CHUNKS; i++) + { + const u8* headers_ptr = chunk_ptr + 4; + const u8* words_ptr = chunk_ptr + 16; + + for (u32 block = 0; block < NUM_BLOCKS; block++) + { + const XA_ADPCMBlockHeader block_header{headers_ptr[block]}; + const u8 shift = block_header.GetShift(); + const u8 filter = block_header.GetFilter(); + const s32 filter_pos = s_xa_adpcm_filter_table_pos[filter]; + const s32 filter_neg = s_xa_adpcm_filter_table_neg[filter]; + + s16* out_samples_ptr = + IS_STEREO ? &samples[(block / 2) * (WORDS_PER_BLOCK * 2) + (block % 2)] : &samples[block * WORDS_PER_BLOCK]; + constexpr u32 out_samples_increment = IS_STEREO ? 2 : 1; + + for (u32 word = 0; word < 28; word++) + { + // NOTE: assumes LE + u32 word_data; + std::memcpy(&word_data, &words_ptr[word * sizeof(u32)], sizeof(word_data)); + + // extract nibble from block + const u32 nibble = IS_8BIT ? ((word_data >> (block * 8)) & 0xFF) : ((word_data >> (block * 4)) & 0x0F); + const s16 sample = static_cast(Truncate16(nibble << (IS_8BIT ? 8 : 12))) >> shift; + + // mix in previous values + s32* prev = IS_STEREO ? &s_xa_last_samples[(block & 1) * 2] : &s_xa_last_samples[0]; + const s32 interp_sample = std::clamp( + static_cast(sample) + ((prev[0] * filter_pos) >> 6) + ((prev[1] * filter_neg) >> 6), -32767, 32768); + + // update previous values + prev[1] = prev[0]; + prev[0] = interp_sample; + + *out_samples_ptr = static_cast(interp_sample); + out_samples_ptr += out_samples_increment; + } + } + + samples += SAMPLES_PER_CHUNK; + chunk_ptr += CHUNK_SIZE_IN_BYTES; + } +} + template void CDROM::ResampleXAADPCM(const s16* frames_in, u32 num_frames_in) { - // Since the disc reads and SPU are running at different speeds, we might be _slightly_ behind, which is fine, since - // the SPU will over-read in the next batch to catch up. - if (s_audio_fifo.GetSize() > AUDIO_FIFO_LOW_WATERMARK) - { - DEV_LOG("Dropping {} XA frames because audio FIFO still has {} frames", num_frames_in, s_audio_fifo.GetSize()); - return; - } - s16* left_ringbuf = s_xa_resample_ring_buffer[0].data(); s16* right_ringbuf = s_xa_resample_ring_buffer[1].data(); u8 p = s_xa_resample_p; @@ -3219,30 +3336,58 @@ ALWAYS_INLINE_RELEASE void CDROM::ProcessXAADPCMSector(const u8* raw_sector, con if (s_last_sector_subheader.submode.eof) ResetCurrentXAFile(); - std::array sample_buffer; - CDXA::DecodeADPCMSector(raw_sector, sample_buffer.data(), s_xa_last_samples.data()); + // Ensure the SPU is caught up for the test below. + SPU::GeneratePendingSamples(); + + // Since the disc reads and SPU are running at different speeds, we might be _slightly_ behind, which is fine, since + // the SPU will over-read in the next batch to catch up. We also should not process the sector, because it'll affect + // the previous samples used for interpolation/ADPCM. Not doing so causes crackling audio in Simple 1500 Series Vol. + // 92 - The Tozan RPG - Ginrei no Hasha (Japan). + const u32 num_frames = s_last_sector_subheader.codinginfo.GetSamplesPerSector() >> + BoolToUInt8(s_last_sector_subheader.codinginfo.IsStereo()); + if (s_audio_fifo.GetSize() > AUDIO_FIFO_LOW_WATERMARK) + { + DEV_LOG("Dropping {} XA frames because audio FIFO still has {} frames", num_frames, s_audio_fifo.GetSize()); + return; + } + + // If muted, we still need to decode the data, to update the previous samples. + std::array sample_buffer; + const u8* xa_block_start = + raw_sector + CDImage::SECTOR_SYNC_SIZE + sizeof(CDImage::SectorHeader) + sizeof(XASubHeader) * 2; + + if (s_last_sector_subheader.codinginfo.Is8BitADPCM()) + { + if (s_last_sector_subheader.codinginfo.IsStereo()) + DecodeXAADPCMChunks(xa_block_start, sample_buffer.data()); + else + DecodeXAADPCMChunks(xa_block_start, sample_buffer.data()); + } + else + { + if (s_last_sector_subheader.codinginfo.IsStereo()) + DecodeXAADPCMChunks(xa_block_start, sample_buffer.data()); + else + DecodeXAADPCMChunks(xa_block_start, sample_buffer.data()); + } // Only send to SPU if we're not muted. if (s_muted || s_adpcm_muted || g_settings.cdrom_mute_cd_audio) return; - SPU::GeneratePendingSamples(); - if (s_last_sector_subheader.codinginfo.IsStereo()) { - const u32 num_samples = s_last_sector_subheader.codinginfo.GetSamplesPerSector() / 2; if (s_last_sector_subheader.codinginfo.IsHalfSampleRate()) - ResampleXAADPCM(sample_buffer.data(), num_samples); + ResampleXAADPCM(sample_buffer.data(), num_frames); else - ResampleXAADPCM(sample_buffer.data(), num_samples); + ResampleXAADPCM(sample_buffer.data(), num_frames); } else { - const u32 num_samples = s_last_sector_subheader.codinginfo.GetSamplesPerSector(); if (s_last_sector_subheader.codinginfo.IsHalfSampleRate()) - ResampleXAADPCM(sample_buffer.data(), num_samples); + ResampleXAADPCM(sample_buffer.data(), num_frames); else - ResampleXAADPCM(sample_buffer.data(), num_samples); + ResampleXAADPCM(sample_buffer.data(), num_frames); } } diff --git a/src/util/CMakeLists.txt b/src/util/CMakeLists.txt index c555cce3a..67c680c85 100644 --- a/src/util/CMakeLists.txt +++ b/src/util/CMakeLists.txt @@ -17,8 +17,6 @@ add_library(util cd_image_ppf.cpp cd_subchannel_replacement.cpp cd_subchannel_replacement.h - cd_xa.cpp - cd_xa.h cue_parser.cpp cue_parser.h gpu_device.cpp diff --git a/src/util/cd_xa.cpp b/src/util/cd_xa.cpp deleted file mode 100644 index 42db0a300..000000000 --- a/src/util/cd_xa.cpp +++ /dev/null @@ -1,102 +0,0 @@ -// SPDX-FileCopyrightText: 2019-2023 Connor McLaughlin -// SPDX-License-Identifier: (GPL-3.0 OR CC-BY-NC-ND-4.0) - -#include "cd_xa.h" -#include "cd_image.h" - -#include -#include - -namespace CDXA { -static constexpr std::array s_xa_adpcm_filter_table_pos = {{0, 60, 115, 98}}; -static constexpr std::array s_xa_adpcm_filter_table_neg = {{0, 0, -52, -55}}; - -template -ALWAYS_INLINE_RELEASE static void DecodeXA_ADPCMChunk(const u8* chunk_ptr, s16* samples, s32* last_samples) -{ - // The data layout is annoying here. Each word of data is interleaved with the other blocks, requiring multiple - // passes to decode the whole chunk. - constexpr u32 NUM_BLOCKS = IS_8BIT ? 4 : 8; - constexpr u32 WORDS_PER_BLOCK = 28; - - const u8* headers_ptr = chunk_ptr + 4; - const u8* words_ptr = chunk_ptr + 16; - - for (u32 block = 0; block < NUM_BLOCKS; block++) - { - const XA_ADPCMBlockHeader block_header{headers_ptr[block]}; - const u8 shift = block_header.GetShift(); - const u8 filter = block_header.GetFilter(); - const s32 filter_pos = s_xa_adpcm_filter_table_pos[filter]; - const s32 filter_neg = s_xa_adpcm_filter_table_neg[filter]; - - s16* out_samples_ptr = - IS_STEREO ? &samples[(block / 2) * (WORDS_PER_BLOCK * 2) + (block % 2)] : &samples[block * WORDS_PER_BLOCK]; - constexpr u32 out_samples_increment = IS_STEREO ? 2 : 1; - - for (u32 word = 0; word < 28; word++) - { - // NOTE: assumes LE - u32 word_data; - std::memcpy(&word_data, &words_ptr[word * sizeof(u32)], sizeof(word_data)); - - // extract nibble from block - const u32 nibble = IS_8BIT ? ((word_data >> (block * 8)) & 0xFF) : ((word_data >> (block * 4)) & 0x0F); - const s16 sample = static_cast(Truncate16(nibble << 12)) >> shift; - - // mix in previous values - s32* prev = IS_STEREO ? &last_samples[(block & 1) * 2] : last_samples; - const s32 interp_sample = s32(sample) + ((prev[0] * filter_pos) + (prev[1] * filter_neg) + 32) / 64; - - // update previous values - prev[1] = prev[0]; - prev[0] = interp_sample; - - *out_samples_ptr = static_cast(std::clamp(interp_sample, -0x8000, 0x7FFF)); - out_samples_ptr += out_samples_increment; - } - } -} - -template -ALWAYS_INLINE_RELEASE static void DecodeXA_ADPCMChunks(const u8* chunk_ptr, s16* samples, s32* last_samples) -{ - constexpr u32 NUM_CHUNKS = 18; - constexpr u32 CHUNK_SIZE_IN_BYTES = 128; - constexpr u32 WORDS_PER_CHUNK = 28; - constexpr u32 SAMPLES_PER_CHUNK = WORDS_PER_CHUNK * (IS_8BIT ? 4 : 8); - - for (u32 i = 0; i < NUM_CHUNKS; i++) - { - DecodeXA_ADPCMChunk(chunk_ptr, samples, last_samples); - samples += SAMPLES_PER_CHUNK; - chunk_ptr += CHUNK_SIZE_IN_BYTES; - } -} - -} // namespace CDXA - -void CDXA::DecodeADPCMSector(const void* data, s16* samples, s32* last_samples) -{ - const XASubHeader* subheader = reinterpret_cast( - reinterpret_cast(data) + CDImage::SECTOR_SYNC_SIZE + sizeof(CDImage::SectorHeader)); - - // The XA subheader is repeated? - const u8* chunk_ptr = reinterpret_cast(data) + CDImage::SECTOR_SYNC_SIZE + sizeof(CDImage::SectorHeader) + - sizeof(XASubHeader) + 4; - - if (subheader->codinginfo.bits_per_sample != 1) - { - if (subheader->codinginfo.mono_stereo != 1) - DecodeXA_ADPCMChunks(chunk_ptr, samples, last_samples); - else - DecodeXA_ADPCMChunks(chunk_ptr, samples, last_samples); - } - else - { - if (subheader->codinginfo.mono_stereo != 1) - DecodeXA_ADPCMChunks(chunk_ptr, samples, last_samples); - else - DecodeXA_ADPCMChunks(chunk_ptr, samples, last_samples); - } -} diff --git a/src/util/cd_xa.h b/src/util/cd_xa.h deleted file mode 100644 index 3c140fd93..000000000 --- a/src/util/cd_xa.h +++ /dev/null @@ -1,73 +0,0 @@ -// SPDX-FileCopyrightText: 2019-2022 Connor McLaughlin -// SPDX-License-Identifier: (GPL-3.0 OR CC-BY-NC-ND-4.0) - -#pragma once -#include "common/bitfield.h" -#include "common/types.h" - -namespace CDXA { -enum -{ - XA_SUBHEADER_SIZE = 4, - XA_ADPCM_SAMPLES_PER_SECTOR_4BIT = 4032, // 28 words * 8 nibbles per word * 18 chunks - XA_ADPCM_SAMPLES_PER_SECTOR_8BIT = 2016 // 28 words * 4 bytes per word * 18 chunks -}; - -struct XASubHeader -{ - u8 file_number; - u8 channel_number; - union Submode - { - u8 bits; - BitField eor; - BitField video; - BitField audio; - BitField data; - BitField trigger; - BitField form2; - BitField realtime; - BitField eof; - } submode; - union Codinginfo - { - u8 bits; - - BitField mono_stereo; - BitField sample_rate; - BitField bits_per_sample; - BitField emphasis; - - bool IsStereo() const { return mono_stereo == 1; } - bool IsHalfSampleRate() const { return sample_rate == 1; } - u32 GetSampleRate() const { return sample_rate == 1 ? 18900 : 37800; } - u32 GetBitsPerSample() const { return bits_per_sample == 1 ? 8 : 4; } - u32 GetSamplesPerSector() const - { - return bits_per_sample == 1 ? XA_ADPCM_SAMPLES_PER_SECTOR_8BIT : XA_ADPCM_SAMPLES_PER_SECTOR_4BIT; - } - } codinginfo; -}; - -union XA_ADPCMBlockHeader -{ - u8 bits; - - BitField shift; - BitField filter; - - // For both 4bit and 8bit ADPCM, reserved shift values 13..15 will act same as shift=9). - u8 GetShift() const - { - const u8 shift_value = shift; - return (shift_value > 12) ? 9 : shift_value; - } - - u8 GetFilter() const { return filter; } -}; -static_assert(sizeof(XA_ADPCMBlockHeader) == 1, "XA-ADPCM block header is one byte"); - -// Decodes XA-ADPCM samples in an audio sector. Stereo samples are interleaved with left first. -void DecodeADPCMSector(const void* data, s16* samples, s32* last_samples); - -} // namespace CDXA \ No newline at end of file diff --git a/src/util/util.vcxproj b/src/util/util.vcxproj index d9f98898a..8e1c67bd5 100644 --- a/src/util/util.vcxproj +++ b/src/util/util.vcxproj @@ -88,7 +88,6 @@ - @@ -189,7 +188,6 @@ - diff --git a/src/util/util.vcxproj.filters b/src/util/util.vcxproj.filters index 7fa83be8d..dae39c57a 100644 --- a/src/util/util.vcxproj.filters +++ b/src/util/util.vcxproj.filters @@ -3,7 +3,6 @@ - @@ -77,7 +76,6 @@ -