[RFC PATCH] ALSA: compress: Add SND_AUDIOCODEC_BESPOKE

[RFC PATCH] ALSA: compress: Add SND_AUDIOCODEC_BESPOKE

[Charles Keepax]

When working with the compressed framework occasionally vendors will
use esoteric internal audio formats. For such formats it doesn't really
make sense to add an new define to the kernel as their use is not
sufficiently general.

This patch adds a new define SND_AUDIOCODEC_BESPOKE that vendors can use
in such situations.

Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
---
 include/uapi/sound/compress_params.h |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/include/uapi/sound/compress_params.h b/include/uapi/sound/compress_params.h
index d9bd9ca..e284380 100644
--- a/include/uapi/sound/compress_params.h
+++ b/include/uapi/sound/compress_params.h
@@ -73,7 +73,8 @@
 #define SND_AUDIOCODEC_IEC61937              ((__u32) 0x0000000B)
 #define SND_AUDIOCODEC_G723_1                ((__u32) 0x0000000C)
 #define SND_AUDIOCODEC_G729                  ((__u32) 0x0000000D)
-#define SND_AUDIOCODEC_MAX                   SND_AUDIOCODEC_G729
+#define SND_AUDIOCODEC_BESPOKE               ((__u32) 0x0000000E)
+#define SND_AUDIOCODEC_MAX                   SND_AUDIOCODEC_BESPOKE
 
 /*
  * Profile and modes are listed with bit masks. This allows for a
-- 
1.7.2.5

Re: [RFC PATCH] ALSA: compress: Add SND_AUDIOCODEC_BESPOKE

[Pierre-Louis Bossart]

On 10/20/15 8:39 AM, Charles Keepax wrote:
> When working with the compressed framework occasionally vendors will
> use esoteric internal audio formats. For such formats it doesn't really
> make sense to add an new define to the kernel as their use is not
> sufficiently general.
>
> This patch adds a new define SND_AUDIOCODEC_BESPOKE that vendors can use
> in such situations.
>
> Signed-off-by: Charles Keepax <ckeepax@opensource.wolfsonmicro.com>
> ---
>   include/uapi/sound/compress_params.h |    3 ++-
>   1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/include/uapi/sound/compress_params.h b/include/uapi/sound/compress_params.h
> index d9bd9ca..e284380 100644
> --- a/include/uapi/sound/compress_params.h
> +++ b/include/uapi/sound/compress_params.h
> @@ -73,7 +73,8 @@
>   #define SND_AUDIOCODEC_IEC61937              ((__u32) 0x0000000B)
>   #define SND_AUDIOCODEC_G723_1                ((__u32) 0x0000000C)
>   #define SND_AUDIOCODEC_G729                  ((__u32) 0x0000000D)
> -#define SND_AUDIOCODEC_MAX                   SND_AUDIOCODEC_G729
> +#define SND_AUDIOCODEC_BESPOKE               ((__u32) 0x0000000E)

No real objections from me, this is better than overloading PCM for this 
purpose...
Since this is clearly an escape mechanism for a non-standard codec, 
maybe we should use an ID that's not in sequence, eg 0x00001000.
If you need to set parameters maybe we should also change the definition 
of snd_enc_generic so that the reserved fields can be used for custom 
parameters, or document that their use is permitted for this sort of ID.

> +#define SND_AUDIOCODEC_MAX                   SND_AUDIOCODEC_BESPOKE
>
>   /*
>    * Profile and modes are listed with bit masks. This allows for a
>

Re: [RFC PATCH] ALSA: compress: Add SND_AUDIOCODEC_BESPOKE

[Mark Brown]

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

On Tue, Oct 20, 2015 at 12:47:58PM -0500, Pierre-Louis Bossart wrote:

> Since this is clearly an escape mechanism for a non-standard codec, maybe we
> should use an ID that's not in sequence, eg 0x00001000.

0xDEADBEEF.  Also the bikeshed should be purple :)

> If you need to set parameters maybe we should also change the definition of
> snd_enc_generic so that the reserved fields can be used for custom
> parameters, or document that their use is permitted for this sort of ID.

It does mean the core can't grab them but quite what the core is
supposed to usefully do for something like this is unclear to me so...

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 473 bytes --]

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

Re: [RFC PATCH] ALSA: compress: Add SND_AUDIOCODEC_BESPOKE'

[Vinod Koul]

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

On Tue, Oct 20, 2015 at 08:37:11PM +0100, Mark Brown wrote:
> On Tue, Oct 20, 2015 at 12:47:58PM -0500, Pierre-Louis Bossart wrote:

> > If you need to set parameters maybe we should also change the definition of
> > snd_enc_generic so that the reserved fields can be used for custom
> > parameters, or document that their use is permitted for this sort of ID.
> 
> It does mean the core can't grab them but quite what the core is
> supposed to usefully do for something like this is unclear to me so...

Well in case of compressed formats, core doesn't do anything, we just act as
a transport and shove down data and information.

I do like Pierre's idea of giving some meaning to formats and use some
reserved fields, but somehow I can't think of a clean solution for this!

-- 
~Vinod

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

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

Re: [RFC PATCH] ALSA: compress: Add SND_AUDIOCODEC_BESPOKE'

[Charles Keepax]

On Wed, Oct 21, 2015 at 09:02:49PM +0530, Vinod Koul wrote:
> On Tue, Oct 20, 2015 at 08:37:11PM +0100, Mark Brown wrote:
> > On Tue, Oct 20, 2015 at 12:47:58PM -0500, Pierre-Louis Bossart wrote:
> 
> > > If you need to set parameters maybe we should also change the definition of
> > > snd_enc_generic so that the reserved fields can be used for custom
> > > parameters, or document that their use is permitted for this sort of ID.
> > 
> > It does mean the core can't grab them but quite what the core is
> > supposed to usefully do for something like this is unclear to me so...
> 
> Well in case of compressed formats, core doesn't do anything, we just act as
> a transport and shove down data and information.
> 
> I do like Pierre's idea of giving some meaning to formats and use some
> reserved fields, but somehow I can't think of a clean solution for this!

Personally, I feel a bit like doing nothing at the moment might
be the best solution. If BESPOKE is for icky stuff that won't be
generic trying to pull out generic fields to go in snd_enc_generic
seems like a challenging exercise, and we can look submissions
that use it and keep an eye out for anything that might be
generic enough to go in there, rather than adding a bunch of
stuff that ends up never getting used.

Thanks,
Charles

Re: [RFC PATCH] ALSA: compress: Add SND_AUDIOCODEC_BESPOKE'

[Pierre-Louis Bossart]

On 10/21/15 10:36 AM, Charles Keepax wrote:
> On Wed, Oct 21, 2015 at 09:02:49PM +0530, Vinod Koul wrote:
>> On Tue, Oct 20, 2015 at 08:37:11PM +0100, Mark Brown wrote:
>>> On Tue, Oct 20, 2015 at 12:47:58PM -0500, Pierre-Louis Bossart wrote:
>>
>>>> If you need to set parameters maybe we should also change the definition of
>>>> snd_enc_generic so that the reserved fields can be used for custom
>>>> parameters, or document that their use is permitted for this sort of ID.
>>>
>>> It does mean the core can't grab them but quite what the core is
>>> supposed to usefully do for something like this is unclear to me so...
>>
>> Well in case of compressed formats, core doesn't do anything, we just act as
>> a transport and shove down data and information.
>>
>> I do like Pierre's idea of giving some meaning to formats and use some
>> reserved fields, but somehow I can't think of a clean solution for this!
>
> Personally, I feel a bit like doing nothing at the moment might
> be the best solution. If BESPOKE is for icky stuff that won't be
> generic trying to pull out generic fields to go in snd_enc_generic
> seems like a challenging exercise, and we can look submissions
> that use it and keep an eye out for anything that might be
> generic enough to go in there, rather than adding a bunch of
> stuff that ends up never getting used.

I was only suggesting to redefine these fields from 'reserved' to 'use 
for whatever parameters are needed for this BESPOKE codec type'. I 
wasn't trying to come-up with generic fields or predict future usages of 
non-standard codecs, that would be a waste of time indeed.

Re: [RFC PATCH] ALSA: compress: Add SND_AUDIOCODEC_BESPOKE'

[Vinod Koul]

On Wed, Oct 21, 2015 at 04:36:14PM +0100, Charles Keepax wrote:
> On Wed, Oct 21, 2015 at 09:02:49PM +0530, Vinod Koul wrote:
> > On Tue, Oct 20, 2015 at 08:37:11PM +0100, Mark Brown wrote:
> > > On Tue, Oct 20, 2015 at 12:47:58PM -0500, Pierre-Louis Bossart wrote:
> > 
> > > > If you need to set parameters maybe we should also change the definition of
> > > > snd_enc_generic so that the reserved fields can be used for custom
> > > > parameters, or document that their use is permitted for this sort of ID.
> > > 
> > > It does mean the core can't grab them but quite what the core is
> > > supposed to usefully do for something like this is unclear to me so...
> > 
> > Well in case of compressed formats, core doesn't do anything, we just act as
> > a transport and shove down data and information.
> > 
> > I do like Pierre's idea of giving some meaning to formats and use some
> > reserved fields, but somehow I can't think of a clean solution for this!
> 
> Personally, I feel a bit like doing nothing at the moment might
> be the best solution. If BESPOKE is for icky stuff that won't be
> generic trying to pull out generic fields to go in snd_enc_generic
> seems like a challenging exercise, and we can look submissions
> that use it and keep an eye out for anything that might be
> generic enough to go in there, rather than adding a bunch of
> stuff that ends up never getting used.

So what is the conclusion here..? Are we all okay with this approach?

-- 
~Vinod

Re: [RFC PATCH] ALSA: compress: Add SND_AUDIOCODEC_BESPOKE'

[Charles Keepax]

On Thu, Nov 05, 2015 at 11:27:39AM +0530, Vinod Koul wrote:
> On Wed, Oct 21, 2015 at 04:36:14PM +0100, Charles Keepax wrote:
> > On Wed, Oct 21, 2015 at 09:02:49PM +0530, Vinod Koul wrote:
> > > On Tue, Oct 20, 2015 at 08:37:11PM +0100, Mark Brown wrote:
> > > > On Tue, Oct 20, 2015 at 12:47:58PM -0500, Pierre-Louis Bossart wrote:
> > > 
> > > > > If you need to set parameters maybe we should also change the definition of
> > > > > snd_enc_generic so that the reserved fields can be used for custom
> > > > > parameters, or document that their use is permitted for this sort of ID.
> > > > 
> > > > It does mean the core can't grab them but quite what the core is
> > > > supposed to usefully do for something like this is unclear to me so...
> > > 
> > > Well in case of compressed formats, core doesn't do anything, we just act as
> > > a transport and shove down data and information.
> > > 
> > > I do like Pierre's idea of giving some meaning to formats and use some
> > > reserved fields, but somehow I can't think of a clean solution for this!
> > 
> > Personally, I feel a bit like doing nothing at the moment might
> > be the best solution. If BESPOKE is for icky stuff that won't be
> > generic trying to pull out generic fields to go in snd_enc_generic
> > seems like a challenging exercise, and we can look submissions
> > that use it and keep an eye out for anything that might be
> > generic enough to go in there, rather than adding a bunch of
> > stuff that ends up never getting used.
> 
> So what is the conclusion here..? Are we all okay with this approach?

I was planning to send out a v2 shortly, just adding a comment by
the reserved fields in snd_enc_generic to say its ok to use them
if using BESPOKE and see where that gets us.

Thanks,
Charles

Re: [RFC PATCH] ALSA: compress: Add SND_AUDIOCODEC_BESPOKE

[Takashi Iwai]

On Tue, 20 Oct 2015 21:37:11 +0200,
Mark Brown wrote:
> 
> On Tue, Oct 20, 2015 at 12:47:58PM -0500, Pierre-Louis Bossart wrote:
> 
> > Since this is clearly an escape mechanism for a non-standard codec, maybe we
> > should use an ID that's not in sequence, eg 0x00001000.
> 
> 0xDEADBEEF.  Also the bikeshed should be purple :)

Oh, let's make it more brilliant like gold over shocking pink :)

Usually the number like 0x1000 makes sense only when the lower bits
are used for relevant purpose, e.g. as a sub-type definition for
BESPOKE.  So, type_is_bespoke() won't be "type == 0x1000" but
"type & 0x1000".


Takashi