[alsa-devel] [PATCH] alsa-lib/tlv: fix handling of raw value ranges
tiwai at suse.de
Fri Mar 30 15:23:32 CEST 2012
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
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.
More information about the Alsa-devel