Skip to content
This repository has been archived by the owner on Jun 8, 2018. It is now read-only.

Add Hostapi Pulseaudio to Portaudio #1

Open
wants to merge 47 commits into
base: master
Choose a base branch
from
Open

Conversation

illuusio
Copy link
Owner

This Pull Request adds Pulseaudio Hostapi to Portaudio

@daschuer
Copy link

I have failed to build this. What is the receipt?

I have tried to call
autoreconf -if

but its has an error and a lot of warnings:

configure.ac:38: error: required file '../../ltmain.sh' not found

I am on Ubuntu Trusty

@illuusio
Copy link
Owner Author

I use

aclocal
libtoolize -f
autoconf -f
automake -f

but you can also use (if above doesn't work)

aclocal -f
libtoolize -f -c
autoconf -f
automake -f -a

I should check the CMake version because it's should be less pain

@daschuer
Copy link

I have still no luck:

automake -f
-aautomake: warning: autoconf input should be named 'configure.ac', not 'configure.in'
configure.in: error: no proper invocation of AM_INIT_AUTOMAKE was found.
configure.in: You should verify that configure.in invokes AM_INIT_AUTOMAKE,
configure.in: that aclocal.m4 is present in the top-level directory,
configure.in: and that aclocal.m4 was recently regenerated (using aclocal)
automake: error: no 'Makefile.am' found for any configure output

@illuusio
Copy link
Owner Author

Oh there should be './configure' now? Just run it and it compiles. It's very frustrating that autotools configuration is messing up.

@daschuer
Copy link

Ah thanks, I did not expect that this worked after previous error.
Now it woks ... Great :-)

@illuusio
Copy link
Owner Author

Marvellous @daschuer. I'll update 'Makefile.am' and 'README.md'. After 15 years of maintaining autotools in several projects I've have love and hate relationship with it. Glad you made it work!

@daschuer
Copy link

What will be the best way to handle installations without Pulse?
The ditros may provide two versions of Portaudio one with Pulse and one without.
But this requires extra maintenance. How about dynamically load the libpulse?
If it fails, continue without pulse api.

/* PaPulseaudioHostApiRepresentation - host api datastructure specific to this implementation */

#define PULSEAUDIO_TIME_EVENT_USEC 50000
#define PULSEAUDIO_BUFFER_SIZE (88100 * 4 * 2)

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments why you pick these values above would be helpful

illuusio and others added 3 commits January 15, 2016 10:43
…g writing but provides 'pa_stream_writable_size'

for quering how much can we write. So with turning OFF PulseAudio write-callback and just looping until we have
written everything emulates block writing very well.
@illuusio
Copy link
Owner Author

Now there is some callback clicks to investigate (which are caused same that caused the blocking playback clicks) and stopping and starting input stream but otherwise this start to be in very good shape.. Hopefully I didn't mugged up Terminate with last fix 👊

@sqweek
Copy link

sqweek commented Jan 18, 2016

Hm. My program did have a click in callback mode, which I managed to fix by changing my callback to zero the entire buffer -- before that if I didn't have a full buffer's worth of samples I only filled in part of the buffer. Don't know if that helps.

There's a few more problems I'd like to investigate, how would you prefer to manage them? I can make a PR for each, or open an issue, or just make a list here?

@illuusio
Copy link
Owner Author

PR is most convenient I think. List what is wrong could be also good so I can fix some of them also.

@sqweek
Copy link

sqweek commented Jan 19, 2016

Actually the only definite problem I'm currenty aware of is that the left/right channel balance is off when the program opens a stream in mono mode (eg. patest_mono). It should be centered but it is heavily skewed towards one of the channels.

(the other things on my list I need to investigate to see if there is actually a problem before bothering you)

1. Change PulseAudioConvertPortaudioFormatToPulseAudio to return
paSampleFormatNotSupported on failure instead of paNotInitialized.

2. Fix OpenStream to check the return value from:
  * PulseAudioConvertPortaudioFormatToPulseAudio
  * PulseAudioBlockingInitRingBuffer

3. Fix OpenStream to return the actual PaError on failure, instead
of always returning paNotInitialized.

4. Fix OpenStream to not free the host api representation on error
(which resulted in an assertion failure later in Terminate).
@sqweek
Copy link

sqweek commented Mar 7, 2016

stream in mono mode (eg. patest_mono). It should be centered but it is heavily skewed towards one of the channels.

Ok I thought portaudio's buffer processor took care of this kind of channel adaption, but it turns out that's host api specific. eg. alsa has PaAlsaStreamComponent_DoChannelAdaption to copy a user mono stream onto a stereo host stream.

So maybe there's nothing to do there, except document the current behaviour (or run it by the main portaudio devs).

Add support for non-interleaved audio
@illuusio
Copy link
Owner Author

illuusio commented Mar 7, 2016

So they do it 'hard way' I try to find time to implement it. It's not a big thing but import for many cases.

@sqweek
Copy link

sqweek commented Mar 22, 2016

Ah I found another issue - PulseAudioGetStreamWriteAvailableBlock is inaccurate because it still looks at the ring buffer, which of course is no longer used for blocking output streams. Probably all references to outputRing can be removed?

@illuusio
Copy link
Owner Author

Yes output ring buffer should be removed altogether.. probably same goes with input ringbuffer (requires some work). Can you provide some example how it's inaccurate or how to test it? I try to fix mono thing and this before eastern.

@sqweek
Copy link

sqweek commented Mar 22, 2016

Well right now it always returns zero, but that's also because it is calling PaUtil_GetRingBuffer*Read*Available :). I'm not sure about a test case, but I would guess that pulseaudio has some kind of "how many samples can be written without blocking" function that could be used here?

I think the input ring buffer is still used in blocking mode at the moment? The recording side of things may need more testing in general, I know I haven't looked at it.

@sqweek
Copy link

sqweek commented Mar 27, 2016

I ran into this:

Assertion 'o' failed at pulse/operation.c:133, function pa_operation_get_state(). Aborting.

Caused by:

  1. Starting an output stream in blocking mode
  2. Calling Pa_Terminate
  3. Calling Pa_WriteStream

Obviously this order of operations will never work so I don't consider this a serious problem, just wanted to document it. In an ideal world it would return paNotInitialized instead of crashing.

@illuusio
Copy link
Owner Author

illuusio commented Apr 4, 2016

Hmm.. above should be tracked as Issue? Seems to be some kind of blocker.

@illuusio
Copy link
Owner Author

Huh long time no pushed this but now Mono should be converted to Stereo correctly. Does this fix your problem @sqweek? I'll try to fix some input issues that I have found next and that close one too.

@illuusio
Copy link
Owner Author

I'm trying to get this in officialy.. pull request to new shiny Portaudio GIT repo https://app.assembla.com/spaces/portaudio/git/merge_requests/3719213

@Be-ing
Copy link

Be-ing commented Aug 21, 2016

Awesome. Between this and the library GUI redesign, Mixxx could replace Amarok as my casual music player :)

@sqweek
Copy link

sqweek commented Aug 25, 2016

Nice work Tuukka! 👍

@daschuer
Copy link

It is really bad that this is still not merged for long. Can you update this with the latest changes on assembla. I am considering to add this to a Launchpad PPA.

@illuusio
Copy link
Owner Author

I've been toying around to rewrite this. I'll find time to update this

@illuusio
Copy link
Owner Author

Please refer to: https://github.com/illuusio/assembla-mirror-portaudio-pulseaudio as upstream for ths

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants