[alsa-devel] [RFC] Channel mapping API (take 2)
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
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.
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
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.
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?
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
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?
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
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
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
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 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
participants (4)
-
David Henningsson -
Raymond Yau -
Takashi Iwai -
Tanu Kaskinen