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 softvol plugin #158

Closed

Conversation

joerg-krause
Copy link
Contributor

Add support for the ALSA softvol plugin as a mixer. The ALSA softvol plugin allows librespot to control the master volume.

Therefore, switch to the alsa crate from diwic, as this crate already implements the mixer interface, and port the alsa backend to this crate.

The ALSA softvol plugin needs to audio device name. Therefoe, enhance the mixer interface by passing the device name to the mixer instance. The device name is ignored by the softmixer.

Finally, get the initial volume setting from the mixer. This way, the spotify player uses the last volume instead of setting it always to 100%.

@joerg-krause joerg-krause force-pushed the mixer-add-alsa-softvol branch from 22e54f9 to f926ea7 Compare March 5, 2017 10:26
@janderholm
Copy link
Contributor

janderholm commented Mar 5, 2017

This seem to break cross compilation. It fails with:

thread 'main' panicked at 'called `Result::unwrap()` on an `Err` value: "Failed to run `\"pkg-config\" \"--libs\" \"--cflags\" \"alsa\"`: No such file or directory (os error 2)"', /buildslave/rust-buildbot/slave/stable-dist-rustc-linux/build/src/libcore/result.rs:837
note: Run with `RUST_BACKTRACE=1` for a backtrace.

I think you should be able to solve this by making sure pkg-config is properly configured in the docker files. Environment variables like PKG_CONFIG_ALLOW_CROSS and PKG_CONFIG_PATH might do the trick.

@joerg-krause
Copy link
Contributor Author

@Fulkerson I am cross-compiling for a custom ARMv5 target and it works:

CC=arm-unknown-linux-musleabi-sysroot PKG_CONFIG_ALLOW_CROSS=1 PKG_CONFIG_PATH=$SYSROOT/usr/arm-buildroot-linux-musleabi/sysroot/usr/lib/pkgconfig cargo build -j4 --target=armv5te-unknown-linux-musleabi --features "alsa-backend with-tremor" --no-default-features --release

I do not use docker...

@plietar
Copy link
Owner

plietar commented Mar 6, 2017

Thanks for the PR ! I'm quite busy this week, so I probably won't be reviewing it until this weekend.

pkg-config can be disabled through the cargo config.

[target.arm-unknown-linux-gnueabi.alsa]
rustc-link-lib = ["asound2"]

See RustAudio/cpal#152 and http://doc.crates.io/build-script.html#overriding-build-scripts

@joerg-krause I can take care of adding that if you don't want to mess around with the Dockerfile

@plietar
Copy link
Owner

plietar commented Mar 6, 2017

This PR seems to support any ALSA mixer not just softvol. The mixer should just be named alsa then.

@joerg-krause
Copy link
Contributor Author

@plietar I've added support for the ALSA softvol plugin to shairport-sync some time ago: mikebrady/shairport-sync#293. There was already ALSA hardware mixer support implemented, but the ALSA functions to use for handling the dB volume range are different between software and hardware mixer.

I will check, if I have a board with a hardware mixer interface to test with.

@plietar plietar mentioned this pull request Mar 6, 2017
@plietar
Copy link
Owner

plietar commented Mar 6, 2017

Hmm, I'm not very familiar with alsa but my understanding is it should not matter what mixer is configured by the user in the asoundrc. I would have thought ALSA exposes a single API to control them.

@jsopenrb
Copy link
Contributor

jsopenrb commented Mar 7, 2017

Is there a reason for not caching the mixer and selem instances, so you don't have to create a new Mixer instance for each volume get/set? It would be also nice to be able to specify a separate alsa mixer device. This is need for things like room correction where librespot outputs to alsa loopback which then sends samples to the real output device, but volume control must work with the real output.

@joerg-krause
Copy link
Contributor Author

@jsopenrb

Is there a reason for not caching the mixer and selem instances, so you don't have to create a new Mixer instance for each volume get/set?

The reason is my limited knowledge about Rust. If I move the mixer and selem instances to open() Rust demands a lifetime specifier.

It would be also nice to be able to specify a separate alsa mixer device.

This can be done using the --device command line option. For now, the mixer control name is hardcoded to "Master". A follow-up patch is necessary to allow passing the mixer control name to the mixer.

@jsopenrb
Copy link
Contributor

jsopenrb commented Mar 8, 2017

@joerg-krause
Maybe you can create mixer instance in get/set volume if it's not created yet and the used the cached one?
I know about device parameter, but I need another optional parameter because alsa output device for librespot will be loopback but alsa mixer device will be the DAC/AMP. I've been playing around with modifying the code and adding support for getting the volume range from alsa so this can work for both soft and hw mixers, but it's far from being ready.

@romerod
Copy link
Contributor

romerod commented Mar 9, 2017

Instead of passing additional options to the mixers which not all mixers need wouldn't it make sense to add a config file?

Either one specifically for your mixer or even one for the whole application where you could add a section for each "component".

I'm working on an external mixer, which might need different options.

@jsopenrb
Copy link
Contributor

jsopenrb commented Mar 9, 2017

Config file for what exactly? Shairport has this handled properly for alsa: you can specify separate mixer device, which defaults to the output device if not set. I'm suggesting to do the same for librespot alsa backend.

@joerg-krause
Copy link
Contributor Author

@romerod Yes, a config file would be definitely the best. However, this is not implemented yet.
@jsopenrb spotifyd already uses an INI-based config file.

Maybe it's best to use librespot as library and spotifyd as the front-end.


let volume: i64 = selem.get_playback_volume(alsa::mixer::SelemChannelId::FrontLeft).unwrap();

volume as u16 * 256
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The factor 256 is only valid for ALSA mixers having a resolution of 256: 0xFFFF / resolution(256) = 256. If the resolution of the mixer is for example 128, the factor needs to be: 0xFFFF / resolution(128) = 512.

@joerg-krause joerg-krause deleted the mixer-add-alsa-softvol branch March 9, 2017 09:27
@joerg-krause joerg-krause restored the mixer-add-alsa-softvol branch March 9, 2017 09:29
@joerg-krause
Copy link
Contributor Author

Closed in favor of #160.

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.

5 participants