monome / softcut-lib

sample cutting library
GNU General Public License v3.0
35 stars 6 forks source link

Jack client fixes #69

Closed catfact closed 11 months ago

catfact commented 11 months ago

several fixes for the jack/OSC client, with the main goal of making it easier / feasible to build and run on macOS.

catfact commented 11 months ago

oh also this pulls in the rec_once change. i guess that should be propagated to norns if we can be triple-sure that it will not impose a performance problem (new crackles) on cm3.

will have a performance / new features pass soon.

tehn commented 11 months ago

awesome, very happy to see this

schollz commented 11 months ago

@catfact I have a cm3 norns now and can do some rigorous performance testing. fwiw I use rec_once regularly in my script context (seemingly) without issues.

It's been awhile and I also will double check what you merged is up to date. I've been using this version for myself: https://github.com/schollz/softcut-lib/tree/rec-once5

catfact commented 11 months ago

@schollz i believe you that performance is fine. but i want to be sure.

there is another sense in which this change increases technical debt. there is a really specific mechanism for swapping per-sample update functions on each voice depdending on current play/rec flag status at the top of each audio block:

https://github.com/monome/softcut-lib/blob/main/softcut-lib/src/Voice.cpp#L60

if we are now also adding conditionals that do similar behavior changes per sample, it doesn't make sense to keep this function-swapping mechanism: it's probably slightly detrimental to performance since we are now checking flags each frame anyways, but more importantly (to me) it is just nonsensical and a future reader of the code won't be able to determine its intent.

catfact commented 11 months ago

... it looks to me like you did consider this a bit more in further commits on your own fork.

tehn commented 11 months ago

my build is failing on ubuntu, and this error message is a bit inscrutable to me

[ 84%] Building CXX object clients/softcut_jack_osc/CMakeFiles/softcut_jack_osc.dir/src/BufDiskWorker.cpp.o
/home/tehn/local/softcut-lib/clients/softcut_jack_osc/src/BufDiskWorker.cpp:24:75: error: aggregate ‘std::array<softcut_jack_osc::BufDiskWorker::BufDesc, 16> softcut_jack_osc::BufDiskWorker::bufs’ has incomplete type and cannot be defined
   24 | std::array<BufDiskWorker::BufDesc, BufDiskWorker::maxBufs> BufDiskWorker::bufs;
      |                                                                           ^~~~
gmake[2]: *** [clients/softcut_jack_osc/CMakeFiles/softcut_jack_osc.dir/build.make:132: clients/softcut_jack_osc/CMakeFiles/softcut_jack_osc.dir/src/BufDiskWorker.cpp.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:142: clients/softcut_jack_osc/CMakeFiles/softcut_jack_osc.dir/all] Error 2
gmake: *** [Makefile:91: all] Error 2

(boost is installed)

catfact commented 11 months ago

that's odd, i just tried a build on ubuntu with no issues.

are you using a very old version of ubuntu and/or gcc? this needs c++17 support, which means >= gcc 8. i'm on gcc 11 and recently upgraded ubuntu to 22.04.

catfact commented 11 months ago

(boost should not be needed now btw)

tehn commented 11 months ago

excellent re: no boost needed

gcc (Ubuntu 12.3.0-1ubuntu1~23.04) 12.3.0

on ubuntu 23.04


will keep messing around with it

catfact commented 11 months ago

huh. how did you invoke cmake and what is the cmake version?

i did this

cd $SOFTCUT_LIB_DIR
mkdir build
cmake -S . -B build/ -DCMAKE_BUILD_TYPE=Release
cmake --build build/

cmake 3.22.1 (though i'd expect older versions to work fine)

catfact commented 11 months ago

oh maybe check g++ --version to verify its the same?

tehn commented 11 months ago

cmake version 3.25.1 g++ (Ubuntu 12.3.0-1ubuntu1~23.04) 12.3.0

followed your precise build, same messages. clean clone.

catfact commented 11 months ago

tried with clean clone just to be sure, no issues.

honestly this feels like a toolchain bug. i wasn't planning on going up to ubuntu 23, too fresh. but guess i will now

tehn commented 11 months ago

let me try on another system (manjaro)

On Thu, Nov 9, 2023, 4:23 PM ezra buchla @.***> wrote:

tried with clean clone just to be sure, no issues.

honestly this feels like a toolchain bug. i wasn't planning on going up to ubuntu 23, too fresh. but guess i will now

— Reply to this email directly, view it on GitHub https://github.com/monome/softcut-lib/pull/69#issuecomment-1804705996, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAB4I4AXD7CYEGKI3OJ6U2LYDVCVTAVCNFSM6AAAAAA7FBBVNGVHI2DSMVQWIX3LMV43OSLTON2WKQ3PNVWWK3TUHMYTQMBUG4YDKOJZGY . You are receiving this because you commented.Message ID: @.***>

tehn commented 11 months ago

ok, since manjaro is arch, it's all edge versions. ran a system update (because this system doesn't get used a lot, and i needed cmake) so all versions are new

different error now:

In file included from /home/tehn/local/softcut-lib/clients/softcut_jack_osc/src/OscInterface.cpp:13:
/home/tehn/local/softcut-lib/clients/softcut_jack_osc/src/BufDiskWorker.h:37:25: error: field ‘path’ has incomplete type ‘std::string’ {aka ‘std::__cxx11::basic_string<char>’}
   37 |             std::string path;
      |                         ^~~~
In file included from /usr/include/c++/13.2.1/iosfwd:41,
                 from /usr/include/c++/13.2.1/bits/std_thread.h:38,
                 from /usr/include/c++/13.2.1/thread:45,
                 from /home/tehn/local/softcut-lib/clients/softcut_jack_osc/src/OscInterface.cpp:8:
/usr/include/c++/13.2.1/bits/stringfwd.h:72:11: note: declaration of ‘std::string’ {aka ‘class std::__cxx11::basic_string<char>’}
   72 |     class basic_string;
      |           ^~~~~~~~~~~~
make[2]: *** [clients/softcut_jack_osc/CMakeFiles/softcut_jack_osc.dir/build.make:118: clients/softcut_jack_osc/CMakeFiles/softcut_jack_osc.dir/src/OscInterface.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:142: clients/softcut_jack_osc/CMakeFiles/softcut_jack_osc.dir/all] Error 2
make: *** [Makefile:91: all] Error 2
catfact commented 11 months ago

bleh i'm dumb. was just missing standard library includes. (in first case for sure, second looks like it too.) pushed and merged to main (and here, for good measure.) https://github.com/monome/softcut-lib/pull/70

(you didn't need to include these explicitly on older gcc; v12 is stricter. apparently as a byproduct of internal refactoring in the stdlib.)

@tehn this addresses the missing array for me on newbuntu. also added <string> where appropriate for the second issue. if you encounter other errors like this ("undefined" or "incomplete" types from the standard library), check for inclusion of relevant headers i guess.

catfact commented 11 months ago

for reference:

https://gcc.gnu.org/gcc-12/porting_to.html

Some C++ Standard Library headers have been changed to no longer include other headers that were being used internally by the library. As such, C++ programs that used standard library components without including the right headers will no longer compile. The following headers are used less widely in libstdc++ and may need to be included explicitly when compiled with GCC 12: (for std::shared_ptr, std::unique_ptr etc.) (for std::begin, std::end, std::size, std::istream_iterator, std::istreambuf_iterator) (for std::for_each, std::copy etc.) (for std::pair) (for std::array) (for std::atomic) (for std::time, std::mktime etc.) (for pthread_create, pthread_mutex_t etc.)

(it doesn't list string, so idk if that is the actual cause of the 2nd problem, but it looks similar.)

tehn commented 11 months ago

now builds perfectly everywhere, thank you. and i apologize i couldn't figure out that error message. still haven't gotten into c++.