Re: [alsa-devel] [Alsa-devel] Quality resampling code for libasound
At Wed, 21 Mar 2007 08:35:22 +1100, Jean-Marc Valin wrote:
Well, I use that type because that's what is used in the API. And the reason I use it in the API is because this code is meant to work even on platforms with 16-bit integers (e.g. TI C5X DSPs) and no C99 compiler. If that's causing problem, I guess you could still use unsigned int with no problem...
Don't worry, alsa-lib would be very unlikely ported to such an architecture.
Well, my code has to run on those architectures, and it's much simpler if I don't need to change it for ALSA. Now if you don't want the spx_uint32_t in the plugin code, then feel free to change that.
OK, as long as it's in alsa-plugins repository, I basically don't care. But, for merging it to alsa-lib, I'd like to change the code very much (including coding-style unification).
Anyway, I applied your latest codes to HG tree now (with the removal of samplerate.h). Thanks!
Takashi
OK, as long as it's in alsa-plugins repository, I basically don't care. But, for merging it to alsa-lib, I'd like to change the code very much (including coding-style unification).
Well, it's ultimately your call. For rate_speexrate.c, it really doesn't matter what happens because that's the only place it's used. When it comes to the resampler code itself, it depends on how you want to maintain it. I'll be maintaining it from the Speex tree, so it you want to change the style, you'll have to either "backport" improvements/fixes (e.g. optimisations) or do the style change again every time you sync. That is, unless you plan to do an alsa-specific fork of it. (I personally think the simplest way would be not modify the core resampler (resample.c and corresponding header files) and just sync with Speex once in a while.
Anyway, I applied your latest codes to HG tree now (with the removal of samplerate.h). Thanks!
Thanks.
Jean-Marc
At Thu, 22 Mar 2007 00:08:09 +1100, Jean-Marc Valin wrote:
OK, as long as it's in alsa-plugins repository, I basically don't care. But, for merging it to alsa-lib, I'd like to change the code very much (including coding-style unification).
Well, it's ultimately your call. For rate_speexrate.c, it really doesn't matter what happens because that's the only place it's used. When it comes to the resampler code itself, it depends on how you want to maintain it. I'll be maintaining it from the Speex tree, so it you want to change the style, you'll have to either "backport" improvements/fixes (e.g. optimisations) or do the style change again every time you sync. That is, unless you plan to do an alsa-specific fork of it. (I personally think the simplest way would be not modify the core resampler (resample.c and corresponding header files) and just sync with Speex once in a while.
Yeah, this is an interesting question. We often see similar conflicts in the maintenance of linux kernel tree, too. The unified coding style gives clearly better readability of the whole tree. OTOH, if a code chunk is derived from another tree, it requires more additional backport effort.
In the ideal situation, we may link to speex resampler as a dynamic library -- e.g. adding a configure option to let user choose the built-in or system-wide library resampler code. Does libspeex provide this code snip as exported?
Takashi
Yeah, this is an interesting question. We often see similar conflicts in the maintenance of linux kernel tree, too. The unified coding style gives clearly better readability of the whole tree. OTOH, if a code chunk is derived from another tree, it requires more additional backport effort.
I think the main question is: do you plan on making many alsa-specific changes to that code? resampler.c is pretty much as close as you get to pure signal processing code (no "system" stuff), so I don't think many people will actually want to mess with it and any improvements might as well be merged in Speex first to be tested for a while. Of course, if the coding rules are simple, an other approach could be to automatically re-indent the code when syncing. Of course, it still means you need to keep the same code as Speex has. In any case, I'd recommend not forking that code too early because some of the planned improvements (e.g. SSE optimisations) could be a pain to backport.
In the ideal situation, we may link to speex resampler as a dynamic library -- e.g. adding a configure option to let user choose the built-in or system-wide library resampler code. Does libspeex provide this code snip as exported?
Not sure I understand what you mean here. If you're asking whether the symbols for the resampler are exported in the shared library, then the answer is yes -- though the resampler is only in the svn version of Speex for now. The first release to include it will be 1.2beta2, which I'm planning to release in a few weeks.
Jean-Marc
At Thu, 22 Mar 2007 00:39:44 +1100, Jean-Marc Valin wrote:
Yeah, this is an interesting question. We often see similar conflicts in the maintenance of linux kernel tree, too. The unified coding style gives clearly better readability of the whole tree. OTOH, if a code chunk is derived from another tree, it requires more additional backport effort.
I think the main question is: do you plan on making many alsa-specific changes to that code?
Yes (not alsa-specific but less speex-specific), once if it's merged to alsa-lib tree. (The code can't be applied to the tree as it is, BTW, because it must be LGPL, as we discussed...)
resampler.c is pretty much as close as you get to pure signal processing code (no "system" stuff), so I don't think many people will actually want to mess with it and any improvements might as well be merged in Speex first to be tested for a while. Of course, if the coding rules are simple, an other approach could be to automatically re-indent the code when syncing. Of course, it still means you need to keep the same code as Speex has. In any case, I'd recommend not forking that code too early because some of the planned improvements (e.g. SSE optimisations) could be a pain to backport.
Yes, but if the code is in another tree, it's actually a fork. Having codes that may be frequently changed in multiple places is already a risky action from the maintenance POV, regardless of the coding style. That's what I'd like to avoid.
In the ideal situation, we may link to speex resampler as a dynamic library -- e.g. adding a configure option to let user choose the built-in or system-wide library resampler code. Does libspeex provide this code snip as exported?
Not sure I understand what you mean here. If you're asking whether the symbols for the resampler are exported in the shared library, then the answer is yes -- though the resampler is only in the svn version of Speex for now. The first release to include it will be 1.2beta2, which I'm planning to release in a few weeks.
Ah, OK. I asked it because I couldn't find symbols in my libspeex binary.
Takashi
Yes (not alsa-specific but less speex-specific),
Outside of the spx_int* types, what are the speex-specific bits that bother you?
once if it's merged to alsa-lib tree. (The code can't be applied to the tree as it is, BTW, because it must be LGPL, as we discussed...)
Is a COPYING file OK or do you need headers (in which case, what's the smallest header that can be added?)?
Yes, but if the code is in another tree, it's actually a fork. Having codes that may be frequently changed in multiple places is already a risky action from the maintenance POV, regardless of the coding style. That's what I'd like to avoid.
Then the solution would be to just copy from the Speex tree once in a while and get the fixes there first. You can always have an automated formatting tool that you run after importing a new version. Overall, I really see no reason why you'd want to modify the resampler in the first place -- other than fixing bugs, which should go in the Speex tree anyway.
Jean-Marc
At Thu, 22 Mar 2007 01:19:18 +1100, Jean-Marc Valin wrote:
Yes (not alsa-specific but less speex-specific),
Outside of the spx_int* types, what are the speex-specific bits that bother you?
There are lots of macros in arch.h and speex_resampler.h, which try to absorb the difference from non-C99 and non-LP32 platforms.
BTW, I'm not against these codes at all in general. But, between speex and alsa-lib, the target is simply different. Hence the code should be written in a different manner accordingly.
once if it's merged to alsa-lib tree. (The code can't be applied to the tree as it is, BTW, because it must be LGPL, as we discussed...)
Is a COPYING file OK or do you need headers (in which case, what's the smallest header that can be added?)?
Each file in pph/* contains the BSD headers, so we have to change them anyway.
Yes, but if the code is in another tree, it's actually a fork. Having codes that may be frequently changed in multiple places is already a risky action from the maintenance POV, regardless of the coding style. That's what I'd like to avoid.
Then the solution would be to just copy from the Speex tree once in a while and get the fixes there first. You can always have an automated formatting tool that you run after importing a new version. Overall, I really see no reason why you'd want to modify the resampler in the first place -- other than fixing bugs, which should go in the Speex tree anyway.
IMO, it never worked well, looking at history. For example, you can find a piece of zlib code everywhere, but they are not synchronized and well bugfixed (especially thinking of many security fixes in zlib).
The only working solution for synchronized source management is to use the shared library.
Takashi
There are lots of macros in arch.h and speex_resampler.h, which try to absorb the difference from non-C99 and non-LP32 platforms.
Actually, most of the stuff in arch.h is designed to abstract the operators so they can be defined for fixed-point or floating point (e.g. spx_word32_t is defined either as float or as int depending on how you compile). There's nothing about non-C99 stuff and the only non-LP32 stuff are the few spx_int* types.
BTW, I'm not against these codes at all in general. But, between speex and alsa-lib, the target is simply different. Hence the code should be written in a different manner accordingly.
But is it worth increasing the maintenance overhead for code that may never be modified directly in alsa. Note that I'm not talking about the plugin code that uses the resampler here.
Each file in pph/* contains the BSD headers, so we have to change them anyway.
Can't we just add a "Oh and BTW, you can also use it under the LGPL" right after the BSD header. This is something I'd be willing to do in the Speex tree. Apply a new license when syncing code seems like a bad idea to me.
IMO, it never worked well, looking at history. For example, you can find a piece of zlib code everywhere, but they are not synchronized and well bugfixed (especially thinking of many security fixes in zlib).
Well, I can't really think of something better for now.
The only working solution for synchronized source management is to use the shared library.
You could do that too, but it means you'll be forcing libspeex > 1.2beta2 onto all distributions. I don't mind because it should be quite a good release, but maybe some will object. I'd suggest just doing a copy in the short term and then move to the shared library when distros have it.
Jean-Marc
At Thu, 22 Mar 2007 02:17:56 +1100, Jean-Marc Valin wrote:
There are lots of macros in arch.h and speex_resampler.h, which try to absorb the difference from non-C99 and non-LP32 platforms.
Actually, most of the stuff in arch.h is designed to abstract the operators so they can be defined for fixed-point or floating point (e.g. spx_word32_t is defined either as float or as int depending on how you compile). There's nothing about non-C99 stuff and the only non-LP32 stuff are the few spx_int* types.
OK.
BTW, I'm not against these codes at all in general. But, between speex and alsa-lib, the target is simply different. Hence the code should be written in a different manner accordingly.
But is it worth increasing the maintenance overhead for code that may never be modified directly in alsa. Note that I'm not talking about the plugin code that uses the resampler here.
Well, this arises another question: If the code is not allowed to modify, why to bother to include the code in the tree? Why not use a shared library?
Each file in pph/* contains the BSD headers, so we have to change them anyway.
Can't we just add a "Oh and BTW, you can also use it under the LGPL" right after the BSD header. This is something I'd be willing to do in the Speex tree. Apply a new license when syncing code seems like a bad idea to me.
I personally don't like the dual license. It can't work seriously if someone changes/forks the code.
IMO, it never worked well, looking at history. For example, you can find a piece of zlib code everywhere, but they are not synchronized and well bugfixed (especially thinking of many security fixes in zlib).
Well, I can't really think of something better for now.
The only working solution for synchronized source management is to use the shared library.
You could do that too, but it means you'll be forcing libspeex > 1.2beta2 onto all distributions. I don't mind because it should be quite a good release, but maybe some will object. I'd suggest just doing a copy in the short term and then move to the shared library when distros have it.
I feel the easiest way for both of us is just to keep alsa-plugins as it is. (Or, even better, we can modify alsa-plugin configure script to detect libspeex, too. This will decrease the maintenance const.)
Then, change alsa-lib rate plugin to look up the list of plugins, and put "speexrate" to the top of it and "linear" to the bottom in the default configuration, so that speex resampler can be used (if installed) and the built-in linear plugin can be safely used as a fallback.
Takashi
BTW, I'm not against these codes at all in general. But, between speex and alsa-lib, the target is simply different. Hence the code should be written in a different manner accordingly.
But is it worth increasing the maintenance overhead for code that may never be modified directly in alsa. Note that I'm not talking about the plugin code that uses the resampler here.
Well, this arises another question: If the code is not allowed to modify, why to bother to include the code in the tree? Why not use a shared library?
Well, first it's nod forbidden to modify it, just saying there probably isn't much point in doing so. I think using a shared library actually makes sense in the long term, but in the short term it may cause problems for distributions. Another concern is making an optional libspeex dependency and having all the distros skip that and end up with the good 'ol shitty resampler again. As I've mentioned before, my #1 goal here is to make sure nobody ever ends up with that resampler again unless actively attempting to.
I personally don't like the dual license. It can't work seriously if someone changes/forks the code.
Nothing to do with dual-license. I can fork ALSA and make it GPL easily. Making the resampler dual-licensed right in libspeex would actually make it very easy for everyone.
You could do that too, but it means you'll be forcing libspeex > 1.2beta2 onto all distributions. I don't mind because it should be quite a good release, but maybe some will object. I'd suggest just doing a copy in the short term and then move to the shared library when distros have it.
I feel the easiest way for both of us is just to keep alsa-plugins as it is. (Or, even better, we can modify alsa-plugin configure script to detect libspeex, too. This will decrease the maintenance const.)
Then, change alsa-lib rate plugin to look up the list of plugins, and put "speexrate" to the top of it and "linear" to the bottom in the default configuration, so that speex resampler can be used (if installed) and the built-in linear plugin can be safely used as a fallback.
But then we've got the same problem as above. Users will not manually install alsa-plugin and I doubt we can trust distros to do the right thing. Really, I don't understand why the whole thing needs to be made that complicated. At this point, I think plain forking the code (and changing all the style and everything) with no interactions with the Speex trunk is still preferable to the options above. At least it means a decent resampler is actually being used -- 100% of the time.
Jean-Marc
At Thu, 22 Mar 2007 09:08:32 +1100, Jean-Marc Valin wrote:
BTW, I'm not against these codes at all in general. But, between speex and alsa-lib, the target is simply different. Hence the code should be written in a different manner accordingly.
But is it worth increasing the maintenance overhead for code that may never be modified directly in alsa. Note that I'm not talking about the plugin code that uses the resampler here.
Well, this arises another question: If the code is not allowed to modify, why to bother to include the code in the tree? Why not use a shared library?
Well, first it's nod forbidden to modify it, just saying there probably isn't much point in doing so. I think using a shared library actually makes sense in the long term, but in the short term it may cause problems for distributions. Another concern is making an optional libspeex dependency and having all the distros skip that and end up with the good 'ol shitty resampler again. As I've mentioned before, my #1 goal here is to make sure nobody ever ends up with that resampler again unless actively attempting to.
Well, this comes from the difference of thoughs regarding the distribution. See below.
I personally don't like the dual license. It can't work seriously if someone changes/forks the code.
Nothing to do with dual-license. I can fork ALSA and make it GPL easily. Making the resampler dual-licensed right in libspeex would actually make it very easy for everyone.
Fine, then this is no problem at all, then.
You could do that too, but it means you'll be forcing libspeex > 1.2beta2 onto all distributions. I don't mind because it should be quite a good release, but maybe some will object. I'd suggest just doing a copy in the short term and then move to the shared library when distros have it.
I feel the easiest way for both of us is just to keep alsa-plugins as it is. (Or, even better, we can modify alsa-plugin configure script to detect libspeex, too. This will decrease the maintenance const.)
Then, change alsa-lib rate plugin to look up the list of plugins, and put "speexrate" to the top of it and "linear" to the bottom in the default configuration, so that speex resampler can be used (if installed) and the built-in linear plugin can be safely used as a fallback.
But then we've got the same problem as above. Users will not manually install alsa-plugin and I doubt we can trust distros to do the right thing. Really, I don't understand why the whole thing needs to be made that complicated. At this point, I think plain forking the code (and changing all the style and everything) with no interactions with the Speex trunk is still preferable to the options above. At least it means a decent resampler is actually being used -- 100% of the time.
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. 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.
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.
Or, in other words: the sound quality could be a matter of taste, but a security fix can't be so :)
Takashi
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.
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.
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.
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
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?
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.
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?
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 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.
Jean-Marc
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
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
I'm willing to do that, but only if we already get the rest sorted out.
o there won't be too frequent updates that can't be handled as patches
There's two issues: - What's frequent? - Can't be handled as patches depends mainly on how much you change the code in your tree.
o we are allowed to modify codes as we like (mostly for clean ups)
The code is open-source, you do what you want with it. I'm only here to make suggestions.
- 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)...
OK, I have to admit I've got limited experience. Let's just say I don't trust the Ubuntu maintainer :-)
- 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.
About update frequency, keep in mind that you don't need to update every time I change something. I'm expecting to make many changes, but that would be more improvements than bug fixes.
I sincerely hope that we agree with the first case (as my favorite choice), and appreciate any of your helps there.
I also hope we get that to work, but the only real power I have over that is the first issue (license).
Well, some random notes:
- RANDOM_PREFIX isn't sexy, and the inclusion of static functions may give you more optimization
I considered static functions, but that means they can only be used from one file and it means it's very easy to end up with multiple copies of the code when compiling.
- Any need of speex_*alloc?
Well, I need it in the main tree to make it easy for developers to override the allocation function when porting Speex (e.g. on platforms that don't have malloc). Basically, I want to make it possible to build Speex without even linking to libc.
- Ditto for home-baked ABS*() and MIN*() macros
Two things here. They need to be macros because they have different definitions for float and int. Also, I need to be able to override then with inline assembly, or debug versions.
- Why not definitions in stdint.h?
Euh, how about because it's only part of C99? It may not matter to you or ALSA, but it does for some users of Speex.
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.
Well, it's in the plans, but it won't happen overnight.
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.
Actually, how about having the copied code as fallback in case the shlib isn't there?
_From other email:
Agreed. I just thought that fixed-point is used as default in your resampler code but it seems wrong.
No, floating-point is used unless you define FIXED_POINT. As I was saying, on any recent PC, the float version (even without SSE) is faster and has better quality than the fixed-point version.
Jean-Marc
At Mon, 26 Mar 2007 21:47:49 +1000, Jean-Marc Valin wrote:
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
I'm willing to do that, but only if we already get the rest sorted out.
o there won't be too frequent updates that can't be handled as patches
There's two issues:
- What's frequent?
I'd say once per week.
- Can't be handled as patches depends mainly on how much you change the
code in your tree.
Right. As mentioned earlier, I have also no plan to rewrite the codes so urgently. Let's settle down it first.
o we are allowed to modify codes as we like (mostly for clean ups)
The code is open-source, you do what you want with it. I'm only here to make suggestions.
OK, good to hear.
- 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)...
OK, I have to admit I've got limited experience. Let's just say I don't trust the Ubuntu maintainer :-)
Heh, you can simply bug them :)
- 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.
About update frequency, keep in mind that you don't need to update every time I change something. I'm expecting to make many changes, but that would be more improvements than bug fixes.
OK. Then it's no big problem from my side.
I sincerely hope that we agree with the first case (as my favorite choice), and appreciate any of your helps there.
I also hope we get that to work, but the only real power I have over that is the first issue (license).
Yeah, the license is a nasty issue, but it's crucial for open-source at the same time.
Well, some random notes:
- RANDOM_PREFIX isn't sexy, and the inclusion of static functions may give you more optimization
I considered static functions, but that means they can only be used from one file and it means it's very easy to end up with multiple copies of the code when compiling.
- Any need of speex_*alloc?
Well, I need it in the main tree to make it easy for developers to override the allocation function when porting Speex (e.g. on platforms that don't have malloc). Basically, I want to make it possible to build Speex without even linking to libc.
- Ditto for home-baked ABS*() and MIN*() macros
Two things here. They need to be macros because they have different definitions for float and int. Also, I need to be able to override then with inline assembly, or debug versions.
- Why not definitions in stdint.h?
Euh, how about because it's only part of C99? It may not matter to you or ALSA, but it does for some users of Speex.
From the POV of library, I of course agree with such workarounds. The suggestions above are the possible clean-ups that could be applied once after included in alsa-lib tree (again, non-urgent issues).
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.
Well, it's in the plans, but it won't happen overnight.
OK, for such architectures, we can disable the plugin. They are anyway rare, so you won't be bothered.
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.
Actually, how about having the copied code as fallback in case the shlib isn't there?
Yes, that's my idea, too. That is, _adding_ the configure option to make use of libspeex.
From other email: Agreed. I just thought that fixed-point is used as default in your resampler code but it seems wrong.
No, floating-point is used unless you define FIXED_POINT. As I was saying, on any recent PC, the float version (even without SSE) is faster and has better quality than the fixed-point version.
OK.
thanks,
Takashi
- What's frequent?
I'd say once per week.
Don't worry then. Except maybe for the next month or two, I would expect an update every 3-6 months at most and only if you want improvements.
OK, I have to admit I've got limited experience. Let's just say I don't trust the Ubuntu maintainer :-)
Heh, you can simply bug them :)
Yup, last time I did that, it still took them more than 6 months to apply a trivial (and critical) patch I had for Speex (see http://jmspeex.livejournal.com/1974.html ).
Yeah, the license is a nasty issue, but it's crucial for open-source at the same time.
It not that nasty considering the two licenses are perfectly compatible. That being said, I'll try and do something about it. Any idea what extra text needs to be added to say this license OR that license.
Euh, how about because it's only part of C99? It may not matter to you or ALSA, but it does for some users of Speex.
From the POV of library, I of course agree with such workarounds. The suggestions above are the possible clean-ups that could be applied once after included in alsa-lib tree (again, non-urgent issues).
Again, you're free to do what you like, but I tend to think that the minor improvement in the code may not be worth diverging from the main tree.
Well, it's in the plans, but it won't happen overnight.
OK, for such architectures, we can disable the plugin. They are anyway rare, so you won't be bothered.
OK, but in that case, I think it would be good to include a warning at configure time along the lines of "Warning, the backup resampler will eat your audio and may make your cat deaf" :-)
Actually, how about having the copied code as fallback in case the shlib isn't there?
Yes, that's my idea, too. That is, _adding_ the configure option to make use of libspeex.
Sounds good.
Jean-Marc
At Mon, 26 Mar 2007 23:09:33 +1000, Jean-Marc Valin wrote:
Yeah, the license is a nasty issue, but it's crucial for open-source at the same time.
It not that nasty considering the two licenses are perfectly compatible. That being said, I'll try and do something about it. Any idea what extra text needs to be added to say this license OR that license.
As mentioned earlier, I basically don't like a dual license because it's ambiguous when a third person adds/modifies the code. I don't think it would be a big issue in the case of small codes like resampler, though.
Do you already have a patch for inclusion to the alsa-lib tree? If yes, I'd like to merge it before 1.0.14 release very much...
thanks,
Takashi
It not that nasty considering the two licenses are perfectly compatible. That being said, I'll try and do something about it. Any idea what extra text needs to be added to say this license OR that license.
As mentioned earlier, I basically don't like a dual license because it's ambiguous when a third person adds/modifies the code. I don't think it would be a big issue in the case of small codes like resampler, though.
I'm not sure what you expect here... There's no way I'm going to make my code LGPL-only just so it makes it nicer in ALSA.
Do you already have a patch for inclusion to the alsa-lib tree? If yes, I'd like to merge it before 1.0.14 release very much...
What kind of patch do you mean? I thought it was just a matter of moving the files around from alsa-plugins to alsa-lib. I've also made a couple changes to the code recently, so I'll send you a patch for that as well. Let me know when I need to send them for the 1.0.14 release.
Jean-Marc
At Thu, 05 Apr 2007 09:04:09 +1000, Jean-Marc Valin wrote:
It not that nasty considering the two licenses are perfectly compatible. That being said, I'll try and do something about it. Any idea what extra text needs to be added to say this license OR that license.
As mentioned earlier, I basically don't like a dual license because it's ambiguous when a third person adds/modifies the code. I don't think it would be a big issue in the case of small codes like resampler, though.
I'm not sure what you expect here... There's no way I'm going to make my code LGPL-only just so it makes it nicer in ALSA.
Well, the above is my very personal opinion, and in practice, the dual license can work for cases like this regardless of like and dislike. So I hope we get LGPL-only at best but LGPL/BSD at least.
Do you already have a patch for inclusion to the alsa-lib tree? If yes, I'd like to merge it before 1.0.14 release very much...
What kind of patch do you mean? I thought it was just a matter of moving the files around from alsa-plugins to alsa-lib. I've also made a couple changes to the code recently, so I'll send you a patch for that as well. Let me know when I need to send them for the 1.0.14 release.
I cannot copy the code as it's still BSD-only. At least, the modification by you is needed in this regard. Also, it'd be nice to update the code before merging, too.
thanks,
Takashi
On 03/21/2007 02:14 PM, Takashi Iwai wrote:
In the ideal situation, we may link to speex resampler as a dynamic library -- e.g. adding a configure option to let user choose the built-in or system-wide library resampler code. Does libspeex provide this code snip as exported?
There's one problem with that solution though; people generally don't compile and install alsa-lib themselves these days and since distributions may not be too keen on having their alsa-lib package dependent on their speex package they might just compile with the builtin code.
Users of these distributions would then have to be fairly familiar with alsa to know they could improve sound by recompiling alsa-lib against the speex libraries, but given that it's (also) dirt cheap soundcards that need the resampling, their users aren't too likely to _be_ fairly familiar. They'd just observe (still) that their sound is "much better on windows".
If the standard code is as lousy as I've read in this thread, keeping it as default is probably not the best thing.
Rene.
Users of these distributions would then have to be fairly familiar with alsa to know they could improve sound by recompiling alsa-lib against the speex libraries, but given that it's (also) dirt cheap soundcards that need the resampling, their users aren't too likely to _be_ fairly familiar. They'd just observe (still) that their sound is "much better on windows".
Oh, I meant using a copy of the pph code in the mean time, not the current linear interpolation resampler.
If the standard code is as lousy as I've read in this thread, keeping it as default is probably not the best thing.
It's worse than you think :-) Try playing an 8 kHz file to a soundcard that only does 44.1/48. It's just horrible.
Jean-Marc
On 03/21/2007 04:34 PM, Jean-Marc Valin wrote:
Users of these distributions would then have to be fairly familiar with alsa to know they could improve sound by recompiling alsa-lib against the speex libraries, but given that it's (also) dirt cheap soundcards that need the resampling, their users aren't too likely to _be_ fairly familiar. They'd just observe (still) that their sound is "much better on windows".
Oh, I meant using a copy of the pph code in the mean time, not the current linear interpolation resampler.
Mmm, I believe Takashi Iwai was though. If I interpreted him correctly he proposed to optionally link libasound against libspeex (libresample?) if so ./config-ured and found at alsa-lib compile time but to keep using the current resampler when not.
Given the idea that distributions probably don't want their alsa-lib package dependent on their speex package (alsa-lib is right above the kernel and mandatory on any Linux system wanting to do anything with sound while speex is significantly higher up on the chain) I worried this would mean your code wouldn't be used in practice.
If the standard code is as lousy as I've read in this thread, keeping it as default is probably not the best thing.
It's worse than you think :-) Try playing an 8 kHz file to a soundcard that only does 44.1/48. It's just horrible.
Trouble is that I don't have a soundcard that can only do 44.1/48. I'll go hack up a driver to pretend I do though and try. Have a nice 8 kHz file I can try with? :)
Rene.
At Wed, 21 Mar 2007 17:38:21 +0100, Rene Herman wrote:
On 03/21/2007 04:34 PM, Jean-Marc Valin wrote:
Users of these distributions would then have to be fairly familiar with alsa to know they could improve sound by recompiling alsa-lib against the speex libraries, but given that it's (also) dirt cheap soundcards that need the resampling, their users aren't too likely to _be_ fairly familiar. They'd just observe (still) that their sound is "much better on windows".
Oh, I meant using a copy of the pph code in the mean time, not the current linear interpolation resampler.
Mmm, I believe Takashi Iwai was though. If I interpreted him correctly he proposed to optionally link libasound against libspeex (libresample?) if so ./config-ured and found at alsa-lib compile time but to keep using the current resampler when not.
Given the idea that distributions probably don't want their alsa-lib package dependent on their speex package (alsa-lib is right above the kernel and mandatory on any Linux system wanting to do anything with sound while speex is significantly higher up on the chain) I worried this would mean your code wouldn't be used in practice.
Well, my (latest) prpoposal is:
- keep speex resampler in alsa-plugins tree - installing this plugin automatically enables it as the default rate converter without touching any configuration
The second part is missing right now.
In this way, distro simply needs to build alsa-plugins-pph package and install it as one of default packages. alsa-lib tree could be still built without speex. So, the package dependency is no longer big problem.
Takashi
At Wed, 21 Mar 2007 17:57:38 +0100, I wrote:
At Wed, 21 Mar 2007 17:38:21 +0100, Rene Herman wrote:
On 03/21/2007 04:34 PM, Jean-Marc Valin wrote:
Users of these distributions would then have to be fairly familiar with alsa to know they could improve sound by recompiling alsa-lib against the speex libraries, but given that it's (also) dirt cheap soundcards that need the resampling, their users aren't too likely to _be_ fairly familiar. They'd just observe (still) that their sound is "much better on windows".
Oh, I meant using a copy of the pph code in the mean time, not the current linear interpolation resampler.
Mmm, I believe Takashi Iwai was though. If I interpreted him correctly he proposed to optionally link libasound against libspeex (libresample?) if so ./config-ured and found at alsa-lib compile time but to keep using the current resampler when not.
Given the idea that distributions probably don't want their alsa-lib package dependent on their speex package (alsa-lib is right above the kernel and mandatory on any Linux system wanting to do anything with sound while speex is significantly higher up on the chain) I worried this would mean your code wouldn't be used in practice.
Well, my (latest) prpoposal is:
- keep speex resampler in alsa-plugins tree
- installing this plugin automatically enables it as the default rate converter without touching any configuration
The second part is missing right now.
... and the patch below supports it.
With this patch, you can set the list of multiple default PCM rate converters. For example,
defaults.pcm.rate_converter [ "speexrate" "samplerate" "linear" ]
will look for speex plugin, then libsamplerate plugin, lastly it will take the built-in linear converter as a fallback. So, we can define this safely in the default alsa.conf file.
Takashi
diff -r c40d95b55851 include/pcm_plugin.h --- a/include/pcm_plugin.h Wed Mar 21 12:21:38 2007 +0100 +++ b/include/pcm_plugin.h Wed Mar 21 17:12:30 2007 +0100 @@ -156,7 +156,8 @@ int _snd_pcm_route_open(snd_pcm_t **pcmp * Rate plugin for linear formats */ int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name, - snd_pcm_format_t sformat, unsigned int srate, const char *converter, + snd_pcm_format_t sformat, unsigned int srate, + const snd_config_t *converter, snd_pcm_t *slave, int close_slave); int _snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name, snd_config_t *root, snd_config_t *conf, diff -r c40d95b55851 src/pcm/pcm_local.h --- a/src/pcm/pcm_local.h Wed Mar 21 12:21:38 2007 +0100 +++ b/src/pcm/pcm_local.h Wed Mar 21 17:11:34 2007 +0100 @@ -756,7 +756,7 @@ int snd_pcm_hw_open_fd(snd_pcm_t **pcmp,
int snd_pcm_wait_nocheck(snd_pcm_t *pcm, int timeout);
-const char *snd_pcm_rate_get_default_converter(snd_config_t *root); +const snd_config_t *snd_pcm_rate_get_default_converter(snd_config_t *root);
#define SND_PCM_HW_PARBIT_ACCESS (1U << SND_PCM_HW_PARAM_ACCESS) #define SND_PCM_HW_PARBIT_FORMAT (1U << SND_PCM_HW_PARAM_FORMAT) diff -r c40d95b55851 src/pcm/pcm_plug.c --- a/src/pcm/pcm_plug.c Wed Mar 21 12:21:38 2007 +0100 +++ b/src/pcm/pcm_plug.c Wed Mar 21 18:11:36 2007 +0100 @@ -50,7 +50,7 @@ typedef struct { snd_pcm_format_t sformat; int schannels; int srate; - const char *rate_converter; + const snd_config_t *rate_converter; enum snd_pcm_plug_route_policy route_policy; snd_pcm_route_ttable_entry_t *ttable; int ttable_ok, ttable_last; @@ -1015,7 +1015,7 @@ int snd_pcm_plug_open(snd_pcm_t **pcmp, int snd_pcm_plug_open(snd_pcm_t **pcmp, const char *name, snd_pcm_format_t sformat, int schannels, int srate, - const char *rate_converter, + const snd_config_t *rate_converter, enum snd_pcm_plug_route_policy route_policy, snd_pcm_route_ttable_entry_t *ttable, unsigned int tt_ssize, @@ -1133,7 +1133,7 @@ int _snd_pcm_plug_open(snd_pcm_t **pcmp, unsigned int cused, sused; snd_pcm_format_t sformat = SND_PCM_FORMAT_UNKNOWN; int schannels = -1, srate = -1; - const char *rate_converter = NULL; + const snd_config_t *rate_converter = NULL;
snd_config_for_each(i, next, conf) { snd_config_t *n = snd_config_iterator_entry(i); @@ -1177,12 +1177,7 @@ int _snd_pcm_plug_open(snd_pcm_t **pcmp, #endif #ifdef BUILD_PCM_PLUGIN_RATE if (strcmp(id, "rate_converter") == 0) { - const char *str; - if ((err = snd_config_get_string(n, &str)) < 0) { - SNDERR("Invalid type for %s", id); - return -EINVAL; - } - rate_converter = str; + rate_converter = n; continue; } #endif diff -r c40d95b55851 src/pcm/pcm_rate.c --- a/src/pcm/pcm_rate.c Wed Mar 21 12:21:38 2007 +0100 +++ b/src/pcm/pcm_rate.c Wed Mar 21 18:18:35 2007 +0100 @@ -1254,17 +1254,44 @@ static snd_pcm_ops_t snd_pcm_rate_ops = * \param root Root configuration node * \retval A const string if found, or NULL */ -const char *snd_pcm_rate_get_default_converter(snd_config_t *root) +const snd_config_t *snd_pcm_rate_get_default_converter(snd_config_t *root) { snd_config_t *n; /* look for default definition */ - if (snd_config_search(root, "defaults.pcm.rate_converter", &n) >= 0) { - const char *str; - if (snd_config_get_string(n, &str) >= 0) - return str; - } + if (snd_config_search(root, "defaults.pcm.rate_converter", &n) >= 0) + return n; return NULL; } + +#ifdef PIC +static int rate_open_func(snd_pcm_rate_t *rate, const char *type) +{ + char open_name[64]; + snd_pcm_rate_open_func_t open_func; + + snprintf(open_name, sizeof(open_name), "_snd_pcm_rate_%s_open", type); + open_func = snd_dlobj_cache_lookup(open_name); + if (!open_func) { + void *h; + char lib_name[128], *lib = NULL; + if (strcmp(type, "linear")) { + snprintf(lib_name, sizeof(lib_name), + "%s/libasound_module_rate_%s.so", PKGLIBDIR, type); + lib = lib_name; + } + h = snd_dlopen(lib, RTLD_NOW); + if (!h) + return -ENOENT; + open_func = dlsym(h, open_name); + if (!open_func) { + snd_dlclose(h); + return -ENOENT; + } + snd_dlobj_cache_add(open_name, h, open_func); + } + return open_func(SND_PCM_RATE_PLUGIN_VERSION, &rate->obj, &rate->ops); +} +#endif
/** * \brief Creates a new rate PCM @@ -1280,15 +1307,17 @@ const char *snd_pcm_rate_get_default_con * of compatibility reasons. The prototype might be freely * changed in future. */ -int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name, snd_pcm_format_t sformat, - unsigned int srate, const char *type, snd_pcm_t *slave, int close_slave) +int snd_pcm_rate_open(snd_pcm_t **pcmp, const char *name, + snd_pcm_format_t sformat, unsigned int srate, + const snd_config_t *converter, + snd_pcm_t *slave, int close_slave) { snd_pcm_t *pcm; snd_pcm_rate_t *rate; - snd_pcm_rate_open_func_t open_func; - char open_name[64]; + const char *type; int err; #ifndef PIC + snd_pcm_rate_open_func_t open_func; extern int SND_PCM_RATE_PLUGIN_ENTRY(linear) (unsigned int version, void **objp, snd_pcm_rate_ops_t *ops); #endif
@@ -1306,49 +1335,47 @@ int snd_pcm_rate_open(snd_pcm_t **pcmp, rate->sformat = sformat; snd_atomic_write_init(&rate->watom);
- if (! type || ! *type) - type = "linear"; - -#ifdef PIC - snprintf(open_name, sizeof(open_name), "_snd_pcm_rate_%s_open", type); - open_func = snd_dlobj_cache_lookup(open_name); - if (! open_func) { - void *h; - char lib_name[128], *lib = NULL; - if (strcmp(type, "linear")) { - snprintf(lib_name, sizeof(lib_name), - "%s/libasound_module_rate_%s.so", PKGLIBDIR, type); - lib = lib_name; - } - h = snd_dlopen(lib, RTLD_NOW); - if (! h) { - SNDERR("Cannot open library for type %s", type); - free(rate); - return -ENOENT; - } - open_func = dlsym(h, open_name); - if (! open_func) { - SNDERR("Cannot find function %s", open_name); - snd_dlclose(h); - free(rate); - return -ENOENT; - } - snd_dlobj_cache_add(open_name, h, open_func); - } -#else - open_func = SND_PCM_RATE_PLUGIN_ENTRY(linear); -#endif - err = snd_pcm_new(&pcm, SND_PCM_TYPE_RATE, name, slave->stream, slave->mode); if (err < 0) { free(rate); return err; } + +#ifdef PIC + err = -ENOENT; + if (!converter) + err = rate_open_func(rate, "linear"); + else if (!snd_config_get_string(converter, &type)) + err = rate_open_func(rate, type); + else if (snd_config_get_type(converter) == SND_CONFIG_TYPE_COMPOUND) { + snd_config_iterator_t i, next; + snd_config_for_each(i, next, converter) { + snd_config_t *n = snd_config_iterator_entry(i); + if (snd_config_get_string(n, &type) < 0) + break; + err = rate_open_func(rate, type); + if (!err) + break; + } + } else { + SNDERR("Invalid type for rate converter"); + snd_pcm_close(pcm); + return -EINVAL; + } + if (err < 0) { + SNDERR("Cannot find rate converter"); + snd_pcm_close(pcm); + return -ENOENT; + } +#else + open_func = SND_PCM_RATE_PLUGIN_ENTRY(linear); err = open_func(SND_PCM_RATE_PLUGIN_VERSION, &rate->obj, &rate->ops); if (err < 0) { snd_pcm_close(pcm); return err; } +#endif + if (! rate->ops.init || ! (rate->ops.convert || rate->ops.convert_s16) || ! rate->ops.input_frames || ! rate->ops.output_frames) { SNDERR("Inproper rate plugin %s initialization", type); @@ -1424,7 +1451,7 @@ int _snd_pcm_rate_open(snd_pcm_t **pcmp, snd_config_t *slave = NULL, *sconf; snd_pcm_format_t sformat = SND_PCM_FORMAT_UNKNOWN; int srate = -1; - const char *type = NULL; + const snd_config_t *converter = NULL;
snd_config_for_each(i, next, conf) { snd_config_t *n = snd_config_iterator_entry(i); @@ -1438,12 +1465,7 @@ int _snd_pcm_rate_open(snd_pcm_t **pcmp, continue; } if (strcmp(id, "converter") == 0) { - const char *str; - if ((err = snd_config_get_string(n, &str)) < 0) { - SNDERR("invalid converter string"); - return -EINVAL; - } - type = str; + converter = n; continue; } SNDERR("Unknown field %s", id); @@ -1470,7 +1492,7 @@ int _snd_pcm_rate_open(snd_pcm_t **pcmp, if (err < 0) return err; err = snd_pcm_rate_open(pcmp, name, sformat, (unsigned int) srate, - type, spcm, 1); + converter, spcm, 1); if (err < 0) snd_pcm_close(spcm); return err;
On 03/21/2007 06:24 PM, Takashi Iwai wrote:
Well, my (latest) prpoposal is:
- keep speex resampler in alsa-plugins tree - installing this
plugin automatically enables it as the default rate converter without touching any configuration
The second part is missing right now.
... and the patch below supports it.
With this patch, you can set the list of multiple default PCM rate converters. For example,
defaults.pcm.rate_converter [ "speexrate" "samplerate" "linear" ]
will look for speex plugin, then libsamplerate plugin, lastly it will take the built-in linear converter as a fallback. So, we can define this safely in the default alsa.conf file.
I guess this should work. This would be "import the speex version unchanged" only with s/alsa-lib/alsa-plugins/. It would depend on how many distributions would have no issue with making alsa-lib dependent on alsa-plugins.
Keeping it in alsa-plugins would as far as I've understood have two reasons:
a) if it were to be put into alsa-lib directly, you'd want (style) changes that would have to be kept in place over possible frequent updates, and
b) some fear and doubt with respect to licenses?
As to b), if alsa-lib is dependent on alsa-plugins in distributions, this might mean all the same license questions arise at least for them. Given that Jean-Marc himself is advocating just pulling it into alsa-lib directly, I assume something could be worked out that would quiet any worries instead?
Not really any coments on a). How many updates can be expected? I saw Jean-Marc say things about SSE optimizations, but optimizations aren't vital...
I did try the effect of upsampling 8000 to 44100 using the current ALSA resampler today and that made for a quite significantly different sounding file. If you spend your time on getting the sound quality of a low bandwidth codec such as speex better, I can see why you'd not be too thrilled with then having the final step in the chain undo much of your efforts again ;-\
Rene.
I guess this should work. This would be "import the speex version unchanged" only with s/alsa-lib/alsa-plugins/. It would depend on how many distributions would have no issue with making alsa-lib dependent on alsa-plugins.
Keeping it in alsa-plugins would as far as I've understood have two reasons:
a) if it were to be put into alsa-lib directly, you'd want (style) changes that would have to be kept in place over possible frequent updates, and
OK, first question here. Is any alsa developer even qualified *and* interested in maintaining that code? I feel like this thread is taking up more time that any maintenance that would be done within alsa and time lost to indentation and whatnot.
b) some fear and doubt with respect to licenses?
What's the license FUD about now?
As to b), if alsa-lib is dependent on alsa-plugins in distributions, this might mean all the same license questions arise at least for them. Given that Jean-Marc himself is advocating just pulling it into alsa-lib directly, I assume something could be worked out that would quiet any worries instead?
OK, so I never quite understood why you don't want BSD-licensed code in alsa, but I've already said I'm willing to allow relicensing to LGPL as long as it doesn't mean too much pollution in my source files. Note BTW that if you want to link with libspeex directly then you'll need to live with the BSD license because there are some parts of Speex that cannot be dual-licensed because I don't own the copyright.
Not really any coments on a). How many updates can be expected? I saw Jean-Marc say things about SSE optimizations, but optimizations aren't vital...
There's still quite a bit to do, even though ALSA probably doesn't care about it. SSE is one, but there's also finishing the fixed-point (which still requires float in the initialisations), and possibly improving the quality of the fixed-point code.
I did try the effect of upsampling 8000 to 44100 using the current ALSA resampler today and that made for a quite significantly different sounding file. If you spend your time on getting the sound quality of a low bandwidth codec such as speex better, I can see why you'd not be too thrilled with then having the final step in the chain undo much of your efforts again ;-\
Exactly. Plus, consider that 8 kHz is used widely in VoIP and that new machines now ship mostly with HDA-based codecs that only do 44.1 kHz and 48 kHz. That's why I'm really worried. I consider the current behaviour to be a bug, because the driver is essentially corrupting the audio signal that it receives.
Jean-Marc
At Fri, 23 Mar 2007 20:36:52 +1100, Jean-Marc Valin wrote:
I guess this should work. This would be "import the speex version unchanged" only with s/alsa-lib/alsa-plugins/. It would depend on how many distributions would have no issue with making alsa-lib dependent on alsa-plugins.
Keeping it in alsa-plugins would as far as I've understood have two reasons:
a) if it were to be put into alsa-lib directly, you'd want (style) changes that would have to be kept in place over possible frequent updates, and
OK, first question here. Is any alsa developer even qualified *and* interested in maintaining that code? I feel like this thread is taking up more time that any maintenance that would be done within alsa and time lost to indentation and whatnot.
Of course, once after it's merged to the alsa-lib tree, this becomes our responsibility. This is why coping code makes the maintenance harder. It means the maintenance action is done in paralllel, and conflicts may happen very likely from both sides.
b) some fear and doubt with respect to licenses?
What's the license FUD about now?
As to b), if alsa-lib is dependent on alsa-plugins in distributions, this might mean all the same license questions arise at least for them. Given that Jean-Marc himself is advocating just pulling it into alsa-lib directly, I assume something could be worked out that would quiet any worries instead?
OK, so I never quite understood why you don't want BSD-licensed code in alsa, but I've already said I'm willing to allow relicensing to LGPL as long as it doesn't mean too much pollution in my source files.
That's good. Keeping the single license in the whole tree is very important.
BTW, what one calls "pollution" can be "sanitization" for others. That's the opensource development...
Note BTW that if you want to link with libspeex directly then you'll need to live with the BSD license because there are some parts of Speex that cannot be dual-licensed because I don't own the copyright.
If we reuse the libspeex shlib, we won't do it from alsa-lib but from the existing plugin in alsa-plugins package. So, alsa-lib will stay unchanged.
Not really any coments on a). How many updates can be expected? I saw Jean-Marc say things about SSE optimizations, but optimizations aren't vital...
There's still quite a bit to do, even though ALSA probably doesn't care about it. SSE is one, but there's also finishing the fixed-point (which still requires float in the initialisations), and possibly improving the quality of the fixed-point code.
Actually, any optimization would be interesting for us, too.
Takashi
OK, so I never quite understood why you don't want BSD-licensed code in alsa, but I've already said I'm willing to allow relicensing to LGPL as long as it doesn't mean too much pollution in my source files.
That's good. Keeping the single license in the whole tree is very important.
BTW, what one calls "pollution" can be "sanitization" for others. That's the opensource development...
Here's the deal. You give me a paragraph that adds the LGPL license in no more then 5 lines and I'll add it to the resampler header now.
Note BTW that if you want to link with libspeex directly then you'll need to live with the BSD license because there are some parts of Speex that cannot be dual-licensed because I don't own the copyright.
If we reuse the libspeex shlib, we won't do it from alsa-lib but from the existing plugin in alsa-plugins package. So, alsa-lib will stay unchanged.
No more plans to have the resampler in alsa-lib?
Actually, any optimization would be interesting for us, too.
Good. Actually, if you know anyone good at SSE code, I'm interested :-)
Jean-Marc
At Sat, 24 Mar 2007 08:43:58 +1100, Jean-Marc Valin wrote:
OK, so I never quite understood why you don't want BSD-licensed code in alsa, but I've already said I'm willing to allow relicensing to LGPL as long as it doesn't mean too much pollution in my source files.
That's good. Keeping the single license in the whole tree is very important.
BTW, what one calls "pollution" can be "sanitization" for others. That's the opensource development...
Here's the deal. You give me a paragraph that adds the LGPL license in no more then 5 lines and I'll add it to the resampler header now.
Hmm, from where comes this "5 lines" rule? Usually you need to put whole sentenses to the code you release under GPL such as below:
This library is free software; you can redistribute it and/or modify it under the terms of the GNU Lesser General Public License as published by the Free Software Foundation; either version 2.1 of the License, or (at your option) any later version.
This program is distributed in the hope that it will be useful, but WITHOUT ANY WARRANTY; without even the implied warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Lesser General Public License for more details.
You should have received a copy of the GNU Lesser General Public License along with this library; if not, write to the Free Software Foundation, Inc., 59 Temple Place, Suite 330, Boston, MA 02111-1307 USA
You can put into a single line if you don't fold lines, though :)
Note BTW that if you want to link with libspeex directly then you'll need to live with the BSD license because there are some parts of Speex that cannot be dual-licensed because I don't own the copyright.
If we reuse the libspeex shlib, we won't do it from alsa-lib but from the existing plugin in alsa-plugins package. So, alsa-lib will stay unchanged.
No more plans to have the resampler in alsa-lib?
No merge unless we agree with all issues, of course. Otherwise someone might blame later.
Actually, any optimization would be interesting for us, too.
Good. Actually, if you know anyone good at SSE code, I'm interested :-)
Actually, I'm not sure whether SSE would be much helpful for fixed point integer operations. But, perhaps you may know better :)
thanks,
Takashi
Hmm, from where comes this "5 lines" rule? Usually you need to put whole sentenses to the code you release under GPL such as below:
_From me -- I don't particularly like having huge headers at the beginning of every file.
No more plans to have the resampler in alsa-lib?
No merge unless we agree with all issues, of course. Otherwise someone might blame later.
So what are the issues left?
Actually, any optimization would be interesting for us, too.
Good. Actually, if you know anyone good at SSE code, I'm interested :-)
Actually, I'm not sure whether SSE would be much helpful for fixed point integer operations. But, perhaps you may know better :)
I've been considering SSE for the float version, not the fixed point. On any new PC, floating-point is now much faster than fixed-point anyway.
Jean-Marc
At Mon, 26 Mar 2007 19:53:34 +1000, Jean-Marc Valin wrote:
Hmm, from where comes this "5 lines" rule? Usually you need to put whole sentenses to the code you release under GPL such as below:
From me -- I don't particularly like having huge headers at the beginning of every file.
Me too personally, but I'm afraid it's almost mandatory to have it all. If you find a shorter version with legally validity, please let us know.
No more plans to have the resampler in alsa-lib?
No merge unless we agree with all issues, of course. Otherwise someone might blame later.
So what are the issues left?
See my previous post.
Actually, any optimization would be interesting for us, too.
Good. Actually, if you know anyone good at SSE code, I'm interested :-)
Actually, I'm not sure whether SSE would be much helpful for fixed point integer operations. But, perhaps you may know better :)
I've been considering SSE for the float version, not the fixed point. On any new PC, floating-point is now much faster than fixed-point anyway.
Agreed. I just thought that fixed-point is used as default in your resampler code but it seems wrong.
Takashi
On 03/23/2007 10:36 AM, Jean-Marc Valin wrote:
I feel like this thread is taking up more time that any maintenance that would be done within alsa and time lost to indentation and whatnot.
Probably, and so as to not further expand on that, I'm butting out again. Takashi is perfectly capable of speaking for himself.
Only wanted to point out that any license worries would likely cross the lib/plugins border anyway once alsa-plugins becomes an alsa-lib package dependency in distributions but I see that worry's gone anyway from the followups. Yes, I hope alsa-lib just incorporates your resampler (or _a_ better resampler at least).
Rene.
On 03/21/2007 05:38 PM, Rene Herman wrote:
On 03/21/2007 04:34 PM, Jean-Marc Valin wrote:
It's worse than you think :-) Try playing an 8 kHz file to a soundcard that only does 44.1/48. It's just horrible.
Trouble is that I don't have a soundcard that can only do 44.1/48. I'll go hack up a driver to pretend I do though and try. Have a nice 8 kHz file I can try with? :)
I grabbed http://www.speex.org/samples/audio/male.wav and tried using a Crystal CS4236 based card (hw:1). With an ~/.asoundrc containing:
pcm.up1 { type plug slave { pcm "hw:1,0" rate 44100 } }
the difference between
aplay -D hw:1 male.wav
and
aplay -D up1 male.wav,
Is indeed very significant.
When I tried with hw:0, an "ESS Canyon3D" (ES1970MS-3D) based card, I was in for a bit of a suprise though. Have never been aware of it, but it seems this card upsamples internally anyway. Both hw:0 and up0 (same as above) sounded almost if not completely the same. I never really play anyting other than 44100 or 48000 to the card but it's not nice. It's a (once) expensive "TerraTec DMX"...
Resampling upfront with:
sox male.wav -r 48000 male_sox.wav resample -q
sounds somewhat like male.wav on hw:1 again (for some reason, it won't do 44100).
Anyways, yes, a very significant difference...
Rene.
participants (3)
-
Jean-Marc Valin -
Rene Herman -
Takashi Iwai