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.

Sign up for GitHub

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

Jump to bottom

Portaudio with Oboe Implementation #840

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

Portaudio with Oboe Implementation #840

wants to merge 133 commits into from

Conversation

hopefulGiupplo
Copy link

As I said in this issue, I finally can say that this PaOboe implementation seems to be working.
Instruction to how to build PortAudio with Oboe are detailed in src/hostapi/oboe/Readme.md

Carlo Benfatti and others added 30 commits June 7, 2023 12:10
Preparation to start patching - TODOS and directory src/hostapi/Oboe
9b6ceeb
Added src/hostapi/oboe/README.md
aff3b5a
Added include/pa_oboe.h and src/hostapi/oboe/pa_oboe.cpp
7d2c0c9
Added PA_USE_OBOE section to CMakeLists.txt
8ab4dd5
Added PA_USE_OBOE section to CMakeLists.txt
3aed6fe
Heavily reworked CMake dependencies, added FindOboe.cmake, updated pa…
f5a89e1 
…_oboe.cpp, more work needed
Included build_all_PaOboe.sh, more work needed on CMake
51b3757
Included build_all_PaOboe.sh, more work needed on CMake
0e05b66
@hopefulGiupplo
Update src/hostapi/oboe/README.md
ed974aa
Built shared library
af46735
Polished bits
cdbb27e
Fixing old problems
816e7f5
Added some debug messages
948cb3d
added new build script
71e0fcb
Added paOboe in the paHostApi array
e96d724
Added paOboe in pa_unix_hostapis.c
194c597
Working patch
313d759
Ready for other users to compile/build
dd40f71
Updated oboe/Readme.md
5cca69c
Added sharing mode selection
402fc77
Fixed minor issue with sharing mode selection
5c24f22
Removed references to KCTI
75605db
Fixed error callback, added performance mode autoselection
a6e4301
Minor change to CMakeLists, added low latency costant in pa_oboe.h
3618f11
Deleted Build_PaOboe.sh
746b3a1
Ready to push (removed my paths to Oboe directory and Android NDK)
4a2bf79
@hopefulGiupplo
Update README.md
b5ca5f9
@hopefulGiupplo
Update README.md
07b8347
Merge remote-tracking branch 'destination/PaOboe_Patch' into PaOboe_P…
11abb8f 
…atch

# Conflicts:
#	src/hostapi/oboe/README.md
Merge remote-tracking branch 'destination/PaOboe_Patch' into PaOboe_P…
16fa52a 
…atch
@hopefulGiupplo hopefulGiupplo deleted the Rebase branch October 9, 2023 11:09
@hopefulGiupplo hopefulGiupplo restored the Rebase branch October 9, 2023 11:14
@hopefulGiupplo hopefulGiupplo reopened this Oct 9, 2023
Carlo Benfatti added 13 commits October 9, 2023 14:12
@hopefulGiupplo hopefulGiupplo marked this pull request as ready for review October 19, 2023 09:12
@philburk philburk self-assigned this Nov 10, 2023
@hopefulGiupplo
Copy link
Author

hopefulGiupplo commented Nov 30, 2023 •
edited
Loading

Hello @philburk, I saw you assigned this issue to yourself. Please let me know if I can help :)

@philburk philburk added the P2 Priority: High label Dec 1, 2023
@hopefulGiupplo
Copy link
Author

Hello @philburk and PA team, I'm here to ask if I can help with any changes, and to ask if you're planning to include pa_oboe in portaudio's next release :) I rebased this work onto the last additions, I hope the library is still correctly built.
Please ping me if there's anything I can do!

@hopefulGiupplo hopefulGiupplo requested a review from philburk May 29, 2024 08:42
@philburk
Copy link
Collaborator

Thanks for rebasing this code.
We are trying to get the next release out soon.
This PR will need a deep review and then after it is merged it will need some soak time for other to try it.
So I don't think it will make the next release.

I will try to review in depth soon. The code looks very good so far.

philburk
philburk requested changes Jun 14, 2024
View reviewed changes
Copy link
Collaborator

@philburk philburk left a comment

Choose a reason for hiding this comment

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

The missing comma is causing it to fail CI.

include/portaudio.h Outdated Show resolved Hide resolved
@RossBencina
Copy link
Collaborator

Hi @hopefulGiupplo, thanks for your contribution, it's looking good but I agree that Phil needs to do a thorough review since he is the Oboe expert. Some other things that would help:

  • It's not passing CI at the moment, you will need to keep on top of that
  • It would help if there were some additional testers for your code, if you think that it's ready (and once CI is passing) you could post to the mailing list and request that other people try it
  • If anyone else is already using this it would be good to hear their feedback here (both in terms of using the implementation and reviewing the source code).

Once Phil is satisfied I will also do a code review.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@philburk philburk philburk requested changes

Requested changes must be addressed to merge this pull request.

Assignees

@philburk philburk

Labels
P2 Priority: High
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@hopefulGiupplo @philburk @RossBencina @illuusio