ALSA: core: Add 384kHz Support

Commit Message

Austin, Brian Jan. 27, 2016, 1:53 p.m. UTC

From: Brian Austin <[e-mail]>

Adding support for 384kHz audio sample rates

Signed-off-by: Brian Austin <[e-mail]>
---
 include/sound/pcm.h     | 3 +++
 sound/core/pcm_native.c | 4 ++--
 2 files changed, 5 insertions(+), 2 deletions(-)

Comments

Clemens Ladisch Jan. 27, 2016, 1:59 p.m. UTC | #1

[e-mail] wrote:
> Adding support for 384kHz audio sample rates

384 kHz already is supported (see the KNOT bit).

>  #define SNDRV_PCM_RATE_96000		(1<<10)		/* 96000Hz */
>  #define SNDRV_PCM_RATE_176400		(1<<11)		/* 176400Hz */
>  #define SNDRV_PCM_RATE_192000		(1<<12)		/* 192000Hz */
> +#define SNDRV_PCM_RATE_384000		(1<<13)		/* 384000Hz */

These bits are shortcuts for often-used rates.
384 kHz is not often used.


Regards,
Clemens

Takashi Iwai Jan. 27, 2016, 2:01 p.m. UTC | #2

On Wed, 27 Jan 2016 14:53:45 +0100,
<[e-mail]> wrote:
> 
> From: Brian Austin <[e-mail]>
> 
> Adding support for 384kHz audio sample rates

The most important information is missing: why this is needed.

(Note that the support of 384kHz itself can be done without this
 addition :) 


Takashi

> 
> Signed-off-by: Brian Austin <[e-mail]>
> ---
>  include/sound/pcm.h     | 3 +++
>  sound/core/pcm_native.c | 4 ++--
>  2 files changed, 5 insertions(+), 2 deletions(-)
> 
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index b0be092..050c0b7 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -129,6 +129,7 @@ struct snd_pcm_ops {
>  #define SNDRV_PCM_RATE_96000		(1<<10)		/* 96000Hz */
>  #define SNDRV_PCM_RATE_176400		(1<<11)		/* 176400Hz */
>  #define SNDRV_PCM_RATE_192000		(1<<12)		/* 192000Hz */
> +#define SNDRV_PCM_RATE_384000		(1<<13)		/* 384000Hz */
>  
>  #define SNDRV_PCM_RATE_CONTINUOUS	(1<<30)		/* continuous range */
>  #define SNDRV_PCM_RATE_KNOT		(1<<31)		/* supports more non-continuos rates */
> @@ -141,6 +142,8 @@ struct snd_pcm_ops {
>  					 SNDRV_PCM_RATE_88200|SNDRV_PCM_RATE_96000)
>  #define SNDRV_PCM_RATE_8000_192000	(SNDRV_PCM_RATE_8000_96000|SNDRV_PCM_RATE_176400|\
>  					 SNDRV_PCM_RATE_192000)
> +#define SNDRV_PCM_RATE_8000_384000	(SNDRV_PCM_RATE_8000_192000|SNDRV_PCM_RATE_384000)
> +
>  #define _SNDRV_PCM_FMTBIT(fmt)		(1ULL << (__force int)SNDRV_PCM_FORMAT_##fmt)
>  #define SNDRV_PCM_FMTBIT_S8		_SNDRV_PCM_FMTBIT(S8)
>  #define SNDRV_PCM_FMTBIT_U8		_SNDRV_PCM_FMTBIT(U8)
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index a8b27cd..2e10c3f 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -1969,12 +1969,12 @@ static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params,
>  	return snd_interval_refine(hw_param_interval(params, rule->var), &t);
>  }
>  
> -#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12
> +#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_384000 != 1 << 13
>  #error "Change this table"
>  #endif
>  
>  static unsigned int rates[] = { 5512, 8000, 11025, 16000, 22050, 32000, 44100,
> -                                 48000, 64000, 88200, 96000, 176400, 192000 };
> +			48000, 64000, 88200, 96000, 176400, 192000, 384000 };
>  
>  const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
>  	.count = ARRAY_SIZE(rates),
> -- 
> 1.9.1
>

Austin, Brian Jan. 27, 2016, 2:11 p.m. UTC | #3

On Wed, 27 Jan 2016, Takashi Iwai wrote:

> On Wed, 27 Jan 2016 14:53:45 +0100,
> <[e-mail]> wrote:
> > 
> > From: Brian Austin <[e-mail]>
> > 
> > Adding support for 384kHz audio sample rates
> 
> The most important information is missing: why this is needed.
So I can specify that I want 384kHz. 
> 
> (Note that the support of 384kHz itself can be done without this
>  addition :) 
> 
So we stay at SNDRV_PCM_RATE_192000 always for sample rate defines?
And everything else is just covered with SNDRV_PCM_RATE_KNOT?

Austin, Brian Jan. 27, 2016, 2:16 p.m. UTC | #4

On Wed, 27 Jan 2016, Brian Austin wrote:

> On Wed, 27 Jan 2016, Takashi Iwai wrote:
> 
> > On Wed, 27 Jan 2016 14:53:45 +0100,
> > <[e-mail]> wrote:
> > > 
> > > From: Brian Austin <[e-mail]>
> > > 
> > > Adding support for 384kHz audio sample rates
> > 
> > The most important information is missing: why this is needed.
> So I can specify that I want 384kHz. 
> > 
> > (Note that the support of 384kHz itself can be done without this
> >  addition :) 
> > 
> So we stay at SNDRV_PCM_RATE_192000 always for sample rate defines?
> And everything else is just covered with SNDRV_PCM_RATE_KNOT? 
> 
If this is the case then we have to have a contraint in the driver to 
support higher rates and leave others out? Is that understanding correct?

Takashi Iwai Jan. 27, 2016, 2:36 p.m. UTC | #5

On Wed, 27 Jan 2016 15:11:10 +0100,
Brian Austin wrote:
> 
> On Wed, 27 Jan 2016, Takashi Iwai wrote:
> 
> > On Wed, 27 Jan 2016 14:53:45 +0100,
> > <[e-mail]> wrote:
> > > 
> > > From: Brian Austin <[e-mail]>
> > > 
> > > Adding support for 384kHz audio sample rates
> > 
> > The most important information is missing: why this is needed.
> So I can specify that I want 384kHz. 
> > 
> > (Note that the support of 384kHz itself can be done without this
> >  addition :) 
> > 
> So we stay at SNDRV_PCM_RATE_192000 always for sample rate defines?
> And everything else is just covered with SNDRV_PCM_RATE_KNOT? 

Depends.  If a new sample rate is (or will be) demanded by many
drivers, it's worth to add it, of course, as it would simplify the
code.

That's why I mentioned "why" is most important information; you need
to convince others about the necessity of this change, after all.
Then you see that "because I-wanna-it" doesn't sound convincing
enough, right?


Takashi

Austin, Brian Jan. 27, 2016, 3:02 p.m. UTC | #6

On Wed, 27 Jan 2016, Takashi Iwai wrote:

> > So we stay at SNDRV_PCM_RATE_192000 always for sample rate defines?
> > And everything else is just covered with SNDRV_PCM_RATE_KNOT? 
> 
> Depends.  If a new sample rate is (or will be) demanded by many
> drivers, it's worth to add it, of course, as it would simplify the
> code.
> 
> That's why I mentioned "why" is most important information; you need
> to convince others about the necessity of this change, after all.
> Then you see that "because I-wanna-it" doesn't sound convincing
> enough, right?
> 
> 
> Takashi
> 
I really, really, really want it :)

I understand. Going forward, from our perspective, 384 and 
other high sample rates are going to be defaults for devices as the market 
is moving that way. I just wanted to make it easier to use those instead 
of doing all the contraint coding. Now my understanding on the KNOT define 
may be wrong. I can add the other rates to this, but for now just wanted 
to add one we use currently.

Does that make sense?

Takashi Iwai Jan. 27, 2016, 3:51 p.m. UTC | #7

On Wed, 27 Jan 2016 16:02:35 +0100,
Brian Austin wrote:
> 
> On Wed, 27 Jan 2016, Takashi Iwai wrote:
> 
> > > So we stay at SNDRV_PCM_RATE_192000 always for sample rate defines?
> > > And everything else is just covered with SNDRV_PCM_RATE_KNOT? 
> > 
> > Depends.  If a new sample rate is (or will be) demanded by many
> > drivers, it's worth to add it, of course, as it would simplify the
> > code.
> > 
> > That's why I mentioned "why" is most important information; you need
> > to convince others about the necessity of this change, after all.
> > Then you see that "because I-wanna-it" doesn't sound convincing
> > enough, right?
> > 
> > 
> > Takashi
> > 
> I really, really, really want it :)
> 
> I understand. Going forward, from our perspective, 384 and 
> other high sample rates are going to be defaults for devices as the market 
> is moving that way. I just wanted to make it easier to use those instead 
> of doing all the contraint coding.

Yeah that's the reason I could *guess*, but it wasn't mentioned.

> Now my understanding on the KNOT define 
> may be wrong. I can add the other rates to this, but for now just wanted 
> to add one we use currently.
> 
> Does that make sense? 

Just resubmit the patch with mo' better advertisement :)


Takashi

Lars-Peter Clausen Jan. 27, 2016, 4:55 p.m. UTC | #8

On 01/27/2016 04:51 PM, Takashi Iwai wrote:
> On Wed, 27 Jan 2016 16:02:35 +0100,
> Brian Austin wrote:
>>
>> On Wed, 27 Jan 2016, Takashi Iwai wrote:
>>
>>>> So we stay at SNDRV_PCM_RATE_192000 always for sample rate defines?
>>>> And everything else is just covered with SNDRV_PCM_RATE_KNOT? 
>>>
>>> Depends.  If a new sample rate is (or will be) demanded by many
>>> drivers, it's worth to add it, of course, as it would simplify the
>>> code.
>>>
>>> That's why I mentioned "why" is most important information; you need
>>> to convince others about the necessity of this change, after all.
>>> Then you see that "because I-wanna-it" doesn't sound convincing
>>> enough, right?
>>>
>>>
>>> Takashi
>>>
>> I really, really, really want it :)
>>
>> I understand. Going forward, from our perspective, 384 and 
>> other high sample rates are going to be defaults for devices as the market 
>> is moving that way. I just wanted to make it easier to use those instead 
>> of doing all the contraint coding.
> 
> Yeah that's the reason I could *guess*, but it wasn't mentioned.
> 
>> Now my understanding on the KNOT define 
>> may be wrong. I can add the other rates to this, but for now just wanted 
>> to add one we use currently.
>>
>> Does that make sense? 
> 
> Just resubmit the patch with mo' better advertisement :)

For symmetry reasons maybe also include the matching 44.1kHz based rate.
Unless you think that is not going to be a thing.

Charles Keepax Jan. 27, 2016, 5:44 p.m. UTC | #9

On Wed, Jan 27, 2016 at 05:55:40PM +0100, Lars-Peter Clausen wrote:
> On 01/27/2016 04:51 PM, Takashi Iwai wrote:
> > On Wed, 27 Jan 2016 16:02:35 +0100,
> > Brian Austin wrote:
> >>
> >> On Wed, 27 Jan 2016, Takashi Iwai wrote:
> >>
> >>>> So we stay at SNDRV_PCM_RATE_192000 always for sample rate defines?
> >>>> And everything else is just covered with SNDRV_PCM_RATE_KNOT? 
> >>>
> >>> Depends.  If a new sample rate is (or will be) demanded by many
> >>> drivers, it's worth to add it, of course, as it would simplify the
> >>> code.
> >>>
> >>> That's why I mentioned "why" is most important information; you need
> >>> to convince others about the necessity of this change, after all.
> >>> Then you see that "because I-wanna-it" doesn't sound convincing
> >>> enough, right?
> >>>
> >>>
> >>> Takashi
> >>>
> >> I really, really, really want it :)
> >>
> >> I understand. Going forward, from our perspective, 384 and 
> >> other high sample rates are going to be defaults for devices as the market 
> >> is moving that way. I just wanted to make it easier to use those instead 
> >> of doing all the contraint coding.
> > 
> > Yeah that's the reason I could *guess*, but it wasn't mentioned.
> > 
> >> Now my understanding on the KNOT define 
> >> may be wrong. I can add the other rates to this, but for now just wanted 
> >> to add one we use currently.
> >>
> >> Does that make sense? 
> > 
> > Just resubmit the patch with mo' better advertisement :)
> 
> For symmetry reasons maybe also include the matching 44.1kHz based rate.
> Unless you think that is not going to be a thing.

Everything I have seen using >192k sample rates has been using
the 48k based rate families. My guess would be that the matching
44.1k rate is unlikely to be a thing.

Thanks,
Charles

Patch

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index b0be092..050c0b7 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -129,6 +129,7 @@  struct snd_pcm_ops {
 #define SNDRV_PCM_RATE_96000		(1<<10)		/* 96000Hz */
 #define SNDRV_PCM_RATE_176400		(1<<11)		/* 176400Hz */
 #define SNDRV_PCM_RATE_192000		(1<<12)		/* 192000Hz */
+#define SNDRV_PCM_RATE_384000		(1<<13)		/* 384000Hz */
 
 #define SNDRV_PCM_RATE_CONTINUOUS	(1<<30)		/* continuous range */
 #define SNDRV_PCM_RATE_KNOT		(1<<31)		/* supports more non-continuos rates */
@@ -141,6 +142,8 @@  struct snd_pcm_ops {
 					 SNDRV_PCM_RATE_88200|SNDRV_PCM_RATE_96000)
 #define SNDRV_PCM_RATE_8000_192000	(SNDRV_PCM_RATE_8000_96000|SNDRV_PCM_RATE_176400|\
 					 SNDRV_PCM_RATE_192000)
+#define SNDRV_PCM_RATE_8000_384000	(SNDRV_PCM_RATE_8000_192000|SNDRV_PCM_RATE_384000)
+
 #define _SNDRV_PCM_FMTBIT(fmt)		(1ULL << (__force int)SNDRV_PCM_FORMAT_##fmt)
 #define SNDRV_PCM_FMTBIT_S8		_SNDRV_PCM_FMTBIT(S8)
 #define SNDRV_PCM_FMTBIT_U8		_SNDRV_PCM_FMTBIT(U8)
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index a8b27cd..2e10c3f 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1969,12 +1969,12 @@  static int snd_pcm_hw_rule_sample_bits(struct snd_pcm_hw_params *params,
 	return snd_interval_refine(hw_param_interval(params, rule->var), &t);
 }
 
-#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_192000 != 1 << 12
+#if SNDRV_PCM_RATE_5512 != 1 << 0 || SNDRV_PCM_RATE_384000 != 1 << 13
 #error "Change this table"
 #endif
 
 static unsigned int rates[] = { 5512, 8000, 11025, 16000, 22050, 32000, 44100,
-                                 48000, 64000, 88200, 96000, 176400, 192000 };
+			48000, 64000, 88200, 96000, 176400, 192000, 384000 };
 
 const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
 	.count = ARRAY_SIZE(rates),