From 4fc4eb8a66c8fc4bfca7ea3c3e2c751b94fe7090 Mon Sep 17 00:00:00 2001 From: Stenzek Date: Sat, 27 Jan 2024 15:20:14 +1000 Subject: [PATCH] USB: Rewrite RingBuffer class It was overflowing and corrupting the heap... --- pcsx2/USB/shared/ringbuffer.cpp | 195 ++++++++------------------- pcsx2/USB/shared/ringbuffer.h | 84 ++---------- pcsx2/USB/usb-mic/audiodev-cubeb.cpp | 34 ++--- 3 files changed, 82 insertions(+), 231 deletions(-) diff --git a/pcsx2/USB/shared/ringbuffer.cpp b/pcsx2/USB/shared/ringbuffer.cpp index c53a2ff396..32fb97648f 100644 --- a/pcsx2/USB/shared/ringbuffer.cpp +++ b/pcsx2/USB/shared/ringbuffer.cpp @@ -1,175 +1,88 @@ -// SPDX-FileCopyrightText: 2002-2023 PCSX2 Dev Team +// SPDX-FileCopyrightText: 2002-2024 PCSX2 Dev Team // SPDX-License-Identifier: LGPL-3.0+ #include "ringbuffer.h" #include #include +#include - -RingBuffer::RingBuffer() - : m_begin(0) - , m_end(0) - , m_capacity(0) - , m_data(nullptr) - , m_overrun(false) -{ -} +RingBuffer::RingBuffer() = default; RingBuffer::RingBuffer(size_t capacity) : RingBuffer() { - reserve(capacity); + reset(capacity); } -RingBuffer::~RingBuffer() -{ - delete[] m_data; -} +RingBuffer::~RingBuffer() = default; -void RingBuffer::reserve(size_t capacity) +void RingBuffer::reset(size_t capacity) { - delete[] m_data; - m_data = new char[capacity]; - memset(m_data, 0, capacity); - m_capacity = capacity; + m_rpos = 0; + m_wpos = 0; + m_full = false; + m_data.reset(); + if ((m_capacity = capacity) > 0) + m_data = std::make_unique(capacity); } size_t RingBuffer::size() const { - size_t size = 0; - if (m_begin == m_end) - { - if (m_overrun) - size = m_capacity; - else - size = 0; - } - else if (m_begin < m_end) - size = m_end - m_begin; // [ b...e ] + if (m_wpos == m_rpos) + return m_full ? m_capacity : 0; + else if (m_wpos > m_rpos) + return m_wpos - m_rpos; else - size = m_capacity - m_begin + m_end; // [...e b...] - - return size; + return (m_capacity - m_rpos) + m_wpos; } -size_t RingBuffer::read(uint8_t* dst, size_t nbytes) +size_t RingBuffer::read(void* dst, size_t nbytes) { + uint8_t* bdst = static_cast(dst); + size_t to_read = nbytes; - while (to_read > 0 && size() > 0) + while (to_read > 0) { - size_t bytes = std::min(to_read, peek_read()); - memcpy(dst, front(), bytes); - read(bytes); - dst += bytes; - to_read -= bytes; + size_t available; + if (m_wpos == m_rpos) + available = m_full ? (m_capacity - m_rpos) : 0; + else if (m_wpos > m_rpos) + available = m_wpos - m_rpos; + else + available = m_capacity - m_rpos; + + if (available == 0) + break; + + const size_t copy = std::min(available, to_read); + std::memcpy(bdst, m_data.get() + m_rpos, copy); + bdst += copy; + to_read -= copy; + + m_rpos = (m_rpos + copy) % m_capacity; + m_full = false; } + return nbytes - to_read; } -void RingBuffer::write(uint8_t* src, size_t nbytes) +void RingBuffer::write(const void* src, size_t nbytes) { + const uint8_t* bsrc = static_cast(src); while (nbytes > 0) { - size_t bytes = std::min(nbytes, m_capacity - m_end); - memcpy(back(), src, bytes); - write(bytes); - src += bytes; - nbytes -= bytes; - } -} - -size_t RingBuffer::peek_write(bool overwrite) const -{ - size_t peek = 0; - - if (overwrite) - return m_capacity - m_end; - - if (m_end < m_begin) // [...e b...] - peek = m_begin - m_end; - else if (m_end < m_capacity) // [ b...e ] - peek = m_capacity - m_end; - else - peek = m_begin; // [ b.......e] - - return peek; -} - -size_t RingBuffer::peek_read() const -{ - size_t peek = 0; - if (m_begin == m_end) - { - if (m_overrun) - peek = m_capacity - m_begin; + size_t free; + if (m_wpos >= m_rpos) + free = m_capacity - m_wpos; else - peek = 0; - } - else if (m_begin < m_end) // [ b...e ] - peek = m_end - m_begin; - else if (m_begin < m_capacity) // [...e b...] - peek = m_capacity - m_begin; - else - peek = m_end; // [...e b] + free = m_rpos - m_wpos; - return peek; -} - -/*size_t RingBuffer::write(const char *data, size_t bytes) -{ - size_t bytes_to_write; - - if (m_end < m_begin) - { - bytes_to_write = std::min(m_begin - m_end, bytes); - memcpy(m_data + m_end, data, bytes_to_write); - m_end += bytes_to_write; - return bytes_to_write; - } - else - { - size_t in_bytes = bytes; - while (in_bytes > 0) - { - bytes_to_write = std::min(m_capacity - m_end, in_bytes); - if (m_end < m_begin && m_end + bytes_to_write > m_begin) - m_begin = (m_end + bytes_to_write + 1) % m_capacity; - - memcpy(m_data + m_end, data, bytes_to_write); - in_bytes -= bytes_to_write; - m_end = (m_end + bytes_to_write) % m_capacity; - } - return bytes; - } -}*/ - -void RingBuffer::write(size_t bytes) -{ - //assert( bytes <= m_capacity - size() ); - - // push m_begin forward if m_end overlaps it - if ((m_end < m_begin && m_end + bytes > m_begin) || - m_end + bytes >= m_begin + m_capacity) - { - m_overrun = true; - m_begin = (m_end + bytes) % m_capacity; - m_end = m_begin; - } - else - m_end = (m_end + bytes) % m_capacity; -} - -void RingBuffer::read(size_t bytes) -{ - assert(bytes <= size()); - - m_overrun = false; - if ((m_begin < m_end && m_begin + bytes > m_end) || - m_begin + bytes > m_end + m_capacity) - { - m_begin = m_end = 0; - return; - } - - m_begin = (m_begin + bytes) % m_capacity; + const size_t copy = std::min(free, nbytes); + std::memcpy(m_data.get() + m_wpos, bsrc, copy); + bsrc += copy; + nbytes -= copy; + + m_wpos = (m_wpos + copy) % m_capacity; + m_full = m_full || (m_wpos == m_rpos); + } } diff --git a/pcsx2/USB/shared/ringbuffer.h b/pcsx2/USB/shared/ringbuffer.h index bd6da23bb7..bb126f0011 100644 --- a/pcsx2/USB/shared/ringbuffer.h +++ b/pcsx2/USB/shared/ringbuffer.h @@ -1,89 +1,33 @@ -// SPDX-FileCopyrightText: 2002-2023 PCSX2 Dev Team +// SPDX-FileCopyrightText: 2002-2024 PCSX2 Dev Team // SPDX-License-Identifier: LGPL-3.0+ #pragma once -#include // for std::min #include +#include class RingBuffer { - RingBuffer(RingBuffer&) = delete; + RingBuffer(const RingBuffer&) = delete; + RingBuffer& operator=(const RingBuffer&) = delete; public: RingBuffer(); RingBuffer(size_t capacity); ~RingBuffer(); - //size_t write(const char *data, size_t bytes); - //size_t read(char *data, size_t bytes); + void reset(size_t size); + size_t capacity() const; + size_t size() const; // Overwrites old data if nbytes > size() - void write(uint8_t* src, size_t nbytes); - size_t read(uint8_t* dst, size_t nbytes); - - // just move pointers around - void write(size_t bytes); - void read(size_t bytes); - - template - void write(size_t samples) - { - write(samples * sizeof(T)); - } - - template - void read(size_t samples) - { - read(samples * sizeof(T)); - } - - void reserve(size_t size); - // if you care about old data, check how much can be written - // may need to call available/write twice in case write pointer wraps - size_t peek_write(bool overwrite = false) const; - size_t peek_read() const; - - template - size_t peek_write(bool overwrite = false) const - { - return peek_write(overwrite) / sizeof(T); - } - - template - size_t peek_read() const - { - return peek_read() / sizeof(T); - } - - // amount of valid data, may need to read twice - size_t size() const; - template - size_t size() const - { - return size() / sizeof(T); - } - size_t capacity() const { return m_capacity; } - char* front() { return m_data + m_begin; } - char* back() { return m_data + m_end; } - - template - T* front() - { - return (T*)(m_data + m_begin); - } - - template - T* back() - { - return (T*)(m_data + m_end); - } + void write(const void* src, size_t nbytes); + size_t read(void* dst, size_t nbytes); private: - size_t m_begin; - size_t m_end; - size_t m_capacity; - char* m_data; - bool m_overrun; + std::unique_ptr m_data; + size_t m_capacity = 0; + size_t m_rpos = 0; + size_t m_wpos = 0; + bool m_full = false; }; - diff --git a/pcsx2/USB/usb-mic/audiodev-cubeb.cpp b/pcsx2/USB/usb-mic/audiodev-cubeb.cpp index 2d9effdf71..5a5a42ad49 100644 --- a/pcsx2/USB/usb-mic/audiodev-cubeb.cpp +++ b/pcsx2/USB/usb-mic/audiodev-cubeb.cpp @@ -182,24 +182,12 @@ namespace usb_mic uint32_t CubebAudioDevice::GetBuffer(short* buff, uint32_t frames) { if (!mStream) - return frames; + return 0; std::lock_guard lk(mMutex); - u32 samples_to_read = frames * GetChannels(); - short* pDst = (short*)buff; - pxAssert(samples_to_read <= mBuffer.size()); - - while (samples_to_read > 0) - { - u32 samples = std::min(samples_to_read, static_cast(mBuffer.peek_read())); - if (!samples) - break; - memcpy(pDst, mBuffer.front(), samples * sizeof(short)); - mBuffer.read(samples); - pDst += samples; - samples_to_read -= samples; - } - return (frames - (samples_to_read / GetChannels())); + const size_t read_size = frames * sizeof(buff[0]) * GetChannels(); + const size_t bytes_read = mBuffer.read(buff, read_size); + return (bytes_read / sizeof(buff[0]) / GetChannels()); } uint32_t CubebAudioDevice::SetBuffer(short* buff, uint32_t frames) @@ -220,7 +208,7 @@ namespace usb_mic return true; std::lock_guard lk(mMutex); - *size = mBuffer.size() / GetChannels(); + *size = mBuffer.size() / sizeof(short) / GetChannels(); return true; } @@ -242,7 +230,7 @@ namespace usb_mic { std::lock_guard lk(mMutex); const u32 samples = std::max(((mSampleRate * mChannels) * mLatency) / 1000u, mStreamLatency * mChannels); - mBuffer.reserve(sizeof(u16) * samples); + mBuffer.reset(sizeof(u16) * samples); } long CubebAudioDevice::DataCallback( @@ -253,9 +241,15 @@ namespace usb_mic std::lock_guard lk(ad->mMutex); if (ad->mAudioDir == AUDIODIR_SOURCE) - ad->mBuffer.write((u8*)input_buffer, bytes); + { + ad->mBuffer.write(input_buffer, bytes); + } else - ad->mBuffer.read((u8*)output_buffer, bytes); + { + const size_t written = ad->mBuffer.read(output_buffer, bytes); + if (written < bytes) + std::memset(static_cast(output_buffer) + written, 0, bytes - written); + } return nframes; }