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

[Takashi Iwai]

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