Re: [PATCH] ALSA: scarlett2: Add Vocaster speaker/headphone mute controls

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]



On Fri, 05 Sep 2025 14:05:25 +0200,
Takashi Iwai wrote:
> 
> On Fri, 05 Sep 2025 13:51:28 +0200,
> Geoffrey D. Bennett wrote:
> > 
> > On Fri, Sep 05, 2025 at 10:36:28AM +0200, Takashi Iwai wrote:
> > > On Thu, 04 Sep 2025 17:53:51 +0200,
> > > Geoffrey D. Bennett wrote:
> > > > 
> > > > Add support for the speaker and headphone mute controls on Focusrite
> > > > Vocaster interfaces. Unlike other Focusrite interfaces, these mute
> > > > controls are per-output, not per-channel.
> > > > 
> > > > Signed-off-by: Geoffrey D. Bennett <g@xxxxx>
> > > > ---
> > > >  sound/usb/mixer_scarlett2.c | 97 ++++++++++++++++++++++++++++++++++++-
> > > >  1 file changed, 96 insertions(+), 1 deletion(-)
> > > > 
> > > > diff --git a/sound/usb/mixer_scarlett2.c b/sound/usb/mixer_scarlett2.c
> > > > index e762d6110b1a..f2446bf3982c 100644
> > > > --- a/sound/usb/mixer_scarlett2.c
> > > > +++ b/sound/usb/mixer_scarlett2.c
> > > > @@ -10,8 +10,9 @@
> > > >   *   - Solo/2i2/4i4 Gen 4
> > > >   *   - Clarett 2Pre/4Pre/8Pre USB
> > > >   *   - Clarett+ 2Pre/4Pre/8Pre
> > > > + *   - Vocaster One/Two
> > > >   *
> > > > - *   Copyright (c) 2018-2024 by Geoffrey D. Bennett <g at b4.vu>
> > > > + *   Copyright (c) 2018-2025 by Geoffrey D. Bennett <g at b4.vu>
> > > >   *   Copyright (c) 2020-2021 by Vladimir Sadovnikov <sadko4u@xxxxxxxxx>
> > > >   *   Copyright (c) 2022 by Christian Colglazier <christian@xxxxxxxxxxxxxxxx>
> > > >   *
> > > > @@ -75,6 +76,9 @@
> > > >   * to many LinuxMusicians people and to Focusrite for hardware
> > > >   * donations).
> > > >   *
> > > > + * Support for Vocaster One and Two added in Mar 2024 (thanks to many
> > > > + * LinuxMusicians people and to Focusrite for hardware donations).
> > > > + *
> > > >   * This ALSA mixer gives access to (model-dependent):
> > > >   *  - input, output, mixer-matrix muxes
> > > >   *  - mixer-matrix gain stages
> > > > @@ -364,6 +368,21 @@ static const char *const scarlett2_dim_mute_names[SCARLETT2_DIM_MUTE_COUNT] = {
> > > >  	"Mute Playback Switch", "Dim Playback Switch"
> > > >  };
> > > >  
> > > > +/* Vocaster One speaker/headphone mute names */
> > > > +static const char *const vocaster_one_sp_hp_mute_names[] = {
> > > > +	"Speaker Mute Playback Switch",
> > > > +	"Headphones Mute Playback Switch",
> > > 
> > > Usually "XXX Playback Switch" indicates that it's a switch for
> > > mute/unmute, hence we don't put "Mute" here.
> > > 
> > > Actually, having a "mute" switch makes difficult to judge how it
> > > behaves: when the mixer switch is "on", is it muted, i.e. the sound
> > > goes off?  It's a contradiction to other mixer controls.
> > > 
> > > 
> > > thanks,
> > > 
> > > Takashi
> > 
> > Hi Takashi,
> > 
> > Calling it "XXX Mute Playback Switch" distinguishes it from the other
> > "XXX Playback Switch" controls created by the scarlett2 driver which
> > are not mute controls, and it also follows the existing pattern for
> > the other switch controls in the scarlett2 driver.
> > 
> > Some examples:
> > 
> > $ grep 'Playback Switch' *.state
> > ...
> > [actual mutes:]
> > Clarett Plus 2Pre.state:                name 'Line 01 Mute Playback Switch'
> > Clarett Plus 2Pre.state:                name 'Line 02 Mute Playback Switch'
> > Clarett Plus 2Pre.state:                name 'Line 03 Mute Playback Switch'
> > Clarett Plus 2Pre.state:                name 'Line 04 Mute Playback Switch'
> > Clarett Plus 2Pre.state:                name 'Mute Playback Switch'
> > ...
> > [other boolean controls, not mutes:]
> > Clarett Plus 2Pre.state:                name 'Dim Playback Switch'
> > Scarlett Gen 3 Solo.state:              name 'Direct Monitor Playback Switch'
> > Scarlett Gen 3 18i20.state:             name 'Talkback Mix A Playback Switch'
> > ...
> > Scarlett Gen 3 18i20.state:             name 'Talkback Mix L Playback Switch'
> > Scarlett Gen 4 16i16.state:             name 'Speaker Switching Playback Switch'
> > Scarlett Gen 4 16i16.state:             name 'Speaker Switching Alt Playback Switch'
> > 
> > I have been consistent since I first added mute support in
> > 0c88f9db1910 ("ALSA: usb-audio: scarlett2: Add mute support") in the
> > use of the terminology "XXX Mute Playback Switch" and the clear logic
> > of:
> > - mute on means that it's muted (no sound).
> > - mute off means that it's not muted.
> > 
> > I think usually "XXX Playback Switch" in other drivers is more of an
> > "enable audio" boolean rather than a "mute" boolean, so there it makes
> > sense that off = muted, on = not muted.
> > 
> > On the capture side in the scarlett2 driver, it is similar:
> > 
> > $ grep Switch V*
> > Vocaster One.state:             name 'MSD Mode Switch'
> > Vocaster One.state:             name 'Line In 1 DSP Capture Switch'
> > Vocaster One.state:             name 'Line In 1 Mute Capture Switch'
> > Vocaster One.state:             name 'Line In 1 Phantom Power Capture Switch'
> > Vocaster One.state:             name 'Line In 1 Autogain Capture Switch'
> > Vocaster Two.state:             name 'MSD Mode Switch'
> > Vocaster Two.state:             name 'Line In 1 DSP Capture Switch'
> > Vocaster Two.state:             name 'Line In 2 DSP Capture Switch'
> > Vocaster Two.state:             name 'Line In 1 Mute Capture Switch'
> > Vocaster Two.state:             name 'Line In 2 Mute Capture Switch'
> > Vocaster Two.state:             name 'Line In 1 Phantom Power Capture Switch'
> > Vocaster Two.state:             name 'Line In 2 Phantom Power Capture Switch'
> > Vocaster Two.state:             name 'Line In 1 Autogain Capture Switch'
> > Vocaster Two.state:             name 'Line In 2 Autogain Capture Switch'
> > 
> > alsamixer removes "Switch" and "Capture" from the name and displays
> > e.g.:
> > - Line In 1 DSP
> > - Line In 1 Mute
> > - Line In 1 Phantom Power
> > 
> > For each of those, off means function disabled and on means function
> > enabled, so the naming in my patch maintains consistency with all the
> > existing device controls. It also reflects the hardware where they are
> > called mute controls, and 0 = not muted, 1 = muted.
> > 
> > Does this address your concerns? The alternative would be to rename to
> > "XXX Playback Switch" and make off = mute. I think that would make it
> > more consistent with other drivers, but less consistent with the
> > existing device controls.
> 
> Hm, so it was no right choice to accept the changes in the past.
> The negative switch is always confusing.  But maybe we should accept
> again once after a similar stuff was already merged; otherwise it'll
> lead to more inconsistency.

So I took the patch for now as is.


thanks,

Takashi




[Index of Archives]     [Pulseaudio]     [Linux Audio Users]     [ALSA Devel]     [Fedora Desktop]     [Fedora SELinux]     [Big List of Linux Books]     [Yosemite News]     [KDE Users]

  Powered by Linux