[alsa-devel] [Alsa-devel] Quality resampling code for libasound

[Takashi Iwai]

At Sat, 24 Mar 2007 08:33:18 +1100,
Jean-Marc Valin wrote:
> 
> > Well, maybe that's the whole point -- how to manage the included
> > code.  If it's linked to libspeex, yes, the plugin would rely on
> > libspeex in 100%.  This allows the automatic addition of new features
> > like SIMD support.  But, this is a drawback, too, if the library gets
> > broken although in this case, the possible problem could be easily
> > removed by removing the offending plugin.  (The alsa-lib itself won't
> > be linked to libspeex, anyway.  Only the plugin will.)
> 
> I thought the idea was to move the resampler from alsa-plugin to alsa-lib?
> 
> > Oh, I'm in very favor of using a better resampler as default, too.
> 
> So are you willing to make the resampler (in one form or another) a
> mandatory dependency?

OK, let's get things straight again:

- I prefer the built-in speex resampler in alsa-lib IFF...
  o all related codes are released under LPGL explicitly
  o there won't be too frequent updates that can't be handled as
    patches
  o we are allowed to modify codes as we like (mostly for clean ups)

- If one/many of these conditions isn't agreed, we can keep speex
  resampler peacefully in alsa-plugins tree.  The default
  configuration can be changed to use speexrate in the first
  priority.  The rest would be a role of distro vendors.
  Apparently you don't trust them but I do (at least it's one of my
  jobs)...

- If the update of resampler code may happen frequently, we can change
  alsa-plugins/pph code to link with libspeex shlib to make the
  maintenance easier.

I sincerely hope that we agree with the first case (as my favorite
choice), and appreciate any of your helps there.


> > Regarding code-rewriting, I think you misunderstood my early comment.
> > It's not the urgent issue to rewrite the code indeed.  But, it may
> > (likely) happen to get rid of the speex-specific redundant codes
> 
> I still haven't seen any example of redundant code outside of the few
> lines where I define the spx_int* types.

Well, some random notes:

- RANDOM_PREFIX isn't sexy, and the inclusion of static functions may
  give you more optimization
- Any need of speex_*alloc?
- Ditto for home-baked ABS*() and MIN*() macros
- Why not definitions in stdint.h?

> > afterwards or change the indentation and coding styles in near future
> > in a long run afterwards.  This is how the code is being maintained
> > just like in the linux kernel tree.
> 
> I thought the Linux kernel was simply copying the alsa tree once in a
> while. Are they really doing independent modifications?

Yes.  We have to occasionally sync the modifications done individually
in the upstream.

> > So, as long as we'll be able to play with the code in alsa-lib tree as
> > we like, and you agree with LGPL, the code inclusion can happen
> > easily.  Of course, for reducing the future maintenance cost, we
> > should double-check the code before inclusion.  But, in the case of
> > resampler, the code flow is easy, so it wouldn't be a big obstacle :)
> > 
> > BTW, one remaining concern is architectures that don't like math
> > library.  Could sin() and floating point operations be removed from 
> > resampler code?  Otherwise, I can make the built of pph resampler
> > conditonal to softfloat configure option.
> 
> It is my plan to get rid of any float operation in there, but it will
> likely take a little while. sin(x)/x can be easily replaced by a lookup
> table, but the remaining float operations might take a while to remove.
> How many architectures don't have soft float anyway?

I don't know exactly, but there have been requests, and we have to
deal with it.

> >> I don't see much security issues here. The code is really simple and
> >> (unless I've done something really obviously stupid -- which I don't
> >> think I have) you can easily show that there's no buffer overflow possible.
> > 
> > Well, you should check the return value of speex_*alloc() at least ;)
> 
> Sure I was going to fix that (still need to return error values), but
> there's no security issue there, as it will just crash in case of OOM
> condition.

Oh no, I don't blame that I found any security issues.  Otherwise I
would have fixed it.

What I meant is that any serious problems could be much easily fixed
in one place if we use a shlib approach.  Of course, the drawback is
the breakage can happen easily as well.  But, still the plugin can be
disabled at any time as long as we have a fallback.


thanks,

Takashi