[alsa-devel] [alsa-lib][PATCH 3/4] pcm: linear, route: handle linear formats with 20-bit sample on 4 bytes

[Takashi Iwai]

On Tue, 05 Dec 2017 17:14:18 +0100,
Maciej S. Szmigiero wrote:
> 
> On 05.12.2017 15:57, Takashi Iwai wrote:
> > On Tue, 05 Dec 2017 15:37:09 +0100,
> > Takashi Sakamoto wrote:
> >>
> >> Hi,
> >>
> (..)
> >> In my opinion, addition of constraint of PCM hardware parameters to
> >> these PCM plugins is preferable to insertion of the assertions,
> >> because abort inner library is bad idea itself. Proper errors should
> >> be returned
> >> to applications in protocol via alsa-lib PCM APIs. It's more friendly to
> >> usual developers and users.
> > 
> > Well, it's basically moot to discuss about it, as
> > snd_pcm_lib_linear_get_index() is an internal function, i.e. it's not
> > supposed to be called from applications.
> > 
> > In that sense, assert() wouldn't be too bad.   However, the primary
> > question is -- would it really catch anything in practice?  What drove
> > you to put that assert() at all?
> > 
> > If a check of the valid formats, then calling snd_pcm_format_linear()
> > is the safest.
> 
> The reason I added these asserts is that snd_pcm_linear_{get,put}_index()
> use result from snd_pcm_format_width() directly to calculate an array
> index.
> If, for any reason, this function returns an unexpected width a subtle
> bug will emerge.
> 
> Three reasons that I could think of why an unexpected width would be
> returned is a bug in a plugin that uses these functions (currently 9
> plugins use them) that fail to properly restrict source / destination
> format space to linear formats only, a bug in general format refining
> code or somebody adding a format to linear formats mask without
> adapting conversion arrays, too.
> 
> I know that these may sound like rare occurrences but since this
> check is close to zero-cost (it only happens on plugin stack
> initialization time and not, for example, at every sample) I think it
> is worth the extra time (and one can always compile the library with
> NDEBUG to skip it).

So it has nothing to do with the 20bit PCM support itself.
Please put in another patch with more contexts.


thanks,

Takashi