[RFC] Channel mapping API (take 2)

[RFC] Channel mapping API (take 2)

[Takashi Iwai]

Hi,

my proposal for channel mapping API seems accepted fairly well through
discussions at Plumbers, so I continued to work on it and uploaded the
updated version.

The kernel part is found in sound-unstable tree topic/tlv-chmap branch
  git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-unstable.git

and the alsa-lib part is found in github tree topic/chmap branch
  git://git.github.com/tiwai/alsa-lib.git

The updated parts are:

- SNDRV_CHMAP_NA was newly added, indicating the channel is not
  available or silent.

- The channel position value is masked in lower 16bit.
  The upper bits are used for more channel attributes as bit flags.

  #define SNDRV_CHMAP_POSITION_MASK	0xffff

  The phase inverted channel has this bit:
  #define SNDRV_CHMAP_PHASE_INVERSE	(0x01 << 16)

  And the driver-specific non-standard channel position has this:
  #define SNDRV_CHMAP_DRIVER_SPEC	(0x02 << 16)

- The alsa-lib API functions use snd_pcm_chmap_query_t and
  snd_pcm_chmap_t instead of ambiguous integer arrays.

================================================================
/** the channel map header */
typedef struct snd_pcm_chmap {
	unsigned int channels;
	unsigned int pos[0];
} snd_pcm_chmap_t;

/** the header of array items returned from snd_pcm_query_chmaps() */
typedef struct snd_pcm_chmap_query {
	enum snd_pcm_chmap_type type;
	snd_pcm_chmap_t map;
} snd_pcm_chmap_query_t;


snd_pcm_chmap_query_t **snd_pcm_query_chmaps(snd_pcm_t *pcm);
void snd_pcm_free_chmaps(snd_pcm_chmap_query_t **maps);
snd_pcm_chmap_t *snd_pcm_get_chmap(snd_pcm_t *pcm);
int snd_pcm_set_chmap(snd_pcm_t *pcm, const snd_pcm_chmap_t *map);
================================================================

The rest aren't so much changed.  Only slight bug fixes.

A big remaining question is whether the current kernel-side
implementation is OK.  We may add some PCM ops pointer instead of the
current style using the direct control tlv/read/write override.
But the kernel-side API can be changed even later, so I don't worry
about it so much for now, as long as we can keep the ABI definition.


thanks,

Takashi

Re: [RFC] Channel mapping API (take 2)

[Tanu Kaskinen]

On Tue, 2012-09-04 at 17:47 +0200, Takashi Iwai wrote:
> The rest aren't so much changed.  Only slight bug fixes.

I have a couple of comments about "the rest". I haven't been following
the development of the channel map API very closely, so I'm relying on
the pcm.h file only.


/** channel map list type */
enum snd_pcm_chmap_type {
	SND_CHMAP_NONE = 0,	/** unspecified channel position */
	SND_CHMAP_FIXED,	/** fixed channel position */
	SND_CHMAP_VAR,		/** freely swappable channel position */
	SND_CHMAP_PAIRED,	/** pair-wise swappable channel position */
};

It's not clear to me what the VAR and PAIRED types actually mean. It
would be nice to explain in more detail the concepts in the header.


The snd_pcm_chmap_position enum doesn't contain an entry for mono. I
think it would be a useful channel to have. Up/downmixing decisions may
be different with mono than, for example, a single channel declared as
front left.

-- 
Tanu

Re: [RFC] Channel mapping API (take 2)

[Takashi Iwai]

At Wed, 05 Sep 2012 09:00:12 +0300,
Tanu Kaskinen wrote:
> 
> On Tue, 2012-09-04 at 17:47 +0200, Takashi Iwai wrote:
> > The rest aren't so much changed.  Only slight bug fixes.
> 
> I have a couple of comments about "the rest". I haven't been following
> the development of the channel map API very closely, so I'm relying on
> the pcm.h file only.
> 
> 
> /** channel map list type */
> enum snd_pcm_chmap_type {
> 	SND_CHMAP_NONE = 0,	/** unspecified channel position */
> 	SND_CHMAP_FIXED,	/** fixed channel position */
> 	SND_CHMAP_VAR,		/** freely swappable channel position */
> 	SND_CHMAP_PAIRED,	/** pair-wise swappable channel position */
> };
> 
> It's not clear to me what the VAR and PAIRED types actually mean. It
> would be nice to explain in more detail the concepts in the header.

VAR is for devices that allow swapping the channel position in the
stream arbitrarily, such as HDMI or DP on HD-audio.  For example, with
4.0 stream, you can set like "RL/FR/RR/FL", "FR/RL/FL/RR" or
whatever you like.

Meanwhile, PAIRED is the case where you can swap the channel positions
but the pair must be retained.  That is, you can swap FL/FR and RL/RR
pairs, but not FL and RR.


> The snd_pcm_chmap_position enum doesn't contain an entry for mono. I
> think it would be a useful channel to have. Up/downmixing decisions may
> be different with mono than, for example, a single channel declared as
> front left.

Well, this raises a question "What is mono".
If this is supposed to be a channel position, then we should add the
definition.  But is it really so?  If it's a mono stream mixed from
front left and front right, isn't it a front center?

I don't mind so much to add more definitions, but I'd like rather hear
whether this gives more confusion than its win from the application
POV (especially PA and gstreamer).


Takashi

Re: [RFC] Channel mapping API (take 2)

[Tanu Kaskinen]

On Wed, 2012-09-05 at 08:17 +0200, Takashi Iwai wrote:
> Well, this raises a question "What is mono".
> If this is supposed to be a channel position, then we should add the
> definition.  But is it really so?  If it's a mono stream mixed from
> front left and front right, isn't it a front center?

If it's a playback stream and alsa knows that it will be routed to a
center speaker on a surround system, then yes, it should be front
center. But if the audio ends up in a lone speaker, mixing left and
right doesn't really result in a center position.

I think mono means pretty much "single channel without defined
position", for which SND_CHMAP_UNKNOWN could be a useful choice, except
that I think PulseAudio will end up ignoring channels with
SND_CHMAP_UNKNOWN, because there's no sane definition for how channel
remapping should be done for such channels. But maybe the single channel
case could be treated as an exception - a single channel with
SND_CHMAP_UNKNOWN could be treated as a mono device, for which
downmixing (or upmixing in case of capture) is well defined.

-- 
Tanu

Re: [RFC] Channel mapping API (take 2)

[David Henningsson]

On 09/04/2012 05:47 PM, Takashi Iwai wrote:
> Hi,
>
> my proposal for channel mapping API seems accepted fairly well through
> discussions at Plumbers, so I continued to work on it and uploaded the
> updated version.
>
> The kernel part is found in sound-unstable tree topic/tlv-chmap branch
>    git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-unstable.git
>
> and the alsa-lib part is found in github tree topic/chmap branch
>    git://git.github.com/tiwai/alsa-lib.git
>
> The updated parts are:
>
> - SNDRV_CHMAP_NA was newly added, indicating the channel is not
>    available or silent.
>
> - The channel position value is masked in lower 16bit.
>    The upper bits are used for more channel attributes as bit flags.
>
>    #define SNDRV_CHMAP_POSITION_MASK	0xffff
>
>    The phase inverted channel has this bit:
>    #define SNDRV_CHMAP_PHASE_INVERSE	(0x01 << 16)
>
>    And the driver-specific non-standard channel position has this:
>    #define SNDRV_CHMAP_DRIVER_SPEC	(0x02 << 16)

Just a comment here:

If this is meant to be used for those (mostly pro-audio) cards that just 
have their outputs in simple numbers, maybe we should call it

#define SNDRV_CHMAP_NUMBERED

instead?

>
> - The alsa-lib API functions use snd_pcm_chmap_query_t and
>    snd_pcm_chmap_t instead of ambiguous integer arrays.
>
> ================================================================
> /** the channel map header */
> typedef struct snd_pcm_chmap {
> 	unsigned int channels;
> 	unsigned int pos[0];
> } snd_pcm_chmap_t;
>
> /** the header of array items returned from snd_pcm_query_chmaps() */
> typedef struct snd_pcm_chmap_query {
> 	enum snd_pcm_chmap_type type;
> 	snd_pcm_chmap_t map;
> } snd_pcm_chmap_query_t;
>
>
> snd_pcm_chmap_query_t **snd_pcm_query_chmaps(snd_pcm_t *pcm);
> void snd_pcm_free_chmaps(snd_pcm_chmap_query_t **maps);
> snd_pcm_chmap_t *snd_pcm_get_chmap(snd_pcm_t *pcm);
> int snd_pcm_set_chmap(snd_pcm_t *pcm, const snd_pcm_chmap_t *map);
> ================================================================
>
> The rest aren't so much changed.  Only slight bug fixes.
>
> A big remaining question is whether the current kernel-side
> implementation is OK.  We may add some PCM ops pointer instead of the
> current style using the direct control tlv/read/write override.

That seems nicer, probably.

Also, if nothing is implemented on the kernel side for the specific 
driver (e g we have many older drivers that I suspect no one will write 
an implementation for), what will alsa-lib return in that case?


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

Re: [RFC] Channel mapping API (take 2)

[Takashi Iwai]

At Wed, 05 Sep 2012 08:49:29 +0200,
David Henningsson wrote:
> 
> On 09/04/2012 05:47 PM, Takashi Iwai wrote:
> > Hi,
> >
> > my proposal for channel mapping API seems accepted fairly well through
> > discussions at Plumbers, so I continued to work on it and uploaded the
> > updated version.
> >
> > The kernel part is found in sound-unstable tree topic/tlv-chmap branch
> >    git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-unstable.git
> >
> > and the alsa-lib part is found in github tree topic/chmap branch
> >    git://git.github.com/tiwai/alsa-lib.git
> >
> > The updated parts are:
> >
> > - SNDRV_CHMAP_NA was newly added, indicating the channel is not
> >    available or silent.
> >
> > - The channel position value is masked in lower 16bit.
> >    The upper bits are used for more channel attributes as bit flags.
> >
> >    #define SNDRV_CHMAP_POSITION_MASK	0xffff
> >
> >    The phase inverted channel has this bit:
> >    #define SNDRV_CHMAP_PHASE_INVERSE	(0x01 << 16)
> >
> >    And the driver-specific non-standard channel position has this:
> >    #define SNDRV_CHMAP_DRIVER_SPEC	(0x02 << 16)
> 
> Just a comment here:
> 
> If this is meant to be used for those (mostly pro-audio) cards that just 
> have their outputs in simple numbers, maybe we should call it
> 
> #define SNDRV_CHMAP_NUMBERED
> 
> instead?

Well, "numbered" sounds something like sequential numbers, so I don't
think it's clearer.  The standard channel position is just a number,
too, after all.


> > - The alsa-lib API functions use snd_pcm_chmap_query_t and
> >    snd_pcm_chmap_t instead of ambiguous integer arrays.
> >
> > ================================================================
> > /** the channel map header */
> > typedef struct snd_pcm_chmap {
> > 	unsigned int channels;
> > 	unsigned int pos[0];
> > } snd_pcm_chmap_t;
> >
> > /** the header of array items returned from snd_pcm_query_chmaps() */
> > typedef struct snd_pcm_chmap_query {
> > 	enum snd_pcm_chmap_type type;
> > 	snd_pcm_chmap_t map;
> > } snd_pcm_chmap_query_t;
> >
> >
> > snd_pcm_chmap_query_t **snd_pcm_query_chmaps(snd_pcm_t *pcm);
> > void snd_pcm_free_chmaps(snd_pcm_chmap_query_t **maps);
> > snd_pcm_chmap_t *snd_pcm_get_chmap(snd_pcm_t *pcm);
> > int snd_pcm_set_chmap(snd_pcm_t *pcm, const snd_pcm_chmap_t *map);
> > ================================================================
> >
> > The rest aren't so much changed.  Only slight bug fixes.
> >
> > A big remaining question is whether the current kernel-side
> > implementation is OK.  We may add some PCM ops pointer instead of the
> > current style using the direct control tlv/read/write override.
> 
> That seems nicer, probably.
> 
> Also, if nothing is implemented on the kernel side for the specific 
> driver (e g we have many older drivers that I suspect no one will write 
> an implementation for),

Actually most of drivers with AC97 do support the channel mapping.
ISA drivers are still missing, but they support rarely multi channels,
thus no big merit by this API.

> what will alsa-lib return in that case?

Just an error, NULL.  I can change the function but passing triple
pointer (snd_pcm_chmap_query_t ***) would look really ugly.


Takashi

Re: [RFC] Channel mapping API (take 2)

[David Henningsson]

On 09/04/2012 05:47 PM, Takashi Iwai wrote:
>    The phase inverted channel has this bit:
>    #define SNDRV_CHMAP_PHASE_INVERSE	(0x01 << 16)

Hmm. On second thought, as mics are plugged in and unplugged during 
recording, the channel map will change. (E g, changing a digital mic 
with phase inverse, to a stereo line in)

Will we also need a notify callback in alsa-lib to know changes in the 
channel map?

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

Re: [RFC] Channel mapping API (take 2)

[Takashi Iwai]

At Wed, 05 Sep 2012 10:32:24 +0200,
David Henningsson wrote:
> 
> On 09/04/2012 05:47 PM, Takashi Iwai wrote:
> >    The phase inverted channel has this bit:
> >    #define SNDRV_CHMAP_PHASE_INVERSE	(0x01 << 16)
> 
> Hmm. On second thought, as mics are plugged in and unplugged during 
> recording, the channel map will change. (E g, changing a digital mic 
> with phase inverse, to a stereo line in)
> 
> Will we also need a notify callback in alsa-lib to know changes in the 
> channel map?

It's possible to implement (just a call of snd_ctl_notify()), but I
don't know whether this is expected by the user-space.

Obviously the configuration isn't expected to be changed dynamically
without hw_params change.  So basically ditto for the channel map.

I guess the phase inverse thing makes sense once after we implement
separate PCM streams for d-mic.  Other than that, dynamic switching
the channel map doesn't sound like a helpful scenario.


Takashi

Re: [RFC] Channel mapping API (take 2)

[Raymond Yau]

2012-9-4 下午11:47 於 "Takashi Iwai" <tiwai@suse.de> 寫道：
>
> Hi,
>
> my proposal for channel mapping API seems accepted fairly well through
> discussions at Plumbers, so I continued to work on it and uploaded the
> updated version.
>
> The kernel part is found in sound-unstable tree topic/tlv-chmap branch
>   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-unstable.git
>
> and the alsa-lib part is found in github tree topic/chmap branch
>   git://git.github.com/tiwai/alsa-lib.git
>
> The updated parts are:
>
> - SNDRV_CHMAP_NA was newly added, indicating the channel is not
>   available or silent.
>
> - The channel position value is masked in lower 16bit.
>   The upper bits are used for more channel attributes as bit flags.
>
>   #define SNDRV_CHMAP_POSITION_MASK     0xffff
>
>   The phase inverted channel has this bit:
>   #define SNDRV_CHMAP_PHASE_INVERSE     (0x01 << 16)
>
>   And the driver-specific non-standard channel position has this:
>   #define SNDRV_CHMAP_DRIVER_SPEC       (0x02 << 16)
>
> - The alsa-lib API functions use snd_pcm_chmap_query_t and
>   snd_pcm_chmap_t instead of ambiguous integer arrays.
>
> ================================================================
> /** the channel map header */
> typedef struct snd_pcm_chmap {
>         unsigned int channels;
>         unsigned int pos[0];
> } snd_pcm_chmap_t;
>
> /** the header of array items returned from snd_pcm_query_chmaps() */
> typedef struct snd_pcm_chmap_query {
>         enum snd_pcm_chmap_type type;
>         snd_pcm_chmap_t map;
> } snd_pcm_chmap_query_t;
>
>
> snd_pcm_chmap_query_t **snd_pcm_query_chmaps(snd_pcm_t *pcm);
> void snd_pcm_free_chmaps(snd_pcm_chmap_query_t **maps);
> snd_pcm_chmap_t *snd_pcm_get_chmap(snd_pcm_t *pcm);
> int snd_pcm_set_chmap(snd_pcm_t *pcm, const snd_pcm_chmap_t *map);
> ================================================================
>
> The rest aren't so much changed.  Only slight bug fixes.
>
> A big remaining question is whether the current kernel-side
> implementation is OK.  We may add some PCM ops pointer instead of the
> current style using the direct control tlv/read/write override.
> But the kernel-side API can be changed even later, so I don't worry
> about it so much for now, as long as we can keep the ABI definition.
>
>
some hda codec support AC_WCAP_LR_SWAP (e.g. the "Swap Center/LFE" switch)

git.kernel.org/?p=linux/kernel/git/tiwai/sound.git;a=commit;h=0fb87bb474f978446786263deff6263284e6e011

does your implementation have two channel maps for surround 5.1 for these
idt codecs?

how about the channal map of those internal subwoofer of notebook and those
external subwoofer (e.g. sonicmaster subwoofer )  alc663 using asus-mode4

https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/871808
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [RFC] Channel mapping API (take 2)

[Takashi Iwai]

At Thu, 6 Sep 2012 19:47:16 +0800,
Raymond Yau wrote:
> 
> 2012-9-4 下午11:47 於 "Takashi Iwai" <tiwai@suse.de> 寫道：
> >
> > Hi,
> >
> > my proposal for channel mapping API seems accepted fairly well through
> > discussions at Plumbers, so I continued to work on it and uploaded the
> > updated version.
> >
> > The kernel part is found in sound-unstable tree topic/tlv-chmap branch
> >   git://git.kernel.org/pub/scm/linux/kernel/git/tiwai/sound-unstable.git
> >
> > and the alsa-lib part is found in github tree topic/chmap branch
> >   git://git.github.com/tiwai/alsa-lib.git
> >
> > The updated parts are:
> >
> > - SNDRV_CHMAP_NA was newly added, indicating the channel is not
> >   available or silent.
> >
> > - The channel position value is masked in lower 16bit.
> >   The upper bits are used for more channel attributes as bit flags.
> >
> >   #define SNDRV_CHMAP_POSITION_MASK     0xffff
> >
> >   The phase inverted channel has this bit:
> >   #define SNDRV_CHMAP_PHASE_INVERSE     (0x01 << 16)
> >
> >   And the driver-specific non-standard channel position has this:
> >   #define SNDRV_CHMAP_DRIVER_SPEC       (0x02 << 16)
> >
> > - The alsa-lib API functions use snd_pcm_chmap_query_t and
> >   snd_pcm_chmap_t instead of ambiguous integer arrays.
> >
> > ================================================================
> > /** the channel map header */
> > typedef struct snd_pcm_chmap {
> >         unsigned int channels;
> >         unsigned int pos[0];
> > } snd_pcm_chmap_t;
> >
> > /** the header of array items returned from snd_pcm_query_chmaps() */
> > typedef struct snd_pcm_chmap_query {
> >         enum snd_pcm_chmap_type type;
> >         snd_pcm_chmap_t map;
> > } snd_pcm_chmap_query_t;
> >
> >
> > snd_pcm_chmap_query_t **snd_pcm_query_chmaps(snd_pcm_t *pcm);
> > void snd_pcm_free_chmaps(snd_pcm_chmap_query_t **maps);
> > snd_pcm_chmap_t *snd_pcm_get_chmap(snd_pcm_t *pcm);
> > int snd_pcm_set_chmap(snd_pcm_t *pcm, const snd_pcm_chmap_t *map);
> > ================================================================
> >
> > The rest aren't so much changed.  Only slight bug fixes.
> >
> > A big remaining question is whether the current kernel-side
> > implementation is OK.  We may add some PCM ops pointer instead of the
> > current style using the direct control tlv/read/write override.
> > But the kernel-side API can be changed even later, so I don't worry
> > about it so much for now, as long as we can keep the ABI definition.
> >
> >
> some hda codec support AC_WCAP_LR_SWAP (e.g. the "Swap Center/LFE" switch)
> 
> git.kernel.org/?p=linux/kernel/git/tiwai/sound.git;a=commit;h=0fb87bb474f978446786263deff6263284e6e011
> 
> does your implementation have two channel maps for surround 5.1 for these
> idt codecs?

No.  Feel free to send a patch.

> how about the channal map of those internal subwoofer of notebook and those
> external subwoofer (e.g. sonicmaster subwoofer )  alc663 using asus-mode4
> 
> https://bugs.launchpad.net/ubuntu/+source/alsa-driver/+bug/871808

This should be advertised to match with the current configuration.
It's yet another homework after the framework is integrated.


Takashi
_______________________________________________
Alsa-devel mailing list
Alsa-devel@alsa-project.org
http://mailman.alsa-project.org/mailman/listinfo/alsa-devel

Re: [RFC] Channel mapping API (take 2)

[Tanu Kaskinen]

Hi,

One additional wish for the channel map API: functions for converting
between snd_pcm_chmaps and (machine-readable) strings. If channel map
information is added to UCM, the UCM code will have to parse the channel
map specifications, or if they are exposed as unfiltered strings to
applications, then applications will have to do the parsing. In either
case, having readily available functions to do the parsing would be
useful. Applications may also want to convert the chmap into a string
for logging purposes, for example.

-- 
Tanu

Re: [RFC] Channel mapping API (take 2)

[Takashi Iwai]

At Fri, 07 Sep 2012 12:16:14 +0300,
Tanu Kaskinen wrote:
> 
> Hi,
> 
> One additional wish for the channel map API: functions for converting
> between snd_pcm_chmaps and (machine-readable) strings. If channel map
> information is added to UCM, the UCM code will have to parse the channel
> map specifications, or if they are exposed as unfiltered strings to
> applications, then applications will have to do the parsing. In either
> case, having readily available functions to do the parsing would be
> useful. Applications may also want to convert the chmap into a string
> for logging purposes, for example.

At least, I already planned to provide conversion functions between a
channel and a string.  One concern is that I'd need to give two
different strings, in a short form like "FR" and a long form like
"Front Right".

For printing the whole chmap, I'd take only the shorter form so that
it can be separated by space letters.  Maybe I can provide a parser
from a string, and in this case, also only a short expression will be
allowed, I guess.


Takashi