From afcf0795c44260f54e74ec854297de439e7562c3 Mon Sep 17 00:00:00 2001
From: Max Kellermann <max@duempel.org>
Date: Sun, 22 Dec 2013 17:39:26 +0100
Subject: [PATCH] pcm/Volume: improved dithering
Instead of just adding a rectangular random value before shifting back
to the normal scale, use the existing PcmDither library.
---
NEWS | 2 ++
src/pcm/PcmDither.cxx | 13 +++++++++++
src/pcm/PcmDither.hxx | 12 ++++++++++
src/pcm/Volume.cxx | 47 +++++++++++++++++++++++-----------------
src/pcm/Volume.hxx | 2 ++
test/test_pcm_volume.cxx | 5 +++--
6 files changed, 59 insertions(+), 22 deletions(-)
diff --git a/NEWS b/NEWS
index 387a1a597..bd7ed3e91 100644
--- a/NEWS
+++ b/NEWS
@@ -3,6 +3,8 @@ ver 0.19 (not yet released)
- new commands "addtagid", "cleartagid"
* input
- alsa: new input plugin
+* filter
+ - volume: improved software volume dithering
* new resampler option using libsoxr
ver 0.18.6 (2013/12/24)
diff --git a/src/pcm/PcmDither.cxx b/src/pcm/PcmDither.cxx
index f07d0b343..d9d4d28ca 100644
--- a/src/pcm/PcmDither.cxx
+++ b/src/pcm/PcmDither.cxx
@@ -62,6 +62,19 @@ PcmDither::Dither(T sample)
return output >> scale_bits;
}
+template<typename ST, unsigned SBITS, unsigned DBITS>
+inline ST
+PcmDither::DitherShift(ST sample)
+{
+ static_assert(sizeof(ST) * 8 > SBITS, "Source type too small");
+ static_assert(SBITS > DBITS, "Non-positive scale_bits");
+
+ static constexpr ST MIN = -(ST(1) << (SBITS - 1));
+ static constexpr ST MAX = (ST(1) << (SBITS - 1)) - 1;
+
+ return Dither<ST, MIN, MAX, SBITS - DBITS>(sample);
+}
+
template<typename ST, typename DT>
inline typename DT::value_type
PcmDither::DitherConvert(typename ST::value_type sample)
diff --git a/src/pcm/PcmDither.hxx b/src/pcm/PcmDither.hxx
index fdcdb1f6a..473a45ac3 100644
--- a/src/pcm/PcmDither.hxx
+++ b/src/pcm/PcmDither.hxx
@@ -32,6 +32,18 @@ public:
constexpr PcmDither()
:error{0, 0, 0}, random(0) {}
+ /**
+ * Shift the given sample by #SBITS-#DBITS to the right, and
+ * apply dithering.
+ *
+ * @param ST the input sample type
+ * @param SBITS the input bit width
+ * @param DBITS the output bit width
+ * @param sample the input sample value
+ */
+ template<typename ST, unsigned SBITS, unsigned DBITS>
+ ST DitherShift(ST sample);
+
void Dither24To16(int16_t *dest, const int32_t *src,
const int32_t *src_end);
diff --git a/src/pcm/Volume.cxx b/src/pcm/Volume.cxx
index 0ccfe03b5..6347657c6 100644
--- a/src/pcm/Volume.cxx
+++ b/src/pcm/Volume.cxx
@@ -25,62 +25,69 @@
#include "util/ConstBuffer.hxx"
#include "util/Error.hxx"
+#include "PcmDither.cxx" // including the .cxx file to get inlined templates
+
#include <stdint.h>
#include <string.h>
template<SampleFormat F, class Traits=SampleTraits<F>>
static inline typename Traits::value_type
-pcm_volume_sample(typename Traits::value_type _sample,
+pcm_volume_sample(PcmDither &dither,
+ typename Traits::value_type _sample,
int volume)
{
typename Traits::long_type sample(_sample);
- sample = (sample * volume + pcm_volume_dither() +
- PCM_VOLUME_1S / 2)
- >> PCM_VOLUME_BITS;
-
- return PcmClamp<F, Traits>(sample);
+ return dither.DitherShift<typename Traits::long_type,
+ Traits::BITS + PCM_VOLUME_BITS,
+ Traits::BITS>(sample * volume);
}
template<SampleFormat F, class Traits=SampleTraits<F>>
static void
-pcm_volume_change(typename Traits::pointer_type dest,
+pcm_volume_change(PcmDither &dither,
+ typename Traits::pointer_type dest,
typename Traits::const_pointer_type src,
typename Traits::const_pointer_type end,
int volume)
{
while (src < end) {
const auto sample = *src++;
- *dest++ = pcm_volume_sample<F, Traits>(sample, volume);
+ *dest++ = pcm_volume_sample<F, Traits>(dither, sample, volume);
}
}
static void
-pcm_volume_change_8(int8_t *dest, const int8_t *src, const int8_t *end,
+pcm_volume_change_8(PcmDither &dither,
+ int8_t *dest, const int8_t *src, const int8_t *end,
int volume)
{
- pcm_volume_change<SampleFormat::S8>(dest, src, end, volume);
+ pcm_volume_change<SampleFormat::S8>(dither, dest, src, end, volume);
}
static void
-pcm_volume_change_16(int16_t *dest, const int16_t *src, const int16_t *end,
+pcm_volume_change_16(PcmDither &dither,
+ int16_t *dest, const int16_t *src, const int16_t *end,
int volume)
{
- pcm_volume_change<SampleFormat::S16>(dest, src, end, volume);
+ pcm_volume_change<SampleFormat::S16>(dither, dest, src, end, volume);
}
static void
-pcm_volume_change_24(int32_t *dest, const int32_t *src, const int32_t *end,
+pcm_volume_change_24(PcmDither &dither,
+ int32_t *dest, const int32_t *src, const int32_t *end,
int volume)
{
- pcm_volume_change<SampleFormat::S24_P32>(dest, src, end, volume);
+ pcm_volume_change<SampleFormat::S24_P32>(dither, dest, src, end,
+ volume);
}
static void
-pcm_volume_change_32(int32_t *dest, const int32_t *src, const int32_t *end,
+pcm_volume_change_32(PcmDither &dither,
+ int32_t *dest, const int32_t *src, const int32_t *end,
int volume)
{
- pcm_volume_change<SampleFormat::S32>(dest, src, end, volume);
+ pcm_volume_change<SampleFormat::S32>(dither, dest, src, end, volume);
}
static void
@@ -143,28 +150,28 @@ PcmVolume::Apply(ConstBuffer<void> src)
gcc_unreachable();
case SampleFormat::S8:
- pcm_volume_change_8((int8_t *)data,
+ pcm_volume_change_8(dither, (int8_t *)data,
(const int8_t *)src.data,
(const int8_t *)end,
volume);
break;
case SampleFormat::S16:
- pcm_volume_change_16((int16_t *)data,
+ pcm_volume_change_16(dither, (int16_t *)data,
(const int16_t *)src.data,
(const int16_t *)end,
volume);
break;
case SampleFormat::S24_P32:
- pcm_volume_change_24((int32_t *)data,
+ pcm_volume_change_24(dither, (int32_t *)data,
(const int32_t *)src.data,
(const int32_t *)end,
volume);
break;
case SampleFormat::S32:
- pcm_volume_change_32((int32_t *)data,
+ pcm_volume_change_32(dither, (int32_t *)data,
(const int32_t *)src.data,
(const int32_t *)end,
volume);
diff --git a/src/pcm/Volume.hxx b/src/pcm/Volume.hxx
index c31aafb6e..50742225f 100644
--- a/src/pcm/Volume.hxx
+++ b/src/pcm/Volume.hxx
@@ -23,6 +23,7 @@
#include "PcmPrng.hxx"
#include "AudioFormat.hxx"
#include "PcmBuffer.hxx"
+#include "PcmDither.hxx"
#include <stdint.h>
#include <stddef.h>
@@ -87,6 +88,7 @@ class PcmVolume {
unsigned volume;
PcmBuffer buffer;
+ PcmDither dither;
public:
PcmVolume()
diff --git a/test/test_pcm_volume.cxx b/test/test_pcm_volume.cxx
index bba5d3bbf..c3a261f7a 100644
--- a/test/test_pcm_volume.cxx
+++ b/test/test_pcm_volume.cxx
@@ -60,8 +60,9 @@ TestVolume(G g=G())
const auto _dest = ConstBuffer<value_type>::FromVoid(dest);
for (unsigned i = 0; i < N; ++i) {
- CPPUNIT_ASSERT(_dest.data[i] >= (_src[i] - 1) / 2);
- CPPUNIT_ASSERT(_dest.data[i] <= _src[i] / 2 + 1);
+ const auto expected = (_src[i] + 1) / 2;
+ CPPUNIT_ASSERT(_dest.data[i] >= expected - 4);
+ CPPUNIT_ASSERT(_dest.data[i] <= expected + 4);
}
pv.Close();