-
Notifications
You must be signed in to change notification settings - Fork 591
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 initial support for ALSA mixer #45
Comments
Comment by jsopenrb Volume calculation fails for maximum volume values. If the range is 0..255 then at full volume the result will be 256 instead of 255. I suppose it should not use integer math because volume range can have very different values. |
Comment by joerg-krause Not sure what do you mean. The mixer uses |
Comment by jsopenrb Yes, but then you add one to that range, so for 0..255 at max volume the On Wednesday, March 15, 2017, Jörg Krause notifications@github.com wrote:
|
Comment by joerg-krause Yes, 256 is the resolution if the range is from 0 to 255. The resolution is used to get the factor wrt the spotify resolution, which is 65536 / 256 = 256 in this case. If the max spotify volume of 65535 is set this results in 65535 / 256 = 255.99..., which is truncated to 255 because of the Integer. But I can see now what you mean. I need to add 1 to the spotify volume and substract it after the division: ((65535 + 1) / 256) - 1 = 255. I will update the commit tomorrow. |
Comment by joerg-krause @jsopenrb I've updated the volume calculation... |
Comment by jsopenrb Better, but it will still fail for certain volume ranges like 190. I think the proper solution it to switch to float calculations. Not a big deal at the moment. |
Comment by plietar Unfortunately I don't have much time at the moment to test this, it'll have to wait another week or two. FWIW, I've previously used the following code to set the volume, which seemed to produced good results on my Pi's builtin audio output let (min, max) = elem.get_playback_db_range();
let db_volume = if volume == 0 {
min
} else {
let volume = volume as f64 / 65535.;
let volume = volume.log10() * 6000.;
MilliBel(volume as i64) + max
};
self.elem.set_playback_db_all(db_volume, alsa::Round::Ceil).unwrap(); |
Comment by joerg-krause No, problem! Take your time. I'll check your volume setting... |
Comment by jsopenrb That volume setting assumes that minimal dB value is mute which is not true for all cards. |
Comment by joerg-krause
This is the relevant code, how the handling of hardware and software mixer is done in shairtport-sync: https://github.com/mikebrady/shairport-sync/blob/master/audio_alsa.c#L358-L396. The ALSA softvol plugin is handled as a special case of a hardware mixer. The software mixer is used if no hardware mixer and no ALSA softvol plugin is used. Note, shairport-sync uses an own attenuation curve, which is mapped onto the mixer. That's make the volume handling more complex than it is necessary for librespot. For hardware mixer, we need a logarithmic curve, but not for the ALSA softvol plugin, as it already maps the linear volume value to a logarithmic curve. So, maybe we need to split the mixer into two mixers: alsa and alsasoftvol? |
Hi everyone, Has anything been done on the topic? I see mixer still has only softmixer.rs (and mod.rs - what's that?). I'm having mpd and librespot on the same machine and the fact that librespot volume is not linked to alsa mixer is a bit annoying. Not much of a coder myself but maybe there's anything I can do to help? Thanks a lot |
Issue by joerg-krause
Thursday Mar 09, 2017 at 09:35 GMT
Originally opened as plietar/librespot#160
The ALSA mixer module implements a simple linear volume control. Note, that a linear volume control is not ideal for human audio experience as our ears respond to sound on an exponential curve.
The mixer also works fine with the ALSA softvol plugin which offers an logarithmic attenuation curve.
TODO: Use logarithmic curve for hardware mixers.
joerg-krause included the following code: https://github.com/plietar/librespot/pull/160/commits
The text was updated successfully, but these errors were encountered: