Skip to content

Conversation

@Chinry
Copy link
Contributor

@Chinry Chinry commented Oct 5, 2023

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?

@Chinry Chinry changed the title Maximize master volume (#10) Maximize master volume Oct 6, 2023
@gavv
Copy link
Owner

gavv commented Oct 7, 2023

Will look into it in upcoming days, thanks!

@gavv
Copy link
Owner

gavv commented Oct 13, 2023

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
Copy link
Owner

gavv commented Oct 13, 2023

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
Copy link
Contributor Author

Chinry commented Oct 21, 2023

Okay, will take a look at implementing these changes.

@gavv
Copy link
Owner

gavv commented Oct 21, 2023

Thanks! FYI: c5ea5ac

@gavv gavv force-pushed the main branch 11 times, most recently from d08881c to ab00726 Compare October 26, 2023 11:29
@gavv
Copy link
Owner

gavv commented Oct 30, 2023

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.

@gavv gavv force-pushed the main branch 4 times, most recently from a995b02 to a77819f Compare November 1, 2023 12:23
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.

2 participants