Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for ALSA channel mapping API #4868

Merged
merged 4 commits into from
Jul 5, 2014
Merged

Conversation

anssih
Copy link
Member

@anssih anssih commented Jun 8, 2014

Add support for the ALSA channel mapping API. This functionality
requires alsa-lib v1.0.27.

This allows
1) selection of a best-matching channel layout when multiple layouts are
   available, and
2) reporting the actual layout back to AE, instead of the ALSA default
   legacy layout which is sometimes incorrect.

In practice, this means:

1) Devices that do not use the standard ALSA layout will now have
   correct channel mapping without ALSA configuration tricks. Such
   devices include at least:
    - 1st gen NVIDIA ION
    - some 2.1 laptop speakers
    - many embedded audio devices

2) HDMI output is now possible at "proper" 2.1, 5.0 PCM layouts (and any
   of the more unusual ones, such as 4.1, 3f, 2f+1r) without resorting to
   adding empty channels as such layouts are handled currently. This
   means that the A/V receiver will now see the correct format, instead
   of the "virtual" 5.1/7.1.

3) HDMI output is now configured to use the exact layout given from AE,
   when possible, so that AE does not need to do any remapping if it
   does not want to (the audio hardware will perform that instead).

4) Layouts containing the more unusual channel positions such as
   AE_CH_TFL, AE_CH_TC, etc, are now supported by the ALSA sink.
   However, currently CActiveAE::ApplySettingsToFormat() always
   force-resolves the stream layout to a standard layout, preventing
   the playback of such channels for now.

2,3,4 above require this post-v1.0.27.2 alsa-lib fix in most cases:
http://git.alsa-project.org/?p=alsa-lib.git;a=commitdiff;h=042223c4ee9b17ba8a24bcfc4019f24b69b8310b

Some Linux kernels between 3.7 and 3.12 contain bugs in the HDA HDMI
driver causing incorrect multichannel mapping in some cases. The most
severe issues that affected common channel layouts were fixed in
November 2013 in versions 3.10.20, 3.11.9, 3.12.1, the rest in 3.12.15
and 3.13.

To avoid any regressions, blacklist everything between 3.7 and 3.12.15.

@fritsch, @FernetMenta - opinions and suggestions welcome.

@MartijnKaijser MartijnKaijser added this to the Pending for inclusion milestone Jun 9, 2014
@anssih
Copy link
Member Author

anssih commented Jun 13, 2014

Changes:

  • fixed style
  • reduced new logging (one message moved to verbose sound logging, one folded to an existing one, no other messages)
  • use strverscmp() and g_sysinfo.GetKernelVersionFull()
  • fix a leftover mistake in CAEChannelInfo::BestMatch ("dropped" was bool, should be int)

Add support for the ALSA channel mapping API. This functionality
requires alsa-lib v1.0.27.

This allows
1) selection of a best-matching channel layout when multiple layouts are
   available, and
2) reporting the actual layout back to AE, instead of the ALSA default
   legacy layout which is sometimes incorrect.

In practice, this means:

1) Devices that do not use the standard ALSA layout will now have
   correct channel mapping without ALSA configuration tricks. Such
   devices include at least:
    - 1st gen NVIDIA ION
    - some 2.1 laptop speakers
    - many embedded audio devices

2) HDMI output is now possible at "proper" 2.1, 5.0 PCM layouts (and any
   of the more unusual ones, such as 4.1, 3f, 2f+1r) without resorting to
   adding empty channels as such layouts are handled currently. This
   means that the A/V receiver will now see the correct format, instead
   of the "virtual" 5.1/7.1.

3) HDMI output is now configured to use the exact layout given from AE,
   when possible, so that AE does not need to do any remapping if it
   does not want to (the audio hardware will perform that instead).

4) Layouts containing the more unusual channel positions such as
   AE_CH_TFL, AE_CH_TC, etc, are now supported by the ALSA sink.
   However, currently CActiveAE::ApplySettingsToFormat() always
   force-resolves the stream layout to a standard layout, preventing
   the playback of such channels for now.

2,3,4 above require this post-v1.0.27.2 alsa-lib fix in most cases:
http://git.alsa-project.org/?p=alsa-lib.git;a=commitdiff;h=042223c4ee9b17ba8a24bcfc4019f24b69b8310b

Some Linux kernels between 3.7 and 3.12 contain bugs in the HDA HDMI
driver causing incorrect multichannel mapping in some cases. The most
severe issues that affected common channel layouts were fixed in
November 2013 in versions 3.10.20, 3.11.9, 3.12.1, the rest in 3.12.15
and 3.13.

To avoid any regressions, blacklist channel mapping API on kernel
versions older than 3.12.15.
@FernetMenta
Copy link
Contributor

jenkins build this please

@anssih anssih modified the milestones: Helix 14.0-alpha2, Pending for inclusion Jun 30, 2014
@anssih
Copy link
Member Author

anssih commented Jun 30, 2014

I intend to merge this in during this merge window.

Unless anyone has some suggestions on how to make it cleaner or something, of course... I have to admit I myself feel like the complexity/benefit ratio is a bit high as a whole in this PR.

@FernetMenta
Copy link
Contributor

go ahead. I have already reused ReplaceChannel and BestMatch in #4938

@anssih
Copy link
Member Author

anssih commented Jul 5, 2014

jenkins build this please

anssih added a commit that referenced this pull request Jul 5, 2014
Add support for ALSA channel mapping API
@anssih anssih merged commit f318de6 into xbmc:master Jul 5, 2014
@rhocheck
Copy link

@anssih could you have a look at this case: http://forum.xbmc.org/showthread.php?tid=203120

User reports ALSA is broken after June 28th. Look at the log:

Channel layout looks weird and format: S24NE4 seems wrong because it is not listed in enumeration.

07:55:15 T:140164266624768   DEBUG: CAESinkALSA::GetChannelLayout - Got Layout: FL,FR (ALSA: none)
07:55:15 T:140164266624768   DEBUG: CActiveAESink::OpenSink - ALSA Initialized:
07:55:15 T:140164266624768   DEBUG:   Output Device : Xonar DGX
07:55:15 T:140164266624768   DEBUG:   Sample Rate   : 44100
07:55:15 T:140164266624768   DEBUG:   Sample Format : AE_FMT_S24NE4

@anssih
Copy link
Member Author

anssih commented Aug 28, 2014

@rhocheck Thanks for the notice, I'll take a look today when I get home.

@FernetMenta
Copy link
Contributor

thx, I was logged in with wrong account.

@anssih
Copy link
Member Author

anssih commented Aug 28, 2014

Looks like the reported issue is not related to this pull, opened PR #5297 with a likely fix.

@FernetMenta
Copy link
Contributor

@anssih seems we have an issue related to this:

21:44:03 T:139734798108416    INFO: CAESinkALSA::Initialize - Opened device "hdmi:CARD=NVidia,DEV=1,AES0=0x04,AES1=0x82,AES2=0x00,AES3=0x02"
21:44:03 T:139734798108416    INFO: CAESinkALSA - ALSA: pcm_hw.c:1069:(snd_pcm_query_chmaps_from_hw) Cannot read Channel Map TLV
                                            : Cannot allocate memory

in turn we get only 2 channels.
http://forum.xbmc.org/showthread.php?tid=203536&pid=1787906#pid1787906

@anssih
Copy link
Member Author

anssih commented Sep 5, 2014

@FernetMenta That failure (caused by old alsa-lib with a newish kernel) just means that no channel map will be selected (legacy mode), it does not affect channel count.

From the log looks like the EDID says stereo only (see enumeration), which causes the driver to restrict us to 2 channels.

I'll have more time later today to actually read the thread.

@fritsch
Copy link
Member

fritsch commented Sep 5, 2014

Yes. That user tunes 5 things at once without telling. Currently he forces
a wrong edid. Looks like his tv's rather than avr.

Sorry for the noise.
Am 05.09.2014 10:56 schrieb "Anssi Hannula" notifications@github.com:

@FernetMenta https://github.com/FernetMenta That failure (caused by old
alsa-lib) just means that no channel map will be selected (legacy mode), it
does not affect channel count.

From the log looks like the EDID says stereo only (see enumeration), which
causes the driver to restrict us to 2 channels.

I'll have more time later today to actually read the thread.

Reply to this email directly or view it on GitHub
#4868 (comment).

@anssih
Copy link
Member Author

anssih commented Sep 5, 2014

@fritsch Yeah, I just took a second look a bit ago and noticed he had forced EDID and you had already asked him to produce a log without that :) . I'll keep an eye on the thread.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants