[alsa-devel] Duplicated/weird SND_CHMAP_xx positions
Hi,
I see we have both: SND_CHMAP_FLH, /**< front left high */ SND_CHMAP_FCH, /**< front center high */ SND_CHMAP_FRH, /**< front right high */ and SND_CHMAP_TFL, /**< top front left */ SND_CHMAP_TFR, /**< top front right */ SND_CHMAP_TFC, /**< top front center */
However, these are actually exactly the same AFAICS, both are the speakers above the front speakers (CEA/HDMI uses "high" and USB audio uses "top" nomenclature).
I guess we should deprecate one set? I can't think of anything else than adding comments saying they are deprecated, though...
I guess I'd prefer to deprecate the "high" ones since we have many other "top" and "bottom" channels.
Also, the only in-kernel use of "high" is by HDA HDMI and it only will start working in 3.13-rcX.
We have also these: SND_CHMAP_BC, /**< bottom center */ SND_CHMAP_BLC, /**< bottom left center */ SND_CHMAP_BRC, /**< bottom right center */
I guess "bottom center" is "under the listener" (USB audio v2 spec doesn't clarify, but clearly doesn't use the "Front" specifier as in "front bottom center").
However, the last two ones are weird. The descriptions do not make sense to me (under the listener to the left? wouldn't that be "bottom side left" instead?).
It seems like they were added by mistake - the only user is the USB audio driver which wrongly uses them for "back left/right center" instead of correct RLC/RRC.
For reference, the complete current position list is: /** channel positions */ enum snd_pcm_chmap_position { SND_CHMAP_UNKNOWN = 0, /**< unspecified */ SND_CHMAP_NA, /**< N/A, silent */ SND_CHMAP_MONO, /**< mono stream */ SND_CHMAP_FL, /**< front left */ SND_CHMAP_FR, /**< front right */ SND_CHMAP_RL, /**< rear left */ SND_CHMAP_RR, /**< rear right */ SND_CHMAP_FC, /**< front center */ SND_CHMAP_LFE, /**< LFE */ SND_CHMAP_SL, /**< side left */ SND_CHMAP_SR, /**< side right */ SND_CHMAP_RC, /**< rear center */ SND_CHMAP_FLC, /**< front left center */ SND_CHMAP_FRC, /**< front right center */ SND_CHMAP_RLC, /**< rear left center */ SND_CHMAP_RRC, /**< rear right center */ SND_CHMAP_FLW, /**< front left wide */ SND_CHMAP_FRW, /**< front right wide */ SND_CHMAP_FLH, /**< front left high */ SND_CHMAP_FCH, /**< front center high */ SND_CHMAP_FRH, /**< front right high */ SND_CHMAP_TC, /**< top center */ SND_CHMAP_TFL, /**< top front left */ SND_CHMAP_TFR, /**< top front right */ SND_CHMAP_TFC, /**< top front center */ SND_CHMAP_TRL, /**< top rear left */ SND_CHMAP_TRR, /**< top rear right */ SND_CHMAP_TRC, /**< top rear center */ SND_CHMAP_TFLC, /**< top front left center */ SND_CHMAP_TFRC, /**< top front right center */ SND_CHMAP_TSL, /**< top side left */ SND_CHMAP_TSR, /**< top side right */ SND_CHMAP_LLFE, /**< left LFE */ SND_CHMAP_RLFE, /**< right LFE */ SND_CHMAP_BC, /**< bottom center */ SND_CHMAP_BLC, /**< bottom left center */ SND_CHMAP_BRC, /**< bottom right center */ SND_CHMAP_LAST = SND_CHMAP_BRC, };
Channel map positions FLH, FCH, FRH duplicate positions TFL, TFC, TFR. Both are the speakers above the front speakers (CEA uses "high" and USB audio uses "top" nomenclature).
Since the USB audio code has used the TFx positions since v3.8 (04324ccc75f96, "ALSA: usb-audio: add channel map support") but the HDMI code only just started using FxH in a5b7d510b2220cccb ("ALSA: hda - hdmi: Fix channel maps with less common speakers") which is not yet in any released kernel, standardize on TFx instead.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi --- sound/pci/hda/patch_hdmi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index fe3b150f97d9..4ad8c055c221 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -763,12 +763,12 @@ static struct channel_map_table map_tables[] = { { SNDRV_CHMAP_RC, RC }, { SNDRV_CHMAP_FLC, FLC }, { SNDRV_CHMAP_FRC, FRC }, - { SNDRV_CHMAP_FLH, FLH }, - { SNDRV_CHMAP_FRH, FRH }, + { SNDRV_CHMAP_TFL, FLH }, + { SNDRV_CHMAP_TFR, FRH }, { SNDRV_CHMAP_FLW, FLW }, { SNDRV_CHMAP_FRW, FRW }, { SNDRV_CHMAP_TC, TC }, - { SNDRV_CHMAP_FCH, FCH }, + { SNDRV_CHMAP_TFC, FCH }, {} /* terminator */ };
At Sun, 10 Nov 2013 21:24:04 +0200, Anssi Hannula wrote:
Channel map positions FLH, FCH, FRH duplicate positions TFL, TFC, TFR. Both are the speakers above the front speakers (CEA uses "high" and USB audio uses "top" nomenclature).
Since the USB audio code has used the TFx positions since v3.8 (04324ccc75f96, "ALSA: usb-audio: add channel map support") but the HDMI code only just started using FxH in a5b7d510b2220cccb ("ALSA: hda - hdmi: Fix channel maps with less common speakers") which is not yet in any released kernel, standardize on TFx instead.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi
Applied both patches now. Thanks.
Takashi
sound/pci/hda/patch_hdmi.c | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-)
diff --git a/sound/pci/hda/patch_hdmi.c b/sound/pci/hda/patch_hdmi.c index fe3b150f97d9..4ad8c055c221 100644 --- a/sound/pci/hda/patch_hdmi.c +++ b/sound/pci/hda/patch_hdmi.c @@ -763,12 +763,12 @@ static struct channel_map_table map_tables[] = { { SNDRV_CHMAP_RC, RC }, { SNDRV_CHMAP_FLC, FLC }, { SNDRV_CHMAP_FRC, FRC },
- { SNDRV_CHMAP_FLH, FLH },
- { SNDRV_CHMAP_FRH, FRH },
- { SNDRV_CHMAP_TFL, FLH },
- { SNDRV_CHMAP_TFR, FRH }, { SNDRV_CHMAP_FLW, FLW }, { SNDRV_CHMAP_FRW, FRW }, { SNDRV_CHMAP_TC, TC },
- { SNDRV_CHMAP_FCH, FCH },
- { SNDRV_CHMAP_TFC, FCH }, {} /* terminator */
};
-- 1.8.1.5
According to USB Audio spec v2 bits 25 and 26 of bmChannelConfig are "Back Left of Center - BLC" and "Back Right of Center - BRC", respectively.
They are currently assigned to ALSA channels BLC/BRC. However, the ALSA BLC/BRC are actually the rather nonsensical "bottom left center" and "bottom right center", so the channels will be assigned wrongly. The comments in the USB code are also similarly wrong, so this is not readily apparent without looking at the actual specification.
Fix the channel mapping by mapping bits 25 and 26 to RLC (Rear Left Center) and RRC (Rear Right Center), respectively, instead.
Signed-off-by: Anssi Hannula anssi.hannula@iki.fi --- sound/usb/stream.c | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/sound/usb/stream.c b/sound/usb/stream.c index d737d0e6e558..2fb71be5e100 100644 --- a/sound/usb/stream.c +++ b/sound/usb/stream.c @@ -273,8 +273,8 @@ static struct snd_pcm_chmap_elem *convert_chmap(int channels, unsigned int bits, SNDRV_CHMAP_TSL, /* top side left */ SNDRV_CHMAP_TSR, /* top side right */ SNDRV_CHMAP_BC, /* bottom center */ - SNDRV_CHMAP_BLC, /* bottom left center */ - SNDRV_CHMAP_BRC, /* bottom right center */ + SNDRV_CHMAP_RLC, /* back left of center */ + SNDRV_CHMAP_RRC, /* back right of center */ 0 /* terminator */ }; struct snd_pcm_chmap_elem *chmap;
At Sun, 10 Nov 2013 18:19:26 +0200, Anssi Hannula wrote:
Hi,
I see we have both: SND_CHMAP_FLH, /**< front left high */ SND_CHMAP_FCH, /**< front center high */ SND_CHMAP_FRH, /**< front right high */ and SND_CHMAP_TFL, /**< top front left */ SND_CHMAP_TFR, /**< top front right */ SND_CHMAP_TFC, /**< top front center */
However, these are actually exactly the same AFAICS, both are the speakers above the front speakers (CEA/HDMI uses "high" and USB audio uses "top" nomenclature).
Oh, a good catch.
I guess we should deprecate one set? I can't think of anything else than adding comments saying they are deprecated, though...
Yeah, commenting would be the best effort...
I guess I'd prefer to deprecate the "high" ones since we have many other "top" and "bottom" channels.
Indeed, TOP elements are already used in the recent patch in gstreamer, so we should keep them.
Also, the only in-kernel use of "high" is by HDA HDMI and it only will start working in 3.13-rcX.
Right, and this can be fixed in 3.13 cycle.
We have also these: SND_CHMAP_BC, /**< bottom center */ SND_CHMAP_BLC, /**< bottom left center */ SND_CHMAP_BRC, /**< bottom right center */
I guess "bottom center" is "under the listener" (USB audio v2 spec doesn't clarify, but clearly doesn't use the "Front" specifier as in "front bottom center").
However, the last two ones are weird. The descriptions do not make sense to me (under the listener to the left? wouldn't that be "bottom side left" instead?).
It seems like they were added by mistake - the only user is the USB audio driver which wrongly uses them for "back left/right center" instead of correct RLC/RRC.
These are taken from gstreamer definitions. I'm not quite sure about these distinctions, but added just for completeness in any future use case. So, just keep these unused for now.
Takashi
For reference, the complete current position list is: /** channel positions */ enum snd_pcm_chmap_position { SND_CHMAP_UNKNOWN = 0, /**< unspecified */ SND_CHMAP_NA, /**< N/A, silent */ SND_CHMAP_MONO, /**< mono stream */ SND_CHMAP_FL, /**< front left */ SND_CHMAP_FR, /**< front right */ SND_CHMAP_RL, /**< rear left */ SND_CHMAP_RR, /**< rear right */ SND_CHMAP_FC, /**< front center */ SND_CHMAP_LFE, /**< LFE */ SND_CHMAP_SL, /**< side left */ SND_CHMAP_SR, /**< side right */ SND_CHMAP_RC, /**< rear center */ SND_CHMAP_FLC, /**< front left center */ SND_CHMAP_FRC, /**< front right center */ SND_CHMAP_RLC, /**< rear left center */ SND_CHMAP_RRC, /**< rear right center */ SND_CHMAP_FLW, /**< front left wide */ SND_CHMAP_FRW, /**< front right wide */ SND_CHMAP_FLH, /**< front left high */ SND_CHMAP_FCH, /**< front center high */ SND_CHMAP_FRH, /**< front right high */ SND_CHMAP_TC, /**< top center */ SND_CHMAP_TFL, /**< top front left */ SND_CHMAP_TFR, /**< top front right */ SND_CHMAP_TFC, /**< top front center */ SND_CHMAP_TRL, /**< top rear left */ SND_CHMAP_TRR, /**< top rear right */ SND_CHMAP_TRC, /**< top rear center */ SND_CHMAP_TFLC, /**< top front left center */ SND_CHMAP_TFRC, /**< top front right center */ SND_CHMAP_TSL, /**< top side left */ SND_CHMAP_TSR, /**< top side right */ SND_CHMAP_LLFE, /**< left LFE */ SND_CHMAP_RLFE, /**< right LFE */ SND_CHMAP_BC, /**< bottom center */ SND_CHMAP_BLC, /**< bottom left center */ SND_CHMAP_BRC, /**< bottom right center */ SND_CHMAP_LAST = SND_CHMAP_BRC, };
-- Anssi Hannula
11.11.2013 18:05, Takashi Iwai kirjoitti:
At Sun, 10 Nov 2013 18:19:26 +0200, Anssi Hannula wrote:
[...]
We have also these: SND_CHMAP_BC, /**< bottom center */ SND_CHMAP_BLC, /**< bottom left center */ SND_CHMAP_BRC, /**< bottom right center */
I guess "bottom center" is "under the listener" (USB audio v2 spec doesn't clarify, but clearly doesn't use the "Front" specifier as in "front bottom center").
However, the last two ones are weird. The descriptions do not make sense to me (under the listener to the left? wouldn't that be "bottom side left" instead?).
It seems like they were added by mistake - the only user is the USB audio driver which wrongly uses them for "back left/right center" instead of correct RLC/RRC.
These are taken from gstreamer definitions. I'm not quite sure about these distinctions, but added just for completeness in any future use case. So, just keep these unused for now.
gst-1.0 has: GST_AUDIO_CHANNEL_POSITION_BOTTOM_FRONT_CENTER, GST_AUDIO_CHANNEL_POSITION_BOTTOM_FRONT_LEFT, GST_AUDIO_CHANNEL_POSITION_BOTTOM_FRONT_RIGHT,
So they are clearly labelled "front" and only the first one is "center".
Your gst patch (at least the ml version I saw) mapped them like this: + ITEM(BC, BOTTOM_FRONT_CENTER), + ITEM(BLC, BOTTOM_FRONT_LEFT), + ITEM(BRC, BOTTOM_FRONT_LEFT),
But I was under the assumption that BC is under the listener instead of front center, since otherwise it would be "BFC", not "BC". (though I don't find it unlikely at all that "bottom-center" got to the USB spec by a mistake... but if it were just a case of missing "front" specifier, one'd think the spec would have "bottom-front-left" and "bottom-front-right" as well, which it does not)
Also, clearly BLC is not bottom front left, since the 'C' stands for "center". Not to mention BRC being right, not left. ;)
So I guess the nonsensical BLC/BRC was just a mistake then... I guess we can pretend they are between BC and BSL/BSR (bottom side left/right) or something.
For reference, the complete current position list is: /** channel positions */ enum snd_pcm_chmap_position { SND_CHMAP_UNKNOWN = 0, /**< unspecified */ SND_CHMAP_NA, /**< N/A, silent */ SND_CHMAP_MONO, /**< mono stream */ SND_CHMAP_FL, /**< front left */ SND_CHMAP_FR, /**< front right */ SND_CHMAP_RL, /**< rear left */ SND_CHMAP_RR, /**< rear right */ SND_CHMAP_FC, /**< front center */ SND_CHMAP_LFE, /**< LFE */ SND_CHMAP_SL, /**< side left */ SND_CHMAP_SR, /**< side right */ SND_CHMAP_RC, /**< rear center */ SND_CHMAP_FLC, /**< front left center */ SND_CHMAP_FRC, /**< front right center */ SND_CHMAP_RLC, /**< rear left center */ SND_CHMAP_RRC, /**< rear right center */ SND_CHMAP_FLW, /**< front left wide */ SND_CHMAP_FRW, /**< front right wide */ SND_CHMAP_FLH, /**< front left high */ SND_CHMAP_FCH, /**< front center high */ SND_CHMAP_FRH, /**< front right high */ SND_CHMAP_TC, /**< top center */ SND_CHMAP_TFL, /**< top front left */ SND_CHMAP_TFR, /**< top front right */ SND_CHMAP_TFC, /**< top front center */ SND_CHMAP_TRL, /**< top rear left */ SND_CHMAP_TRR, /**< top rear right */ SND_CHMAP_TRC, /**< top rear center */ SND_CHMAP_TFLC, /**< top front left center */ SND_CHMAP_TFRC, /**< top front right center */ SND_CHMAP_TSL, /**< top side left */ SND_CHMAP_TSR, /**< top side right */ SND_CHMAP_LLFE, /**< left LFE */ SND_CHMAP_RLFE, /**< right LFE */ SND_CHMAP_BC, /**< bottom center */ SND_CHMAP_BLC, /**< bottom left center */ SND_CHMAP_BRC, /**< bottom right center */ SND_CHMAP_LAST = SND_CHMAP_BRC, };
-- Anssi Hannula
Alsa-devel mailing list Alsa-devel@alsa-project.org http://mailman.alsa-project.org/mailman/listinfo/alsa-devel
participants (2)
-
Anssi Hannula
-
Takashi Iwai