struct _snd_pcm_hardware

struct _snd_pcm_hardware

[Guilhem Tardy]

Hi,

I have a problem with supporting multiple audio formats in that my hardware
defines its buffers in a number of frames rather than bytes.

I evaluated the changes required to support description of a hardware's period
contraints in frame AND bytes. Essentially, this would mean adding 'size_t
period_size_min' and 'size_t period_size_max' to the stucture _snd_pcm_hardware
in pcm.h, and then add a constraint in snd_pcm_hw_constraints_complete() in
pcm_native.c such that the period size is within those min and max values.
Other drivers wouldn't be affected if the period_size_min/max default values
are properly chosen (i.e. 0 and MAX_UINT, respectively).

Would you consider such a change in the ALSA code base?

Guilhem.


__________________________________________________
Do You Yahoo!?
Yahoo! Finance - Get real-time stock quotes
http://finance.yahoo.com


-------------------------------------------------------
This sf.net email is sponsored by:ThinkGeek
Welcome to geek heaven.
http://thinkgeek.com/sf

Re: struct _snd_pcm_hardware

[Jaroslav Kysela]

On Thu, 29 Aug 2002, Guilhem Tardy wrote:

> Hi,
> 
> I have a problem with supporting multiple audio formats in that my hardware
> defines its buffers in a number of frames rather than bytes.
> 
> I evaluated the changes required to support description of a hardware's
> period contraints in frame AND bytes. Essentially, this would mean
> adding 'size_t period_size_min' and 'size_t period_size_max' to the
> stucture _snd_pcm_hardware in pcm.h, and then add a constraint in
> snd_pcm_hw_constraints_complete() in pcm_native.c such that the period
> size is within those min and max values. Other drivers wouldn't be
> affected if the period_size_min/max default values are properly chosen
> (i.e. 0 and MAX_UINT, respectively).
> 
> Would you consider such a change in the ALSA code base?

Wouldn't be better to add such constraint to your code in open() 
callback? This constraint is not used frequently.

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
Linux Kernel Sound Maintainer
ALSA Project  http://www.alsa-project.org
SuSE Linux    http://www.suse.com



-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone?  Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390

snd_pcm_hardware_t NOT pre-initialized

[Guilhem Tardy]

[-- Attachment #1: Type: text/plain, Size: 533 bytes --]

Hi,

snd_pcm_hardware_t is NOT pre-initialized, and drivers are responsible to
define ALL fields necessary. This is too bad, because it essentially means that
a new constraint (i.e. period_size_min + period_size_max) will need to be
supported by all drivers.

I have tested successfully such a new constraint (patch in attachment), and
hope that you'll incorporate it in the cvs anyway.

Guilhem.


__________________________________________________
Do You Yahoo!?
Yahoo! Finance - Get real-time stock quotes
http://finance.yahoo.com

[-- Attachment #2: alsa_rc3.diff --]
[-- Type: text/plain, Size: 6559 bytes --]

--- alsa-kernel/include_orig/asound.h	Wed Jun 26 14:07:13 2002
+++ alsa-kernel/include/asound.h	Thu Sep  5 12:08:02 2002
@@ -190,8 +191,15 @@
 	SNDRV_PCM_FORMAT_IMA_ADPCM,
 	SNDRV_PCM_FORMAT_MPEG,
 	SNDRV_PCM_FORMAT_GSM,
-	SNDRV_PCM_FORMAT_SPECIAL = 31,
-	SNDRV_PCM_FORMAT_S24_3LE = 32,	/* in three bytes */
+	SNDRV_PCM_FORMAT_G723_5_3KBPS,
+	SNDRV_PCM_FORMAT_G723_6_3KBPS,
+	SNDRV_PCM_FORMAT_G726_16KBPS,
+	SNDRV_PCM_FORMAT_G726_24KBPS,
+	SNDRV_PCM_FORMAT_G726_32KBPS,
+	SNDRV_PCM_FORMAT_G726_40KBPS,
+	SNDRV_PCM_FORMAT_G729,
+	SNDRV_PCM_FORMAT_SPECIAL = 47,
+	SNDRV_PCM_FORMAT_S24_3LE = 48,	/* in three bytes */
 	SNDRV_PCM_FORMAT_S24_3BE,	/* in three bytes */
 	SNDRV_PCM_FORMAT_U24_3LE,	/* in three bytes */
 	SNDRV_PCM_FORMAT_U24_3BE,	/* in three bytes */
--- alsa-kernel/include_orig/pcm.h	Tue Aug  6 03:28:27 2002
+++ alsa-kernel/include/pcm.h	Wed Sep  4 13:32:19 2002
@@ -75,6 +75,8 @@
 	unsigned int channels_min;	/* min channels */
 	unsigned int channels_max;	/* max channels */
 	size_t buffer_bytes_max;	/* max buffer size */
+	size_t period_size_min;		/* min size size (in samples) */
+	size_t period_size_max;		/* max size size (in samples) */
 	size_t period_bytes_min;	/* min period size */
 	size_t period_bytes_max;	/* max period size */
 	unsigned int periods_min;	/* min # of periods */
@@ -176,6 +178,13 @@
 #define SNDRV_PCM_FMTBIT_IMA_ADPCM	(1ULL << SNDRV_PCM_FORMAT_IMA_ADPCM)
 #define SNDRV_PCM_FMTBIT_MPEG		(1ULL << SNDRV_PCM_FORMAT_MPEG)
 #define SNDRV_PCM_FMTBIT_GSM		(1ULL << SNDRV_PCM_FORMAT_GSM)
+#define SNDRV_PCM_FMTBIT_G723_5_3KBPS	(1ULL << SNDRV_PCM_FORMAT_G723_5_3KBPS)
+#define SNDRV_PCM_FMTBIT_G723_6_3KBPS	(1ULL << SNDRV_PCM_FORMAT_G723_5_3KBPS)
+#define SNDRV_PCM_FMTBIT_G726_16KBPS	(1ULL << SNDRV_PCM_FORMAT_G726_16KBPS)
+#define SNDRV_PCM_FMTBIT_G726_24KBPS	(1ULL << SNDRV_PCM_FORMAT_G726_24KBPS)
+#define SNDRV_PCM_FMTBIT_G726_32KBPS	(1ULL << SNDRV_PCM_FORMAT_G726_32KBPS)
+#define SNDRV_PCM_FMTBIT_G726_40KBPS	(1ULL << SNDRV_PCM_FORMAT_G726_40KBPS)
+#define SNDRV_PCM_FMTBIT_G729		(1ULL << SNDRV_PCM_FORMAT_G729)
 #define SNDRV_PCM_FMTBIT_SPECIAL	(1ULL << SNDRV_PCM_FORMAT_SPECIAL)
 #define SNDRV_PCM_FMTBIT_S24_3LE	(1ULL << SNDRV_PCM_FORMAT_S24_3LE)
 #define SNDRV_PCM_FMTBIT_U24_3LE	(1ULL << SNDRV_PCM_FORMAT_U24_3LE)
--- alsa-kernel/core_orig/pcm.c	Tue Aug 13 12:13:34 2002
+++ alsa-kernel/core/pcm.c	Thu Aug 22 13:35:40 2002
@@ -183,6 +183,13 @@
 	FORMAT(IMA_ADPCM),
 	FORMAT(MPEG),
 	FORMAT(GSM),
+	FORMAT(G723_5_3KBPS),
+	FORMAT(G723_6_3KBPS),
+	FORMAT(G726_16KBPS),
+	FORMAT(G726_24KBPS),
+	FORMAT(G726_32KBPS),
+	FORMAT(G726_40KBPS),
+	FORMAT(G729),
 	FORMAT(SPECIAL),
 	FORMAT(S24_3LE),
 	FORMAT(S24_3BE),
--- alsa-kernel/core_orig/pcm_misc.c	Mon Aug 12 04:43:45 2002
+++ alsa-kernel/core/pcm_misc.c	Fri Aug 23 08:50:20 2002
@@ -188,6 +188,20 @@
 		return 8;
 	case SNDRV_PCM_FORMAT_IMA_ADPCM:
 		return 4;
+	case SNDRV_PCM_FORMAT_G723_5_3KBPS:
+		return 160;
+	case SNDRV_PCM_FORMAT_G723_6_3KBPS:
+		return 192;
+	case SNDRV_PCM_FORMAT_G726_16KBPS:
+		return 2;
+	case SNDRV_PCM_FORMAT_G726_24KBPS:
+		return 3;
+	case SNDRV_PCM_FORMAT_G726_32KBPS:
+		return 4;
+	case SNDRV_PCM_FORMAT_G726_40KBPS:
+		return 5;
+	case SNDRV_PCM_FORMAT_G729:
+		return 80;
 	default:
 		return -EINVAL;
 	}
@@ -238,6 +252,20 @@
 		return 8;
 	case SNDRV_PCM_FORMAT_IMA_ADPCM:
 		return 4;
+	case SNDRV_PCM_FORMAT_G723_5_3KBPS:
+		return 160;
+	case SNDRV_PCM_FORMAT_G723_6_3KBPS:
+		return 192;
+	case SNDRV_PCM_FORMAT_G726_16KBPS:
+		return 2;
+	case SNDRV_PCM_FORMAT_G726_24KBPS:
+		return 3;
+	case SNDRV_PCM_FORMAT_G726_32KBPS:
+		return 4;
+	case SNDRV_PCM_FORMAT_G726_40KBPS:
+		return 5;
+	case SNDRV_PCM_FORMAT_G729:
+		return 80;
 	default:
 		return -EINVAL;
 	}
@@ -291,6 +319,34 @@
 		if (samples & 1)
 			return -EINVAL;
 		return samples / 2;
+	case SNDRV_PCM_FORMAT_G723_5_3KBPS:
+		if (samples % 240)
+			return -EINVAL;
+		return (samples / 240) * 160;
+	case SNDRV_PCM_FORMAT_G723_6_3KBPS:
+		if (samples % 240)
+			return -EINVAL;
+		return (samples / 240) * 192;
+	case SNDRV_PCM_FORMAT_G726_16KBPS:
+		if (samples & 3)
+			return -EINVAL;
+		return samples / 4;
+	case SNDRV_PCM_FORMAT_G726_24KBPS:
+		if (samples & 7)
+			return -EINVAL;
+		return (samples * 3) / 8;
+	case SNDRV_PCM_FORMAT_G726_32KBPS:
+		if (samples & 1)
+			return -EINVAL;
+		return samples / 2;
+	case SNDRV_PCM_FORMAT_G726_40KBPS:
+		if (samples & 7)
+			return -EINVAL;
+		return (samples * 5) / 8;
+	case SNDRV_PCM_FORMAT_G729:
+		if (samples % 80)
+			return -EINVAL;
+		return samples / 8;
 	default:
 		return -EINVAL;
 	}
@@ -428,6 +484,13 @@
 	case SNDRV_PCM_FORMAT_IMA_ADPCM:	/* special case */
 	case SNDRV_PCM_FORMAT_MPEG:
 	case SNDRV_PCM_FORMAT_GSM:
+	case SNDRV_PCM_FORMAT_G723_5_3KBPS:
+	case SNDRV_PCM_FORMAT_G723_6_3KBPS:
+	case SNDRV_PCM_FORMAT_G726_16KBPS:
+	case SNDRV_PCM_FORMAT_G726_24KBPS:
+	case SNDRV_PCM_FORMAT_G726_32KBPS:
+	case SNDRV_PCM_FORMAT_G726_40KBPS:
+	case SNDRV_PCM_FORMAT_G729:
 	case SNDRV_PCM_FORMAT_SPECIAL:
 		return 0;
 	default:
@@ -456,12 +519,39 @@
 	if (samples == 0)
 		return 0;
 	switch (snd_pcm_format_width(format)) {
+	case 2: {
+		u_int8_t silence = snd_pcm_format_silence_64(format);
+		unsigned int samples1;
+		if (samples & 3)
+			return -EINVAL;
+		samples1 = samples / 4;
+		memset(data, silence, samples1);
+		break;
+	}
+	case 3: {
+		u_int8_t silence = snd_pcm_format_silence_64(format);
+		unsigned int samples1;
+		if (samples & 7)
+			return -EINVAL;
+		samples1 = (samples * 3) / 8;
+		memset(data, silence, samples1);
+		break;
+	}
 	case 4: {
 		u_int8_t silence = snd_pcm_format_silence_64(format);
 		unsigned int samples1;
-		if (samples % 2 != 0)
+		if (samples & 1)
 			return -EINVAL;
 		samples1 = samples / 2;
+		memset(data, silence, samples1);
+		break;
+	}
+	case 5: {
+		u_int8_t silence = snd_pcm_format_silence_64(format);
+		unsigned int samples1;
+		if (samples & 7)
+			return -EINVAL;
+		samples1 = (samples * 5) / 8;
 		memset(data, silence, samples1);
 		break;
 	}
--- alsa-kernel/core_orig/pcm_native.c	Tue Aug 13 12:13:34 2002
+++ alsa-kernel/core/pcm_native.c	Wed Sep  4 13:36:41 2002
@@ -1628,6 +1628,10 @@
 					   hw->rate_min, hw->rate_max);
 	snd_assert(err >= 0, return -EINVAL);
 
+	err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
+					   hw->period_size_min, hw->period_size_max);
+	snd_assert(err >= 0, return -EINVAL);
+
 	err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_BYTES,
 					   hw->period_bytes_min, hw->period_bytes_max);
 	snd_assert(err >= 0, return -EINVAL);

Re: snd_pcm_hardware_t NOT pre-initialized

[Takashi Iwai]

At Fri, 6 Sep 2002 09:28:01 -0700 (PDT),
Guilhem Tardy wrote:
> 
> Hi,
> 
> snd_pcm_hardware_t is NOT pre-initialized, and drivers are responsible to
> define ALL fields necessary. This is too bad, because it essentially means that
> a new constraint (i.e. period_size_min + period_size_max) will need to be
> supported by all drivers.

well, but drivers supporting G72x are very rare.
then why bother to change the common code to affect every driver?

> 
> I have tested successfully such a new constraint (patch in attachment), and
> hope that you'll incorporate it in the cvs anyway.

it'd be better to add the constraint only if the period_size_min and
max are non-zero, since most of drivers do NOT want to use this
constraint.
the similar should be applied to the period_bytes constraint if we
add period_size constraint, because they may conflict with each
other.


ciao,

Takashi


-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone?  Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390

Re: snd_pcm_hardware_t NOT pre-initialized

[Guilhem Tardy]

> > snd_pcm_hardware_t is NOT pre-initialized, and drivers are responsible to
> > define ALL fields necessary. This is too bad, because it essentially means
> > that a new constraint (i.e. period_size_min + period_size_max) will need
> > to be supported by all drivers.
> 
> well, but drivers supporting G72x are very rare.
> then why bother to change the common code to affect every driver?

I sent a consolidated patch with the G.72x stuff of last week and the
constraint problem of today. Both matters are unrelated, and I apologize for
the inconvenience.

Please note that the G.72x stuff has no effect on existing drivers, and
therefore I would hope that it is accepted as well (rather than me maintaining
it separately).

> > I have tested successfully such a new constraint (patch in attachment), and
> > hope that you'll incorporate it in the cvs anyway.
> 
> it'd be better to add the constraint only if the period_size_min and
> max are non-zero, since most of drivers do NOT want to use this
> constraint.
> the similar should be applied to the period_bytes constraint if we
> add period_size constraint, because they may conflict with each
> other.

Agreed. I'll change that and send the patch (for the constraint stuff ONLY ;)
to you directly. Note this assumes that the compiler automatically sets
undefined members of the structure to zero.

Cheers!
Guilhem.


__________________________________________________
Do You Yahoo!?
Yahoo! Finance - Get real-time stock quotes
http://finance.yahoo.com


-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone?  Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390

Re: snd_pcm_hardware_t NOT pre-initialized

[Jaroslav Kysela]

On Fri, 6 Sep 2002, Guilhem Tardy wrote:

> Hi,
> 
> snd_pcm_hardware_t is NOT pre-initialized, and drivers are responsible to
> define ALL fields necessary. This is too bad, because it essentially means that
> a new constraint (i.e. period_size_min + period_size_max) will need to be
> supported by all drivers.
> 
> I have tested successfully such a new constraint (patch in attachment), and
> hope that you'll incorporate it in the cvs anyway.

I don't think that we need such contraint in the midlevel code. Please,
add it only to your driver to open() callback. Anyway, it's not too good
to change numbers for formats. BTW: Can compression algorithms be used
only for specific sample rates? Otherwise, I don't see any reason to put
KBPS to format name.

						Jaroslav

-----
Jaroslav Kysela <perex@suse.cz>
SuSE Linux    http://www.suse.com
ALSA project  http://www.alsa-project.org




-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone?  Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390

updated patch for period_size constraint

[Guilhem Tardy]

[-- Attachment #1: Type: text/plain, Size: 445 bytes --]

Hi all,

I modified my previous patch for supporting a period_size (rather than
period_bytes) constraint with Takashi's suggestion that the fields be non-zero
for either constraint to kick in. See file in attachment. It has worked fine on
my hardware for the past few days.

Guilhem.


__________________________________________________
Yahoo! - We Remember
9-11: A tribute to the more than 3,000 lives lost
http://dir.remember.yahoo.com/tribute

[-- Attachment #2: alsa-driver-rc3.diff --]
[-- Type: text/plain, Size: 1355 bytes --]

--- alsa-kernel/include_orig/pcm.h	Tue Aug  6 03:28:27 2002
+++ alsa-kernel/include/pcm.h	Wed Sep  4 13:32:19 2002
@@ -75,6 +75,8 @@
 	unsigned int channels_min;	/* min channels */
 	unsigned int channels_max;	/* max channels */
 	size_t buffer_bytes_max;	/* max buffer size */
+	size_t period_size_min;		/* min size size (in samples) */
+	size_t period_size_max;		/* max size size (in samples) */
 	size_t period_bytes_min;	/* min period size */
 	size_t period_bytes_max;	/* max period size */
 	unsigned int periods_min;	/* min # of periods */
--- alsa-kernel/core_orig/pcm_native.c	Tue Aug 13 12:13:34 2002
+++ alsa-kernel/core/pcm_native.c	Fri Sep  6 12:52:44 2002
@@ -1628,9 +1628,17 @@
 					   hw->rate_min, hw->rate_max);
 	snd_assert(err >= 0, return -EINVAL);
 
+	if (hw->period_size_min || hw->period_size_max) {
+		err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
+						   hw->period_size_min, hw->period_size_max);
+		snd_assert(err >= 0, return -EINVAL);
+	}
+
+	if (hw->period_bytes_min || hw->period_bytes_max) {
 	err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_BYTES,
 					   hw->period_bytes_min, hw->period_bytes_max);
 	snd_assert(err >= 0, return -EINVAL);
+	}
 
 	err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIODS,
 					   hw->periods_min, hw->periods_max);

Re: updated patch for period_size constraint

[Takashi Iwai]

Hi Guilhem,

At Tue, 10 Sep 2002 08:29:59 -0700 (PDT),
Guilhem Tardy wrote:
> 
> Hi all,
> 
> I modified my previous patch for supporting a period_size (rather than
> period_bytes) constraint with Takashi's suggestion that the fields be non-zero
> for either constraint to kick in. See file in attachment. It has worked fine on
> my hardware for the past few days.

you don't need to modify the common code.
my intention was not to change the common code, too, unless other
drivers also need this constraint.

just add the constraint in open callback, such like

int your_pcm_open(snd_pcm_substream_t *substream)
{
	...

	err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
					   your_period_size_min, your_period_size_max);
	...
}

this is the cleanest solution.


ciao,

Takashi


-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone?  Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390

Re: updated patch for period_size constraint

[Guilhem Tardy]

> > I modified my previous patch for supporting a period_size (rather than
> > period_bytes) constraint with Takashi's suggestion that the fields be
> > non-zero for either constraint to kick in. See file in attachment.
> > It has worked fine on my hardware for the past few days.
> 
> you don't need to modify the common code.
> my intention was not to change the common code, too, unless other
> drivers also need this constraint.

I got you wrong, sorry.

> just add the constraint in open callback, such like
> 
> int your_pcm_open(snd_pcm_substream_t *substream)
> {
> ...
> 
> err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
> 			   your_period_size_min, your_period_size_max);
> ...
> }
> 
> this is the cleanest solution.

Sure, but then I also need to define .period_bytes_min and .period_bytes_max in
my snd_pcm_hardware_t, right? This is quite feasible, indeed.

Guilhem.


__________________________________________________
Yahoo! - We Remember
9-11: A tribute to the more than 3,000 lives lost
http://dir.remember.yahoo.com/tribute


-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone?  Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390

Re: updated patch for period_size constraint

[Takashi Iwai]

At Tue, 10 Sep 2002 10:06:31 -0700 (PDT),
Guilhem Tardy wrote:
> 
> > just add the constraint in open callback, such like
> > 
> > int your_pcm_open(snd_pcm_substream_t *substream)
> > {
> > ...
> > 
> > err = snd_pcm_hw_constraint_minmax(runtime, SNDRV_PCM_HW_PARAM_PERIOD_SIZE,
> > 			   your_period_size_min, your_period_size_max);
> > ...
> > }
> > 
> > this is the cleanest solution.
> 
> Sure, but then I also need to define .period_bytes_min and .period_bytes_max in
> my snd_pcm_hardware_t, right? This is quite feasible, indeed.

yes.  they can be 0 and MAX_INT, so that they never restrict the
condition, if you don't want these parameters affecting the behavior.
(this is not tested but should work ;)


Takashi


-------------------------------------------------------
This sf.net email is sponsored by: OSDN - Tired of that same old
cell phone?  Get a new here for FREE!
https://www.inphonic.com/r.asp?r=sourceforge1&refcode1=vs3390