On Fri, Feb 21, 2014 at 08:11:12AM +0100, Takashi Iwai wrote:
Mark Brown wrote:
Interesting one here. On the one hand we always say to present the hardware features as directly as possible to the application layer. On the other hand I'd not be surprised if some userspaces failed to deal constructively with a full 31 bits of linearly mapped volume control (but I'm not sure how common they are and really they ought to be fixed). Anyway, not a problem more just a comment.
A straightforward implementation would be just to expose this 31bit raw value and declare it as a linear dB via TLV. The proper dB conversion is done in alsa-lib.
Yes, exactly. This should work well with most userspaces, the only thing that would worry me would be something basic that makes the user manually step through thousands of tiny volume steps. I don't think any modern application layer should have an issue there though and I'm not sure if I'd care about such application layers myself.
BTW, you need no table for this volume value at all. volume_map[] can be replaced like #define volume_map(i) ((1U << (i) - 1)
It does mean it's easier to update the chosen steps in future though.
- if (!pcm_data->stream) {
pcm_data->volume[0] =hsw_mixer_to_ipc(ucontrol->value.integer.value[0]);pcm_data->volume[1] =hsw_mixer_to_ipc(ucontrol->value.integer.value[1]);mutex_unlock(&pcm_data->mutex);return 0;- }
It looks like we only record the volume when the stream is idle. What happens if the user changes the volume while it's idle, starts playing, changes again then stops playing? It's possible I missed the bit where it gets saved but I did look.
This seems restored in hsw_pcm_open().
Yes, it's restored there - what concerned me was that the variables didn't seem to be being updated while the stream was running, it only wrote to the hardware so if the volume had subsequently been changed on a running stream then the older value from a closed stream would be written out. It'd at least be more obviously correct to just update the variables no matter what.