search

diwic / alsa-rs

Thin but safe ALSA wrappers for Rust
Apache License 2.0
139 stars 66 forks source link

32/64 bit support #41

Closed ashthespy closed 6 years ago

ashthespy commented 6 years ago

I was having some issues with setting the volume via the Mixer api, and after some head scratching realised that my user space was 32bit, so all my i64 volume values were overflowing.

Would it make sense to switch to isize instead of i64 - where the underlying api uses c_long? Like at: https://github.com/diwic/alsa-rs/blob/86660415b9c0d975fb2b2ad113e259b2689a4a83/src/mixer.rs#L339-L341

diwic commented 6 years ago

I'm not sure if that would buy us anything, as AFAIK there is no guarantee that c_long is the same as isize on all platforms?

ashthespy commented 6 years ago

Not entirely sure myself. Any ideas on how best to use these in a platform agnostic manner?

EDIT: isize is determined by target-pointer-width that is defined by the target JSON file. (thanks @1tgr)

diwic commented 6 years ago

So; I think the answer is that you're supposed to use get_playback_volume_range to figure out what the range is, and then never set volume outside that range. If you set a volume that's larger than 2^31 it must have been outside that range for sure?

With that in mind, is there anything we can do better than just adding a comment (feel free to do that in a PR if you like) that valid values are what get_playback_volume_range returns?

ashthespy commented 6 years ago

Hmm, that is what I already do, get the min max and map it to 0 - 1. I will hunt down why it was working with i32 and not i64 - I must have changed some math.

Do you think it really requires a note - most people brave enough to use alsa directly, would already have spend few hours reading the docs ;-)

Sorry for the wild goose chase!