* [RFC] G.723-24 format patch
@ 2010-05-01 15:36 Ben Collins
2010-05-08 9:58 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Ben Collins @ 2010-05-01 15:36 UTC (permalink / raw)
To: alsa-devel
I'm writing a driver for a card that only has hardware support for
G.723-24 ADPCM format. I am working around the lack of support in alsa
at the moment for this format, but wanted to take a crack at
implementing it properly.
G.723-24 is a 3-bit signed sample at a fixed 8khz mono sample rate
(24000 bit rate). Basically means you need to take in at least 3 bytes
(8 samples) minimum for processing the standard packed version.
I don't have hardware that supports it, but since I was in the code, I
added a G723_24_1B format for producing 1 sample per byte (3 lower bits
are actual sample), but for my use I am producing the standard packed
stream.
Only question I had is that since this is signed, should I expect or
want the _1B version to be a 1 byte sign extended version of the 3-bit
sample, or just expect the lower 3 bits of the byte to be the g723-24
sample? Looking at other unpacked examples, I assume the latter rather
than the former.
diff --git a/include/sound/asound.h b/include/sound/asound.h
index 1f57bb9..16fc633 100644
--- a/include/sound/asound.h
+++ b/include/sound/asound.h
@@ -212,7 +212,9 @@ typedef int __bitwise snd_pcm_format_t;
#define SNDRV_PCM_FORMAT_S18_3BE ((__force snd_pcm_format_t) 41) /* in three bytes */
#define SNDRV_PCM_FORMAT_U18_3LE ((__force snd_pcm_format_t) 42) /* in three bytes */
#define SNDRV_PCM_FORMAT_U18_3BE ((__force snd_pcm_format_t) 43) /* in three bytes */
-#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_U18_3BE
+#define SNDRV_PCM_FORMAT_G723_24 ((__force snd_pcm_format_t) 44) /* 8 samples in 3 bytes */
+#define SNDRV_PCM_FORMAT_G723_24_1B ((__force snd_pcm_format_t) 45) /* 1 sample in 1 byte */
+#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_G723_24_1B
#ifdef SNDRV_LITTLE_ENDIAN
#define SNDRV_PCM_FORMAT_S16 SNDRV_PCM_FORMAT_S16_LE
diff --git a/include/sound/pcm.h b/include/sound/pcm.h
index de6d981..031e2cd 100644
--- a/include/sound/pcm.h
+++ b/include/sound/pcm.h
@@ -173,6 +173,8 @@ struct snd_pcm_ops {
#define SNDRV_PCM_FMTBIT_U18_3LE (1ULL << SNDRV_PCM_FORMAT_U18_3LE)
#define SNDRV_PCM_FMTBIT_S18_3BE (1ULL << SNDRV_PCM_FORMAT_S18_3BE)
#define SNDRV_PCM_FMTBIT_U18_3BE (1ULL << SNDRV_PCM_FORMAT_U18_3BE)
+#define SNDRV_PCM_FMTBIT_G723_24 (1ULL << SNDRV_PCM_FORMAT_G723_24)
+#define SNDRV_PCM_FMTBIT_G723_24_1B (1ULL << SNDRV_PCM_FORMAT_G723_24_1B)
#ifdef SNDRV_LITTLE_ENDIAN
#define SNDRV_PCM_FMTBIT_S16 SNDRV_PCM_FMTBIT_S16_LE
diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
index ea2bf82..c7f8da4 100644
--- a/sound/core/pcm_misc.c
+++ b/sound/core/pcm_misc.c
@@ -128,6 +128,10 @@ static struct pcm_format_data pcm_formats[SNDRV_PCM_FORMAT_LAST+1] = {
.width = 4, .phys = 4, .le = -1, .signd = -1,
.silence = {},
},
+ [SNDRV_PCM_FORMAT_G723_24] = {
+ .width = 3, .phys = 3, .le = -1, .signd = 1,
+ .silence = {},
+ },
/* FIXME: the following three formats are not defined properly yet */
[SNDRV_PCM_FORMAT_MPEG] = {
.le = -1, .signd = -1,
@@ -186,6 +190,10 @@ static struct pcm_format_data pcm_formats[SNDRV_PCM_FORMAT_LAST+1] = {
.width = 18, .phys = 24, .le = 0, .signd = 0,
.silence = { 0x02, 0x00, 0x00 },
},
+ [SNDRV_PCM_FORMAT_G723_24_1B] = {
+ .width = 3, .phys = 8, .le = -1, .signd = 1,
+ .silence = {},
+ },
};
^ permalink raw reply related [flat|nested] 4+ messages in thread* Re: [RFC] G.723-24 format patch
2010-05-01 15:36 [RFC] G.723-24 format patch Ben Collins
@ 2010-05-08 9:58 ` Takashi Iwai
2010-05-10 17:50 ` Ben Collins
0 siblings, 1 reply; 4+ messages in thread
From: Takashi Iwai @ 2010-05-08 9:58 UTC (permalink / raw)
To: Ben Collins; +Cc: alsa-devel
At Sat, 01 May 2010 11:36:03 -0400,
Ben Collins wrote:
>
> I'm writing a driver for a card that only has hardware support for
> G.723-24 ADPCM format. I am working around the lack of support in alsa
> at the moment for this format, but wanted to take a crack at
> implementing it properly.
>
> G.723-24 is a 3-bit signed sample at a fixed 8khz mono sample rate
> (24000 bit rate). Basically means you need to take in at least 3 bytes
> (8 samples) minimum for processing the standard packed version.
>
> I don't have hardware that supports it, but since I was in the code, I
> added a G723_24_1B format for producing 1 sample per byte (3 lower bits
> are actual sample), but for my use I am producing the standard packed
> stream.
>
> Only question I had is that since this is signed, should I expect or
> want the _1B version to be a 1 byte sign extended version of the 3-bit
> sample, or just expect the lower 3 bits of the byte to be the g723-24
> sample? Looking at other unpacked examples, I assume the latter rather
> than the former.
I suppose both are non-linear formats, so you shouldn't give
signed/unsigned flags for them. Otherwise snd_pcm_format_linear()
will return 1.
Other than that, addition of these new formats look good to me.
thanks,
Takashi
> diff --git a/include/sound/asound.h b/include/sound/asound.h
> index 1f57bb9..16fc633 100644
> --- a/include/sound/asound.h
> +++ b/include/sound/asound.h
> @@ -212,7 +212,9 @@ typedef int __bitwise snd_pcm_format_t;
> #define SNDRV_PCM_FORMAT_S18_3BE ((__force snd_pcm_format_t) 41) /* in three bytes */
> #define SNDRV_PCM_FORMAT_U18_3LE ((__force snd_pcm_format_t) 42) /* in three bytes */
> #define SNDRV_PCM_FORMAT_U18_3BE ((__force snd_pcm_format_t) 43) /* in three bytes */
> -#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_U18_3BE
> +#define SNDRV_PCM_FORMAT_G723_24 ((__force snd_pcm_format_t) 44) /* 8 samples in 3 bytes */
> +#define SNDRV_PCM_FORMAT_G723_24_1B ((__force snd_pcm_format_t) 45) /* 1 sample in 1 byte */
> +#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_G723_24_1B
>
> #ifdef SNDRV_LITTLE_ENDIAN
> #define SNDRV_PCM_FORMAT_S16 SNDRV_PCM_FORMAT_S16_LE
> diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> index de6d981..031e2cd 100644
> --- a/include/sound/pcm.h
> +++ b/include/sound/pcm.h
> @@ -173,6 +173,8 @@ struct snd_pcm_ops {
> #define SNDRV_PCM_FMTBIT_U18_3LE (1ULL << SNDRV_PCM_FORMAT_U18_3LE)
> #define SNDRV_PCM_FMTBIT_S18_3BE (1ULL << SNDRV_PCM_FORMAT_S18_3BE)
> #define SNDRV_PCM_FMTBIT_U18_3BE (1ULL << SNDRV_PCM_FORMAT_U18_3BE)
> +#define SNDRV_PCM_FMTBIT_G723_24 (1ULL << SNDRV_PCM_FORMAT_G723_24)
> +#define SNDRV_PCM_FMTBIT_G723_24_1B (1ULL << SNDRV_PCM_FORMAT_G723_24_1B)
>
> #ifdef SNDRV_LITTLE_ENDIAN
> #define SNDRV_PCM_FMTBIT_S16 SNDRV_PCM_FMTBIT_S16_LE
> diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
> index ea2bf82..c7f8da4 100644
> --- a/sound/core/pcm_misc.c
> +++ b/sound/core/pcm_misc.c
> @@ -128,6 +128,10 @@ static struct pcm_format_data pcm_formats[SNDRV_PCM_FORMAT_LAST+1] = {
> .width = 4, .phys = 4, .le = -1, .signd = -1,
> .silence = {},
> },
> + [SNDRV_PCM_FORMAT_G723_24] = {
> + .width = 3, .phys = 3, .le = -1, .signd = 1,
> + .silence = {},
> + },
> /* FIXME: the following three formats are not defined properly yet */
> [SNDRV_PCM_FORMAT_MPEG] = {
> .le = -1, .signd = -1,
> @@ -186,6 +190,10 @@ static struct pcm_format_data pcm_formats[SNDRV_PCM_FORMAT_LAST+1] = {
> .width = 18, .phys = 24, .le = 0, .signd = 0,
> .silence = { 0x02, 0x00, 0x00 },
> },
> + [SNDRV_PCM_FORMAT_G723_24_1B] = {
> + .width = 3, .phys = 8, .le = -1, .signd = 1,
> + .silence = {},
> + },
> };
>
>
>
>
>
>
> _______________________________________________
> Alsa-devel mailing list
> Alsa-devel@alsa-project.org
> http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
>
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [RFC] G.723-24 format patch
2010-05-08 9:58 ` Takashi Iwai
@ 2010-05-10 17:50 ` Ben Collins
2010-05-10 20:58 ` Takashi Iwai
0 siblings, 1 reply; 4+ messages in thread
From: Ben Collins @ 2010-05-10 17:50 UTC (permalink / raw)
To: Takashi Iwai; +Cc: alsa-devel
On Sat, 2010-05-08 at 11:58 +0200, Takashi Iwai wrote:
> At Sat, 01 May 2010 11:36:03 -0400,
> Ben Collins wrote:
> >
> > I'm writing a driver for a card that only has hardware support for
> > G.723-24 ADPCM format. I am working around the lack of support in alsa
> > at the moment for this format, but wanted to take a crack at
> > implementing it properly.
> >
> > G.723-24 is a 3-bit signed sample at a fixed 8khz mono sample rate
> > (24000 bit rate). Basically means you need to take in at least 3 bytes
> > (8 samples) minimum for processing the standard packed version.
> >
> > I don't have hardware that supports it, but since I was in the code, I
> > added a G723_24_1B format for producing 1 sample per byte (3 lower bits
> > are actual sample), but for my use I am producing the standard packed
> > stream.
> >
> > Only question I had is that since this is signed, should I expect or
> > want the _1B version to be a 1 byte sign extended version of the 3-bit
> > sample, or just expect the lower 3 bits of the byte to be the g723-24
> > sample? Looking at other unpacked examples, I assume the latter rather
> > than the former.
>
> I suppose both are non-linear formats, so you shouldn't give
> signed/unsigned flags for them. Otherwise snd_pcm_format_linear()
> will return 1.
>
> Other than that, addition of these new formats look good to me.
I may not be understanding the implications of setting them as
signed/unsigned. G.723 decoder treats each sample as signed. Does
alsa-lib do special things when it is signed even though it doesn't
understand the actual encoding?
And my understanding of sound encoding is very limited, so if you could
explain the non-linear part too, I could answer the question better.
Thanks
>
> thanks,
>
> Takashi
>
>
> > diff --git a/include/sound/asound.h b/include/sound/asound.h
> > index 1f57bb9..16fc633 100644
> > --- a/include/sound/asound.h
> > +++ b/include/sound/asound.h
> > @@ -212,7 +212,9 @@ typedef int __bitwise snd_pcm_format_t;
> > #define SNDRV_PCM_FORMAT_S18_3BE ((__force snd_pcm_format_t) 41) /* in three bytes */
> > #define SNDRV_PCM_FORMAT_U18_3LE ((__force snd_pcm_format_t) 42) /* in three bytes */
> > #define SNDRV_PCM_FORMAT_U18_3BE ((__force snd_pcm_format_t) 43) /* in three bytes */
> > -#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_U18_3BE
> > +#define SNDRV_PCM_FORMAT_G723_24 ((__force snd_pcm_format_t) 44) /* 8 samples in 3 bytes */
> > +#define SNDRV_PCM_FORMAT_G723_24_1B ((__force snd_pcm_format_t) 45) /* 1 sample in 1 byte */
> > +#define SNDRV_PCM_FORMAT_LAST SNDRV_PCM_FORMAT_G723_24_1B
> >
> > #ifdef SNDRV_LITTLE_ENDIAN
> > #define SNDRV_PCM_FORMAT_S16 SNDRV_PCM_FORMAT_S16_LE
> > diff --git a/include/sound/pcm.h b/include/sound/pcm.h
> > index de6d981..031e2cd 100644
> > --- a/include/sound/pcm.h
> > +++ b/include/sound/pcm.h
> > @@ -173,6 +173,8 @@ struct snd_pcm_ops {
> > #define SNDRV_PCM_FMTBIT_U18_3LE (1ULL << SNDRV_PCM_FORMAT_U18_3LE)
> > #define SNDRV_PCM_FMTBIT_S18_3BE (1ULL << SNDRV_PCM_FORMAT_S18_3BE)
> > #define SNDRV_PCM_FMTBIT_U18_3BE (1ULL << SNDRV_PCM_FORMAT_U18_3BE)
> > +#define SNDRV_PCM_FMTBIT_G723_24 (1ULL << SNDRV_PCM_FORMAT_G723_24)
> > +#define SNDRV_PCM_FMTBIT_G723_24_1B (1ULL << SNDRV_PCM_FORMAT_G723_24_1B)
> >
> > #ifdef SNDRV_LITTLE_ENDIAN
> > #define SNDRV_PCM_FMTBIT_S16 SNDRV_PCM_FMTBIT_S16_LE
> > diff --git a/sound/core/pcm_misc.c b/sound/core/pcm_misc.c
> > index ea2bf82..c7f8da4 100644
> > --- a/sound/core/pcm_misc.c
> > +++ b/sound/core/pcm_misc.c
> > @@ -128,6 +128,10 @@ static struct pcm_format_data pcm_formats[SNDRV_PCM_FORMAT_LAST+1] = {
> > .width = 4, .phys = 4, .le = -1, .signd = -1,
> > .silence = {},
> > },
> > + [SNDRV_PCM_FORMAT_G723_24] = {
> > + .width = 3, .phys = 3, .le = -1, .signd = 1,
> > + .silence = {},
> > + },
> > /* FIXME: the following three formats are not defined properly yet */
> > [SNDRV_PCM_FORMAT_MPEG] = {
> > .le = -1, .signd = -1,
> > @@ -186,6 +190,10 @@ static struct pcm_format_data pcm_formats[SNDRV_PCM_FORMAT_LAST+1] = {
> > .width = 18, .phys = 24, .le = 0, .signd = 0,
> > .silence = { 0x02, 0x00, 0x00 },
> > },
> > + [SNDRV_PCM_FORMAT_G723_24_1B] = {
> > + .width = 3, .phys = 8, .le = -1, .signd = 1,
> > + .silence = {},
> > + },
> > };
> >
> >
> >
> >
> >
> >
> > _______________________________________________
> > Alsa-devel mailing list
> > Alsa-devel@alsa-project.org
> > http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
> >
^ permalink raw reply [flat|nested] 4+ messages in thread* Re: [RFC] G.723-24 format patch
2010-05-10 17:50 ` Ben Collins
@ 2010-05-10 20:58 ` Takashi Iwai
0 siblings, 0 replies; 4+ messages in thread
From: Takashi Iwai @ 2010-05-10 20:58 UTC (permalink / raw)
To: Ben Collins; +Cc: alsa-devel
At Mon, 10 May 2010 13:50:26 -0400,
Ben Collins wrote:
>
> On Sat, 2010-05-08 at 11:58 +0200, Takashi Iwai wrote:
> > At Sat, 01 May 2010 11:36:03 -0400,
> > Ben Collins wrote:
> > >
> > > I'm writing a driver for a card that only has hardware support for
> > > G.723-24 ADPCM format. I am working around the lack of support in alsa
> > > at the moment for this format, but wanted to take a crack at
> > > implementing it properly.
> > >
> > > G.723-24 is a 3-bit signed sample at a fixed 8khz mono sample rate
> > > (24000 bit rate). Basically means you need to take in at least 3 bytes
> > > (8 samples) minimum for processing the standard packed version.
> > >
> > > I don't have hardware that supports it, but since I was in the code, I
> > > added a G723_24_1B format for producing 1 sample per byte (3 lower bits
> > > are actual sample), but for my use I am producing the standard packed
> > > stream.
> > >
> > > Only question I had is that since this is signed, should I expect or
> > > want the _1B version to be a 1 byte sign extended version of the 3-bit
> > > sample, or just expect the lower 3 bits of the byte to be the g723-24
> > > sample? Looking at other unpacked examples, I assume the latter rather
> > > than the former.
> >
> > I suppose both are non-linear formats, so you shouldn't give
> > signed/unsigned flags for them. Otherwise snd_pcm_format_linear()
> > will return 1.
> >
> > Other than that, addition of these new formats look good to me.
>
> I may not be understanding the implications of setting them as
> signed/unsigned. G.723 decoder treats each sample as signed. Does
> alsa-lib do special things when it is signed even though it doesn't
> understand the actual encoding?
The signed/unsigned information is useful only for linear PCM formats
(e.g. U8, S16, etc), so that they can be converted with each other
simply with bit shift and sign-bit flip. Non-linear formats
(e.g. ADPCM, ulaw, etc) require specific conversions, thus its
conversion to other formats isn't supported always in the kernel, and
signed/unsigned information isn't referred as well.
Similarly, in alsa-lib, signed/unsigned information doesn't give much
merit but for linear PCM formats.
thanks,
Takashi
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2010-05-10 20:58 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2010-05-01 15:36 [RFC] G.723-24 format patch Ben Collins
2010-05-08 9:58 ` Takashi Iwai
2010-05-10 17:50 ` Ben Collins
2010-05-10 20:58 ` Takashi Iwai
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).