-
Notifications
You must be signed in to change notification settings - Fork 758
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
Freebsd compat #2834
Freebsd compat #2834
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for this! There are some issues:
- This fails to build now. Looking over the Travis logs that's because you have some
#elif
s where there should be#else
s. It's also because you missed athread
declaration inUIUGens.mm
. - Instead of decomposing usage of
thread
tostd::thread
, I think it might be better to go the other way andtypedef SC_Thread std::thread;
likeSC_Lock
. This makes it easier to test/swap out thread implementations other thanstd::thread
. We can discuss.
QtCollider/QObjectProxy.cpp
Outdated
#else | ||
#elif defined __FreeBSD__ | ||
# include <stdlib.h> | ||
#elif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be #else
#else | ||
#elif defined __FreeBSD__ | ||
# include <stdlib.h> | ||
#elif |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should be #else
Also forgot to say, this PR should definitely be built on Windows before merging. |
63cca1e
to
adb042c
Compare
I updated my patches. Travis is still unhappy. One of the tests failed:
I do not see how this is relevant to what I have done. |
lang/LangSource/PyrObject.cpp
Outdated
@@ -1169,7 +1169,7 @@ void buildBigMethodMatrix() | |||
double t0 = elapsedTime(); | |||
#endif | |||
|
|||
const int hw_concurrency = thread::hardware_concurrency(); | |||
const int hw_concurrency = std::thread::hardware_concurrency(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(missed one here)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks. I did that intentionally. I do not know C++, so I thought declared type will not work as a namespace. (Or you will not be able to call its class methods, or whatever they called). Will fix it tomorrow.
Thanks! Hm... UnitTest uses forking, so it could be related to a change to threads. I've restarted the Travis build in case it was a fluke. I can build on Windows, I was just making a note for myself or anyone else interested. :) |
That's strange. No changes were done to program logic, just renaming. Oh, I see green check. That is good |
So Windows builds fine with this VS2013/2015 MinGW 4.8 Msys2 64. So no build obstacles. Can somebody explain what that means implementation-wise? How would one go about to find out what this actually means? ;) The usual start sc, boot server, do ().play is not enough, I guess? ;) It's nice to have an occasional update of the support of more exotical Unixes I am curious: which Qt-version does FreeBSD use, which compiler and compiler version? Are FreeBSD, OpenBSD and NetBSD close enough to assume they might be covered as well? As to the necessity of Qt5Positioning, I do think the SC build currently requires a few Qt-libraries which are actually unused. I guess it happened in an optimistic moment Qt-support in SC... |
There are source files which include <sys/sysctl.h> and have "using std::thread" at the same time. This causes name clash on FreeBSD. Define a new type SC_Thread to avoid this.
adb042c
to
ccb379f
Compare
Changed that last std::thread. All must be good now. Can this be merged now? Thanks |
Answering to @bagong. I currently have Qt 5.7.1 from ports. FreeBSD 11's base system compiler is currently clang 3.8, you can also install llvm+clang up to 4.0. GCC is also present both in FreeBSD's source tree and ports, but is not built by default. By myself, I do not have any need in GCC.
All BSD flavors develop independently, but sometimes something from one BSD goes to another. I cannot say if my patch is useful for any other BSD (or even if this problem occurs in any other variant of BSD). |
@bagong The main change is just resolving a namespace collision that happens on FreeBSD. Building on Windows was not strictly necessary but interactions between namespaces and OS headers can be affected in odd ways sometimes. A good illustration of why to never put |
And just to clarify, the reason why I did think it was a good idea was because a file that was only included while building on macOS showed a missed replacement. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks again!
This is ready to merge IMO, I'd rather someone else look it over too. @danstowell @llloret? (you approved the hidapi changes). |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
All fine except for one I have a comment on
@@ -4264,7 +4264,7 @@ void initMIDIPrimitives(); | |||
initMIDIPrimitives(); | |||
#endif | |||
|
|||
#if !defined(_WIN32) && !defined(SC_IPHONE) && !defined(__OpenBSD__) && !defined(__NetBSD__) && !defined(__APPLE__) | |||
#if defined __linux__ |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Are we confident that this change is safe? It's probably better to be conservative and not assume that linux is defined on all platforms that can use LID (i.e. the old way). However I don't know if there are cases in the wild where this change would be a problem. Personally I'd say stick with the verbose way, exclusion rather than inclusion.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@danstowell the LID file is only included if LINUX is defined in Cmake, which is only defined if CMAKE_SYSTEM_NAME is "Linux", so I think this is safe. Do you see any issues arising from that? __linux__
is after all only defined on the Linux kernel.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I raised the question because I wanted this to be a conscious decision, rather than because I had strong views. I'll go along with your argument, yes. No need to hold up the merge.
It breaks in supernova, but that's probably intentional... |
@bagong More info please |
First break is in server/supernova/server/main.cpp:
Which makes sense, as main.cpp 211ff 'resolve home' only covers Linux, Apple, and Windows, other unixes are not covered, although that would be easy in that particular case. There are a few places in supernova main.cpp where non-Apple/Windows just covers Linux. If you create an outlet for FreeBSD in those cases the one way or the other, it will ultimately break in the linker stage (not necessarily at 99%, but always with reference to sc_ugen_factory:load_plugin_folder/close_handles):
Maybe that would be easy to fix. But the fact that FreeBSD doesn't seem support ALSA (?) made me wonder more generally, how SC would cope with the audio backend in FreeBSD land. Btw, there is a SC 3.8 (!) package in FreeBSD, I was surprised to see. I haven't tried it yet, should do so. But I'd like to hear from @shamazmazum first, how he actually runs the server before going through another trial-error odyssey. |
@bagong I run server with "Server.local.boot" or "s.boot". @danstowell What platforms are currently supported? Windows, Linux, Apple family, BSD family, right? From that list only Linux has LID, as I can understand. Look at the commit history. For some reason it started with few !defined and with time it became very long definition. At first it was With 0b623d9 is became My point is that at first it had some sense: both linux and apple had |
It would be. Some code paths are only turned on by the macro See primecoin/primecoin#22 and aspnet/KestrelHttpServer#94 for similar discussions. |
There is no obligation to build SuperNova of course, but I think it's good to keep it in mind, at least for us here. As to the server question: I assumed that you used s.boot ;) My question is about the backend used by jackd. I couldn't just start jackd (to wouldn't "just boot"), I had to define arguments first to make it boot with oss and use the sample rate my hardware was set to. But my main question: does the server play the frequencies you ask it to play? I get a wide difference between what I set and what I hear. Is that not so for you?
Pretty similar ;) |
@brianlheim I was able to build supernova, but got many runtime errors. First of all it cannot find any UGen and also report some errors about threads. Already removed that build, so cannot paste them here. Btw, are UGens loaded with dlopen()? @bagong Jack starts just well with jack -r -d oss. I tried to play Sin wave on 440 Hz and got 440 Hz exactly. (But then you set another sampling frequency on Jack (default is 48kHz), your wave sounds like clipped, suppose that's bug in jack). |
Ah, okay @shamazmazum . What I see is that playback seems to be at a constant samplerate, regardless of what sc and jackd think it is. At 44.100 the audible frequency is about a fifth higher, at 48.000 a minor third, at 88.200 it's a lot lower, at 96.000 even lower. Same when playing back audio files. If you get oddities with the sampleRate switch too, it might be worth to inspect that a bit more. I am not saying it's the fault of your port, of course not, but - while it could be my environment (virtual machine) - it might also be that the chain scsynth -> jackd -> oss -> audiohardware doesn't work as it should on NetBSD. Chromium, btw, plays back the 440Hz test tone in a Youtube video correctly for all of 44.100 up to 96000 on my system. Doesn't mean much, but at least shows that it could work ;). This is more informational of course, but a bit more testing is probably advisable before we assume FreeBSD is a supported platform now. I's suspect 'can be built now' is more apt. And from that point of view it might be nice fix the supernova build as well. We have a similar situation on Raspbian, where supernova can be built, but produces a lot of runtime errors. If the build hurdle is out of the way, somebody might think it worthwhile to have a look what actually happens (although that's likely harder to fix than the build)... This btw is what jackd posts on startup:
And we should mention that |
My is
so I thought the third number is the sample rate. But 63257 seems senseless to me. Btw. jackd is started on that sample rate, which is currently set on virtual channel by audio driver, no matter what jackd is told to set (e.g. jackd -r -d oss -r 192000). So no wonder that you hear tune on different frequency than it should be. And if you set sample rate to 48000 on virtual channel (by sysctl dev.pcm.X.play.vchanrate=48000) you will probably hear tunes as they should be. I think SuperCollider can't do anything about it. Supercollider<->jackd interaction is working fine, it's jackd<->kernel interaction which is broken. |
Yea, it's the sample rate, you are right, also fit's my hear experience (higher below 63256, lower above 63256). Sysctl dev.pcm gives for me:
Trying to set dev.pcm.0.rec.vchanrate on the command line seems not possible on my system. Maybe in some configuration file, which is used at boot time. Or it's broken all together (on my system)? Browsing Google seems to show quite a few postings with this kind of problem. Let's leave it at that, just something I bumped into when trying to reproduce what you were doing. Really has nothing to do with your PR. Except one might feel inspired to inspect SCs limitations on FreeBSD if one facilitates its use ;) |
every single plattform that supercollider may be optimized for, is a plus for the possibility to bring in new users. so i vote fixing that up |
What remains to be done here? I think modulo @danstowell's concern this is near mergeable, right? |
Go for it, IMHO |
Awesome. Thanks again for the discussion everyone, and for the code @shamazmazum |
Hi. This is my second set of commits for FreeBSD compatibility. The first is for hidapi here: supercollider/hidapi#8
With these commits you will able to build Supercollider on FreeBSD smoothly (provided you have Qt5Positioning which is not in the port collection). Btw., what is Qt5Positioning for? Is it really used in Supercollider?