From 01f80d344e206cbb860e0861e06347daec54501a Mon Sep 17 00:00:00 2001 From: Sam Hocevar Date: Mon, 19 Feb 2024 16:25:22 +0100 Subject: [PATCH] audio: fix bugs in the audio sample conversion, and add unit tests --- .gitignore | 3 + include/lol/private/audio/stream.h | 80 +++++++++++---------- t/Makefile | 13 ++++ t/audio.cpp | 107 +++++++++++++++++++++++++++++ 4 files changed, 166 insertions(+), 37 deletions(-) create mode 100644 t/Makefile create mode 100644 t/audio.cpp diff --git a/.gitignore b/.gitignore index 4abed329..72278f39 100644 --- a/.gitignore +++ b/.gitignore @@ -1,3 +1,6 @@ # Editor cruft .*.swp *~ +# Binaries +*.exe +t/test diff --git a/include/lol/private/audio/stream.h b/include/lol/private/audio/stream.h index caa1217e..1b1d8666 100644 --- a/include/lol/private/audio/stream.h +++ b/include/lol/private/audio/stream.h @@ -27,6 +27,48 @@ namespace lol::audio { +class sample +{ +public: + // Convert samples between different types (float, int16_t, uint8_t, …) + template + static inline TO convert(FROM x) + { + constexpr auto from_fp = std::is_floating_point_v; + constexpr auto from_signed = !from_fp && std::is_signed_v; + constexpr auto to_fp = std::is_floating_point_v; + constexpr auto to_signed = !to_fp && std::is_signed_v; + + if constexpr (std::is_same_v || (from_fp && to_fp)) + { + // If types are the same, or both floating-point, no conversion is needed + return TO(x); + } + else if constexpr (from_fp) + { + // From floating point to integer: + // - renormalise to 0…1 + // - multiply by the size of the integer range + // - add min, round down, and clamp to min…max + FROM constexpr min(std::numeric_limits::min()); + FROM constexpr max(std::numeric_limits::max()); + x = (x + 1) * ((max - min + 1) / 2); + return TO(std::max(min, std::min(max, std::floor(x + min)))); + } + else if constexpr (to_fp) + { + TO constexpr min(std::numeric_limits::min()); + TO constexpr max(std::numeric_limits::max()); + return (TO(x) - min) * 2 / (max - min) - 1; + } + else + { + // FIXME: this is better than nothing but we need a better implementation + return convert(convert(x)); + } + } +}; + template class stream { @@ -124,42 +166,6 @@ protected: std::unordered_set>> m_streams; }; -// Convert samples from and to different types (float, int16_t, uint8_t, …) -template -static inline T convert_sample(T0 x) -{ - constexpr auto from_fp = std::is_floating_point_v; - constexpr auto from_signed = !from_fp && std::is_signed_v; - constexpr auto to_fp = std::is_floating_point_v; - constexpr auto to_signed = !to_fp && std::is_signed_v; - - if constexpr (std::is_same_v || (from_fp && to_fp)) - { - // If types are the same, or both floating-point, no conversion is needed - return T(x); - } - else if constexpr (from_fp) - { - // From floating point to integer - if constexpr (to_signed) - x = (x + T0(1.0)) * T0(0.5); - return T(x * std::numeric_limits::max()); - } - else if constexpr (to_fp) - { - // From integer to floating point - if constexpr (from_signed) - return x / -T(std::numeric_limits::min()); - else - return x / (T(std::numeric_limits::max()) * T(0.5)) - T(1.0); - } - else - { - // FIXME: this is better than nothing but we need a better implementation - return convert_sample(convert_sample(x)); - } -} - template class converter : public stream { @@ -179,7 +185,7 @@ public: std::vector tmp(samples); m_in->get(tmp.data(), frames); for (size_t n = 0; n < samples; ++n) - buf[n] = convert_sample(tmp[n]); + buf[n] = sample::convert(tmp[n]); return frames; } diff --git a/t/Makefile b/t/Makefile new file mode 100644 index 00000000..1a89510e --- /dev/null +++ b/t/Makefile @@ -0,0 +1,13 @@ + +SRC = audio.cpp + +all: test + +clean: + rm -f test test.exe + +check: test + ./test + +test: $(SRC) + $(CXX) -I../include $^ -o $@ diff --git a/t/audio.cpp b/t/audio.cpp new file mode 100644 index 00000000..5dc32714 --- /dev/null +++ b/t/audio.cpp @@ -0,0 +1,107 @@ +#include +#include + +TEST_CASE("sample conversion between floating point types") +{ + auto cv1 = lol::audio::sample::convert; + CHECK(cv1(-1.0f) == -1.0f); + CHECK(cv1( 0.0f) == 0.0f); + CHECK(cv1( 1.0f) == 1.0f); + + auto cv2 = lol::audio::sample::convert; + CHECK(cv2(-1.0f) == -1.0); + CHECK(cv2( 0.0f) == 0.0); + CHECK(cv2( 1.0f) == 1.0); + + auto cv3 = lol::audio::sample::convert; + CHECK(cv3(-1.0) == -1.0f); + CHECK(cv3( 0.0) == 0.0f); + CHECK(cv3( 1.0) == 1.0f); + + auto cv4 = lol::audio::sample::convert; + CHECK(cv4(-1.0) == -1.0); + CHECK(cv4( 0.0) == 0.0); + CHECK(cv4( 1.0) == 1.0); +} + +TEST_CASE("sample conversion from float to 8-bit") +{ + auto cv1 = lol::audio::sample::convert; + CHECK(cv1(-1.5f) == -128); + CHECK(cv1(-1.0f) == -128); + CHECK(cv1(-0.5f) == -64); + CHECK(cv1( 0.0f) == 0); + CHECK(cv1( 0.5f) == 64); + CHECK(cv1( 1.0f) == 127); + CHECK(cv1( 1.5f) == 127); + + auto cv2 = lol::audio::sample::convert; + CHECK(cv2(-1.5f) == 0); + CHECK(cv2(-1.0f) == 0); + CHECK(cv2(-0.5f) == 64); + CHECK(cv2( 0.0f) == 128); + CHECK(cv2( 0.5f) == 192); + CHECK(cv2( 1.0f) == 255); + CHECK(cv2( 1.5f) == 255); +} + +TEST_CASE("sample conversion from 8-bit to float") +{ + auto cv1 = lol::audio::sample::convert; + CHECK(cv1(-128) == -1.0f); + CHECK(cv1( 127) == 1.0f); + + auto cv2 = lol::audio::sample::convert; + CHECK(cv2( 0) == -1.0f); + CHECK(cv2(255) == 1.0f); +} + +TEST_CASE("sample conversion from float to 16-bit") +{ + auto cv1 = lol::audio::sample::convert; + CHECK(cv1(-1.5f) == -32768); + CHECK(cv1(-1.0f) == -32768); + CHECK(cv1(-0.5f) == -16384); + CHECK(cv1( 0.0f) == 0); + CHECK(cv1( 0.5f) == 16384); + CHECK(cv1( 1.0f) == 32767); + CHECK(cv1( 1.5f) == 32767); + + auto cv2 = lol::audio::sample::convert; + CHECK(cv2(-1.5f) == 0); + CHECK(cv2(-1.0f) == 0); + CHECK(cv2(-0.5f) == 16384); + CHECK(cv2( 0.0f) == 32768); + CHECK(cv2( 0.5f) == 49152); + CHECK(cv2( 1.0f) == 65535); + CHECK(cv2( 1.5f) == 65535); +} + +TEST_CASE("sample conversion from 16-bit to float") +{ + auto cv1 = lol::audio::sample::convert; + CHECK(cv1(-32768) == -1.0f); + CHECK(cv1( 32767) == 1.0f); + + auto cv2 = lol::audio::sample::convert; + CHECK(cv2( 0) == -1.0f); + CHECK(cv2(65535) == 1.0f); +} + +TEST_CASE("round-trip conversion from 8-bit to 8-bit") +{ + auto cv1 = lol::audio::sample::convert; + auto cv2 = lol::audio::sample::convert; + CHECK(cv2(cv1(-128)) == -128); + CHECK(cv2(cv1(-127)) == -127); + CHECK(cv2(cv1( -64)) == -64); + CHECK(cv2(cv1( -32)) == -32); + CHECK(cv2(cv1( -2)) == -2); + CHECK(cv2(cv1( -1)) == -1); + CHECK(cv2(cv1( 0)) == 0); + CHECK(cv2(cv1( 1)) == 1); + CHECK(cv2(cv1( 2)) == 2); + CHECK(cv2(cv1( 32)) == 32); + CHECK(cv2(cv1( 64)) == 64); + CHECK(cv2(cv1( 127)) == 127); +}