Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Tidy and modernize SampleBuffer #11987

Merged
merged 1 commit into from
Sep 19, 2023
Merged

Tidy and modernize SampleBuffer #11987

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 0 additions & 1 deletion CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1103,7 +1103,6 @@ add_library(mixxx-lib STATIC EXCLUDE_FROM_ALL
src/util/rotary.cpp
src/util/runtimeloggingcategory.cpp
src/util/sample.cpp
src/util/samplebuffer.cpp
src/util/sandbox.cpp
src/util/semanticversion.cpp
src/util/screensaver.cpp
Expand Down
6 changes: 3 additions & 3 deletions src/util/assert.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ inline void mixxx_release_assert(const char* assertion, const char* file, int li
/// corrupting user data. Handle errors gracefully whenever possible.
#define RELEASE_ASSERT(cond) \
do \
if (Q_UNLIKELY(!static_cast<bool>(cond))) { \
if (!static_cast<bool>(cond)) [[unlikely]] { \
mixxx_release_assert(#cond, __FILE__, __LINE__, ASSERT_FUNCTION); \
} \
while (0)
Expand All @@ -56,7 +56,7 @@ inline void mixxx_release_assert(const char* assertion, const char* file, int li
#ifdef MIXXX_DEBUG_ASSERTIONS_ENABLED
#define DEBUG_ASSERT(cond) \
do \
if (Q_UNLIKELY(!static_cast<bool>(cond))) { \
if (!static_cast<bool>(cond)) [[unlikely]] { \
mixxx_debug_assert(#cond, __FILE__, __LINE__, ASSERT_FUNCTION); \
} \
while (0)
Expand All @@ -80,5 +80,5 @@ inline void mixxx_release_assert(const char* assertion, const char* file, int li
if (Q_UNLIKELY(!static_cast<bool>(cond)) && \
mixxx_debug_assert_return_true(#cond, __FILE__, __LINE__, ASSERT_FUNCTION))
#else
#define VERIFY_OR_DEBUG_ASSERT(cond) if (Q_UNLIKELY(!static_cast<bool>(cond)))
#define VERIFY_OR_DEBUG_ASSERT(cond) if (!static_cast<bool>(cond)) [[unlikely]]
#endif
5 changes: 2 additions & 3 deletions src/util/sample.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ class SampleUtil {

// Allocated a buffer of CSAMPLE's with length size. Ensures that the buffer
// is 16-byte aligned for SSE enhancement.
static CSAMPLE* alloc(SINT size);
[[nodiscard]] static CSAMPLE* alloc(SINT size);
abique marked this conversation as resolved.
Show resolved Hide resolved

// Frees a 16-byte aligned buffer allocated by SampleUtil::alloc()
static void free(CSAMPLE* pBuffer);
Expand All @@ -45,7 +45,7 @@ class SampleUtil {
inline
static void fill(CSAMPLE* pBuffer, CSAMPLE value,
SINT numSamples) {
std::fill(pBuffer, pBuffer + numSamples, value);
std::fill_n(pBuffer, numSamples, value);
}

// Copies every sample from pSrc to pDest
Expand Down Expand Up @@ -126,7 +126,6 @@ class SampleUtil {

inline static SINT floorPlayPosToFrame(double playPos) {
return static_cast<SINT>(floor(playPos / kPlayPositionChannels));

}

inline static SINT ceilPlayPosToFrame(double playPos) {
Expand Down
26 changes: 0 additions & 26 deletions src/util/samplebuffer.cpp
View file
Open in desktop

This file was deleted.

110 changes: 70 additions & 40 deletions src/util/samplebuffer.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,11 +2,12 @@


#include <algorithm> // std::swap
#include <utility> // std::exchange

#include "util/sample.h"
#include "util/span.h"
#include "util/types.h"


namespace mixxx {

// A sample buffer with properly aligned memory to enable SSE optimizations.
Expand Down Expand Up @@ -34,79 +35,94 @@ namespace mixxx {
//
class SampleBuffer final {
public:
SampleBuffer()
: m_data(nullptr),
m_size(0) {
constexpr SampleBuffer()
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
constexpr SampleBuffer()
constexpr SampleBuffer() noexcept

: m_data(nullptr),
m_size(0) {
}
explicit SampleBuffer(SINT size)
: m_data((size > 0) ? SampleUtil::alloc(size) : nullptr),
m_size((m_data != nullptr) ? size : 0) {
}
explicit SampleBuffer(SINT size);
SampleBuffer(SampleBuffer&) = delete;
SampleBuffer(SampleBuffer&& that)
: m_data(that.m_data),
m_size(that.m_size) {
that.m_data = nullptr;
that.m_size = 0;
SampleBuffer(SampleBuffer&& that) noexcept
: m_data(std::exchange(that.m_data, nullptr)),
m_size(std::exchange(that.m_size, 0)) {
}
~SampleBuffer() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

TIL destructors are noexcept by default since C++11, so even if we don't add it here, the destructor would std::terminate be default if SampleUtil::free would panic.

SampleUtil::free(m_data);
}
virtual ~SampleBuffer() final;

SampleBuffer& operator=(SampleBuffer& that) = delete;
SampleBuffer& operator=(SampleBuffer&& that) {
swap(that);
SampleBuffer& operator=(SampleBuffer&& that) noexcept {
SampleUtil::free(m_data);
m_data = std::exchange(that.m_data, nullptr);
m_size = std::exchange(that.m_size, 0);
return *this;
}

constexpr SINT size() const {
constexpr SINT size() const noexcept {
abique marked this conversation as resolved.
Show resolved Hide resolved
return m_size;
}

CSAMPLE* data(SINT offset = 0) {
constexpr CSAMPLE* data() noexcept {
return m_data;
}
constexpr const CSAMPLE* data() const noexcept {
return m_data;
}
abique marked this conversation as resolved.
Show resolved Hide resolved
CSAMPLE* data(SINT offset) noexcept {
DEBUG_ASSERT((m_data != nullptr) || (offset == 0));
DEBUG_ASSERT(0 <= offset);
// >=: allow access to one element behind allocated memory
DEBUG_ASSERT(m_size >= offset);
return m_data + offset;
}
const CSAMPLE* data(SINT offset = 0) const {
const CSAMPLE* data(SINT offset) const noexcept {
DEBUG_ASSERT((m_data != nullptr) || (offset == 0));
DEBUG_ASSERT(0 <= offset);
// >=: allow access to one element behind allocated memory
DEBUG_ASSERT(m_size >= offset);
return m_data + offset;
}

std::span<CSAMPLE> span() {
std::span<CSAMPLE> span() noexcept {
return mixxx::spanutil::spanFromPtrLen(m_data, m_size);
}
std::span<const CSAMPLE> span() const {
std::span<const CSAMPLE> span() const noexcept {
return mixxx::spanutil::spanFromPtrLen(m_data, m_size);
}

CSAMPLE& operator[](SINT index) {
CSAMPLE& operator[](SINT index) noexcept {
return *data(index);
}
const CSAMPLE& operator[](SINT index) const {
const CSAMPLE& operator[](SINT index) const noexcept {
return *data(index);
}

// Exchanges the members of two buffers in conformance with the
// implementation of all STL containers. Required for exception
// safe programming and as a workaround for the missing resize
// operation.
void swap(SampleBuffer& that) {
void swap(SampleBuffer& that) noexcept {
std::swap(m_data, that.m_data);
std::swap(m_size, that.m_size);
}

// Fills the whole buffer with zeroes
void clear();
void clear() noexcept {
SampleUtil::clear(data(), size());
}

// Fills the whole buffer with the same value
void fill(CSAMPLE value);
void fill(CSAMPLE value) noexcept {
SampleUtil::fill(data(), value, size());
}

class ReadableSlice {
class ReadableSlice final {
public:
ReadableSlice()
: m_data(nullptr),
m_length(0) {
constexpr ReadableSlice()
: m_data(nullptr),
m_length(0) {
}
ReadableSlice(const CSAMPLE* data, SINT length)
: m_data(data),
Expand All @@ -119,35 +135,42 @@ class SampleBuffer final {
m_length(length) {
DEBUG_ASSERT((buffer.size() - offset) >= length);
}
const CSAMPLE* data(SINT offset = 0) const {
const CSAMPLE* data() const noexcept {
return m_data;
}
const CSAMPLE* data(SINT offset) const {
DEBUG_ASSERT((m_data != nullptr) || (offset == 0));
DEBUG_ASSERT(0 <= offset);
// >=: allow access to one element behind allocated memory
DEBUG_ASSERT(m_length >= offset);
return m_data + offset;
}
SINT length(SINT offset = 0) const {
SINT length() const noexcept {
return m_length;
}
SINT length(SINT offset) const noexcept {
DEBUG_ASSERT(0 <= offset);
// >=: allow access to one element behind allocated memory
DEBUG_ASSERT(m_length >= offset);
return m_length - offset;
}
bool empty() const {
bool empty() const noexcept {
return (m_data == nullptr) || (m_length <= 0);
}
const CSAMPLE& operator[](SINT index) const {
const CSAMPLE& operator[](SINT index) const noexcept {
return *data(index);
}

private:
const CSAMPLE* m_data;
SINT m_length;
};

class WritableSlice {
class WritableSlice final {
public:
WritableSlice()
: m_data(nullptr),
m_length(0) {
constexpr WritableSlice()
: m_data(nullptr),
m_length(0) {
}
WritableSlice(CSAMPLE* data, SINT length)
: m_data(data),
Expand All @@ -164,25 +187,32 @@ class SampleBuffer final {
m_length(length) {
DEBUG_ASSERT((buffer.size() - offset) >= length);
}
CSAMPLE* data(SINT offset = 0) const {
CSAMPLE* data() const noexcept {
return m_data;
}
CSAMPLE* data(SINT offset) const noexcept {
DEBUG_ASSERT((m_data != nullptr) || (offset == 0));
DEBUG_ASSERT(0 <= offset);
// >=: allow access to one element behind allocated memory
DEBUG_ASSERT(m_length >= offset);
return m_data + offset;
}
SINT length(SINT offset = 0) const {
SINT length() const noexcept {
return m_length;
}
SINT length(SINT offset) const noexcept {
DEBUG_ASSERT(0 <= offset);
// >=: allow access to one element behind allocated memory
DEBUG_ASSERT(m_length >= offset);
return m_length - offset;
}
bool empty() const {
bool empty() const noexcept {
return (m_data == nullptr) || (m_length <= 0);
}
CSAMPLE& operator[](SINT index) const {
CSAMPLE& operator[](SINT index) const noexcept {
return *data(index);
}

private:
CSAMPLE* m_data;
SINT m_length;
Expand All @@ -199,7 +229,7 @@ namespace std {

// Template specialization of std::swap() for SampleBuffer
template<>
inline void swap(::mixxx::SampleBuffer& lhs, ::mixxx::SampleBuffer& rhs) {
inline void swap(::mixxx::SampleBuffer& lhs, ::mixxx::SampleBuffer& rhs) noexcept {
lhs.swap(rhs);
}

Expand Down
4 changes: 2 additions & 2 deletions src/util/span.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,7 @@ namespace spanutil {
/// At the same time, the function provides appropriate lower bound checking
/// for signed data types.
template<typename T, typename S, typename T2 = typename std::span<T>::size_type>
constexpr T2 castToSizeType(S size) {
constexpr T2 castToSizeType(S size) noexcept {
if constexpr (std::is_signed_v<S> && std::is_unsigned_v<T2>) {
VERIFY_OR_DEBUG_ASSERT(size >= 0) {
size = 0;
Expand All @@ -27,7 +27,7 @@ constexpr T2 castToSizeType(S size) {
/// In most cases, the pointer to the raw data of a data structure
/// is used, and the size of the data structure.
template<typename T, typename S>
constexpr std::span<T> spanFromPtrLen(T* ptr, S size) {
constexpr std::span<T> spanFromPtrLen(T* ptr, S size) noexcept {
return std::span<T>{ptr, mixxx::spanutil::castToSizeType<T>(size)};
}

Expand Down