pcm/Volume: improved dithering

Instead of just adding a rectangular random value before shifting back
to the normal scale, use the existing PcmDither library.

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();