[PATCH] ASoC: hdmi-codec: Fix broken channel map reporting

[PATCH] ASoC: hdmi-codec: Fix broken channel map reporting

[Matthias Reichl]

Commit 4e0871333661 ("ASoC: hdmi-codec: fix channel info for
compressed formats") accidentally changed hcp->chmap_idx from
ca_id, the CEA channel allocation ID, to idx, the index to
the table of channel mappings ordered by preference.

This resulted in wrong channel maps being reported to userspace,
eg for 5.1 "FL,FR,LFE,FC" was reported instead of the expected
"FL,FR,LFE,FC,RL,RR":

~ # speaker-test -c 6 -t sine
...
 0 - Front Left
 3 - Front Center
 1 - Front Right
 2 - LFE
 4 - Unknown
 5 - Unknown

~ # amixer cget iface=PCM,name='Playback Channel Map' | grep ': values'
  : values=3,4,8,7,0,0,0,0

Revert this incorrect change so that channel maps are properly
reported again.

Fixes: 4e0871333661 ("ASoC: hdmi-codec: fix channel info for compressed formats")
Cc: stable@vger.kernel.org
Signed-off-by: Matthias Reichl <hias@horus.com>
---
 sound/soc/codecs/hdmi-codec.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/sound/soc/codecs/hdmi-codec.c b/sound/soc/codecs/hdmi-codec.c
index 13689e718d36f..c8e48225598f8 100644
--- a/sound/soc/codecs/hdmi-codec.c
+++ b/sound/soc/codecs/hdmi-codec.c
@@ -531,7 +531,7 @@ static int hdmi_codec_fill_codec_params(struct snd_soc_dai *dai,
 	hp->sample_rate = sample_rate;
 	hp->channels = channels;
 
-	hcp->chmap_idx = idx;
+	hcp->chmap_idx = ca_id;
 
 	return 0;
 }
-- 
2.39.2

Re: [PATCH] ASoC: hdmi-codec: Fix broken channel map reporting

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 645 bytes --]

On Sat, Sep 09, 2023 at 01:46:33PM +0200, Matthias Reichl wrote:
> Commit 4e0871333661 ("ASoC: hdmi-codec: fix channel info for
> compressed formats") accidentally changed hcp->chmap_idx from
> ca_id, the CEA channel allocation ID, to idx, the index to
> the table of channel mappings ordered by preference.
> 
> This resulted in wrong channel maps being reported to userspace,
> eg for 5.1 "FL,FR,LFE,FC" was reported instead of the expected
> "FL,FR,LFE,FC,RL,RR":

Presumably this will cause a regression for people using compressed
formats - isn't the fix here to make this behaviour conditional on if
the format is compressed?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: hdmi-codec: Fix broken channel map reporting

[Matthias Reichl]

On Sat, Sep 09, 2023 at 01:45:38PM +0100, Mark Brown wrote:
> On Sat, Sep 09, 2023 at 01:46:33PM +0200, Matthias Reichl wrote:
> > Commit 4e0871333661 ("ASoC: hdmi-codec: fix channel info for
> > compressed formats") accidentally changed hcp->chmap_idx from
> > ca_id, the CEA channel allocation ID, to idx, the index to
> > the table of channel mappings ordered by preference.
> > 
> > This resulted in wrong channel maps being reported to userspace,
> > eg for 5.1 "FL,FR,LFE,FC" was reported instead of the expected
> > "FL,FR,LFE,FC,RL,RR":
> 
> Presumably this will cause a regression for people using compressed
> formats - isn't the fix here to make this behaviour conditional on if
> the format is compressed?

This change won't affect passthrough, the values of the HDMI audio
infoframe are stored separately, in hdmi_codec_params.cea.

chmap_idx in hdmi_codec_priv is only used by the get PCM channel map
control - which userspace shouldn't use in the non-PCM case.

Before the "fix channel info for compressed formats" change the control
would always return a matching channel map, i.e. "FL FR" in the 2-channel
case, regardless if PCM or non-PCM was used.

When using high bitrate compressed streams, which are passed through as
8ch (containing 4 consecutive frames), this leads to a problem though as
the sink might not announce support for 8 speakers (eg on TVs with 2 speakers
and "virtual surround") and thus finding a channel map would fail.

The "fix channel info" patch addressed this issue by only searching for a
channel map in the PCM case.

In the non-PCM case ca_id is set to 0 which means "FL, FR" - CTA doesn't
have a separate value for "n/a" but specifies that the channel allocation
only applies to PCM streams with more than 2 channels. Although the spec
says the value doesn't apply to non-PCM some TVs seem to look at it and
refuse to output HBR (TrueHD etc) if it's set to something other than 0.

My plan was to return the exact same info via the channel map control
as we set in the info frame but unfortunately I messed up and accidentally
used the wrong value which this patch tries to rectify.

I'm not really sure though if that's the best (or even proper) way for
the channel map control to behave in the non-PCM case - it'll either return
"FL, FR" or "FL, FR, UNKNOWN, UNKNOWN, UNKNOWN, UNKNOWN, UNKNOWN, UNKNOWN"
with this patch.

An alternative would be to set chmap_idx to HDMI_CODEC_CHMAP_IDX_UNKNOWN
in the non-PCM case so the channel map control will return UNKNOWN for
all channels. i.e. use this code instead:

        if (pcm_audio)
                hcp->chmap_idx = ca_id;
        else
                hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;

Any input on that topic is highly appreciated.

so long & thanks,

Hias

Re: [PATCH] ASoC: hdmi-codec: Fix broken channel map reporting

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 619 bytes --]

On Sun, Sep 10, 2023 at 02:14:34PM +0200, Matthias Reichl wrote:

> An alternative would be to set chmap_idx to HDMI_CODEC_CHMAP_IDX_UNKNOWN
> in the non-PCM case so the channel map control will return UNKNOWN for
> all channels. i.e. use this code instead:

>         if (pcm_audio)
>                 hcp->chmap_idx = ca_id;
>         else
>                 hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;

> Any input on that topic is highly appreciated.

This is going to depend a bit on what userspace is expecting which I
don't have a good feel for.  The above does look reasonable TBH, I'm
tempted to go for that.

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]

Re: [PATCH] ASoC: hdmi-codec: Fix broken channel map reporting

[Mark Brown]

[-- Attachment #1: Type: text/plain, Size: 507 bytes --]

On Sun, Sep 10, 2023 at 02:14:34PM +0200, Matthias Reichl wrote:

> An alternative would be to set chmap_idx to HDMI_CODEC_CHMAP_IDX_UNKNOWN
> in the non-PCM case so the channel map control will return UNKNOWN for
> all channels. i.e. use this code instead:

>         if (pcm_audio)
>                 hcp->chmap_idx = ca_id;
>         else
>                 hcp->chmap_idx = HDMI_CODEC_CHMAP_IDX_UNKNOWN;

> Any input on that topic is highly appreciated.

Can you send this version as a real patch please?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]