[PATCH] ALSA: compress: add num_sample_rates in snd_codec_desc

[PATCH] ALSA: compress: add num_sample_rates in snd_codec_desc

[Vinod Koul]

this gives ability to convey the valid values of supported rates in
sample_rates array

Signed-off-by: Vinod Koul <vinod.koul@intel.com>
---
 include/uapi/sound/compress_params.h |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)

diff --git a/include/uapi/sound/compress_params.h b/include/uapi/sound/compress_params.h
index b62b24b..165e705 100644
--- a/include/uapi/sound/compress_params.h
+++ b/include/uapi/sound/compress_params.h
@@ -326,6 +326,7 @@ union snd_codec_options {
 /** struct snd_codec_desc - description of codec capabilities
  * @max_ch: Maximum number of audio channels
  * @sample_rates: Sampling rates in Hz, use values like 48000 for this
+ * @num_sample_rates: Number of valid values in sample_rates array
  * @bit_rate: Indexed array containing supported bit rates
  * @num_bitrates: Number of valid values in bit_rate array
  * @rate_control: value is specified by SND_RATECONTROLMODE defines.
@@ -348,6 +349,7 @@ union snd_codec_options {
 struct snd_codec_desc {
 	__u32 max_ch;
 	__u32 sample_rates[MAX_NUM_SAMPLE_RATES];
+	__u32 num_sample_rates;
 	__u32 bit_rate[MAX_NUM_BITRATES];
 	__u32 num_bitrates;
 	__u32 rate_control;
-- 
1.7.0.4

Re: [PATCH] ALSA: compress: add num_sample_rates in snd_codec_desc

[Vinod Koul]

On Tue, Jan 07, 2014 at 05:35:10PM +0000, Mark Brown wrote:
> On Tue, Jan 07, 2014 at 09:55:42PM +0530, Vinod Koul wrote:
> 
> >  struct snd_codec_desc {
> >  	__u32 max_ch;
> >  	__u32 sample_rates[MAX_NUM_SAMPLE_RATES];
> > +	__u32 num_sample_rates;
> >  	__u32 bit_rate[MAX_NUM_BITRATES];
> >  	__u32 num_bitrates;
> >  	__u32 rate_control;
> 
> This is adding a field to the userspace ABI which will break it...  not
> sure if we care or not but it probably ought to be noted in the commit
> log at the very least; people do build things like distros with headers
> from older kernels than the ones they run.
I am not aware is any users of desciptors yet :) so thats why went ahead and
modifed... This is long term plan to query dynamic capablities of device using
this

--
~Vinod

Re: [PATCH] ALSA: compress: add num_sample_rates in snd_codec_desc

[Vinod Koul]

On Tue, Jan 07, 2014 at 06:34:14PM +0100, Takashi Iwai wrote:
> At Tue,  7 Jan 2014 21:55:42 +0530,
> Vinod Koul wrote:
> > 
> > this gives ability to convey the valid values of supported rates in
> > sample_rates array
> > 
> > Signed-off-by: Vinod Koul <vinod.koul@intel.com>
> 
> Thanks, applied.

Thanks, does this one get the record for quicked apply :)

--
~Vinod
> 
> 
> Takashi
> 
> > ---
> >  include/uapi/sound/compress_params.h |    2 ++
> >  1 files changed, 2 insertions(+), 0 deletions(-)
> > 
> > diff --git a/include/uapi/sound/compress_params.h b/include/uapi/sound/compress_params.h
> > index b62b24b..165e705 100644
> > --- a/include/uapi/sound/compress_params.h
> > +++ b/include/uapi/sound/compress_params.h
> > @@ -326,6 +326,7 @@ union snd_codec_options {
> >  /** struct snd_codec_desc - description of codec capabilities
> >   * @max_ch: Maximum number of audio channels
> >   * @sample_rates: Sampling rates in Hz, use values like 48000 for this
> > + * @num_sample_rates: Number of valid values in sample_rates array
> >   * @bit_rate: Indexed array containing supported bit rates
> >   * @num_bitrates: Number of valid values in bit_rate array
> >   * @rate_control: value is specified by SND_RATECONTROLMODE defines.
> > @@ -348,6 +349,7 @@ union snd_codec_options {
> >  struct snd_codec_desc {
> >  	__u32 max_ch;
> >  	__u32 sample_rates[MAX_NUM_SAMPLE_RATES];
> > +	__u32 num_sample_rates;
> >  	__u32 bit_rate[MAX_NUM_BITRATES];
> >  	__u32 num_bitrates;
> >  	__u32 rate_control;
> > -- 
> > 1.7.0.4
> > 

-- 

Re: [PATCH] ALSA: compress: add num_sample_rates in snd_codec_desc

[Takashi Iwai]

At Tue,  7 Jan 2014 21:55:42 +0530,
Vinod Koul wrote:
> 
> this gives ability to convey the valid values of supported rates in
> sample_rates array
> 
> Signed-off-by: Vinod Koul <vinod.koul@intel.com>

Thanks, applied.


Takashi

> ---
>  include/uapi/sound/compress_params.h |    2 ++
>  1 files changed, 2 insertions(+), 0 deletions(-)
> 
> diff --git a/include/uapi/sound/compress_params.h b/include/uapi/sound/compress_params.h
> index b62b24b..165e705 100644
> --- a/include/uapi/sound/compress_params.h
> +++ b/include/uapi/sound/compress_params.h
> @@ -326,6 +326,7 @@ union snd_codec_options {
>  /** struct snd_codec_desc - description of codec capabilities
>   * @max_ch: Maximum number of audio channels
>   * @sample_rates: Sampling rates in Hz, use values like 48000 for this
> + * @num_sample_rates: Number of valid values in sample_rates array
>   * @bit_rate: Indexed array containing supported bit rates
>   * @num_bitrates: Number of valid values in bit_rate array
>   * @rate_control: value is specified by SND_RATECONTROLMODE defines.
> @@ -348,6 +349,7 @@ union snd_codec_options {
>  struct snd_codec_desc {
>  	__u32 max_ch;
>  	__u32 sample_rates[MAX_NUM_SAMPLE_RATES];
> +	__u32 num_sample_rates;
>  	__u32 bit_rate[MAX_NUM_BITRATES];
>  	__u32 num_bitrates;
>  	__u32 rate_control;
> -- 
> 1.7.0.4
> 

Re: [PATCH] ALSA: compress: add num_sample_rates in snd_codec_desc

[Mark Brown]

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

On Tue, Jan 07, 2014 at 09:55:42PM +0530, Vinod Koul wrote:

>  struct snd_codec_desc {
>  	__u32 max_ch;
>  	__u32 sample_rates[MAX_NUM_SAMPLE_RATES];
> +	__u32 num_sample_rates;
>  	__u32 bit_rate[MAX_NUM_BITRATES];
>  	__u32 num_bitrates;
>  	__u32 rate_control;

This is adding a field to the userspace ABI which will break it...  not
sure if we care or not but it probably ought to be noted in the commit
log at the very least; people do build things like distros with headers
from older kernels than the ones they run.

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

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

Re: [PATCH] ALSA: compress: add num_sample_rates in snd_codec_desc

[Takashi Iwai]

At Tue, 7 Jan 2014 17:35:10 +0000,
Mark Brown wrote:
> 
> On Tue, Jan 07, 2014 at 09:55:42PM +0530, Vinod Koul wrote:
> 
> >  struct snd_codec_desc {
> >  	__u32 max_ch;
> >  	__u32 sample_rates[MAX_NUM_SAMPLE_RATES];
> > +	__u32 num_sample_rates;
> >  	__u32 bit_rate[MAX_NUM_BITRATES];
> >  	__u32 num_bitrates;
> >  	__u32 rate_control;
> 
> This is adding a field to the userspace ABI which will break it...  not
> sure if we care or not but it probably ought to be noted in the commit
> log at the very least; people do build things like distros with headers
> from older kernels than the ones they run.

The previous commit for sample_rates[] already broke it, so it doesn't
matter for this commit :)  I applied the update patches since
snd_codec_desc isn't used anywhere in the upstream code, and this is
the request from Vinod, so he must know well the ABI regression if it
really matters.


Takashi

Re: [PATCH] ALSA: compress: add num_sample_rates in snd_codec_desc

[Mark Brown]

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

On Tue, Jan 07, 2014 at 10:08:18PM +0530, Vinod Koul wrote:

> I am not aware is any users of desciptors yet :) so thats why went ahead and
> modifed... This is long term plan to query dynamic capablities of device using
> this

Right, but might be worth tagging it to stable as a result - I can see
someone building their userspace with the LTS headers for example.

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

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

Re: [PATCH] ALSA: compress: add num_sample_rates in snd_codec_desc

[Takashi Iwai]

At Tue, 7 Jan 2014 17:43:32 +0000,
Mark Brown wrote:
> 
> On Tue, Jan 07, 2014 at 10:08:18PM +0530, Vinod Koul wrote:
> 
> > I am not aware is any users of desciptors yet :) so thats why went ahead and
> > modifed... This is long term plan to query dynamic capablities of device using
> > this
> 
> Right, but might be worth tagging it to stable as a result - I can see
> someone building their userspace with the LTS headers for example.

Sorry, too late, the previous patch was already applied days ago.

Vinod, if the update on stable kernel is required, inform Greg later
once after the code is merged to 3.14 tree.  It'd be enough just to
mail him the commit ids to cherry pick.


thanks,

Takashi

Re: [PATCH] ALSA: compress: add num_sample_rates in snd_codec_desc

[Mark Brown]

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

On Tue, Jan 07, 2014 at 06:41:35PM +0100, Takashi Iwai wrote:

> The previous commit for sample_rates[] already broke it, so it doesn't
> matter for this commit :)  I applied the update patches since
> snd_codec_desc isn't used anywhere in the upstream code, and this is
> the request from Vinod, so he must know well the ABI regression if it
> really matters.

Yeah, it's not just Vinod though.  Like I say pushing to stable so
anyone trying to build on top of it might be the best thing, can't
remember if that happend with the previous change or not.

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

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

Re: [PATCH] ALSA: compress: add num_sample_rates in snd_codec_desc

[Vinod Koul]

On Tue, Jan 07, 2014 at 05:47:51PM +0000, Mark Brown wrote:
> On Tue, Jan 07, 2014 at 06:41:35PM +0100, Takashi Iwai wrote:
> 
> > The previous commit for sample_rates[] already broke it, so it doesn't
> > matter for this commit :)  I applied the update patches since
> > snd_codec_desc isn't used anywhere in the upstream code, and this is
> > the request from Vinod, so he must know well the ABI regression if it
> > really matters.
> 
> Yeah, it's not just Vinod though.  Like I say pushing to stable so
> anyone trying to build on top of it might be the best thing, can't
> remember if that happend with the previous change or not.
The changes to othere like 64bit ones were marked to stable. The descriptor ones
werent as we dont have users. But yes agree to your point that folks might be
still on LTS kernel. I will send commit ids to Greg after it gets merged to Linus
tree

-- 
~Vinod