RFC: support for 12 & 24Khz

RFC: support for 12 & 24Khz

[Vinod Koul]

Hey Takashi,

For compressed audio we also need to support the PCM rates of 12 and 24KHz.

Looking at pcm.h these are not defined so we can simply add them at the end.
But am worried about wider impact of adding these rates.

Can you let me know if more is required to be done or below is fine

diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index 84b10f9..e418d8d 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -126,6 +126,8 @@ 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_12000           <1<<13>         /* 12000Hz */
+#define SNDRV_PCM_RATE_24000           <1<<14>         /* 24000Hz */

 #define SNDRV_PCM_RATE_CONTINUOUS      (1<<30)         /* continuous range */
 #define SNDRV_PCM_RATE_KNOT            (1<<31)         /* supports more
non-continuos rates */
diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
index a68d4c6..42600b0 100644
--- a/sound/core/pcm_native.c
+++ b/sound/core/pcm_native.c
@@ -1779,12 +1779,13 @@ 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_24000 != 1 << 14
 #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,
+                                 12000, 24000 };

 const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
        .count = ARRAY_SIZE(rates),

~Vinod
-- 

Re: RFC: support for 12 & 24Khz

[David Henningsson]

On 07/24/2013 07:26 PM, Vinod Koul wrote:
> Hey Takashi,
>
> For compressed audio we also need to support the PCM rates of 12 and 24KHz.
>
> Looking at pcm.h these are not defined so we can simply add them at the end.
> But am worried about wider impact of adding these rates.
>
> Can you let me know if more is required to be done or below is fine
>
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index 84b10f9..e418d8d 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -126,6 +126,8 @@ 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_12000           <1<<13>         /* 12000Hz */
> +#define SNDRV_PCM_RATE_24000           <1<<14>         /* 24000Hz */

You probably meant (1<<13), not <1<<13> ?

>
>   #define SNDRV_PCM_RATE_CONTINUOUS      (1<<30)         /* continuous range */
>   #define SNDRV_PCM_RATE_KNOT            (1<<31)         /* supports more
> non-continuos rates */
> diff --git a/sound/core/pcm_native.c b/sound/core/pcm_native.c
> index a68d4c6..42600b0 100644
> --- a/sound/core/pcm_native.c
> +++ b/sound/core/pcm_native.c
> @@ -1779,12 +1779,13 @@ 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_24000 != 1 << 14
>   #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,
> +                                 12000, 24000 };
>
>   const struct snd_pcm_hw_constraint_list snd_pcm_known_rates = {
>          .count = ARRAY_SIZE(rates),
>
> ~Vinod
>



-- 
David Henningsson, Canonical Ltd.
https://launchpad.net/~diwic

Re: RFC: support for 12 & 24Khz

[Vinod Koul]

On Thu, Jul 25, 2013 at 08:45:15AM +0200, David Henningsson wrote:
> On 07/24/2013 07:26 PM, Vinod Koul wrote:
> >Hey Takashi,
> >
> >For compressed audio we also need to support the PCM rates of 12 and 24KHz.
> >
> >Looking at pcm.h these are not defined so we can simply add them at the end.
> >But am worried about wider impact of adding these rates.
> >
> >Can you let me know if more is required to be done or below is fine
> >
> >diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> >index 84b10f9..e418d8d 100644
> >--- a/include/sound/pcm.h
> >+++ b/include/sound/pcm.h
> >@@ -126,6 +126,8 @@ 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_12000           <1<<13>         /* 12000Hz */
> >+#define SNDRV_PCM_RATE_24000           <1<<14>         /* 24000Hz */
> 
> You probably meant (1<<13), not <1<<13> ?
Yup :)

~Vinod

Re: RFC: support for 12 & 24Khz

[Clemens Ladisch]

Vinod Koul wrote:
> For compressed audio we also need to support the PCM rates of 12 and 24KHz.
>
> Looking at pcm.h these are not defined

You don't need such symbols to use a rate.  These symbols are intended
for often-used rates.  As long as only one or two drivers use a rate,
they can just set KNOT and install a constraint.

> But am worried about wider impact of adding these rates.
>
> Can you let me know if more is required to be done or below is fine

Grep for 176400 in sound/core/, include/, and in alsa-lib.


Regards,
Clemens

Re: RFC: support for 12 & 24Khz

[Vinod Koul]

On Thu, Jul 25, 2013 at 08:50:40AM +0200, Clemens Ladisch wrote:
> Vinod Koul wrote:
> > For compressed audio we also need to support the PCM rates of 12 and 24KHz.
> >
> > Looking at pcm.h these are not defined
> 
> You don't need such symbols to use a rate.  These symbols are intended
> for often-used rates.  As long as only one or two drivers use a rate,
> they can just set KNOT and install a constraint.
okay so how exactly is the rate passed to driver and converted and sent to
drivers?


> 
> > But am worried about wider impact of adding these rates.
> >
> > Can you let me know if more is required to be done or below is fine
> 
> Grep for 176400 in sound/core/, include/, and in alsa-lib.
SNDRV_PCM_RATE_176400?

~Vinod

-- 

Re: RFC: support for 12 & 24Khz

[Takashi Iwai]

At Thu, 25 Jul 2013 21:15:01 +0530,
Vinod Koul wrote:
> 
> On Thu, Jul 25, 2013 at 08:50:40AM +0200, Clemens Ladisch wrote:
> > Vinod Koul wrote:
> > > For compressed audio we also need to support the PCM rates of 12 and 24KHz.
> > >
> > > Looking at pcm.h these are not defined
> > 
> > You don't need such symbols to use a rate.  These symbols are intended
> > for often-used rates.  As long as only one or two drivers use a rate,
> > they can just set KNOT and install a constraint.
> okay so how exactly is the rate passed to driver and converted and sent to
> drivers?

Define the own hw_constraint.  Usually it's implemented via
snd_pcm_hw_constraint_list().

> > > But am worried about wider impact of adding these rates.
> > >
> > > Can you let me know if more is required to be done or below is fine
> > 
> > Grep for 176400 in sound/core/, include/, and in alsa-lib.
> SNDRV_PCM_RATE_176400?

Heh, this was an already added one.  Rather take a look at
SNDRV_PCM_RATE_KNOT.


Takashi

Re: RFC: support for 12 & 24Khz

[Vinod Koul]

On Thu, Jul 25, 2013 at 06:31:33PM +0200, Takashi Iwai wrote:
> At Thu, 25 Jul 2013 21:15:01 +0530,
> Vinod Koul wrote:
> > 
> > On Thu, Jul 25, 2013 at 08:50:40AM +0200, Clemens Ladisch wrote:
> > > Vinod Koul wrote:
> > > > For compressed audio we also need to support the PCM rates of 12 and 24KHz.
> > > >
> > > > Looking at pcm.h these are not defined
> > > 
> > > You don't need such symbols to use a rate.  These symbols are intended
> > > for often-used rates.  As long as only one or two drivers use a rate,
> > > they can just set KNOT and install a constraint.
> > okay so how exactly is the rate passed to driver and converted and sent to
> > drivers?
> 
> Define the own hw_constraint.  Usually it's implemented via
> snd_pcm_hw_constraint_list().
Thanks
> 
> > > > But am worried about wider impact of adding these rates.
> > > >
> > > > Can you let me know if more is required to be done or below is fine
> > > 
> > > Grep for 176400 in sound/core/, include/, and in alsa-lib.
> > SNDRV_PCM_RATE_176400?
> 
> Heh, this was an already added one.  Rather take a look at
> SNDRV_PCM_RATE_KNOT.
Okay so the orignal problem is that we use SNDRV_PCM_RATE_XXX to send the sample
rate to decoder. Some decoders very commonly use 12 and 24Khz, so I would like
them to be added.

I think 12 and 24 are fairly common rates and should be added. Do you agree?

~Vinod
-- 

Re: RFC: support for 12 & 24Khz

[Takashi Iwai]

At Fri, 26 Jul 2013 11:39:08 +0530,
Vinod Koul wrote:
> 
> On Thu, Jul 25, 2013 at 06:31:33PM +0200, Takashi Iwai wrote:
> > At Thu, 25 Jul 2013 21:15:01 +0530,
> > Vinod Koul wrote:
> > > 
> > > On Thu, Jul 25, 2013 at 08:50:40AM +0200, Clemens Ladisch wrote:
> > > > Vinod Koul wrote:
> > > > > For compressed audio we also need to support the PCM rates of 12 and 24KHz.
> > > > >
> > > > > Looking at pcm.h these are not defined
> > > > 
> > > > You don't need such symbols to use a rate.  These symbols are intended
> > > > for often-used rates.  As long as only one or two drivers use a rate,
> > > > they can just set KNOT and install a constraint.
> > > okay so how exactly is the rate passed to driver and converted and sent to
> > > drivers?
> > 
> > Define the own hw_constraint.  Usually it's implemented via
> > snd_pcm_hw_constraint_list().
> Thanks
> > 
> > > > > But am worried about wider impact of adding these rates.
> > > > >
> > > > > Can you let me know if more is required to be done or below is fine
> > > > 
> > > > Grep for 176400 in sound/core/, include/, and in alsa-lib.
> > > SNDRV_PCM_RATE_176400?
> > 
> > Heh, this was an already added one.  Rather take a look at
> > SNDRV_PCM_RATE_KNOT.
> Okay so the orignal problem is that we use SNDRV_PCM_RATE_XXX to send the sample
> rate to decoder.

Relying only on SNDRV_PCM_RATE_* bits doesn't scale.
What if more other rates are needed at the next time?

> Some decoders very commonly use 12 and 24Khz, so I would like
> them to be added.
> 
> I think 12 and 24 are fairly common rates and should be added. Do you agree?

It's no solution.  It makes things easier, though.
Better to fix the code to be able to handle uncommon rates, at first.


Takashi

Re: RFC: support for 12 & 24Khz

[Vinod Koul]

On Fri, Jul 26, 2013 at 09:09:09AM +0200, Takashi Iwai wrote:
> Relying only on SNDRV_PCM_RATE_* bits doesn't scale.
> What if more other rates are needed at the next time?
Agreed, this is my thinking as well. Remove the usage of macro and use raw rate
values...

~Vinod
-- 

Re: RFC: support for 12 & 24Khz

[Mark Brown]

[-- Attachment #1.1: Type: text/plain, Size: 599 bytes --]

On Thu, Jul 25, 2013 at 09:15:01PM +0530, Vinod Koul wrote:
> On Thu, Jul 25, 2013 at 08:50:40AM +0200, Clemens Ladisch wrote:

> > > For compressed audio we also need to support the PCM rates of 12 and 24KHz.

> > > Looking at pcm.h these are not defined

> > You don't need such symbols to use a rate.  These symbols are intended
> > for often-used rates.  As long as only one or two drivers use a rate,
> > they can just set KNOT and install a constraint.

> okay so how exactly is the rate passed to driver and converted and sent to
> drivers?

Having said that 24kHz sounds awfully familiar...

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: RFC: support for 12 & 24Khz

[Vinod Koul]

[-- Attachment #1.1: Type: text/plain, Size: 260 bytes --]

On Thu, Jul 25, 2013 at 08:36:44PM +0100, Mark Brown wrote:
> 
> Having said that 24kHz sounds awfully familiar...
Yup it is :)

I think todays rates defined in ALSA are HD-Audio based. IIRC HDA doesnt use
12/24 they were not added here!

~Vinod
-- 

[-- Attachment #1.2: Digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

[-- Attachment #2: Type: text/plain, Size: 0 bytes --]

Re: RFC: support for 12 & 24Khz

[Takashi Iwai]

At Fri, 26 Jul 2013 11:41:03 +0530,
Vinod Koul wrote:
> 
> On Thu, Jul 25, 2013 at 08:36:44PM +0100, Mark Brown wrote:
> > 
> > Having said that 24kHz sounds awfully familiar...
> Yup it is :)
> 
> I think todays rates defined in ALSA are HD-Audio based. IIRC HDA doesnt use
> 12/24 they were not added here!

No, the values were defined at the time of AC97.


Takashi