Hide Forgot
Description of problem: The summary says it all. I've been looking into fixing this, and that won't be easy, the current alsa code in portaudio onl works with alsa devices which support mmap, and adding support for new output devices to portaudio seems for from easy. I think the best solution is to add non mmap (regular read/write) support to portaudio's alsa support, if you look for MMAP and mmap you will find all the mmap specific code. I was thinking about writing a fix for this myself, but I currently simply do not have the time for this.
Ah, Matthias, I see this is yours, I know how busy you are, so I'll reassign this to me. Shall we co-maintain this?
(In reply to comment #1) > Matthias, I see this is yours, I know how busy you are, so I'll reassign this > to me. Shall we co-maintain this? Perfectly fine by me, go for it! Oh, and... thanks :-)
Changing version to '9' as part of upcoming Fedora 9 GA. More information and reason for this action is here: http://fedoraproject.org/wiki/BugZappers/HouseKeeping
Any progress on this? Im a software dev, trying to use wiresharks rtp player to listen to a captured rtp audio stream. Ive discovered that I cant because it uses portaudio. I found this bug report, and considered ditching pulseaudio from my system, but it seems too deeply integrated to risk trying.
Ok, I found out all i needed was a "killall pulseaudio" to use wiresharks rtpplayer. Still would be nice if the extra compatibility was there, though.
(In reply to comment #4) > Any progress on this? Im a software dev, trying to use wiresharks rtp player to > listen to a captured rtp audio stream. Ive discovered that I cant because it > uses portaudio. I found this bug report, and considered ditching pulseaudio > from my system, but it seems too deeply integrated to risk trying. I'm afraid not portaudio is a mighty complex beast (esp for what it does). I've looked into how to handle this, but haven't gotten any further then that (-ENOTIME).
FYI, I'm currently working on this (I'm trying to fix PortAudio to work with _any_ non-mmap ALSA device, including PulseAudio, with PulseAudio being the one I do my testing with).
Here's a patch, can you please try this? http://techweb.rfa.org/pipermail/portaudio/2008-December/009023.html Tested with espeak (by me), Audacity (by David Henningsson) and the basic PortAudio test programs (by me). It will also have to be applied to the audacity and audacity-freeworld packages because they ship private patched copies of PortAudio.
See also http://techweb.rfa.org/pipermail/portaudio/2008-November/008990.html for some context (but that's an OLD version of the patch).
(In reply to comment #8) > Here's a patch, can you please try this? > http://techweb.rfa.org/pipermail/portaudio/2008-December/009023.html Awesome! I'm very happy you found the time to this, thanks! > Tested with espeak (by me), Audacity (by David Henningsson) and the basic > PortAudio test programs (by me). I've gotten involved of this because of a game using portaudio (wordwarvi) and I've just done a build of portaudio with this patch and testes it. wordwarvi crashes with the patched portaudio :( I'll make some time to look into this asap.
Kevin, Good news after too much debugging I've found the issue with wordwarvi and it is wordwarvi's fault, with the bug in wordwarvi fixed things work fine with your patch, However during debugging I noticed that snd_pcm_writei was being called with 8193 samples instead of 8192. This is also the reason why you need the + 1 to the buffer-size for the non-mmap buffers. This is caused by this part of the patch: @@ -1831,7 +1837,7 @@ PA_UNLESS( framesPerHostBuffer != 0, paInternalError ); self->maxFramesPerHostBuffer = framesPerHostBuffer; - if( !accurate ) + if( !self->playback.canMmap || !accurate ) { /* Don't know the exact size per host buffer */ *hostBufferSizeMode = paUtilBoundedHostBufferSize; And can (I believe) be fixed by simply removing this hunk. The accurate thing is to handle cards where periodsize is a not integer, for example 500.5 samples / period. I really see no reason to always tread things as if we have a non integer period size in the read / write case. There really is no difference with regards to periodsize handling between mmap / rw mode. Writing 8193 samples instead of 8192 (in my test case) really is quite in-efficient and I see no reason for this. Even if we drop this chunk we still need the + 1 in the allocs / buffersize calculations for the nonMMAP buffer for the case where accurate is not true. --- I've prepared a new portaudio release with your patch (minus the hunk) applied, and if you're ok with the removal of the hunk I'll build it for rawhide and F-10 and push it to F-10 updates-testing.
We cannot drop this hunk entirely! This hunk is the fix for the stuttering I had with the first version of the patch. The if condition contains 2 lines (plus comments): /* Don't know the exact size per host buffer */ *hostBufferSizeMode = paUtilBoundedHostBufferSize; /* Raise upper bound */ ++self->maxFramesPerHostBuffer; The first one is what we need - we must use paUtilBoundedHostBufferSize instead of paUtilFixedHostBufferSize, because we must be prepared to request different sizes from the callback, we cannot wait until we have the fixed size because that's too late. The second line (and the associated +1s in the allocs) is the problem. I'm preparing a fixed patch.
Created attachment 326162 [details] Fixed patch Please try this one. The only change from my previous patch is this: /* Raise upper bound */ - ++self->maxFramesPerHostBuffer; + if( !accurate ) + ++self->maxFramesPerHostBuffer; Compared to the patch you were planning to use, it adds this hunk: @@ -1831,12 +1837,13 @@ PA_UNLESS( framesPerHostBuffer != 0, paInternalError ); self->maxFramesPerHostBuffer = framesPerHostBuffer; - if( !accurate ) + if( !self->playback.canMmap || !accurate ) { /* Don't know the exact size per host buffer */ *hostBufferSizeMode = paUtilBoundedHostBufferSize; /* Raise upper bound */ - ++self->maxFramesPerHostBuffer; + if( !accurate ) + ++self->maxFramesPerHostBuffer; } error: This should fix the bogus increase from 4092 to 4093 and still use the correct mode (paUtilBoundedHostBufferSize) for rw mode. Using the fixed host buffer size sounds horrible in all my tests, it just doesn't work. The reason is that a fixed host buffer size means requests from callbacks look like this: /* We've committed to a fixed host buffer size, stick to that */ framesGot = framesGot >= stream->maxFramesPerHostBuffer ? stream->maxFramesPerHostBuffer : 0; which means it will wait until it has all of stream->maxFramesPerHostBuffer available before calling the callback, and that's too late, it will invariably lead to an xrun.
(In reply to comment #13) > Created an attachment (id=326162) [details] > Fixed patch > Looks good, I already thought we might end up with having to do things this way, but in my test case it worked fine without the hunk. I've done a new build of portaudio with your latest patch in for rawhide: http://koji.fedoraproject.org/koji/buildinfo?buildID=73915 I would like to also fix this in F-10, so I plan on also doing a build for F-10 and push it to updates-testing, is that ok with you or do you think that is a bad idea?
I'm fine with it. I'd push it all the way down to F8, but that's just me. :-)
portaudio-19-6.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/portaudio-19-6.fc10
portaudio-19-6.fc10 has been pushed to the Fedora 10 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update portaudio'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2008-11164
portaudio-19-6.fc10, wordwarvi-0.23-2.fc10 has been pushed to the Fedora 10 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing update portaudio wordwarvi'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F10/FEDORA-2008-11164
portaudio-19-6.fc10, wordwarvi-0.23-2.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.
audacity-1.3.5-0.9.beta.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/audacity-1.3.5-0.9.beta.fc10
IMHO this should also get fixed in Fedora 9 (which is what this bug was filed against in the first place).
(In reply to comment #21) > IMHO this should also get fixed in Fedora 9 (which is what this bug was filed > against in the first place). As you wish an update is on its way to f9-updates-testing.
portaudio-19-6.fc9 has been pushed to the Fedora 9 testing repository. If problems still persist, please make note of it in this bug report. If you want to test the update, you can install it with su -c 'yum --enablerepo=updates-testing-newkey update portaudio'. You can provide feedback for this update here: http://admin.fedoraproject.org/updates/F9/FEDORA-2009-0967
portaudio-19-6.fc9 has been pushed to the Fedora 9 stable repository. If problems still persist, please make note of it in this bug report.
Looks like the wordwarvi in F9 also suffers from the same bug as the one in F10 (despite being a much older version). :-( So an update for that is also needed.
(In reply to comment #25) > Looks like the wordwarvi in F9 also suffers from the same bug as the one in F10 > (despite being a much older version). :-( So an update for that is also needed. Aw, good point, I'm pushing a fixed version straight to updates (stable).
audacity-1.3.7-0.3.beta.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/audacity-1.3.7-0.3.beta.fc10
For Audacity, this part of my patch should be removed now: diff -Nur audacity-src-1.3.7-orig/lib-src/portaudio-v19/src/os/unix/pa_unix_hostapis.c audacity-src-1.3.7/lib-src/portaudio-v19/src/os/unix/pa_unix_hostapis.c --- audacity-src-1.3.7-orig/lib-src/portaudio-v19/src/os/unix/pa_unix_hostapis.c 2009-01-27 21:51:40.000000000 +0100 +++ audacity-src-1.3.7/lib-src/portaudio-v19/src/os/unix/pa_unix_hostapis.c 2009-02-02 19:08:22.000000000 +0100 @@ -103,4 +103,8 @@ 0 /* NULL terminated array */ }; +#if defined(PA_USE_OSS) && defined(PA_USE_ALSA) +int paDefaultHostApiIndex = 1; +#else int paDefaultHostApiIndex = 0; +#endif because ALSA now has index 0 on __linux__: /** Note that ALSA is placed before OSS so that the latter is preferred over the * latter on Linux. */ PaUtilHostApiInitializer *paHostApiInitializers[] = { #ifdef __linux__ #ifdef PA_USE_ALSA PaAlsa_Initialize, #endif #ifdef PA_USE_OSS PaOSS_Initialize, #endif #else #ifdef PA_USE_OSS PaOSS_Initialize, #endif #ifdef PA_USE_ALSA PaAlsa_Initialize, #endif #endif /* __linux__ */ I'm not convinced it was a good idea to change the indexes of the APIs (due to compatibility), but it's how upstream does it. So please remove that portion of my patch, otherwise OSS is the default (which is exactly what my patch was fixing)! This will presumably also affect portaudio itself if updated to a newer snapshot.
Actually, I took the liberty to just fix this myself. (Devel and F-10 builds now running.) Note that this change only affects Audacity >= 1.3.7 and portaudio >= around December 2008, the current portaudio package should either keep the patch as is or change the hunk to look like the upstream solution instead.
Hard-coding an array index like that is fragile. The default index could have been determined at run-time.
Well, I suggest you take that up with PortAudio upstream. My paDefaultHostApiIndex change is no longer necessary because ALSA is the default in current snapshots (i.e. what Audacity >= 1.3.7 is using), so I think it's best we don't touch upstream's default device code, there's no immediate need. If you think it can or should be improved, upstream is probably the best place. Unfortunately, upstream doesn't look very responsive to me, given that I got no feedback at all from upstream about the non-mmap ALSA patch. :-( (Several people from other distributions showed interest and some other distros now ship my patch as well, but upstream didn't even bother rejecting it.) That's really annoying, I'd really like that fix to go upstream!
audacity-1.3.7-0.7.beta.fc10 has been submitted as an update for Fedora 10. http://admin.fedoraproject.org/updates/audacity-1.3.7-0.7.beta.fc10
audacity-1.3.7-0.7.beta.fc10 has been pushed to the Fedora 10 stable repository. If problems still persist, please make note of it in this bug report.