At Fri, 30 Mar 2012 15:07:27 +0200 (CEST), Benoît Thébaudeau wrote:
Takashi Iwai wrote:
At Fri, 30 Mar 2012 13:44:43 +0200, Takashi Iwai wrote:
At Fri, 30 Mar 2012 12:41:44 +0200 (CEST), Benoît Thébaudeau wrote:
Clemens Ladisch wrote:
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.
Not always. For instance, if a codec register has the following volume values: 0x00 to 0x20: prohibited 0x21 to 0x40: some dB values 0x41 to 0x60: prohibited 0x61 to 0xff: some dB values
You can't prohibit the values that are exposed as accessible. The driver may return an error -EINVAL or such, but it doesn't mean that it's allowed to give a hole in the TLV dB definition.
But, don't get me wrong with the statement above. Your fix can be an improvement indeed. The fact that the function relies on range_min/max blindly isn't described anywhere, and it'd be better to parse the range properly, of course, especially it's an exported function, not only used as an internal function.
There may be minor coding issues as Clemens suggested, but they can be easily fixed.
So, the motivation here is to make the code more robust and consistent
Yes, that's what this patch aims at.
although the examples you raised seem like rather buggy driver implementations.
Why? And you say that there shouldn't be holes in TLV ranges. I don't see why either.
Because holes have been never defined to be allowed. Rather, in the API level, all values are supposed to be accessible.
It's possible that a driver temporarily returns an error when accessing some value. But, it's supposed to be an temporay error. Some drivers may abuse this to implement a hole.
I have not read anywhere that the ALSA APIs forbid this,
"Not forbidden" doesn't mean it's always allowed and recommended :)
and by the way it works. Moreover, the example above with some prohibited ranges would be a hardware example, not a driver example. It should be possible to handle all hardware cases.
The ALSA control values don't have to be exactly as same as the hardware raw values at all. Rather they should be converted to be human-friendly form. If there is a hole, it should be omitted and converted in the driver side to be continous.
Takashi