[PATCH v2 3/8] ALSA: usb-audio: Improve volume range checks

Rong Zhang posted 8 patches 8 hours ago
[PATCH v2 3/8] ALSA: usb-audio: Improve volume range checks
Posted by Rong Zhang 8 hours ago
Currently the volume range check is only meant to discover quirky
microphone on webcam devices and facing these issues:

- The check is only meaningful for dB volume, but it doesn't check if
  the TLV callback is the corresponding one
- A common quirky pattern "val = 0/100/1" doesn't trigger any warning
- Some modern devices trigger the check, but they are legit
- The warning message doesn't apply to some quirky messages with linear
  volume
- The term "range" in the warning message is confusing. At readers'
  first glance it should be (max - min), but it turns out to be
  ((max - min) / res)

Solve these issues by improving the checking logic to:

- Ignore mixers with non-dB TLV
- Warn on unlikely small volume ranges (max - min < 256)
- Add some heuristics to determine if the volume range is unlikely big
- Rephrase the warning message to mention linear volume
- Rephrase the warning message in correct wording

Signed-off-by: Rong Zhang <i@rong.moe>
---
 sound/usb/mixer.c | 54 +++++++++++++++++++++++++++++++++++++++++------
 1 file changed, 48 insertions(+), 6 deletions(-)

diff --git a/sound/usb/mixer.c b/sound/usb/mixer.c
index f52ca0d7e6653..7007e0c9489b4 100644
--- a/sound/usb/mixer.c
+++ b/sound/usb/mixer.c
@@ -1664,20 +1664,62 @@ static bool check_insane_volume_range(struct usb_mixer_interface *mixer,
 				      struct snd_kcontrol *kctl,
 				      struct usb_mixer_elem_info *cval)
 {
-	int range = (cval->max - cval->min) / cval->res;
+	int range, steps, threshold;
 
 	/*
-	 * Are there devices with volume range more than 255? I use a bit more
-	 * to be sure. 384 is a resolution magic number found on Logitech
-	 * devices. It will definitively catch all buggy Logitech devices.
+	 * If a device quirk has overrode our TLV callback, no warning should
+	 * be generated since our checks are only meaningful for dB volume.
 	 */
-	if (range > 384) {
+	if (!(kctl->vd[0].access & SNDRV_CTL_ELEM_ACCESS_TLV_CALLBACK) ||
+	    kctl->tlv.c != snd_usb_mixer_vol_tlv)
+		return false;
+
+	/*
+	 * Meaningless volume control capability (<1dB). This should cover
+	 * devices mapping their volume to val = 0/100/1, which are very likely
+	 * to be quirky.
+	 */
+	range = cval->max - cval->min;
+	if (range < 256) {
 		usb_audio_warn(mixer->chip,
-			       "Warning! Unlikely big volume range (=%u), cval->res is probably wrong.",
+			       "Warning! Unlikely small volume range (=%u), linear volume or custom curve?",
 			       range);
 		return true;
 	}
 
+	steps = range / cval->res;
+
+	/*
+	 * There are definitely devices with ~20,000 ranges (e.g., HyperX Cloud
+	 * III with val = -18944/0/1), so we use some heuristics here:
+	 *
+	 * min < 0 < max: Attenuator + amplifier? Likely to be sane
+	 *
+	 * min < 0 = max: DSP? Voltage attenuator with FW conversion to dB?
+	 * Likely to be sane
+	 *
+	 * min < max < 0: Measured values? Neutral
+	 *
+	 * min = 0 < max: Oversimplified FW conversion? Linear volume? Likely to
+	 * be quirky (e.g., MV-SILICON)
+	 *
+	 * 0 < min < max: Amplifier with fixed gains? Likely to be quirky
+	 * (e.g., Logitech webcam)
+	 */
+	if (cval->min < 0 && 0 <= cval->max)
+		threshold = 24576; /* 65535 * (3 / 8) */
+	else if (cval->min < cval->max && cval->max < 0)
+		threshold = 1024;
+	else
+		threshold = 384;
+
+	if (steps > threshold) {
+		usb_audio_warn(mixer->chip,
+			       "Warning! Unlikely big volume step count (=%u), linear volume or wrong cval->res?",
+			       steps);
+		return true;
+	}
+
 	return false;
 }
 
-- 
2.51.0