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

[Takashi Iwai]

At Fri, 23 Mar 2007 00:36:00 +1100,
Jean-Marc Valin wrote:
> 
> > This is different understanding between us.
> > 
> > I'm working at a distro vendor, and as a responsible person for
> > distribution packages, the first thought is maintainability.
> 
> So far we agree.

Good.

> > For that purpase, I prefer very much a shared library approach because
> > I know the hell of vice of copying codes.  Copying code works well if
> > it's 100% safe and mature code, which requires no longer updates and
> > fixes.  But a code being developed shouldn't be copied.
> 
> Not sure I agree here. Do you really want to depend on libspeex
> "blindly" not knowing what changes I make (while the code is evolving).
> Code copying is not necessarily bad. After all, that what the kernel
> does with the ALSA drivers -- sync up every once and then.

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.)


> > That is, the only criteria for inclusion is whether the code is well
> > tested and no more fix/update is needed.  In this case, whether
> > resample code has to be further sync'ed with speex codebase or not.
> > If it should be kept synced, then shlib approach is clearly better.
> > Otherwise we have to take care of two places at the same time.
> 
> I guess my main question would be: are you willing to make libspeex a
> *mandatory* dependency? If not, please include a copy of the resampler.
> From now on, there's no way alsa-lib should ever be built with linear
> interpolation resampling unless the user specifically configures it with
> --enable-awful-audio-mutilation-code or something like that.

Oh, I'm in very favor of using a better resampler as default, too.

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
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.

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.

> > Or, in other words:  the sound quality could be a matter of taste, but
> > a security fix can't be so :)
> 
> 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 ;)


thanks,

Takashi