Tidy and modernize SampleBuffer

src/util/assert.h

@@ -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)
@@ -56,14 +56,16 @@ 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)
+#define MIXXX_RELEASE_NOEXCEPT
 #else
 #define DEBUG_ASSERT(cond) \
     do {                   \
     } while (0)
+#define MIXXX_RELEASE_NOEXCEPT noexcept
 #endif
 
 /// Same as DEBUG_ASSERT, but if MIXXX_DEBUG_ASSERTIONS_FATAL is disabled run
@@ -80,5 +82,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

src/util/sample.h

@@ -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);
 
     // Frees a 16-byte aligned buffer allocated by SampleUtil::alloc()
     static void free(CSAMPLE* pBuffer);
@@ -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
@@ -83,53 +83,53 @@ class SampleUtil {
     }
 
     // Limits a CSAMPLE value to the valid range [-CSAMPLE_PEAK, CSAMPLE_PEAK]
-    inline static CSAMPLE clampSample(CSAMPLE in) {
+    [[nodiscard]] inline static CSAMPLE clampSample(CSAMPLE in) {
         return CSAMPLE_clamp(in);
     }
 
     // Limits a CSAMPLE_GAIN value to the valid range [CSAMPLE_GAIN_MIN, CSAMPLE_GAIN_MAX]
-    inline static CSAMPLE clampGain(CSAMPLE_GAIN in) {
+    [[nodiscard]] inline static CSAMPLE clampGain(CSAMPLE_GAIN in) {
         return CSAMPLE_GAIN_clamp(in);
     }
 
-    inline static SINT roundPlayPosToFrameStart(
+    [[nodiscard]] inline static SINT roundPlayPosToFrameStart(
             double playPos, mixxx::audio::ChannelCount numChannels) {
         SINT playPosFrames = static_cast<SINT>(round(playPos / numChannels));
         return playPosFrames * numChannels;
     }
 
-    inline static SINT truncPlayPosToFrameStart(
+    [[nodiscard]] inline static SINT truncPlayPosToFrameStart(
             double playPos, mixxx::audio::ChannelCount numChannels) {
         SINT playPosFrames = static_cast<SINT>(playPos / numChannels);
         return playPosFrames * numChannels;
     }
 
-    inline static SINT floorPlayPosToFrameStart(
+    [[nodiscard]] inline static SINT floorPlayPosToFrameStart(
             double playPos, mixxx::audio::ChannelCount numChannels) {
         SINT playPosFrames = static_cast<SINT>(floor(playPos / numChannels));
         return playPosFrames * numChannels;
     }
 
-    inline static SINT ceilPlayPosToFrameStart(
+    [[nodiscard]] inline static SINT ceilPlayPosToFrameStart(
             double playPos, mixxx::audio::ChannelCount numChannels) {
         SINT playPosFrames = static_cast<SINT>(ceil(playPos / numChannels));
         return playPosFrames * numChannels;
     }
 
-    inline static SINT roundPlayPosToFrame(double playPos) {
+    [[nodiscard]] inline static SINT roundPlayPosToFrame(double playPos) {
         return static_cast<SINT>(round(playPos / kPlayPositionChannels));
     }
 
-    inline static SINT truncPlayPosToFrame(double playPos) {
+    [[nodiscard]] inline static SINT truncPlayPosToFrame(double playPos) {
         return static_cast<SINT>(playPos / kPlayPositionChannels);
     }
 
-    inline static SINT floorPlayPosToFrame(double playPos) {
+    [[nodiscard]] inline static SINT floorPlayPosToFrame(double playPos) {
         return static_cast<SINT>(floor(playPos / kPlayPositionChannels));
 
     }
 
-    inline static SINT ceilPlayPosToFrame(double playPos) {
+    [[nodiscard]] inline static SINT ceilPlayPosToFrame(double playPos) {
         return static_cast<SINT>(ceil(playPos / kPlayPositionChannels));
     }
 
@@ -223,12 +223,12 @@ class SampleUtil {
             const CSAMPLE* pBuffer, SINT numSamples);
 
     // Returns the sum of the squared values of the buffer.
-    static CSAMPLE sumSquared(const CSAMPLE* pBuffer, SINT numSamples);
+    [[nodiscard]] static CSAMPLE sumSquared(const CSAMPLE* pBuffer, SINT numSamples);
 
     // Returns the root mean square of the values of the buffer.
-    static CSAMPLE rms(const CSAMPLE* pBuffer, SINT numSamples);
+    [[nodiscard]] static CSAMPLE rms(const CSAMPLE* pBuffer, SINT numSamples);
 
-    static CSAMPLE maxAbsAmplitude(const CSAMPLE* pBuffer, SINT numSamples);
+    [[nodiscard]] static CSAMPLE maxAbsAmplitude(const CSAMPLE* pBuffer, SINT numSamples);
 
     // Copies every sample in pSrc to pDest, limiting the values in pDest
     // to the valid range of CSAMPLE. pDest and pSrc must not overlap.

src/util/samplebuffer.cpp

@@ -2,7 +2,6 @@
 
 #include "util/sample.h"
 
-
 namespace mixxx {
 
 SampleBuffer::SampleBuffer(SINT size)
@@ -12,6 +11,17 @@ SampleBuffer::SampleBuffer(SINT size)
 
 SampleBuffer::~SampleBuffer() {
     SampleUtil::free(m_data);
+
+    /* cleanup the memory, just in case of use after free */
+    m_data = nullptr;
+    m_size = 0;
+}
+
+SampleBuffer& SampleBuffer::operator=(SampleBuffer&& that) {
+    SampleUtil::free(m_data);
+    m_data = std::exchange(that.m_data, nullptr);
+    m_size = std::exchange(that.m_size, 0);
+    return *this;
 }
 
 void SampleBuffer::clear() {
@@ -22,5 +32,4 @@ void SampleBuffer::clear() {
 void SampleBuffer::fill(CSAMPLE value) {
     SampleUtil::fill(data(), value, size());
 }
-
 } // namespace mixxx

src/util/samplebuffer.h

@@ -34,64 +34,62 @@ namespace mixxx {
 //
 class SampleBuffer final {
   public:
-    SampleBuffer()
-        : m_data(nullptr),
-          m_size(0) {
-    }
+    constexpr SampleBuffer() = default;
     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;
+        : m_data(std::exchange(that.m_data, nullptr)),
+          m_size(std::exchange(that.m_size, 0)) {
     }
-    virtual ~SampleBuffer() final;
+    ~SampleBuffer();
 
     SampleBuffer& operator=(SampleBuffer& that) = delete;
-    SampleBuffer& operator=(SampleBuffer&& that) {
-        swap(that);
-        return *this;
-    }
+    SampleBuffer& operator=(SampleBuffer&& that);
 
-    constexpr SINT size() const {
+    [[nodiscard]] constexpr SINT size() const noexcept {
         return m_size;
     }
 
-    CSAMPLE* data(SINT offset = 0) {
+    [[nodiscard]] constexpr CSAMPLE* data() noexcept {
+        return m_data;
+    }
+    [[nodiscard]] constexpr const CSAMPLE* data() const noexcept {
+        return m_data;
+    }
+    [[nodiscard]] CSAMPLE* data(SINT offset) MIXXX_RELEASE_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 {
+    [[nodiscard]] const CSAMPLE* data(SINT offset) const MIXXX_RELEASE_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() {
+    [[nodiscard]] std::span<CSAMPLE> span() {
         return mixxx::spanutil::spanFromPtrLen(m_data, m_size);
     }
-    std::span<const CSAMPLE> span() const {
+    [[nodiscard]] std::span<const CSAMPLE> span() const {
         return mixxx::spanutil::spanFromPtrLen(m_data, m_size);
     }
 
-    CSAMPLE& operator[](SINT index) {
+    [[nodiscard]] CSAMPLE& operator[](SINT index) MIXXX_RELEASE_NOEXCEPT {
         return *data(index);
     }
-    const CSAMPLE& operator[](SINT index) const {
+    [[nodiscard]] const CSAMPLE& operator[](SINT index) const MIXXX_RELEASE_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);
     }
@@ -102,12 +100,9 @@ class SampleBuffer final {
     // Fills the whole buffer with the same value
     void fill(CSAMPLE value);
 
-    class ReadableSlice {
+    class ReadableSlice final {
       public:
-        ReadableSlice()
-            : m_data(nullptr),
-              m_length(0) {
-        }
+        constexpr ReadableSlice() = default;
         ReadableSlice(const CSAMPLE* data, SINT length)
             : m_data(data),
               m_length(length) {
@@ -119,36 +114,39 @@ class SampleBuffer final {
               m_length(length) {
             DEBUG_ASSERT((buffer.size() - offset) >= length);
         }
-        const CSAMPLE* data(SINT offset = 0) const {
+        [[nodiscard]] const CSAMPLE* data() const noexcept {
+            return m_data;
+        }
+        [[nodiscard]] 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 {
+        [[nodiscard]] SINT length() const noexcept {
+            return m_length;
+        }
+        [[nodiscard]] SINT length(SINT offset) const MIXXX_RELEASE_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 {
+        [[nodiscard]] bool empty() const noexcept {
             return (m_data == nullptr) || (m_length <= 0);
         }
-        const CSAMPLE& operator[](SINT index) const {
+        [[nodiscard]] const CSAMPLE& operator[](SINT index) const MIXXX_RELEASE_NOEXCEPT {
             return *data(index);
         }
       private:
-        const CSAMPLE* m_data;
-        SINT m_length;
+        const CSAMPLE* m_data = nullptr;
+        SINT m_length = 0;
     };
 
-    class WritableSlice {
+    class WritableSlice final {
       public:
-        WritableSlice()
-            : m_data(nullptr),
-              m_length(0) {
-        }
+        constexpr WritableSlice() = default;
         WritableSlice(CSAMPLE* data, SINT length)
             : m_data(data),
               m_length(length) {
@@ -164,33 +162,39 @@ class SampleBuffer final {
               m_length(length) {
             DEBUG_ASSERT((buffer.size() - offset) >= length);
         }
-        CSAMPLE* data(SINT offset = 0) const {
+        [[nodiscard]] CSAMPLE* data() const noexcept {
+            return m_data;
+        }
+        [[nodiscard]] CSAMPLE* data(SINT offset) const MIXXX_RELEASE_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 {
+        [[nodiscard]] SINT length() const noexcept {
+            return m_length;
+        }
+        [[nodiscard]] SINT length(SINT offset) const MIXXX_RELEASE_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 {
+        [[nodiscard]] bool empty() const noexcept {
             return (m_data == nullptr) || (m_length <= 0);
         }
-        CSAMPLE& operator[](SINT index) const {
+        [[nodiscard]] CSAMPLE& operator[](SINT index) const MIXXX_RELEASE_NOEXCEPT {
             return *data(index);
         }
       private:
-        CSAMPLE* m_data;
-        SINT m_length;
+        CSAMPLE* m_data = nullptr;
+        SINT m_length = 0;
     };
 
   private:
-    CSAMPLE* m_data;
-    SINT m_size;
+    CSAMPLE* m_data = nullptr;
+    SINT m_size = 0;
 };
 
 } // namespace mixxx
@@ -199,7 +203,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);
 }