After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 129709 - esound segfaults while use old version of alsa pcm api
esound segfaults while use old version of alsa pcm api
Status: RESOLVED FIXED
Product: esound
Classification: Deprecated
Component: general
0.2.31
Other Linux
: High critical
: ---
Assigned To: Esound Maintainers
Esound Maintainers
: 130012 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2003-12-20 02:49 UTC by £ukasz Mach
Modified: 2004-12-22 21:47 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
new alsa api patch (989 bytes, patch)
2003-12-20 02:50 UTC, £ukasz Mach
none Details | Review
Patch for acconfig.h. Need to run autoheader to propagate change to config.h. (343 bytes, patch)
2004-01-10 07:15 UTC, Eddy Mulyono
none Details | Review
Patch for configure.in. Need to run autoconf to propagate changes to configure script. (780 bytes, patch)
2004-01-10 07:16 UTC, Eddy Mulyono
none Details | Review
Modified patch for audio_alsa09.c (1.17 KB, patch)
2004-01-10 07:17 UTC, Eddy Mulyono
none Details | Review
revised patch for configure.in. Patch against original configure.in (esound 0.2.32). (851 bytes, patch)
2004-01-10 07:59 UTC, Eddy Mulyono
none Details | Review

Description £ukasz Mach 2003-12-20 02:49:37 UTC
I'm using esound 0.2.32 (it isn't listed on version choose list in
bugzilla, so I've choosen most recent), there is new alsa pcm api up from 
1.0.0.pre1. 

esound compiles properly (with 2 warnings about conversion from int to int
*) , but (as expected) segfaults while using new functions (which needs int
* parameters) with old params (int). 

here is patch for it:
--- ../esound-0.2.32.orig/audio_alsa09.c        Thu Mar 20 09:34:19 2003
+++ ./audio_alsa09.c    Sat Dec 20 03:01:11 2003
@@ -136,15 +136,17 @@
                alsaerr = -1;
                return handle;
        }
-
-       err = snd_pcm_hw_params_set_rate_near(handle, hwparams, speed, 0);
+
+       int t_dir=0;
+       int t_speed=speed;
+       err = snd_pcm_hw_params_set_rate_near(handle, hwparams, &t_speed,
&t_dir);
        if (err < 0) {
                if (alsadbg)
                        fprintf(stderr, "%s\n", snd_strerror(err));
                alsaerr = -1;
                return handle;
        }
-       if (err != speed) {
+       if (t_speed != speed) {
                if (alsadbg)
                        fprintf(stderr, "Rate not avaliable %i != %i\n",
speed, err);
                alsaerr = -1;
@@ -176,8 +178,9 @@
                alsaerr = -1;
                return handle;
        }
-
-       err = snd_pcm_hw_params_set_buffer_size_near(handle, hwparams,
BUFFERSIZE);
+
+       snd_pcm_uframes_t t_bufsize=BUFFERSIZE;
+       err = snd_pcm_hw_params_set_buffer_size_near(handle, hwparams,
&t_bufsize);
        if (err < 0) {
                if (alsadbg)
                        fprintf(stderr, "Buffersize:%s\n", snd_strerror(err));

(BTW, there could be some kind of 'add file' option in gnome bugzilla)
Comment 1 £ukasz Mach 2003-12-20 02:50:21 UTC
Created attachment 22585 [details] [review]
new alsa api patch
Comment 2 Luis Villa 2003-12-29 23:45:54 UTC
Adding the PATCH keyword. Fredric? :) 
Comment 3 Frederic Crozat 2003-12-30 16:18:07 UTC
*** Bug 130012 has been marked as a duplicate of this bug. ***
Comment 4 Frederic Crozat 2003-12-30 16:22:14 UTC
Thanks for the patch, but we need to add some test in configure.in to
allow use of esound with older version of alsa. 
Comment 5 £ukasz Mach 2003-12-30 20:00:10 UTC
I'm not familiar with autoconf/automake, so I cannot apply patch to
it, but I think that patch is clear and one should know what to change. 
Comment 6 Eddy Mulyono 2004-01-03 07:17:53 UTC
At what version does the ALSA API change?

(trying to make autoconf-automake patch)

-Eddy
Comment 7 £ukasz Mach 2004-01-03 10:24:24 UTC
I'm not sure, but based on the cvs page
( http://cvs.sourceforge.net/viewcvs.py/alsa/alsa-lib/src/pcm/pcm.c )
I think that api change was at 0.9.6->1.0.0pre1

maHo
Comment 8 Eddy Mulyono 2004-01-10 05:25:24 UTC
After digging in ALSA's CVS, the functions started using pointers from
pcm.c revision 1.220 (0.9.0_rc4?).

I think it makes sense to patch audio_alsa09.c altogether, since these
new functions started to appear in 0.9.0_rc4, which is very early in
0.9.x.

So... I guess we can do it in 2 ways:
1. Change audio_alsa09.c altogether.
2. Use an autoconf macro to test these functions.

I'll try to do option 2, since £ukasz had done option 1... :p

-Eddy
Comment 9 Eddy Mulyono 2004-01-10 07:15:08 UTC
Created attachment 23190 [details] [review]
Patch for acconfig.h. Need to run autoheader to propagate change to config.h.
Comment 10 Eddy Mulyono 2004-01-10 07:16:03 UTC
Created attachment 23191 [details] [review]
Patch for configure.in. Need to run autoconf to propagate changes to configure script.
Comment 11 Eddy Mulyono 2004-01-10 07:17:50 UTC
Created attachment 23192 [details] [review]
Modified patch for audio_alsa09.c
Comment 12 Eddy Mulyono 2004-01-10 07:28:21 UTC
The previous 3 patches enables the configure script to correctly
detect system's settings. I have the configure script to try to link
one of the (offending) function. When tested in my
kernel-2.6.0-gentoo-r1 system, config.log shows:

configure:7818: gcc -o conftest -g -O2   conftest.c  1>&5
configure: In function `main':
configure:7813: warning: passing arg 3 of
`snd_pcm_hw_params_set_rate_near' makes pointer from integer without a
cast
configure:7813: warning: passing arg 4 of
`snd_pcm_hw_params_set_rate_near' makes pointer from integer without a
cast
/tmp/ccu3LO0K.o(.text+0x19): In function `main':
/home/eddy/lab/esdPelennor/configure:7813: undefined reference to
`snd_pcm_hw_params_set_rate_near'
collect2: ld returned 1 exit status

This causes DRIVER_ALSA_09_AFTER_RC4 to be #defined. When the source
audio_alsa09.c is preprocessed, then the correct parameters are passed.

(I don't know how to coalesce all 3 files into a patch file... sorry.
This is my first Bugzilla "effort", too. So I'll be happy if someone
can give me pointers to "the right way of doing it").   :p

-Eddy
Comment 13 Eddy Mulyono 2004-01-10 07:59:07 UTC
Created attachment 23193 [details] [review]
revised patch for configure.in. Patch against original configure.in (esound 0.2.32).
Comment 14 Eddy Mulyono 2004-01-10 08:00:35 UTC
Description about attachment 29193 [details] [review]:

Oops. Just realized some mistake. First, I decided to put the
AC_TRY_LINK after "-lasound" is already in ${LIBS}. Secondly, I
decided to append "-Werror" to CFLAGS, to treat warnings as errors
(because it causes segfaults in esound). If I didn't, apparently the
macro AC_TRY_LINK still perceive the warnings as a succesful compile,
and ended up not #define-ing DRIVER_ALSA_09_AFTER_RC4.

-Eddy
Comment 15 Frederic Crozat 2004-01-15 12:49:31 UTC
I've committed all your patches, somehow fixed :)