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

Attempt to use Portaudio (v1) #821

Closed
wants to merge 1 commit into from

Conversation

npostavs
Copy link
Contributor

@npostavs npostavs commented Jan 9, 2021

In #116 (comment) @corrados says

But right now I do not see an urgent need of including portaudio.

For me, getting non-computer savvy musicians to work with asio4all has been pretty challenging, to the point where I'm looking at adding portaudio support to make things easier. I banged something together to get an idea of what would needed to be changed. I don't expect the code in this PR to be merged, it's just a starting point for discussion.

Here are something things that need to be worked on:

  1. I've put the portaudio support in place of the Windows ASIO code, which is obviously the wrong place.
  2. It probably fails for mono devices, my grasp of the audio buffer formatting is a bit shaky
  3. The current user interaction of opening the ASIO config modally and refusing to open Jamulus until the sound is functioning doesn't really work for portaudio devices, so I just stopped throwing an exception there to allow Jamulus to proceed. The UI definitely still needs some more reworking.
  4. I completely failed to understand what the GetSelCHAndAddCH stuff is about.
  5. I know haven't followed the indent/paren spacing very well.

Maybe some other things; I haven't use Qt before, and my C++ skills are somewhat out of date.

This is a messy, not ready for prime time, proof of concept.  I've
banged on it until I managed to hear myself when connecting to a
Jamulus server with an MME and then WASAPI devices selected.
@corrados
Copy link
Contributor

corrados commented Jan 9, 2021

Hi npostavs, thank you for your initial attempt to include portaudio to Jamulus.

My idea was not to replace the current ASIO sound interface but be placed parallel to the existing Windows ASIO interface as a start. This has the advantage that we could use portaudio also for MacOS and Linux.

In a first step I would keep the changes in the GUI (i.e. client.cpp/.h, clientdlg.cpp/.h) to an absolute minimum and start with only portaudio ASIO support to see if and how it works. I.e. do not introduce GUI elements for selecting the Api but set it fix to ASIO in your portaudiosound. The idea would be to keep the GUI as simple as possible. I could think of an automation that an algorithm tries to find the sound interface with lowest latency in your system. If ASIO is present, it picks ASIO. If another low-latency Windows audio interface is available, it picks that one. If an experienced user wants to pick a specific API, we could later on introduce a command line argument for that.

Since portaudio may be used for all OSs, the correct place for the new "sound" files are in the src directory. We already have files "vstsound" there. So it would make sense to name it "portaudiosound". Try to keep soundbase as is since this is used for all OSs. Changes in that file will most probably break functionality.

How do you integrate the portaudio code? Do you use it as a Git submodule? Or do you want to include the source like we did for the OPUS codec?

@npostavs
Copy link
Contributor Author

npostavs commented Jan 9, 2021

Okay, I'm not sure if keeping the GUI unchanged will be viable in the end, but I agree that using just ASIO is a good first step.

My idea was not to replace the current ASIO sound interface but be placed parallel to the existing Windows ASIO interface as a start.
Since portaudio may be used for all OSs, the correct place for the new "sound" files are in the src directory. We already have files "vstsound" there.

It would have to be selected at compile time with an ifdef, since it looks like Jamulus only supports one kind of CSound at a time, right?

How do you integrate the portaudio code?

Currently I just make installed it and use it as a library. I also had to backport a little patch to make it build with WASAPI support: npostavs/portaudio@d7e7916

@corrados
Copy link
Contributor

corrados commented Jan 9, 2021

It would have to be selected at compile time with an ifdef, since it looks like Jamulus only supports one kind of CSound at a time, right?

As a first start, yes. Just implement portaudio as an additional sound interface like the Winodws/MacOS/Android which is already existing. These are also selected with ifdefs. Then you can do the implementation without any change in the GUI.

@ghost
Copy link

ghost commented Jan 10, 2021

I hope that extra dependencies would be controllable with build-configuration options. If portaudio is not currently used in Jamulus, Jamulus should be forever buildable without portaudio as Jamulus is very useful and mature as is. Note that I'm not opposed to a portaudio build-option.
Edit: I'm speaking more specifically for *nix/linux portaudio (maybe portaudio is the unconditionally logical choice in MS Windows).

@npostavs
Copy link
Contributor Author

Any advice on how to add new build options? I haven't used the qmake/qtcreator build system before.

@npostavs
Copy link
Contributor Author

I've hit a bit of a snag, portaudio doesn't seem to provide an equivalent for asioMessages to detect buffer size changes in its API (in their implementation, they pass in a no-op callback for this). So if I change the buffer size in the ASIO control panel, the Jamulus sound is distorted until I click around in the settings window to trigger a reset.

I guess if we include the portaudio source into jamulus then we could modify it to support this. What do you think?

@nefarius2001
Copy link
Contributor

where is a buffer size change to be detected? the parameter of the normal callback is not a good thing?

@npostavs
Copy link
Contributor Author

@nefarius2001 I don't think I understand your question. But you can compare how Jamulus's current code handles this, vs portaudio's no-op.

By the way, I have made some progress on the qmake configuration this weekend in my np/portaudio.v2 branch. Adding support for the buffer size change callback still pending.

@npostavs
Copy link
Contributor Author

Closing in favour of #870.

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.

3 participants