Re: alsa-lib rate plugins - float samples - patches

[Takashi Iwai <tiwai@suse.de>]

At Fri, 01 Jul 2011 11:27:03 +0200,
Pavel Hofman wrote:
> 
> 
> Dne 30.6.2011 14:48, Takashi Iwai napsal(a):
> > At Tue, 28 Jun 2011 09:11:02 +0200,
> > 
> > Thanks for the patches.
> > The basic strategy looks good to me, but a few things to be considered:
> > 
> > - The extension of API should be done in binary-compatible way; i.e.
> >   append new stuff to the existing structure, not inserting in the
> >   middle.  In that way, you can use still old plugins.
> 
> Hi Takashi, thanks a lot for your reply. Would moving the new method to
> the end of snd_pcm_rate_ops struct suffice?

This is needed together with the API version bump.
Increase SND_PCM_RATE_PLUGIN_VERSION, change rate_open_func() to call
all older versions, and don't use the new methods for older versions.

> Or do I have to create a new
> API number with corresponding checks? I am afraid the latter is the case :)
> 
> > 
> > - From the performance POV, we may need to have the built-in
> >   float conversion in rate plugin indeed.  Anyway, if rate plugin
> >   supports float, it should add FLOAT as the supported format.
> >   This needs changes in the parameter setup in rate plugin.
> >   Then it should accept the float format as is without conversions.
> 
> If I understand correctly, the rate plugin accepts only linear formats
> now. What was the original reason to exclude float?

Because the existing rate-converter are only for linear formats :)

> The "convert" API
> method is generic enough, perhaps I should add float support to all the
> rate converters using the API "convert" method. Again there is the
> question of new API version - the new one would offer convert_float and
> allow float samples in the generic convert method.
> 
> The specific convert_s16/float methods in pcm_rate.c can be modified to
> accept float too.

I don't mind how it'll be implemented as long as the backward
compatibility is kept somehow.

> > - The float-support has to be disabled when the float is suppressed
> >   via configure option.  Also this needs to be exposed somehow to the
> >   plugin.
> 
> Is softfloat/HAVE_SOFT_FLOAT the corresponding configure option? BTW, is
> the ifndef check in pcm_softvol.c correct?

HAVE_SOFT_FLOAT is a bit confusing name.  It means actually NO_FLOAT,
or better to say, NO_FLOAT_CALCULATION.  Thus ifndef is correct
below.

> #ifndef HAVE_SOFT_FLOAT
>                 svol->dB_value = calloc(resolution, sizeof(unsigned int));
>                 if (! svol->dB_value) {
>                         SNDERR("cannot allocate dB table");
>                         return -ENOMEM;
>                 }
>                 svol->min_dB = min_dB;
>                 svol->max_dB = max_dB;
>                 for (i = 0; i <= svol->max_val; i++) {
>                         double db = svol->min_dB + (i * (svol->max_dB -
> svol->min_dB)) / svol->max_val;
>                         double v = (pow(10.0, db / 20.0) * (double)(1 <<
> VOL_SCALE_SHIFT));
>                         svol->dB_value[i] = (unsigned int)v;
>                 }
>                 if (svol->zero_dB_val)
>                         svol->dB_value[svol->zero_dB_val] = 65535;
> #else
>                 SNDERR("Cannot handle the given dB range and resolution");
>                 return -EINVAL;
> #endif
> 
> It occurs to me the check should be ifdef, but perhaps I just get it
> wrong :)
> 
> > 
> > - At best, the rate converters (plugin of plugin) should support both
> >   cases with and without float.
> 
> Well, the libsamplerate is using float internally, I am afraid the
> float-less case would require disabling this rate converter alltogether.

Currently yes, but who knows in future?  The library internal can
change.


Takashi