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