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

Add support for non-interleaved audio #5

Merged
merged 4 commits into from
Mar 7, 2016

Conversation

sqweek
Copy link

@sqweek sqweek commented Mar 4, 2016

Hi Tuukka! Sorry to disappear on you, a lot of things have been demanding my attention (as I guess has also been the case with you). Hope all is well :)

I found this message in PulseAudioConvertPortaudioFormatToPulseAudio:

    case paNonInterleaved:
        PA_DEBUG(("PulseAudio %s: THIS IS NOT SUPPORTED BY PULSEAUDIO!\n",

Which is fine, but actually it doesn't matter if pulse audio supports non-interleaved audio because port audio's buffer processor can adapt it for us (so the user deals with non-interleaved samples but pulse audio sees interleaved samples).

There doesn't seem to be a patest case that uses non-interleaved samples so I modified patest_sine_formats - if curious you can find the modified version on my patest-sine-non-interleaved branch.

I also uncovered some problems with the error handling in OpenStream - at first my modified test failed like so:

Assertion 'm' failed at pulsecore/mutex-posix.c:88, function pa_mutex_lock(). Aborting.

Which was OpenStream freeing the entire host api representation on failure causing Terminate to hit this assertion. The first commit fixes this along with several other error handling problems. After that the test fails like so:

An error occured while using the portaudio stream
Error number: -9999
Error message: Unanticipated host error

Which is expected since it's only in the second commit that non-interleaved audio support is added. The patch works because PaUtil_SelectClosestAvailableFormat discards the paNonInterleaved flag - ie. hostOutputSampleFormat will never be non-interleaved.

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).
@illuusio
Copy link
Owner

illuusio commented Mar 4, 2016

Nice to have you back. I have also been just using this and not doing anything else.. I'll take a look at them and comment.

@illuusio
Copy link
Owner

illuusio commented Mar 6, 2016

Very clean and welcome changes. One thing mainly it just me thing is can you add {} to one-line ifs after that this is mergable. It think I'll start to push this to main Portaudio because I haven't run in issues for a while.

@sqweek
Copy link
Author

sqweek commented Mar 7, 2016

Ok I've added the {}s. Coding style might become an issue when pushing to main portaudio -- you've used if (...), while (...) etc etc, whereas the portaudio convention uses spaces inside the paren like if( ... ), while( ... ). See https://www.assembla.com/spaces/portaudio/wiki/ImplementationStyleGuidelines

@illuusio
Copy link
Owner

illuusio commented Mar 7, 2016

@sqweek thanks to pointing that out. Just a habit of mine but have to convert it to portaudio one.

illuusio added a commit that referenced this pull request Mar 7, 2016
Add support for non-interleaved audio
@illuusio illuusio merged commit 6cb7384 into illuusio:hostapi-pulseaudio Mar 7, 2016
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.

2 participants