Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Build with PortAudio support if the "portaudio" CONFIG flag is specified #3338

Merged
merged 1 commit into from Feb 13, 2018

Conversation

@davidebeatrici
Copy link
Member

commented Feb 11, 2018

No description provided.

@mkrautz

This comment has been minimized.

Copy link
Member

commented Feb 12, 2018

We don't recommend the use of the PortAudio backend in production. It's not as mature as the other backends. Though I understand that you're having issues with PulseAudio that aren't there in the PortAudio backend.

I'd be happier if this was a CONFIG flag. I don't have a particular thing in mind. Simply CONFIG+=portaudio wouldn't work. (Pulse's config block would still disable it.)

@davidebeatrici davidebeatrici changed the title Don't disable PortAudio support if PulseAudio is available Build with PortAudio support if the "portaudio" CONFIG flag is specified Feb 12, 2018

@mkrautz

This comment has been minimized.

Copy link
Member

commented Feb 12, 2018

This possibly breaks OSX Universal. We'll need to redo the build script.

@davidebeatrici

This comment has been minimized.

Copy link
Member Author

commented Feb 12, 2018

Because it relies on PortAudio being automatically enabled?

@mkrautz

This comment has been minimized.

Copy link
Member

commented Feb 12, 2018

@mkrautz

This comment has been minimized.

Copy link
Member

commented Feb 12, 2018

Hmmm. Not actually sure. Maybe we use CoreAudio on that build now?

@@ -446,12 +446,11 @@ unix {
HAVE_PULSEAUDIO=$$system(pkg-config --modversion --silence-errors libpulse)
HAVE_PORTAUDIO=$$system(pkg-config --modversion --silence-errors portaudio-2.0)

!isEmpty(HAVE_PORTAUDIO):!CONFIG(no-portaudio) {
!isEmpty(HAVE_PORTAUDIO):CONFIG(portaudio) {

This comment has been minimized.

Copy link
@mkrautz

mkrautz Feb 12, 2018

Member

This doesn't make any sense. If CONFIG(config)... add it again? :-)

We can drop this whole section.

Drop HAVE_PORTAUDIO.
Drop this check.

This comment has been minimized.

Copy link
@davidebeatrici

davidebeatrici Feb 12, 2018

Author Member

I read it as "Enable PortAudio" instead of "Add flag"...

@mkrautz
Copy link
Member

left a comment

Drop HAVE_PORTAUDIO assignment.
Drop the condition that sets CONFIG+=portaudio.

Just leave the portaudio {} block later in the file.

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:port-audio branch from c025738 to 7e77a1a Feb 12, 2018

@mkrautz

This comment has been minimized.

Copy link
Member

commented Feb 13, 2018

@davidebeatrici Could you squash?

@mkrautz

This comment has been minimized.

Copy link
Member

commented Feb 13, 2018

@davidebeatrici Also, will you do the mumble-releng PR?

@davidebeatrici davidebeatrici force-pushed the davidebeatrici:port-audio branch from 7e77a1a to a0e2797 Feb 13, 2018

davidebeatrici added a commit to davidebeatrici/mumble-releng that referenced this pull request Feb 13, 2018

@mkrautz mkrautz merged commit 48277cb into mumble-voip:master Feb 13, 2018

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.