search

gavv / signal-estimator

Measure characteristics of a looped back signal.
MIT License
63 stars 18 forks source link

Maximize master volume #46

Open Chinry opened 11 months ago

Chinry commented 11 months ago

Implementing #10

When AlsaWriter::open() is run a call to a volume setting function is made. Storing the previous state of the mixer element volume is nontrivial because it can have 9 different channels shown here. I see two options. One option is to assume that there is only mono or stereo. The other would be to store values in a container by getting the channels present. Do you think this is worth pursuing?

gavv commented 11 months ago

Will look into it in upcoming days, thanks!

gavv commented 11 months ago

Thanks for PR, looks great!

Here are my thoughts:

  1. I think yes, it makes sense to save per-channel volume and then restore it. It would be annoying if running signal-estimator would always reset sound card volume.

    For example, in some of my tests stands, I often switch between using signal-estimator and playing sound using aplay, on the same card. On some sound cards, I need to set volume to non-default value to use aplay (e.g. on one of my sound card default volume or 100% volume is too loud). It would be quite inconvenient if after each time I run signal-estimator I'll need to open alsamixer and reconfigure volume. I imagine that others may have similar use cases and I'm not the only one :)

  2. I guess we could iterate channels numbers from 0 to SND_MIXER_SCHN_LAST-1 and store volumes in container, as you suggested, then restore it.

  3. Hopefully this logic will work for most users, however I think we need to provide options to deal with corner cases. I suggest to add two CLI options:

    • --hw-volume FLOAT - override which volume to set for ALSA; default value is 1.0 (which means 100%); float is used for consistency with --volume
    • --no-hw-volume - disable touching ALSA volume

  4. Could you please add logging of errors for calls like snd_mixer_attach() and others?

  5. Do you know if "Master" element is always present? Maybe we need to add a CLI option for element name as well?

gavv commented 11 months ago

Forgot to mention, assuming mono or stereo is not great because we allow to specify arbitrary channel count in CLI and mostly are not bound to number of channels.

Chinry commented 11 months ago

Okay, will take a look at implementing these changes.

gavv commented 11 months ago

Thanks! FYI: https://github.com/gavv/signal-estimator/commit/c5ea5acd5c909787db40ae57ac4e569b7f74fe16

gavv commented 10 months ago

Update: since we now have separate options for output and input devices (--out-latency/--in-latency, --out-periods/--in-periods), I think it makes sense to make separate options for volume too. E.g. we can have three options: --out-volume, --in-volume, --no-volume.