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

Implement Patch for Portaudio from Upstream #193

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

Conversation

mariaWitch
Copy link
Contributor

This change moves the old implementation of PortAudio's PaUtil_GetTime() into Audacity and renames it util_GetTime() because audioport removed said function in an upstream commit.

See: audacity/audacity#946 for further info.

With this change, we should be able to update portaudio to the upstream version.

(This commits may need to be updated slightly to include credit.)

This change moves the old implementation of PortAudio's PaUtil_GetTime() into Audacity and renames it util_GetTime() because it was removed in an upstream commit.

See audacity/audacity#946 for further reference.

(This is not ready for Merging.)
@mariaWitch mariaWitch changed the title Let Audacity compile with unmodified portaudio sources Implement Patch for Portaudio from Upstream Jul 11, 2021
@Ckath
Copy link
Contributor

Ckath commented Jul 11, 2021

wonder if this will help with that portaudio specific issue found in #75, at least two people have gotten it with debian/ubuntu iirc.

@mariaWitch
Copy link
Contributor Author

mariaWitch commented Jul 11, 2021

We'll need to test midi playback/timing to confirm that it isn't jittery.

@mariaWitch mariaWitch marked this pull request as ready for review July 11, 2021 20:19
@mariaWitch
Copy link
Contributor Author

mariaWitch commented Jul 11, 2021

I also believe that we should confirm whether or not upstream Portaudio works with this patch applied.

@Ckath
Copy link
Contributor

Ckath commented Jul 13, 2021

added this as well from other posts in the threads to get portaudio from system working

diff --git a/cmake-proxies/CMakeLists.txt b/cmake-proxies/CMakeLists.txt
index 268e8e098..00b86211b 100644
--- a/cmake-proxies/CMakeLists.txt
+++ b/cmake-proxies/CMakeLists.txt
@@ -121,7 +121,7 @@ set_conan_vars_to_parent()
 #       directory          option      symbol      req   chk   version
 addlib( libsndfile         sndfile     SNDFILE     YES   YES   "sndfile >= 1.0.28" )
 addlib( libsoxr            soxr        SOXR        YES   YES   "soxr >= 0.1.1" )
-addlib( portaudio-v19      portaudio   PORTAUDIO   YES   YES   "" )
+addlib( portaudio-v19      portaudio   PORTAUDIO   YES   YES   "portaudio-2.0" )
 addlib( sqlite             sqlite      SQLITE      YES   YES   "sqlite3 >= 3.32.0" )
 
 # Optional libraries
diff --git a/src/AudioIOBase.cpp b/src/AudioIOBase.cpp
index 7d9e8e745..9fdbeb534 100644
--- a/src/AudioIOBase.cpp
+++ b/src/AudioIOBase.cpp
@@ -12,6 +12,7 @@ Paul Licameli split from AudioIO.cpp
 #include "AudioIOBase.h"
 
 
+#include <portaudio.h>
 
 #include <wx/log.h>
 #include <wx/sstream.h>

after this it builds with system portaudio with -Dsneedacity_use_portaudio=system -Dsneedacity_use_portmixer=off
ldd output: libportaudio.so.2 => /usr/lib/libportaudio.so.2 (0x00007f1f5d2a0000)

@Ckath
Copy link
Contributor

Ckath commented Jul 13, 2021

seems system portaudio does indeed ruin the midi devices:
with -Dsneedacity_use_portaudio=system -Dsneedacity_use_portmixer=off:
image

regular build:
image

@mariaWitch
Copy link
Contributor Author

I'm going to post on the upstream PR about how to actually test this fix.

@Ckath
Copy link
Contributor

Ckath commented Jul 13, 2021

the situation now is, from my testing:

  • with extra patch I posted I can build from system portaudio, but it breaks midi devices
  • just upstream portaudio doesnt work because of portmixer
  • disabling portmixer without going to system portaudio fails the build as well
  • yet to be tested but kinda defeats the point of 'accounting for not having to patch' is patching the latest portaudio

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

Successfully merging this pull request may close these issues.

2 participants