alsa-project / alsa-lib

The Advanced Linux Sound Architecture (ALSA) - library
GNU Lesser General Public License v2.1
366 stars 177 forks source link

Config parsing of pcm_rate broken since 1.1.7 (at least for a52 pcm plugin) #154

Closed quequotion closed 3 years ago

quequotion commented 3 years ago

As noted in the mailing list back in 2019, config parsing (for alsa-plugins a52 pcm at least) has been broken since alsa-lib 1.1.7.

As far as I know this was never bisected to a single commit in either alsa-lib or alsa-plugins.

Looking at the mailing list discussion, it occurs to me that it may have given the impression that the config parser was broken when using a patch that ports alsa-plugins to libswresample.

Config parsing is broken regardless of that patch.

@IdleGandalf worked around this by applying a patch to alsa-lib, in src/pcm/pcm_rate.c, but never got a response if this was an appropriate method to resolve the issue.

Should this be fixed in alsa-lib or in alsa-plugins?

Note: above links are to mirrors of the original patches, also in a repository owned by @IdleGandalf as far as I can tell. The links in the mailing list discussion are now 404.

perexg commented 3 years ago

The fix does not seem relevant. What does not work? Could you show your configuration and describe the problem?

quequotion commented 3 years ago

The same configuration most everyone across the internet uses to enable the a52 plugin:

# Digital 5.1 AC3 Output
pcm.a52 {
  @args [CARD]
  @args.CARD {
    type string
  }
  type rate
  slave {
    pcm {
      type a52
      bitrate 448
      channels 6
      card $CARD
    }
    rate 48000
  }
}

This configuration worked up to alsa-lib 1.1.6, and then it did not. There doesn't seem to be any configuration that works after alsa-lib 1.1.7. What @IdleGandalf did was to filter out all keywords to work around the problem.

Nothing has changed since: the configuration (using vanilla, stable releases of alsa-plugins and alsa-lib) still fails with "Unknown field card" today just as it did back then.

perexg commented 3 years ago

The applied filter was for the pcm.a52 { } compound, so it's not relevant for pcm.a52.slave { } compound.

This one does work:

pcm.aloop48000 {
  type rate
  slave {
     pcm hw:Loopback
     rate 48000
  }
}

EDIT: Show the error message, please.

quequotion commented 3 years ago

This may be limited to the a52 plugin; that is one of the things I am trying to find out with this issue report.

Here it is in an excerpt from pulseaudio's startup:

D: [pulseaudio] alsa-mixer.c: Looking at profile output:iec958-ac3-surround-51
D: [pulseaudio] alsa-mixer.c: Checking for playback on Digital Surround 5.1 (IEC958/AC3) (iec958-ac3-surround-51)
D: [pulseaudio] alsa-util.c: Trying a52:0 with SND_PCM_NO_AUTO_FORMAT ...
I: [pulseaudio] (alsa-lib)pcm_rate.c: Unknown field card
I: [pulseaudio] alsa-util.c: Error opening PCM device a52:0: Invalid argument
D: [pulseaudio] alsa-mixer.c: Caching failure to open output:iec958-ac3-surround-51
D: [pulseaudio] alsa-mixer.c: Skipping profile output:iec958-ac3-surround-51+input:analog-stereo - will not be able to open output:iec958-ac3-surround-51
D: [pulseaudio] alsa-mixer.c: Skipping profile output:iec958-ac3-surround-51+input:iec958-stereo - will not be able to open output:iec958-ac3-surround-51

edit, and here it is from speaker-test with pulseaudio disabled:

speaker-test -c6 -D a52

speaker-test 1.2.5

Playback device is a52
Stream parameters are 48000Hz, S16_LE, 6 channels
Using 16 octaves of pink noise
ALSA lib pcm_rate.c:1575:(_snd_pcm_rate_open) Unknown field card
Playback open error: -22,Invalid argument
perexg commented 3 years ago

Could you check the merged config dump (with 1.2.5) ? alsactl dump-cfg - show only the a52 config compound .

perexg commented 3 years ago

And btw, a52 is already defined in 60-a52-encoder.conf ! So if you want to override it, you should create pcm.!a52 { } config.

quequotion commented 3 years ago

This?

pcm.a52 {
    @args.0 CARD
    @args.1 SLAVE
    @args.2 RATE
    @args.3 BITRATE
    @args.4 CHANNELS
    @args.CARD {
        type string
        default {
            @func refer
            name 'defaults.pcm.card'
        }
    }
    @args.SLAVE {
        type string
    }
    @args.RATE {
        type integer
        default 48000
    }
    @args.BITRATE {
        type integer
        default 448
    }
    @args.CHANNELS {
        type string
        default 6
    }
    @args.5 CARD
    type rate
    card $CARD
    slavepcm $SLAVE
    rate $RATE
    bitrate $BITRATE
    channels $CHANNELS
    hint {
        show {
            @func refer
            name 'defaults.namehint.basic'
        }
        description 'Plugin to convert multichannel stream to A52 (AC3) bitstream'
    }
    slave {
        pcm {
            type a52
            bitrate 448
            channels 6
            card $CARD
        }
        rate 48000
    }
}
perexg commented 3 years ago

Yes, and you see the culprit. The configs are merged!

quequotion commented 3 years ago

And btw, a52 is already defined in 60-a52-encoder.conf ! So if you want to override it, you should create pcm.!a52 { } config.

I don't think anyone is aware of this.

perexg commented 3 years ago

https://www.alsa-project.org/alsa-doc/alsa-lib/conf.html - "Operation modes for parsing nodes"

quequotion commented 3 years ago

I mean that there is an existing configuration for a52.

The plugin is not activated unless using the configuration posted above, so no users of the plugin would be aware that a conflicting configuration to be merged with this configuration exists.

edit: brb; trying pcm.!a52: initial results after alsactl init, speaker-test did not immediately fail, but did halt and zombifiy while testing "Front Left"; rebooting.

edit again: interesting. The plugin now works through pulseaudio, but speaker-test is broken: freezes when testing the first speaker, has to be killed with kill -9 $(pidof speaker-test)

perexg commented 3 years ago

It would be also nice, if you can check the default 60-a52-encoder.conf , so we can eventually fix the problem with it (rename your config for tests). The rate resampling should not be mandatory for pulseaudio.

quequotion commented 3 years ago

Okay, now we are getting somewhere.

So, after another reboot, the plugin works in pulseaudio without any additional configuration (using 60-a52-encoder.conf as provided by alsa-plugins). Unfortunately, this leaves no pcm device to be tested with speaker-test when pulseaudio is disabled, but I can live with that.

I am curious if there is a way to access the a52 pcm without pulseaudio. What the cargo-cult configuration did was to establish this device in alsa, which some were using instead.

edit: I should note that I have an advantage in setting up the plugin to work with pulseaudio as my /etc/pulse/default.pa is already configured to load the card with profile output:iec958-ac3-surround-51+input:analog-stereo (I don't use any autodetection). It may be possible for people to find "Built-in Audio Digital Surround 5.1 (IEC958/AC3)" in pavucontrol, but otherwise there's not really anywhere to see that the plugin is active.

perexg commented 3 years ago

speaker-test -Dplug:a52 ?

quequotion commented 3 years ago

speaker-test -Dplug:a52 ?

Still getting stuck on "Front Left"

Edit: then again, that could be because we still need to upgrade to 32bit sampling.

quequotion commented 3 years ago

In any case, I am satisfied that parsing the configuration was not, or at least is not now, the issue: the way of configuring this plugin has changed (a manual configuration is no longer required to use the plugin, at least with pulseaudio)

manio commented 3 years ago

@quequotion two questions:

  1. you did all above tests with https://github.com/alsa-project/alsa-plugins/pull/23/ applied?
  2. so you are telling me that pulseaudio is able to use the a52 plugin and the sound is magically working without 32bit sampling?

ps. I am not using pulseaudio at all so I am also not so familiar with it.... maybe it is a time to change this habit :D

quequotion commented 3 years ago
  1. you did all above tests with alsa-project/alsa-plugins#23 applied?

Yes

  1. so you are telling me that pulseaudio is able to use the a52 plugin and the sound is magically working without 32bit sampling?

Not that well, the audio is as you described in your tests: pretty terrible really (choppy, chipmunk voice effect).

ps. I am not using pulseaudio at all so I am also not so familiar with it.... maybe it is a time to change this habit :D

There are still many people who distrust pulseaudio or think of it as bloat. From my experience, unless you are running on an embedded system or otherwise have some critical need to save a few CPU cycles and megabytes of RAM, pulseaudio is a very reasonable thing to use. It enables quite a lot of advanced features and provides a relatively easy way to configure them over ALSA on its own.

On the other hand, your current setup is a good test bed: we do need to make sure standalone ALSA setups are supported.

perexg commented 3 years ago

If you use plug:a52 then the automatic rate / format conversion is activated. If something does not work, it's problem is probably in the end plugin (a52). You may dump the chain using aplay -Dplug:a52 -v <your_file>.

quequotion commented 3 years ago

I tried this with this test file.

Normally, this announces each channel (Front Left, Center, Front Right, Rear Left, Rear Right, noise through LFE).

aplay -D plug:a52 -v www_lynnemusic_com_surround_test.ac3
Playing raw data 'www_lynnemusic_com_surround_test.ac3' : Signed 16 bit Little Endian, Rate 8000 Hz, Mono
Plug PCM: Rate conversion PCM (48000, sformat=S16_LE)
Converter: libspeex (external)
Protocol version: 10002
Its setup is:
  stream       : PLAYBACK
  access       : RW_INTERLEAVED
  format       : S16_LE
  subformat    : STD
  channels     : 1
  rate         : 8000
  exact rate   : 8000 (8000/1)
  msbits       : 16
  buffer_size  : 4096
  period_size  : 256
  period_time  : 32000
  tstamp_mode  : NONE
  tstamp_type  : GETTIMEOFDAY
  period_step  : 1
  avail_min    : 256
  period_event : 0
  start_threshold  : 4096
  stop_threshold   : 4096
  silence_threshold: 0
  silence_size : 0
  boundary     : 1152921504606846976
Slave: Route conversion PCM (sformat=S16_LE)
  Transformation table:
    0 <- 0
    1 <- 0
    2 <- 0
    3 <- 0
    4 <- 0
    5 <- 0
Its setup is:
  stream       : PLAYBACK
  access       : MMAP_INTERLEAVED
  format       : S16_LE
  subformat    : STD
  channels     : 1
  rate         : 48000
  exact rate   : 48000 (48000/1)
  msbits       : 16
  buffer_size  : 24576
  period_size  : 1536
  period_time  : 32000
  tstamp_mode  : NONE
  tstamp_type  : GETTIMEOFDAY
  period_step  : 1
  avail_min    : 1536
  period_event : 0
  start_threshold  : 24576
  stop_threshold   : 24576
  silence_threshold: 0
  silence_size : 0
  boundary     : 6917529027641081856
Slave: A52 Output Plugin
Its setup is:
  stream       : PLAYBACK
  access       : MMAP_NONINTERLEAVED
  format       : S16_LE
  subformat    : STD
  channels     : 6
  rate         : 48000
  exact rate   : 48000 (48000/1)
  msbits       : 16
  buffer_size  : 24576
  period_size  : 1536
  period_time  : 32000
  tstamp_mode  : NONE
  tstamp_type  : GETTIMEOFDAY
  period_step  : 1
  avail_min    : 1536
  period_event : 0
  start_threshold  : 24576
  stop_threshold   : 24576
  silence_threshold: 0
  silence_size : 0
  boundary     : 6917529027641081856

The sound produced by aplay this time was an atrocious grinding noise through all channels simultaneously (note: turn your volume down).

I am not entirely sure if that is only because of the encoding, or if aplay needs additional parameters to use the plugin (or play this file) properly (-c 6 made no difference).

Playing the same file with mpv through pulseaudio produces a listenable playback of the channel announcements through their proper channels, but with a choppy chimpunk effect.

mpv www_lynnemusic_com_surround_test.ac3
[ffmpeg/demuxer] ac3: Estimating duration from bitrate, this may be inaccurate
 (+) Audio --aid=1 (ac3 6ch 48000Hz)
AO: [pulse] 48000Hz 5.1(side) 6ch float
A: 00:00:08 / 00:00:09 (96%)

Exiting... (End of file)
perexg commented 3 years ago

All is ok here, let continue in https://github.com/alsa-project/alsa-plugins/pull/23 .