[alsa-devel] Pull request for alsa-plugins
Takashi, Jaroslav,
please pull and merge a series of 26 patches for alsa-plugins' PulseAudio driver which I prepared in this repository of mine:
git://git.0pointer.de/alsa-plugins.git
gitweb is here:
http://git.0pointer.de/?p=alsa-plugins.git
There are quite a few bug fixes and feature enhancements among those patches. It also includes some clean-ups in the coding style. i.e. previously the indenting of the PA driver was pure chaos, every single patch seemed to introduce a new indentation width. The first patch in this series simply reindents the driver to Linux style. All other patches are on top of that one.
Here's the shortlog:
Lennart Poettering (26): Reindent to Linux kernel style Add Emacs-style /*-*- linux-c -*-*/ header comment Make pulse_new() a proper C function Don't modify the SIGPIPE handler Call pa_context_disconnect() explicitly use SNDERR instead of fprintf to print error messages Support S32 sample types Add trailing NUL character to snprintf output Get rid of pulse_poll_revents() Add more error checking Remove fix for bug 0003470 Rework hardware parameter selection A bag of minor clean ups for ctl_pulse.c Make pulse_ext_callback const Drop our own implementation of the poll() callbacks A bag of minor clean-ups for pulse.c Split out O_NONBLOCK setting into seperate function Save a byte of memory Adjust buffering metrics to match what PA internally uses Make sure we always have a sensible channel mapping Use PA_STREAM_EARLY_REQUESTS if available Use S32/FLOAT32 only where available in the PA libs Add const to our snd_pcm_ioplug_callback_t instances Don't implement our own poll handlers, we can use the default ones Remove our own poll handler implementation entirely A bag of clean-ups for pcm_ctl.c
Those patches only touch the pulse/ subdir.
The tree is freshly rebased against current alsa-plugins master.
May I ask you to make me the "semi-official" maintainer of this driver? I.e. I'd like to be consulted (as in 'Signed-Off-by') before any patches for it are merged?
If requested I can also mail this as series of 26 seperate patches.
Thank you,
Lennart
At Wed, 3 Sep 2008 21:17:41 +0200, Lennart Poettering wrote:
Takashi, Jaroslav,
please pull and merge a series of 26 patches for alsa-plugins' PulseAudio driver which I prepared in this repository of mine:
git://git.0pointer.de/alsa-plugins.git
Thanks, pulled in, pushed out now.
At the next time, could you add our sign-off to each commit? I don't care much about it for non-kernel codes, but certainly better with it.
Also, please add the branch name in the same line of git://... above. The recent git requires the branch name explicitly to pull.
gitweb is here:
http://git.0pointer.de/?p=alsa-plugins.gitThere are quite a few bug fixes and feature enhancements among those patches. It also includes some clean-ups in the coding style. i.e. previously the indenting of the PA driver was pure chaos, every single patch seemed to introduce a new indentation width. The first patch in this series simply reindents the driver to Linux style. All other patches are on top of that one.
Oh yes, that was ugly.
Here's the shortlog:
Lennart Poettering (26): Reindent to Linux kernel style Add Emacs-style /*-*- linux-c -*-*/ header comment Make pulse_new() a proper C function Don't modify the SIGPIPE handler Call pa_context_disconnect() explicitly use SNDERR instead of fprintf to print error messages Support S32 sample types Add trailing NUL character to snprintf output Get rid of pulse_poll_revents() Add more error checking Remove fix for bug 0003470 Rework hardware parameter selection A bag of minor clean ups for ctl_pulse.c Make pulse_ext_callback const Drop our own implementation of the poll() callbacks A bag of minor clean-ups for pulse.c Split out O_NONBLOCK setting into seperate function Save a byte of memory Adjust buffering metrics to match what PA internally uses Make sure we always have a sensible channel mapping Use PA_STREAM_EARLY_REQUESTS if available Use S32/FLOAT32 only where available in the PA libs Add const to our snd_pcm_ioplug_callback_t instances Don't implement our own poll handlers, we can use the default ones Remove our own poll handler implementation entirely A bag of clean-ups for pcm_ctl.c
Thanks for your work.
One of my concerns in the current pulse code is the many use of assert(). In many cases, assert() is a wrong choice. For example, checking the return value of malloc in assert() is definitely wrong. It should be always checked, and should return an error instead of aborting the program.
Yes, alsa-lib has also many assert() although I removed already many. Some of them may be still wrong ones. Hopefully this can be sorted out later.
Those patches only touch the pulse/ subdir.
The tree is freshly rebased against current alsa-plugins master.
May I ask you to make me the "semi-official" maintainer of this driver? I.e. I'd like to be consulted (as in 'Signed-Off-by') before any patches for it are merged?
Well, the patches can go in by any ALSA developers, so it can't be forced *always* through you. But, I'll mail you pulse-related patches before reviewing and merging.
Takashi
Takashi Iwai wrote:
One of my concerns in the current pulse code is the many use of assert(). In many cases, assert() is a wrong choice. For example, checking the return value of malloc in assert() is definitely wrong. It should be always checked, and should return an error instead of aborting the program.
Yes, alsa-lib has also many assert() although I removed already many. Some of them may be still wrong ones. Hopefully this can be sorted out later.
There has been a rather large thread on this on PA mailing list over the last couple days :)
http://thread.gmane.org/gmane.comp.audio.pulseaudio.general/2014
I personally got some stuff wrong, but learned from Lennart's responses.
Certainly I think a lot of the cases where this plugin could have previously hit an assert have now been removed. Not sure about the malloc asserts() tho'...
Col
On Thu, 04.09.08 09:48, Takashi Iwai (tiwai@suse.de) wrote:
please pull and merge a series of 26 patches for alsa-plugins' PulseAudio driver which I prepared in this repository of mine:
git://git.0pointer.de/alsa-plugins.gitThanks, pulled in, pushed out now.
At the next time, could you add our sign-off to each commit? I don't care much about it for non-kernel codes, but certainly better with it.
Sure, will do.
Also, please add the branch name in the same line of git://... above. The recent git requires the branch name explicitly to pull.
Will do.
One of my concerns in the current pulse code is the many use of assert(). In many cases, assert() is a wrong choice. For example, checking the return value of malloc in assert() is definitely wrong. It should be always checked, and should return an error instead of aborting the program.
I think I fixed every single assert() on malloc, OOM should now be handled in similar way to most other code in in alsa-lib/-plugins.
Still, there are quite a few asserts left in the pulse driver which check validity of function parameters. But they should only be hit on internal programming errors and as such I think it makes a lot of sense to leave them in.
Those patches only touch the pulse/ subdir.
The tree is freshly rebased against current alsa-plugins master.
May I ask you to make me the "semi-official" maintainer of this driver? I.e. I'd like to be consulted (as in 'Signed-Off-by') before any patches for it are merged?
Well, the patches can go in by any ALSA developers, so it can't be forced *always* through you. But, I'll mail you pulse-related patches before reviewing and merging.
Thanks a lot!
Lennart
participants (3)
-
Colin Guthrie -
Lennart Poettering -
Takashi Iwai