Rewrite ALSA device enumeration and selection code #987

Merged
merged 2 commits into from Jul 10, 2012

Conversation

Projects
None yet
7 participants
Member

anssih commented May 20, 2012

The current ALSA device enumeration code steps through all the hardware
ALSA cards and devices while trying to guess the symbolic "hdmi" and
"iec958" names that will be used with ALSA. It misses any virtual
devices, such as "pulse" or "default". In addition, it didn't have any
handling for "surroundXX", which is needed on many cards to get
multi-channel analog output. Also, it didn't find "iec958" devices that
have no separate hardware device from the analog device, like on USB
sound cards.

Solve those issues by rewriting the ALSA device handling code to use the
ALSA device name hints to determine which devices should be used. This
is the same method "aplay -L" uses.

The code will automatically use the correct front/surround40/surround51/
surround71 device depending on the amount of channels requested. This is
achieved with a magic "@" in the beginning of the device string, which
will be replaced with the wanted base device name ("front",
"surround51", etc.). In addition, the "sysdefault" and "default"
dmix-enabled (allowing shared usage) outputs will be automatically used
when available, for stereo streams that have the same sample rate as the
ALSA dmix mixer has (normally 48000Hz).

The enumeration code will now add a preferred "Default" device, which is
the ALSA default device. This allows XBMC to follow the system-wide
configuration, so that when the user e.g. modifies /etc/asound.conf or
changes environment variables ($ALSA_CARD etc.), XBMC will follow those.
The device name of the "Default" device will be either "default" or "@",
depending on if the default device needs "surroundXX" mangling for
multichannel support (normal analog devices need it, but if the ALSA
default is e.g. the pulseaudio sound server, it is not needed nor
wanted).

Also, additional differentation (device number and/or card id) is now
added to the device names when identical names were found by
enumeration.

Additionally, the AES parameters are now set even when not using
passthrough for S/PDIF and HDMI devices as specified by IEC 60958.
The code will fallback to not setting the parameters since it may
sometimes fail if e.g. using a custom device that does not accept them.

The behavior of setting parameters (channel count, sample rate) is not
altered.

anssih added some commits May 20, 2012

@anssih anssih rewritten: ALSA device enumeration and selection code
The current ALSA device enumeration code steps through all the hardware
ALSA cards and devices while trying to guess the symbolic "hdmi" and
"iec958" names that will be used with ALSA. It misses any virtual
devices, such as "pulse" or "default". In addition, it didn't have any
handling for "surroundXX", which is needed on many cards to get
multi-channel analog output. Also, it didn't find "iec958" devices that
have no separate hardware device from the analog device, like on USB
sound cards.

Solve those issues by rewriting the ALSA device handling code to use the
ALSA device name hints to determine which devices should be used. This
is the same method "aplay -L" uses.

The code will automatically use the correct front/surround40/surround51/
surround71 device depending on the amount of channels requested. This is
achieved with a magic "@" in the beginning of the device string, which
will be replaced with the wanted base device name ("front",
"surround51", etc.). In addition, the "sysdefault" and "default"
dmix-enabled (allowing shared usage) outputs will be automatically used
when available, for stereo streams that have the same sample rate as the
ALSA dmix mixer has (normally 48000Hz).

The enumeration code will now add a preferred "Default" device, which is
the ALSA default device. This allows XBMC to follow the system-wide
configuration, so that when the user e.g. modifies /etc/asound.conf or
changes environment variables ($ALSA_CARD etc.), XBMC will follow those.
The device name of the "Default" device will be either "default" or "@",
depending on if the default device needs "surroundXX" mangling for
multichannel support (normal analog devices need it, but if the ALSA
default is e.g. the pulseaudio sound server, it is not needed nor
wanted).

Also, additional differentation (device number and/or card id) is now
added to the device names when identical names were found by
enumeration.

Additionally, the AES parameters are now set even when not using
passthrough for S/PDIF and HDMI devices as specified by IEC 60958.
The code will fallback to not setting the parameters since it may
sometimes fail if e.g. using a custom device that does not accept them.

The behavior of setting parameters (channel count, sample rate) is not
altered.
4c1bcc7
@anssih anssih added: log wrapper for ALSA errors b73081c
Member

gnif commented May 20, 2012

I agree, I discovered the same issue a few days ago.

I've merged in these changes to my HTPC and can now get HDMI working using the standard GUI options, e.g. without needing to use plughw:1,7.

I still don't get HDMI device for passthrough on ATI (fglrx).
aplay -L:

hdmi:CARD=Generic,DEV=0
    HD-Audio Generic, HDMI 0
    HDMI Audio Output

This device is not listed under enumerated ALSA devices, only plughw:CARD=Generic,DEV=3 which I can select as audio device.

Owner

anssih replied May 21, 2012

@FernetMenta, yes, but I think I know what the issue could be, was waiting for you to come in IRC so that I could get more info so that I could fix it in the best way possible :)

Specifically, could you give the output of "amixer contents -c Generic"?

I am stuck in telephone conferences ....

xbmc@AD02:~$ amixer contents -c Generic
numid=1,iface=CARD,name='HDMI/DP,pcm=3 Jack'
  ; type=BOOLEAN,access=r-------,values=1
  : values=off
numid=2,iface=MIXER,name='IEC958 Playback Con Mask'
  ; type=IEC958,access=r-------,values=1
  : values=[AES0=0x0f AES1=0xff AES2=0x00 AES3=0x00]
numid=3,iface=MIXER,name='IEC958 Playback Pro Mask'
  ; type=IEC958,access=r-------,values=1
  : values=[AES0=0x0f AES1=0x00 AES2=0x00 AES3=0x00]
numid=4,iface=MIXER,name='IEC958 Playback Default'
  ; type=IEC958,access=rw------,values=1
  : values=[AES0=0x04 AES1=0x82 AES2=0x00 AES3=0x02]
numid=5,iface=MIXER,name='IEC958 Playback Switch'
  ; type=BOOLEAN,access=rw------,values=1
  : values=on
numid=6,iface=PCM,name='ELD',device=3
  ; type=BYTES,access=r-------,values=0
  : values=

Owner

anssih replied May 21, 2012

Member

anssih commented May 21, 2012

One thing I'm not sure of is the magic placeholder for the "front/default/surround51/surround71" string in the device name. I've currently used "@", but would something else be more clear maybe, like $foo$ with something interesting as "foo"? I couldn't think think of any descriptive word so I used a non-descriptive "@" :)

This is the thing that will appear in the device names like "@" or "@:CARD=Intel,DEV=0".

Contributor

gyunaev commented Jun 19, 2012

I can confirm that applying this merge fixes the playback on my machine which has both the analog output (which I use) and the HDMI output (which I do not use). Without it the current master doesn't play any sound at all nor it allows selecting the analog playback.

anssih was assigned Jul 1, 2012

@elupus elupus commented on the diff Jul 3, 2012

xbmc/cores/AudioEngine/Sinks/AESinkALSA.cpp
@@ -144,26 +151,40 @@ bool CAESinkALSA::Initialize(AEAudioFormat &format, std::string &device)
format.m_channelLayout = m_channelLayout;
- /* if passthrough we need the additional AES flags */
- if (m_passthrough)
- GetPassthroughDevice(format, device);
+ std::string AESParams;
+ /* digital interfaces should have AESx set, though in practice most
+ * receivers don't care */
+ if (m_passthrough || device.substr(0, 6) == "iec958" || device.substr(0, 4) == "hdmi")
@elupus

elupus Jul 3, 2012

Member

I think some systems expose a aplay -L pcm called spdif, think it shouldn't hurt to add it?

@anssih

anssih Jul 7, 2012

Member

Yep, will add in a separate commit.

Thanks for reviewing the code, it is really appreciated :)

@elupus elupus commented on the diff Jul 3, 2012

xbmc/cores/AudioEngine/Sinks/AESinkALSA.cpp
- if (snd_ctl_open_lconf(&ctlhandle, strHwName.c_str(), 0, config) != 0)
+bool CAESinkALSA::OpenPCMDevice(const std::string &name, const std::string &params, int channels, snd_pcm_t **pcmp, snd_config_t *lconf, bool preferDmixStereo)
+{
+ /* Special name denoting surroundXX mangling. This is needed for some
+ * devices for multichannel to work. */
+ if (name == "@" || name.substr(0, 2) == "@:")
@elupus

elupus Jul 3, 2012

Member

Is there a reason why it's not just replacing @ with the device name instead of just the first char being that?

@anssih

anssih Jul 7, 2012

Member

Can't think of one at least immediately, guess I'll change that.

While on the subject, do you have any better suggestion than using "@"?

@elupus elupus commented on the diff Jul 3, 2012

xbmc/cores/AudioEngine/Sinks/AESinkALSA.cpp
- /* some HDMI devices (intel) report Digital for HDMI also */
- if (devname.find("Digital") != std::string::npos)
- maybeHDMI = true;
+ for (AEDeviceInfoList::iterator it1 = list.begin(); it1 != list.end(); ++it1)
+ {
+ for (AEDeviceInfoList::iterator it2 = it1+1; it2 != list.end(); ++it2)
+ {
+ if (it1->m_displayName == it2->m_displayName
+ && it1->m_displayNameExtra == it2->m_displayNameExtra)
+ {
+ /* something needs to be done */
+ std::string cardString1;
+ std::string cardString2;
+ GetParamFromName(it1->m_deviceName, "CARD", cardString1);
+ GetParamFromName(it2->m_deviceName, "CARD", cardString2);
@elupus

elupus Jul 3, 2012

Member

Cleaner if this would just return the string instead of as a parameter, then these two line things could be combined. But it's just a suggestion, not important.

@anssih

anssih Jul 7, 2012

Member

Right, will change in a separate commit (probably after merge).

anssih merged commit 2de2058 into xbmc:master Jul 10, 2012

This sample rate comparision leads to problems on my ALSA-only system. I had to remove it otherwise my 44.1kHz music play only on front speakers (CAESinkALSA::Initialize - Opened device "front:CARD=Audigy,DEV=0") while all 48kHz audio play correctly on 5.1 (CAESinkALSA::Initialize - Opened device "sysdefault:CARD=Audigy"). More info in http://trac.xbmc.org/ticket/13381

@tru tru added a commit to RasPlex/plex-home-theatre that referenced this pull request Feb 20, 2014

@tru @LongChair tru + LongChair Add button for hiding cloud sync servers.
Fixes #987
43582e8

@tru tru added a commit to plexinc/plex-home-theater-public that referenced this pull request Feb 25, 2014

@tru tru Add button for hiding cloud sync servers.
Fixes #987
5a82138
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment