diff mbox

Bugfix: Fix resampling when client and slave both use format float

Message ID 530B41B4.7000106@ivitera.com (mailing list archive)
State Changes Requested
Headers show

Commit Message

Pavel Hofman Feb. 24, 2014, 12:57 p.m. UTC 
On 24.2.2014 13:13, Maarten Baert wrote:
> On 24/02/14 10:17, Takashi Iwai wrote:
>> Can S32 work instead of S16?  Then we won't lose the accuracy so much.
>> Of course, handling float directly would be the best option.
> The samplerate and speexrate plugins currently take S16 (see
> pcm_src_convert_s16 in alsa-plugins/rate/rate_samplerate.c and
> alsa-plugins/rate/rate_speexrate.c), so just using S32 will not improve
> the accuracy. It would be easy to replace those functions with
> pcm_src_convert_float (both resampler libraries have functions that take
> float directly), but that will break the plugin ABI. Is that acceptable?

Here is my older patch for the convertor, I know it needs a bit of work
before accepting (originally discussed at
http://mailman.alsa-project.org/pipermail/alsa-devel/2011-June/041368.html )

If this project gets done, there is a CPU-efficient libsoxr convertor
plugin in the pipeline too.

Thanks a lot,

Pavel.

Comments

Takashi Iwai Feb. 24, 2014, 1:38 p.m. UTC | #1 
At Mon, 24 Feb 2014 13:57:24 +0100,
Pavel Hofman wrote:
> 
> On 24.2.2014 13:13, Maarten Baert wrote:
> > On 24/02/14 10:17, Takashi Iwai wrote:
> >> Can S32 work instead of S16?  Then we won't lose the accuracy so much.
> >> Of course, handling float directly would be the best option.
> > The samplerate and speexrate plugins currently take S16 (see
> > pcm_src_convert_s16 in alsa-plugins/rate/rate_samplerate.c and
> > alsa-plugins/rate/rate_speexrate.c), so just using S32 will not improve
> > the accuracy. It would be easy to replace those functions with
> > pcm_src_convert_float (both resampler libraries have functions that take
> > float directly), but that will break the plugin ABI. Is that acceptable?
> 
> Here is my older patch for the convertor, I know it needs a bit of work
> before accepting (originally discussed at
> http://mailman.alsa-project.org/pipermail/alsa-devel/2011-June/041368.html )
> 
> If this project gets done, there is a CPU-efficient libsoxr convertor
> plugin in the pipeline too.

Yeah, we'll need some change like this, but need the version checks
and process the new process_float ops only for the new protocol
version for keeping the backward compatibility, too.


thanks,

Takashi

> 
> Thanks a lot,
> 
> Pavel.
> >From 628930dabaa8b8e3f79a03a092a63ee44bfdb98e Mon Sep 17 00:00:00 2001
> From: Pavel Hofman <pavel.hofman@ivitera.com>
> Date: Tue, 28 Jun 2011 08:50:23 +0200
> Subject: [ALSA-LIB 1/1] Support for float samples in rate converter plugins
> 
> Some rate converters have native float resolution, no need
> to loose information by converting to s16.
> 
> Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>
> 
> diff --git a/include/pcm_rate.h b/include/pcm_rate.h
> index 4d70df2..e92900b 100644
> --- a/include/pcm_rate.h
> +++ b/include/pcm_rate.h
> @@ -91,6 +91,11 @@ typedef struct snd_pcm_rate_ops {
>  	void (*convert_s16)(void *obj, int16_t *dst, unsigned int dst_frames,
>  			    const int16_t *src, unsigned int src_frames);
>  	/**
> +	 * convert a float interleaved-data array; exclusive with convert
> +	 */
> +	void (*convert_float)(void *obj, float *dst, unsigned int dst_frames,
> +			    const float *src, unsigned int src_frames);
> +	/**
>  	 * compute the frame size for input
>  	 */
>  	snd_pcm_uframes_t (*input_frames)(void *obj, snd_pcm_uframes_t frames);
> diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c
> index 54a3e67..2831f23 100644
> --- a/src/pcm/pcm_rate.c
> +++ b/src/pcm/pcm_rate.c
> @@ -35,7 +35,7 @@
>  #include "iatomic.h"
>  
>  #include "plugin_ops.h"
> -
> +typedef float float_t;
>  #if 0
>  #define DEBUG_REFINE
>  #endif
> @@ -66,8 +66,8 @@ struct _snd_pcm_rate {
>  	snd_pcm_rate_ops_t ops;
>  	unsigned int get_idx;
>  	unsigned int put_idx;
> -	int16_t *src_buf;
> -	int16_t *dst_buf;
> +	void *src_buf;
> +	void *dst_buf;
>  	int start_pending; /* start is triggered but not commited to slave */
>  	snd_htimestamp_t trigger_tstamp;
>  	unsigned int plugin_version;
> @@ -310,13 +310,23 @@ static int snd_pcm_rate_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
>  		rate->sareas[chn].step = swidth;
>  	}
>  
> -	if (rate->ops.convert_s16) {
> +	if (rate->ops.convert_float) {
> +		rate->get_idx = snd_pcm_linear_get_index(rate->info.in.format, SND_PCM_FORMAT_S32);
> +		rate->put_idx = snd_pcm_linear_put_index(SND_PCM_FORMAT_S32, rate->info.out.format);
> +		free(rate->src_buf);
> +		rate->src_buf = (float_t *) malloc(channels * rate->info.in.period_size * sizeof(float_t));
> +		free(rate->dst_buf);
> +		rate->dst_buf = (float_t *) malloc(channels * rate->info.out.period_size * sizeof(float_t));
> +		if (! rate->src_buf || ! rate->dst_buf)
> +			goto error;
> +	}
> +	else if (rate->ops.convert_s16) {
>  		rate->get_idx = snd_pcm_linear_get_index(rate->info.in.format, SND_PCM_FORMAT_S16);
>  		rate->put_idx = snd_pcm_linear_put_index(SND_PCM_FORMAT_S16, rate->info.out.format);
>  		free(rate->src_buf);
> -		rate->src_buf = malloc(channels * rate->info.in.period_size * 2);
> +		rate->src_buf = (int16_t *) malloc(channels * rate->info.in.period_size * 2);
>  		free(rate->dst_buf);
> -		rate->dst_buf = malloc(channels * rate->info.out.period_size * 2);
> +		rate->dst_buf = (int16_t *) malloc(channels * rate->info.out.period_size * 2);
>  		if (! rate->src_buf || ! rate->dst_buf)
>  			goto error;
>  	}
> @@ -503,6 +513,85 @@ static void convert_from_s16(snd_pcm_rate_t *rate, const int16_t *buf,
>  		}
>  	}
>  }
> +static void convert_to_float(snd_pcm_rate_t *rate, float_t *buf,
> +			   const snd_pcm_channel_area_t *areas,
> +			   snd_pcm_uframes_t offset, unsigned int frames,
> +			   unsigned int channels)
> +{
> +#ifndef DOC_HIDDEN
> +#define GET32_LABELS
> +#include "plugin_ops.h"
> +#undef GET32_LABELS
> +#endif /* DOC_HIDDEN */
> +	void *get32 = get32_labels[rate->get_idx];
> +	const char *src;
> +	int32_t sample;
> +	const char *srcs[channels];
> +	int src_step[channels];
> +	unsigned int c;
> +
> +	for (c = 0; c < channels; c++) {
> +		srcs[c] = snd_pcm_channel_area_addr(areas + c, offset);
> +		src_step[c] = snd_pcm_channel_area_step(areas + c);
> +	}
> +
> +	while (frames--) {
> +		for (c = 0; c < channels; c++) {
> +			src = srcs[c];
> +			// converting the source format to int32 first
> +			goto *get32;
> +#ifndef DOC_HIDDEN
> +#define GET32_END after_get
> +#include "plugin_ops.h"
> +#undef GET32_END
> +#endif /* DOC_HIDDEN */
> +		after_get:
> +			// converting to float for the plugin
> +			*buf++ = (float_t) sample;
> +			srcs[c] += src_step[c];
> +		}
> +	}
> +}
> +
> +static void convert_from_float(snd_pcm_rate_t *rate, const float_t *buf,
> +			     const snd_pcm_channel_area_t *areas,
> +			     snd_pcm_uframes_t offset, unsigned int frames,
> +			     unsigned int channels)
> +{
> +#ifndef DOC_HIDDEN
> +#define PUT32_LABELS
> +#include "plugin_ops.h"
> +#undef PUT32_LABELS
> +#endif /* DOC_HIDDEN */
> +	void *put32 = put32_labels[rate->put_idx];
> +	char *dst;
> +	int32_t sample;
> +	char *dsts[channels];
> +	int dst_step[channels];
> +	unsigned int c;
> +
> +	for (c = 0; c < channels; c++) {
> +		dsts[c] = snd_pcm_channel_area_addr(areas + c, offset);
> +		dst_step[c] = snd_pcm_channel_area_step(areas + c);
> +	}
> +
> +	while (frames--) {
> +		for (c = 0; c < channels; c++) {
> +			dst = dsts[c];
> +			// plugin returned float, converting to int32 first
> +			sample = (int32_t) *buf++;
> +			// converting int32 to the destination format
> +			goto *put32;
> +#ifndef DOC_HIDDEN
> +#define PUT32_END after_put
> +#include "plugin_ops.h"
> +#undef PUT32_END
> +#endif /* DOC_HIDDEN */
> +		after_put:
> +			dsts[c] += dst_step[c];
> +		}
> +	}
> +}
>  
>  static void do_convert(const snd_pcm_channel_area_t *dst_areas,
>  		       snd_pcm_uframes_t dst_offset, unsigned int dst_frames,
> @@ -511,18 +600,36 @@ static void do_convert(const snd_pcm_channel_area_t *dst_areas,
>  		       unsigned int channels,
>  		       snd_pcm_rate_t *rate)
>  {
> -	if (rate->ops.convert_s16) {
> +	if (rate->ops.convert_float) {
> +		const float_t *src;
> +		float_t *dst;
> +		if (! rate->src_buf)
> +			src = src_areas->addr + src_offset * sizeof(float_t) * channels;
> +		else {
> +			convert_to_float(rate, rate->src_buf, src_areas, src_offset,
> +				       src_frames, channels);
> +			src = rate->src_buf;
> +		}
> +		if (! rate->dst_buf)
> +			dst = dst_areas->addr + dst_offset * sizeof(float_t) * channels;
> +		else
> +			dst = rate->dst_buf;
> +		rate->ops.convert_float(rate->obj, dst, dst_frames, src, src_frames);
> +		if (dst == rate->dst_buf)
> +			convert_from_float(rate, rate->dst_buf, dst_areas, dst_offset,
> +					 dst_frames, channels);
> +	} else if (rate->ops.convert_s16) {
>  		const int16_t *src;
>  		int16_t *dst;
>  		if (! rate->src_buf)
> -			src = src_areas->addr + src_offset * 2 * channels;
> +			src = src_areas->addr + src_offset * sizeof(int16_t) * channels;
>  		else {
>  			convert_to_s16(rate, rate->src_buf, src_areas, src_offset,
>  				       src_frames, channels);
>  			src = rate->src_buf;
>  		}
>  		if (! rate->dst_buf)
> -			dst = dst_areas->addr + dst_offset * 2 * channels;
> +			dst = dst_areas->addr + dst_offset * sizeof(int16_t) * channels;
>  		else
>  			dst = rate->dst_buf;
>  		rate->ops.convert_s16(rate->obj, dst, dst_frames, src, src_frames);
> @@ -1416,7 +1523,7 @@ int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name,
>  	}
>  #endif
>  
> -	if (! rate->ops.init || ! (rate->ops.convert || rate->ops.convert_s16) ||
> +	if (! rate->ops.init || ! (rate->ops.convert || rate->ops.convert_s16 || rate->ops.convert_float) ||
>  	    ! rate->ops.input_frames || ! rate->ops.output_frames) {
>  		SNDERR("Inproper rate plugin %s initialization", type);
>  		snd_pcm_free(pcm);
> -- 
> 1.7.1
> 
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
diff mbox

Patch

From 628930dabaa8b8e3f79a03a092a63ee44bfdb98e Mon Sep 17 00:00:00 2001
From: Pavel Hofman <pavel.hofman@ivitera.com>
Date: Tue, 28 Jun 2011 08:50:23 +0200
Subject: [ALSA-LIB 1/1] Support for float samples in rate converter plugins

Some rate converters have native float resolution, no need
to loose information by converting to s16.

Signed-off-by: Pavel Hofman <pavel.hofman@ivitera.com>

diff --git a/include/pcm_rate.h b/include/pcm_rate.h
index 4d70df2..e92900b 100644
--- a/include/pcm_rate.h
+++ b/include/pcm_rate.h
@@ -91,6 +91,11 @@  typedef struct snd_pcm_rate_ops {
 	void (*convert_s16)(void *obj, int16_t *dst, unsigned int dst_frames,
 			    const int16_t *src, unsigned int src_frames);
 	/**
+	 * convert a float interleaved-data array; exclusive with convert
+	 */
+	void (*convert_float)(void *obj, float *dst, unsigned int dst_frames,
+			    const float *src, unsigned int src_frames);
+	/**
 	 * compute the frame size for input
 	 */
 	snd_pcm_uframes_t (*input_frames)(void *obj, snd_pcm_uframes_t frames);
diff --git a/src/pcm/pcm_rate.c b/src/pcm/pcm_rate.c
index 54a3e67..2831f23 100644
--- a/src/pcm/pcm_rate.c
+++ b/src/pcm/pcm_rate.c
@@ -35,7 +35,7 @@ 
 #include "iatomic.h"
 
 #include "plugin_ops.h"
-
+typedef float float_t;
 #if 0
 #define DEBUG_REFINE
 #endif
@@ -66,8 +66,8 @@  struct _snd_pcm_rate {
 	snd_pcm_rate_ops_t ops;
 	unsigned int get_idx;
 	unsigned int put_idx;
-	int16_t *src_buf;
-	int16_t *dst_buf;
+	void *src_buf;
+	void *dst_buf;
 	int start_pending; /* start is triggered but not commited to slave */
 	snd_htimestamp_t trigger_tstamp;
 	unsigned int plugin_version;
@@ -310,13 +310,23 @@  static int snd_pcm_rate_hw_params(snd_pcm_t *pcm, snd_pcm_hw_params_t * params)
 		rate->sareas[chn].step = swidth;
 	}
 
-	if (rate->ops.convert_s16) {
+	if (rate->ops.convert_float) {
+		rate->get_idx = snd_pcm_linear_get_index(rate->info.in.format, SND_PCM_FORMAT_S32);
+		rate->put_idx = snd_pcm_linear_put_index(SND_PCM_FORMAT_S32, rate->info.out.format);
+		free(rate->src_buf);
+		rate->src_buf = (float_t *) malloc(channels * rate->info.in.period_size * sizeof(float_t));
+		free(rate->dst_buf);
+		rate->dst_buf = (float_t *) malloc(channels * rate->info.out.period_size * sizeof(float_t));
+		if (! rate->src_buf || ! rate->dst_buf)
+			goto error;
+	}
+	else if (rate->ops.convert_s16) {
 		rate->get_idx = snd_pcm_linear_get_index(rate->info.in.format, SND_PCM_FORMAT_S16);
 		rate->put_idx = snd_pcm_linear_put_index(SND_PCM_FORMAT_S16, rate->info.out.format);
 		free(rate->src_buf);
-		rate->src_buf = malloc(channels * rate->info.in.period_size * 2);
+		rate->src_buf = (int16_t *) malloc(channels * rate->info.in.period_size * 2);
 		free(rate->dst_buf);
-		rate->dst_buf = malloc(channels * rate->info.out.period_size * 2);
+		rate->dst_buf = (int16_t *) malloc(channels * rate->info.out.period_size * 2);
 		if (! rate->src_buf || ! rate->dst_buf)
 			goto error;
 	}
@@ -503,6 +513,85 @@  static void convert_from_s16(snd_pcm_rate_t *rate, const int16_t *buf,
 		}
 	}
 }
+static void convert_to_float(snd_pcm_rate_t *rate, float_t *buf,
+			   const snd_pcm_channel_area_t *areas,
+			   snd_pcm_uframes_t offset, unsigned int frames,
+			   unsigned int channels)
+{
+#ifndef DOC_HIDDEN
+#define GET32_LABELS
+#include "plugin_ops.h"
+#undef GET32_LABELS
+#endif /* DOC_HIDDEN */
+	void *get32 = get32_labels[rate->get_idx];
+	const char *src;
+	int32_t sample;
+	const char *srcs[channels];
+	int src_step[channels];
+	unsigned int c;
+
+	for (c = 0; c < channels; c++) {
+		srcs[c] = snd_pcm_channel_area_addr(areas + c, offset);
+		src_step[c] = snd_pcm_channel_area_step(areas + c);
+	}
+
+	while (frames--) {
+		for (c = 0; c < channels; c++) {
+			src = srcs[c];
+			// converting the source format to int32 first
+			goto *get32;
+#ifndef DOC_HIDDEN
+#define GET32_END after_get
+#include "plugin_ops.h"
+#undef GET32_END
+#endif /* DOC_HIDDEN */
+		after_get:
+			// converting to float for the plugin
+			*buf++ = (float_t) sample;
+			srcs[c] += src_step[c];
+		}
+	}
+}
+
+static void convert_from_float(snd_pcm_rate_t *rate, const float_t *buf,
+			     const snd_pcm_channel_area_t *areas,
+			     snd_pcm_uframes_t offset, unsigned int frames,
+			     unsigned int channels)
+{
+#ifndef DOC_HIDDEN
+#define PUT32_LABELS
+#include "plugin_ops.h"
+#undef PUT32_LABELS
+#endif /* DOC_HIDDEN */
+	void *put32 = put32_labels[rate->put_idx];
+	char *dst;
+	int32_t sample;
+	char *dsts[channels];
+	int dst_step[channels];
+	unsigned int c;
+
+	for (c = 0; c < channels; c++) {
+		dsts[c] = snd_pcm_channel_area_addr(areas + c, offset);
+		dst_step[c] = snd_pcm_channel_area_step(areas + c);
+	}
+
+	while (frames--) {
+		for (c = 0; c < channels; c++) {
+			dst = dsts[c];
+			// plugin returned float, converting to int32 first
+			sample = (int32_t) *buf++;
+			// converting int32 to the destination format
+			goto *put32;
+#ifndef DOC_HIDDEN
+#define PUT32_END after_put
+#include "plugin_ops.h"
+#undef PUT32_END
+#endif /* DOC_HIDDEN */
+		after_put:
+			dsts[c] += dst_step[c];
+		}
+	}
+}
 
 static void do_convert(const snd_pcm_channel_area_t *dst_areas,
 		       snd_pcm_uframes_t dst_offset, unsigned int dst_frames,
@@ -511,18 +600,36 @@  static void do_convert(const snd_pcm_channel_area_t *dst_areas,
 		       unsigned int channels,
 		       snd_pcm_rate_t *rate)
 {
-	if (rate->ops.convert_s16) {
+	if (rate->ops.convert_float) {
+		const float_t *src;
+		float_t *dst;
+		if (! rate->src_buf)
+			src = src_areas->addr + src_offset * sizeof(float_t) * channels;
+		else {
+			convert_to_float(rate, rate->src_buf, src_areas, src_offset,
+				       src_frames, channels);
+			src = rate->src_buf;
+		}
+		if (! rate->dst_buf)
+			dst = dst_areas->addr + dst_offset * sizeof(float_t) * channels;
+		else
+			dst = rate->dst_buf;
+		rate->ops.convert_float(rate->obj, dst, dst_frames, src, src_frames);
+		if (dst == rate->dst_buf)
+			convert_from_float(rate, rate->dst_buf, dst_areas, dst_offset,
+					 dst_frames, channels);
+	} else if (rate->ops.convert_s16) {
 		const int16_t *src;
 		int16_t *dst;
 		if (! rate->src_buf)
-			src = src_areas->addr + src_offset * 2 * channels;
+			src = src_areas->addr + src_offset * sizeof(int16_t) * channels;
 		else {
 			convert_to_s16(rate, rate->src_buf, src_areas, src_offset,
 				       src_frames, channels);
 			src = rate->src_buf;
 		}
 		if (! rate->dst_buf)
-			dst = dst_areas->addr + dst_offset * 2 * channels;
+			dst = dst_areas->addr + dst_offset * sizeof(int16_t) * channels;
 		else
 			dst = rate->dst_buf;
 		rate->ops.convert_s16(rate->obj, dst, dst_frames, src, src_frames);
@@ -1416,7 +1523,7 @@  int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name,
 	}
 #endif
 
-	if (! rate->ops.init || ! (rate->ops.convert || rate->ops.convert_s16) ||
+	if (! rate->ops.init || ! (rate->ops.convert || rate->ops.convert_s16 || rate->ops.convert_float) ||
 	    ! rate->ops.input_frames || ! rate->ops.output_frames) {
 		SNDERR("Inproper rate plugin %s initialization", type);
 		snd_pcm_free(pcm);
-- 
1.7.1