[alsa-devel] [PATCH] alsa-lib/tlv: fix handling of raw value ranges

[Clemens Ladisch]

Benoît Thébaudeau wrote:
> In case of a TLV dB range with all items having raw value ranges strictly within
> the main raw value range reported by the driver, snd_tlv_convert_from_dB()
> returned one of the main raw range boundaries, which was outside all dB range
> items.

But the main raw range boundaries _are_ valid values.

I guess the problem you actually have is that convert_to_dB does not
work with these raw values?

> static const unsigned int bd3753x_vol_fader_gain_att_tlv[] = {
> 	TLV_DB_RANGE_HEAD(2),
> 	0, 0, TLV_DB_SCALE_ITEM(TLV_DB_GAIN_MUTE, 0, 1),
> 	48, 142, TLV_DB_SCALE_ITEM(-7900, 100, 0),
> };
> 	SOC_SINGLE_TLV("Main Volume",
> 		       BD3753X_VOL_GAIN,
> 		       0, 255, 1, bd3753x_vol_fader_gain_att_tlv),
>
> snd_tlv_convert_from_dB(tlv, 0, 255, 1500, &value, 1) returned 255 instead of
> the expected 142.

But there is no guarantee that this mixer control will have only one of
those values in the TLV's range.  Furthermore, convert_to_dB will still
fail for any raw value 1..47.

This TLV is just incomplete.  Please report this bug to the driver's
author.

As for the patch, while I'm not sure whether it is needed at all, I also
don't quite understand the reason for all the changes.

> --- alsa-lib/src/control/tlv.c
> +++ alsa-lib/src/control/tlv.c
> @@ -291,41 +291,37 @@ int snd_tlv_convert_from_dB(unsigned int
>  {
>  	switch (tlv[0]) {
>  	case SND_CTL_TLVT_DB_RANGE: {
> +		long dbmin, dbmax, prev_submax;
>  		unsigned int pos, len;
>  		len = int_index(tlv[1]);
> +		if (len < 6 || len > MAX_TLV_RANGE_SIZE)
>  			return -EINVAL;
>  		pos = 2;
> +		prev_submax = 0;
> +		do {

Why is this not a while loop?

> +			long submin, submax;
> +			submin = (int)tlv[pos];
> +			submax = (int)tlv[pos + 1];
> +			if (rangemax < submax)
> +				submax = rangemax;

Why is this check needed?  Any why not return -EINVAL?

>  			if (!snd_tlv_get_dB_range(tlv + pos + 2,
> +						  submin, submax,
>  						  &dbmin, &dbmax) &&
>  			    db_gain >= dbmin && db_gain <= dbmax)
>  				return snd_tlv_convert_from_dB(tlv + pos + 2,
> +							       submin, submax,
>  							       db_gain, value, xdir);
>  			else if (db_gain < dbmin) {
> +				*value = xdir || pos == 2 ? submin : prev_submax;
>  				return 0;
>  			}
> +			prev_submax = submax;
> +			if (rangemax == submax)
> +				break;

Why this check?

>  			pos += int_index(tlv[pos + 3]) + 4;
> +		} while (pos + 4 <= len);
> +		*value = prev_submax;
> +		return 0;
>  	}


Regards,
Clemens