sonic-pi
sonic-pi copied to clipboard
sonic-pi-net
Use built-in RtMidi on Linux too
This replace #2534
The idea is to use the bundled RtMidi on Linux too (using ALSA)
Maybe it could be configurable using an option to use the system library if wanted, to make it easier to make things like .deb/rpm packages?
I have used both versions (system package and bundled RtMidi) and either will work. (In fact the bundled version is version 4.0.0 whereas the package (at least in Debian) is currently 3.0.0 but either is good), depending on how you modify the CMakeLists.txt file. At present there are issues with Sp_Midi on Linux which still need resolving (basically midi connections multiply uncontrollably when you add or remove a hardware midi device) and these issues (which may possibly affect the CMakeLists file) need to be resolved first.
tbf I haven't tested the MIDI feature yet. (I'm also new to Sonic Pi as a user)
I think that using the same version across the board should be easier on support for SonicPi developers.
Fedora 33 also only has 3.0.0.
I don’t have much experience in packaging what-so-ever, but this is what I gather about it: When packaging for e.g. a .deb package, it’s generally recommended to use the system installed libraries and state them as dependencies to be installed with sonic pi. (I believe that’s what the maintainers of the sonic-pi pkg in Debian have done) However, for things like making AppImages or portable builds, including uncommon libraries can be useful. And I don’t know how these sort of library dependencies work in flatpaks. (I imagine some uncommon libraries are bundled?)
Maybe it would be nice to have options to pick between bundled and system installed libraries? (maybe though a cmake option, or command arg like --build-aubio in the current build scripts)
flexibility & ease of use = win! 🙂
For the flatpak whatever is needed will be built.
But on Raspberry Pi OS (64-bits) there is only rtmidi 3.0, so using the system one ain't really an option. Same on Fedora 33.
All in all it's probably the best solution to for consistency across platforms.
Given it's a small-ish library without external deps other than ALSA/JACK and is fairly distro/system agnostic, I think it would be good idea to build RtMidi in by default on Linux to reduce maintenance and support burden
I've made a patch at https://github.com/lilyinstarlight/sp_midi/commit/777c1dc which turns built-in RtMidi on by default but gives the option to use the system-provided library via the CMake option USE_SYSTEM_RTMIDI
I want to PR the above patch, but I would like input from @samaaron and everyone else in this thread about whether that is the right way to go forward with this (specifically on Linux whether to default to using built-in RtMidi and provide an option to exclude it, or whether to leave it out by default and provide an option to include it)
Just a comment: normally package maintainers prefer not to vendor dependencies (that is, use dependencies in-tree), and they encourage to use the system available package.
On Thu, 10 Feb 2022 at 14:32, Lily Foster @.***> wrote:
Given it's a small-ish library without external deps other than ALSA/JACK and is fairly distro/system agnostic, I think it would be good idea to build RtMidi in by default on Linux to reduce maintenance and support burden
I've made a patch at @.*** https://github.com/lilyinstarlight/sp_midi/commit/2d7be76 which turns built-in RtMidi on by default but gives the option to use the system-provided library via the CMake option USE_SYSTEM_RTMIDI
I want to PR the above patch, but I would like input from @samaaron https://github.com/samaaron and everyone else in this thread about whether that is the right way to go forward with this (specifically on Linux whether to default to using built-in RtMidi and provide an option to exclude it, or whether to leave it out by default and provide an option to include it)
— Reply to this email directly, view it on GitHub https://github.com/sonic-pi-net/sonic-pi/pull/2542#issuecomment-1034990080, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJOL44WXI73IO3L74R6LGLU2PD67ANCNFSM4TNXP4YQ . You are receiving this because you are subscribed to this thread.Message ID: @.***>
but when no one has the package, it just doesn't work. when I submitted the patch I did it because I couldn't build on the latest Fedora. Let alone build on distro based on much older code.
@lilyinstarlight solution is more flexible (i.e. better than this one), so I'm personally OK with superceding this PR with it.
But if you want contributions to better support Linux, being able to easily build the project should be priority number 1.
I totally agree with you. This was just a thought because some maintainers are not willing to make packages that have vendors dependencies.
On Thu, 10 Feb 2022, 17:34 Hubert Figuière, @.***> wrote:
but when no one has the package, it just doesn't work. when I submitted the patch I did it because I couldn't build on the latest Fedora. Let alone build on distro based on much older code.
@lilyinstarlight https://github.com/lilyinstarlight solution is more flexible (i.e. better than this one), so I'm personally OK with superceding this PR with it.
But if you want contributions to better support Linux, being able to easily build the project should be priority number 1.
— Reply to this email directly, view it on GitHub https://github.com/sonic-pi-net/sonic-pi/pull/2542#issuecomment-1035212262, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJOL445NLJT6ESJW22LHPLU2PZJPANCNFSM4TNXP4YQ . You are receiving this because you commented.Message ID: @.***>
This was just a thought because some maintainers are not willing to make packages that have vendors dependencies.
As a package maintainer, I made sure to keep the option available to link against the system RtMidi via a CMake option (I just don't think it should do that by default)
If you wanted or if there is demand for it, I could also add a flag to the prebuild script (like the -n flag is now) to set the USE_SYSTEM_RTMIDI CMake option
I think that what you did makes sense, just wanted to highlight this potential issue.
On Thu, 10 Feb 2022 at 19:36, Lily Foster @.***> wrote:
This was just a thought because some maintainers are not willing to make packages that have vendors dependencies.
As a package maintainer, I made sure to keep the option available to link against the system RtMidi via a CMake option (I just don't think it should do that by default)
If you wanted or if there is demand for it, I could also add a flag to the prebuild script (like the -n flag is now https://github.com/sonic-pi-net/sonic-pi/blob/a84e11b/app/linux-prebuild.sh#L11-L14) to set the USE_SYSTEM_RTMIDI CMake option
— Reply to this email directly, view it on GitHub https://github.com/sonic-pi-net/sonic-pi/pull/2542#issuecomment-1035400418, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAJOL4YXDS5BSIOUWT6AWEDU2QHSZANCNFSM4TNXP4YQ . You are receiving this because you commented.Message ID: @.***>
I'm all for using the built-in rtmidi by default with an option for using the system version should package maintainers prefer to use that.
I've opened the PR as sonic-pi-net/sp_midi#29, so further discussion can occur there
So this is solved by https://github.com/sonic-pi-net/sp_midi/pull/29, right?
So this is solved by sonic-pi-net/sp_midi#29, right?
Yep, it should be once that PR gets synced to this repo!