Subject: Re: Propose: improve audio driver
To: None <tech-kern@netbsd.org>
From: Tetsuya Isaki <isaki@par.odn.ne.jp>
List: tech-kern
Date: 03/12/2002 00:45:47
----Next_Part(Tue_Mar_12_00:45:20_2002_595)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit

Hi,

I made a bug fix patch for vs(4) driver again for the audio framework
that tamura-san (kent@netbsd.org) has commited days ago.

This patch extends struct audio_params of audio(9).
Newly I added the member `factor_denom' (was called
reduce_factor last time) which is a denominator of `factor'.
It can let sw_code reduce output data size in audio_write().

vs(4) can play mulaw and ulinear8 correctly with this patch.
and I've also tested yds and auich on i386.
---
Tetsuya Isaki <isaki@par.odn.ne.jp / isaki@NetBSD.org>

On Sun, 24 Feb 2002 13:58:35 +0900,
In Propose: improve audio driver,
Tetsuya Isaki <isaki@par.odn.ne.jp> wrote:
> Hi,
> 
> I'd like to introduce the mechanism of factor=1/n to
> struct audio_params for x68k vs(4) driver.
> 
> for details:
> MI audio driver prepares sw_code and factor to be able
> to play 8bit/sampling format with the hardware which can
> play only 16bit/sampling, for example.
> 
> However, so that factor appoints whether data after
> sw_code become several times of original one with integer,
> MI audio driver does not support the case that data after
> sw_code become smaller than original one.
> 
> Because X68k built-in ADPCM is 4bit/samping, originally
> vs (driver for X68k built-in ADPCM; arch/x68k/dev/vs.c)
> can play no format of 8bit/samping and 16bit/sampling format
> with mechanism of existing sw_code and factor.
> 
> # However, due to a bug (see dev/ic/msm6258.c of attached patch)
> # and the easy fix (sample_rate *= 2 in arch/x68k/dev/vs.c of
> # attached patch), current vs(4) driver can play it fortunately
> # under some conditions.
> 
> Here is my proposal. I've improved MI audio driver to use
> factor=1/n and I've fixed a bug of vs(4). I attach the
> patch to this mail. With this patch, vs(4) can play mulaw and
> ulinear8 correctly.  And I think there is no side effect on
> any other audio drivers.  I've tested only yds driver on i386.
> 
> 
> Recording is untouched.
> Please comments.
> 
> Thanks.
> ---
> Tetsuya Isaki <isaki@par.odn.ne.jp / isaki@NetBSD.org>

----Next_Part(Tue_Mar_12_00:45:20_2002_595)--
Content-Type: Text/Plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Content-Disposition: attachment; filename="vs-reduce-20020310.diff"

Index: sys/arch/x68k/dev/vs.c
===================================================================
RCS file: /home/isaki/cvsroot/vs/src/sys/arch/x68k/dev/vs.c,v
retrieving revision 1.1.1.3
retrieving revision 1.15
diff -u -r1.1.1.3 -r1.15
--- sys/arch/x68k/dev/vs.c	2001/12/24 09:31:20	1.1.1.3
+++ sys/arch/x68k/dev/vs.c	2002/03/10 05:55:25	1.15
@@ -389,13 +389,18 @@
 		rate = p->sample_rate;
 		p->sw_code = NULL;
 		p->factor = 1;
+		p->factor_denom = 1;
+		p->hw_precision = 4;
+		p->hw_encoding = AUDIO_ENCODING_ADPCM;
+		DPRINTF(1, ("vs_set_params: encoding=%d, precision=%d\n",
+			p->encoding, p->precision));
 		switch (p->encoding) {
 		case AUDIO_ENCODING_ULAW:
 			if (p->precision != 8)
 				return EINVAL;
 			if (mode == AUMODE_PLAY) {
 				p->sw_code = msm6258_mulaw_to_adpcm;
-				rate = p->sample_rate * 2;
+				p->factor_denom = 2;
 			} else {
 				p->sw_code = msm6258_adpcm_to_mulaw;
 				p->factor = 2;
@@ -407,7 +412,7 @@
 				return EINVAL;
 			if (mode == AUMODE_PLAY) {
 				p->sw_code = msm6258_ulinear8_to_adpcm;
-				rate = p->sample_rate * 2;
+				p->factor_denom = 2;
 			} else {
 				p->sw_code = msm6258_adpcm_to_ulinear8;
 				p->factor = 2;
Index: sys/dev/audio.c
===================================================================
RCS file: /home/isaki/cvsroot/vs/src/sys/dev/audio.c,v
retrieving revision 1.1.1.6
retrieving revision 1.31
diff -u -r1.1.1.6 -r1.31
--- sys/dev/audio.c	2002/03/10 02:51:09	1.1.1.6
+++ sys/dev/audio.c	2002/03/10 05:09:12	1.31
@@ -83,6 +83,7 @@
 #include <sys/device.h>
 
 #include <dev/audio_if.h>
+#include <dev/auconv.h>
 #include <dev/audiovar.h>
 
 #include <machine/endian.h>
@@ -184,7 +185,7 @@
 
 /* The default audio mode: 8 kHz mono ulaw */
 struct audio_params audio_default = 
-	{ 8000, AUDIO_ENCODING_ULAW, 8, 1, 0, 1 };
+	{ 8000, AUDIO_ENCODING_ULAW, 8, 1, 0, 1, 1 };
 
 struct cfattach audio_ca = {
 	sizeof(struct audio_softc), audioprobe, audioattach, 
@@ -1132,7 +1133,7 @@
 	u_char *outp;
 	int error, s, used, cc, n;
 	const struct audio_params *params;
-	int hw_bytes_per_sample;
+	int hw_bits_per_sample;
 
 	if (cb->mmapped)
 		return EINVAL;
@@ -1146,11 +1147,11 @@
 	case AUDIO_ENCODING_SLINEAR_BE:
 	case AUDIO_ENCODING_ULINEAR_LE:
 	case AUDIO_ENCODING_ULINEAR_BE:
-		hw_bytes_per_sample = params->hw_channels * params->precision/8
+		hw_bits_per_sample = params->hw_channels * params->precision
 			* params->factor;
 		break;
 	default:
-		hw_bytes_per_sample = 1 * params->factor;
+		hw_bits_per_sample = 8 * params->factor / params->factor_denom;
 	}
 	error = 0;
 	/*
@@ -1193,7 +1194,7 @@
 	while (uio->uio_resid > 0 && !error) {
 		if (sc->sc_rconvbuffer_end - sc->sc_rconvbuffer_begin <= 0) {
 			s = splaudio();
-			while (cb->used < hw_bytes_per_sample) {
+			while (cb->used < hw_bits_per_sample / 8) {
 				if (!sc->sc_rbus) {
 					error = audiostartr(sc);
 					if (error) {
@@ -1212,7 +1213,7 @@
 					error = EIO;
 				if (error) {
 					splx(s);
-					return (error);
+					return error;
 				}
 			}
 
@@ -1228,11 +1229,11 @@
 			if (cc > sc->sc_rconvbuffer_size)
 				cc = sc->sc_rconvbuffer_size;
 			/* cc must be aligned by the sample size */
-			cc = (cc / hw_bytes_per_sample) * hw_bytes_per_sample;
+			cc = (cc * 8 / hw_bits_per_sample) * hw_bits_per_sample / 8;
 #ifdef DIAGNOSTIC
 			if (cc == 0)
 				printf("audio_read: cc=0 hw_sample_size=%d\n",
-				       hw_bytes_per_sample);
+				       hw_bits_per_sample / 8);
 #endif
 
 			/*
@@ -1276,11 +1277,13 @@
 					       ": cc=%d factor=%d\n", cc,
 					       params->factor);
 #endif
-				cc /= params->factor;
+				cc = cc * params->factor_denom / params->factor;
 #ifdef DIAGNOSTIC
 				if (cc == 0)
-					printf("audio_read: cc=0 factor=%d\n",
-					       params->factor);
+					printf("audio_read: cc=0 "
+					       "factor=%d/%d\n",
+					       params->factor,
+					       params->factor_denom);
 #endif
 				params->sw_code(sc->hw_hdl, sc->sc_rconvbuffer,
 						cc);
@@ -1451,7 +1454,7 @@
 	u_char *inp, *einp;
 	int saveerror, error, s, n, cc, used;
 	struct audio_params *params;
-	int samples, hw_bytes_per_sample, user_bytes_per_sample;
+	int samples, hw_bits_per_sample, user_bits_per_sample;
 	int input_remain, space;
 
 	DPRINTFN(2,("audio_write: sc=%p count=%lu used=%d(hi=%d)\n", 
@@ -1502,27 +1505,27 @@
 	case AUDIO_ENCODING_SLINEAR_BE:
 	case AUDIO_ENCODING_ULINEAR_LE:
 	case AUDIO_ENCODING_ULINEAR_BE:
-		hw_bytes_per_sample = params->hw_channels * params->precision/8
+		hw_bits_per_sample = params->hw_channels * params->precision
 			* params->factor;
-		user_bytes_per_sample = params->channels * params->precision/8;
+		user_bits_per_sample = params->channels * params->precision;
 		break;
 	default:
-		hw_bytes_per_sample = 1 * params->factor;
-		user_bytes_per_sample = 1;
+		hw_bits_per_sample = 8 * params->factor / params->factor_denom;
+		user_bits_per_sample = 8;
 	}
 #ifdef DIAGNOSTIC
-	if (hw_bytes_per_sample > MAX_SAMPLE_SIZE) {
+	if (hw_bits_per_sample > MAX_SAMPLE_SIZE * 8) {
 		printf("audio_write(): Invalid sample size: cur=%d max=%d\n",
-		       hw_bytes_per_sample, MAX_SAMPLE_SIZE);
+		       hw_bits_per_sample / 8, MAX_SAMPLE_SIZE);
 	}
 #endif
 	space = ((params->hw_sample_rate / params->sample_rate) + 1)
-		* hw_bytes_per_sample;
+		* hw_bits_per_sample / 8;
 	error = 0;
 	while ((input_remain = uio->uio_resid + sc->sc_input_fragment_length) > 0
 	       && !error) {
 		s = splaudio();
-		if (input_remain < user_bytes_per_sample) {
+		if (input_remain < user_bits_per_sample / 8) {
 			n = uio->uio_resid;
 			DPRINTF(("audio_write: fragment uiomove length=%d\n", n));
 			error = uiomove(sc->sc_input_fragment
@@ -1559,7 +1562,7 @@
 			cc = cb->end - cb->start;
 
 		/* cc: # of bytes we can write to the ring buffer */
-		samples = cc / hw_bytes_per_sample;
+		samples = cc * 8 / hw_bits_per_sample;
 #ifdef DIAGNOSTIC
 		if (samples == 0)
 			printf("audio_write: samples (cc/hw_bps) == 0\n");
@@ -1572,22 +1575,26 @@
 			       "usedhigh-used=%d cc/hw_bps=%d/%d "
 			       "rate/hw_rate=%ld/%ld space=%d\n",
 			       cb->usedhigh - cb->used, cc,
-			       hw_bytes_per_sample, params->sample_rate,
+			       hw_bits_per_sample / 8, params->sample_rate,
 			       params->hw_sample_rate, space);
 #endif
 		/* samples: # of samples in source data */
-		cc = samples * user_bytes_per_sample;
+		cc = samples * user_bits_per_sample / 8;
 		/* cc: # of bytes in source data */
 		if (input_remain < cc)	/* and no more than we have */
-			cc = (input_remain / user_bytes_per_sample)
-				* user_bytes_per_sample;
+			cc = (input_remain * 8 / user_bits_per_sample)
+				* user_bits_per_sample / 8;
 #ifdef DIAGNOSTIC
 		if (cc == 0)
 			printf("audio_write: cc == 0\n");
 #endif
-		if (cc * params->factor > sc->sc_pconvbuffer_size) {
-			cc = (sc->sc_pconvbuffer_size / params->factor
-			      / user_bytes_per_sample) * user_bytes_per_sample;
+		if (cc * params->factor / params->factor_denom > sc->sc_pconvbuffer_size) {
+			/*
+			 * cc = (pconv / factor / user_bps ) * user_bps
+			 */
+			cc = (sc->sc_pconvbuffer_size * params->factor_denom
+			      * 8 / params->factor / user_bits_per_sample)
+			     * user_bits_per_sample / 8;
 		}
 
 #ifdef DIAGNOSTIC
@@ -1596,8 +1603,11 @@
 		 * block pointers are always nicely aligned. 
 		 */
 		if (cc == 0) {
-			printf("audio_write: cc == 0, swcode=%p, factor=%d\n",
-			       sc->sc_pparams.sw_code, sc->sc_pparams.factor);
+			printf("audio_write: cc == 0, swcode=%p, factor=%d "
+			       "remain=%d u_bps=%d hw_bps=%d\n",
+			       sc->sc_pparams.sw_code, sc->sc_pparams.factor,
+			       input_remain, user_bits_per_sample,
+			       hw_bits_per_sample);
 			cb->copying = 0;
 			return EINVAL;
 		}
@@ -1636,7 +1646,7 @@
 			sc->sc_pparams.sw_code(sc->hw_hdl,
 					       sc->sc_pconvbuffer, cc);
 			/* Adjust count after the expansion. */
-			cc *= sc->sc_pparams.factor;
+			cc = cc * sc->sc_pparams.factor / sc->sc_pparams.factor_denom;
 			DPRINTFN(1, ("audio_write: expanded cc=%d\n", cc));
 		}
 		/*
@@ -2679,6 +2689,7 @@
 		modechange = cleared = 1;
 		rp.sw_code = 0;
 		rp.factor = 1;
+		rp.factor_denom = 1;
 		rp.hw_sample_rate = rp.sample_rate;
 		rp.hw_encoding = rp.encoding;
 		rp.hw_precision = rp.precision;
@@ -2691,6 +2702,7 @@
 		modechange = cleared = 1;
 		pp.sw_code = 0;
 		pp.factor = 1;
+		pp.factor_denom = 1;
 		pp.hw_sample_rate = pp.sample_rate;
 		pp.hw_encoding = pp.encoding;
 		pp.hw_precision = pp.precision;
Index: sys/dev/audio_if.h
===================================================================
RCS file: /home/isaki/cvsroot/vs/src/sys/dev/audio_if.h,v
retrieving revision 1.1.1.3
retrieving revision 1.8
diff -u -r1.1.1.3 -r1.8
--- sys/dev/audio_if.h	2002/03/10 02:51:09	1.1.1.3
+++ sys/dev/audio_if.h	2002/03/10 05:09:12	1.8
@@ -51,6 +51,7 @@
 	/* Software en/decode functions, set if SW coding required by HW */
 	void	(*sw_code)(void *, u_char *, int);
 	int	factor;				/* coding space change */
+	int factor_denom;		/* coding space change smaller */
 	/*
 	 * The following four members represent what format is used in a
 	 * hardware.  If hw_sample_rate != sample_rate || hw_channels !=
Index: sys/dev/ic/msm6258.c
===================================================================
RCS file: /home/isaki/cvsroot/vs/src/sys/dev/ic/msm6258.c,v
retrieving revision 1.1.1.2
retrieving revision 1.4
diff -u -r1.1.1.2 -r1.4
--- sys/dev/ic/msm6258.c	2001/11/17 12:00:28	1.1.1.2
+++ sys/dev/ic/msm6258.c	2002/03/02 14:11:54	1.4
@@ -44,8 +44,8 @@
 #include <sys/audioio.h>
 
 #include <dev/audio_if.h>
-#include <dev/audiovar.h>
 #include <dev/auconv.h>
+#include <dev/audiovar.h>
 #include <dev/mulaw.h>
 #include <dev/ic/msm6258var.h>
 
@@ -135,11 +135,12 @@
 	char *x = &(mc->mc_estim);
 	short *y = &(mc->mc_amp);
 	register int i;
+	u_char *d = p;
 	u_char f;
 
-	for (i = 0; i < cc; i++) {
-		f = pcm2adpcm_step(p[i], y, x);
-		p[i] = f + (pcm2adpcm_step(p[i], y, x) << 4);
+	for (i = 0; i < cc; ) {
+		f = pcm2adpcm_step(p[i++], y, x);
+		*d++ = f + (pcm2adpcm_step(p[i++], y, x) << 4);
 	}
 }
 
Index: sys/dev/ic/msm6258var.h
===================================================================
RCS file: /home/isaki/cvsroot/vs/src/sys/dev/ic/msm6258var.h,v
retrieving revision 1.1.1.1
retrieving revision 1.2
diff -u -r1.1.1.1 -r1.2
--- sys/dev/ic/msm6258var.h	2001/11/04 13:28:49	1.1.1.1
+++ sys/dev/ic/msm6258var.h	2002/03/02 14:11:54	1.2
@@ -34,8 +34,6 @@
  * OKI MSM6258 ADPCM voice synthesizer codec.
  */
 
-#include <dev/auconv.h>
-
 void *msm6258_codec_init (void);
 void msm6258_ulinear8_to_adpcm (void *, u_char *, int);
 void msm6258_mulaw_to_adpcm (void *, u_char *, int);

----Next_Part(Tue_Mar_12_00:45:20_2002_595)----