| Message ID | 1445348358-23306-1-git-send-email-ckeepax@opensource.wolfsonmicro.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
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 >
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...
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!
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
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.
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
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?
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
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
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(-)