Yeah I think that makes sense and I think I like that format better anyway. I think that’ll be a good move and if it becomes integrated it’ll allow others to help maintain if you need the help.
Ok, I created a branch for the api changes. Please take a look: https://github.com/danomatika/ofxPd/tree/libpd-api-matching
Also, there is a changelog.
I’ve updated to ofxPd to more closely match the libpd Java api. I’ve also spun off most of the functionality into a PdBase class which hopefully can become the official libpd C++ wrapper. Also, ofxPd now accepts a PdMidiReceiver which can filter the midi channels it receives.
Check out the libpd-api-matching branch and changelog in the previous post and/or test the new code and let me know what you think.
This may be slightly off-topic for this forum, but I’ve started using the libpd-api-matching branch within the Cinder framework; it’s a fantastic wrapper around libpd and I think it’d be awesome if this was part of a c++ wrapper within libpd. I’ll post some first impressions here.
At first I thought the absence of locks was worrisome, but I’m liking it, as it leaves it up to the specific locking mechanisms (whether semaphors, mutexes, or whatever) up to the implementation in the specific platform. The locks do ensure that an app won’t crash when opening a file during audio processing (and probably other cases), but people may wish to work around these issues in a more efficient manner…
If it is included in libpd, most of the example projects for iOS will have to be updated in order to avoid the name class between the two versions of PdBase.h (C++ and Obj-C). This could be avoided by renaming the C++ version to PdBase.hpp, although I realize this might rub against you’re own coding customs.
You note that currently only one version of PdBase should be used, due to pd’s current problem with static variables; have you considered making the PdBase a singleton for this reason?
I think it’d be nice to have an overloaded openPatch method that takes the concatenated path/name.pd. Also concerning those methods, the libpd api uses openFile and closeFile (incredibly small gripe, sorry :).
The overloaded operators in PdTypes are missing the return value (return *this; ?).
I’m going to keep using this branch in my project, looking forward to it!
Just wanted to mention I’ve been using the libpd-api-matching branch for a few weeks now and it’s been very stable. Nice work Dan and nice feedback Rich.
That’s the idea. Just waiting for Peter to be ready (and for you all to bug test it).
I deliberated on this a bit. I modeled PdBase on Peter’s Java implementation and Java has the built in thread support. There’s currently no easy way to include thread safety in a “native C++” wrapper, so I decided to leave it out. When C++11 comes around, I’d go ahead and use std::mutex and add a compile flag to disable it.
That’s a good point. I hadn’t thought of it. I figured the folder layout would add my wrapper in the “cpp” folder in the libpd repo. You would only include “cpp” if you wanted it. I can’t imagine people using both the Obj-C and C++ wrapper simultaneously …
Yes and, as I noted on the CreateDigitalNoise forum, I don’t want to force a singleton on users. If you want a singleton, it’s easy enough to add a PdBase object inside of it. Also, creation order is sometimes an issue with singletons and, last, I don’t want to have to explain singletons to newbies.
The ofxPd wrapper does this … it’s easy enough using Poco::Files/ofxFiles in OF, but requires string handling in plain C++ that seems redundant so I didn’t bother. I suppose it could be added, make a pull request.
Yeah I saw that. Originally it was “openFile” then I noticed it was “openPatch” in both Java PdBase and the Processing lib. I like openPatch better and it matches the name most users of the higher-level wrappers will be used to.
Ok thanks. I can fix this.
Operator returns are now fixed in PdTypes.
I was thinking of adding it to the included xcode project as well, so you can just drop that into an existing project and add the correct target in your build phases. Relying on the folder layout will work for this as well, and is probably the better solution, although not how things are set up at the moment. I was a bit lazy with the existing iOS examples; there are needed headers in 3 different locations (objc, libpd_wrapper, and pure-data folders) and it was a bit long winded to instruct people to include all three separately. Not sure what to do about this; using sub-projects is an easy way to get the code into a new project, but then you need all of it (the java stuff is omitted).
I can’t find this forum post at CreateDigitalNoise, can you link to it? Anyways, I’m not a huge fan of singletons either, especially since we’d all like the root of the problem to be fixed so we can have multiple pd’s running (will take a bit of work). Still, the way it is currently set up will also cause problems for ‘newbies’ by not protecting the static PdBase instance.
About the static PdBase instance (it’s documented as such, although the static declaration is missing) pdPtr; every class will set that pointer to itself, so it isn’t much use. Why not check to see if it is set first, or use some other method to protect it from being set more than once? This gives you a single protected instance, without having to change public API.
Hm, I suppose a few things don’t match with the obj-c layer that I hadn’t noticed, and I’m outvoted in this instance. Maybe I should change the name of the obj-c method, if it isn’t yet entrenched.
One thing that I like better about the obj-c layer, however, is that the PdFile object manages the lifetime of a single patch/file, so the patch is closed in the PdFile’s dealloc method (I don’t think Peter wanted to do it this way in Java because of the garbage colleciton scheme). It maintains thread safety by calling to libpd’s C API through PdBase.m. This looks a bit hard to do in the current C++ wrapper, however, since there are no built in locks.
And one more question, why the maxMsgLen = 32? Isn’t this too limiting for all lists and messages passed between pd and the native code? Anyway, there are many changes concerning message callbacks in the new version of libpd coming out, which will get mentioned soon enough.
There’s other comments, but they have to do with implementation details, maybe too much detail for this forum post and would be apt for github, would you agree or is here fine for this as well?
Of course, I’m able to do pull requests, just wanted to discuss things first. It’d be nice to get the Peters in on this discussion, or anyone else who doesn’t use OF but would possibly use libpd in C++.
Really? I find dragging 3 folders into the Xcode tree pretty easy …
Actually, in C++ pdPtr will be different for every instance. It is not overwritten unless its static. Every class will have it’s own and multiple instances do not overwrite it. Try it with a float. I could make it static but, as the comment notes, any use of multiple instances is undefined so your screwed anyway.
Yes, it’s not feasible in plain C++ since I decided against writing custom, cross platform mutex code. When C++11 is supported on all platforms, locks can be added. It’d be easy to add to ofxPd though. Besides, in C++ you have to be used to cleaning up after yourself
As far as I know, the libpd max message len was 32 until Peter changed the api back in Aug to specify the len when sending. It didn’t seem to be a problem until then … You can simply make it larger if you want with setMaxMessageLen().
Keep general discussions here and bug reports, detailed feature requests on Github.
I discussed a bit with the Peters before I refactored ofxPd. The plan is this will be the cpp wrapper.
Thanks for the replies, Dan.
It is much easier to manage library/framework updates with sub-projects. If new files are added that existing files depending on within the framework, your app will surely break, where as (in theory) the sub-project should be self contained. It’s also just a better way (at least in my opinion) to organize code.
That said, using the path hierarchy is working in the sub-project on a compilation level, so it should be fine (both binary outputs will be .o anyway). Now it’s just dealing with the pesky Xcode bugs like it wrongly thinking PdBase.m’s interface file (.h) is PdBase.cpp’s counterpart.
That’s not very useful then. Anyways, your last comment spells it out; you can’t use multiple instances so you shouldn’t be able to do so. This is how all other libpd language wrappers are currently written, so I am of the opinion that the cpp version should be the same. Whether this is through using static functions or by using a singleton, I don’t think it matters much (although the first is what would match the existing API’s, which is the goal right?).
I don’t see the reason for the initial limitation, you’re really not saving much memory by only allowing 32 t_atom pointers as compared to allowing 300. Also, the new version of libpd includes a ringbuffer implementation that should be used with the C++ wrapper as well, to get the message callbacks off of the audio thread. I haven’t mentioned this much because it’s not officially released, but it should be any time now. With this tool, there is an overall buffer length that you can set, rather than limiting every message.
Fair enough. I see your point. I only named the class “PdBase” as I couldn’t think of anything better. You can just use it directly without having to inherit it, so maybe Pd.hpp/Pd.cpp make more sense? In either case changing “.hpp” exts is simple enough.
Ok I’ll make it static. No ambiguity!
Well 32 seemed reasonable and it was Peter’s initial choice. I hadn’t run into a wall over it yet anyway. What is better? 64? 128? 256? I suppose I could try to make it transparent where it always sends whatever it gets … but it’s good to have some sort of limit.
Ha. A settable buffer length in libpd is what I argued for with Peter on the pd-list. Good to see it coming. In any case, the current setup is a reasonable compromise. I haven’t gotten any negative feedback yet.
It’s definitely great work, thanks for building this! I’m just thinking of how we can have the wrapper generic enough to fit in libpd and be used by a multitude of frameworks.
I for one like Pd.hpp/Pd.cpp; short and to the point. But that doesn’t match the existing apis; not sure how important that is to Peter B.
The navigation problem is an Xcode bug that I’ll report when I have time. Even with the header renamed to PdBase.hpp, Xcode is too stupid to know to jump to PdBase.cpp instead of PdBase.hpp (and it shouldn’t even know about PdBase.m, since that folder isn’t included in the search path). Sorry, off topic again…
Ok, I wrapped all the instance variables into a PdContext singleton class. Any/all PdBase instances now use this single context without any api changes. This should allow adding multiple context support (if/when it comes) easier without breaking much code.
Also, the headers are now *.hpp.
Cheers, I’m implementing an audio class for using pd in the cinder framework using your updates, I’ll report back how it goes…
I just got back to this after putting it down for a number of months. I tested in Ubuntu 10.04 and found that I had to limit the sound stream setup to only audio output. After that everything played fine with no buffer overruns (note the 0 in the arguments to disable audio input channels).
// in testApp::setup (for the example) // setup OF sound stream ofSoundStreamSetup(2, 0, this, 44100, ofxPd::getBlockSize()*ticksPerBuffer, 4); // setup the app core core.setup(2, 0, 44100, ticksPerBuffer);
I’m currently getting the following:
OF: OF_LOG_ERROR: ofxPd: Can not start list, message in progress OF: OF_LOG_ERROR: ofxPd: Can not add symbol, message not in progress OF: OF_LOG_ERROR: ofxPd: Can not add float, message not in progress OF: OF_LOG_ERROR: ofxPd: Can not add float, message not in progress OF: OF_LOG_ERROR: ofxPd: Can not add float, message not in progress OF: OF_LOG_ERROR: ofxPd: Can not finish message, message not in progress
Even if I have this wrapped around the code, it still happens.
My thought is this is occurring because I’m sending lists at the same time or something. One list is being sent on a certain frame number and that runs fine without the error. Another list sends x/y movement to PD on a touch move event. If I only run the frame based one OR the touch move one - it runs without error. When combined there becomes a problem. Could they be cancelling eachother out or something? The error also seems to cause some bad audio artifacts on occasion.
I’m now getting some 'error: unpack: type mismatch" errors on occasion. I think somehow the messages are getting mixed up and lists are being sent to the wrong place when multiple are sent at the same time. It’s causing a nasty distortion when it happens in my case.
I also experienced this once:
malloc: *** error for object 0xeee340: pointer being freed was not allocated *** set a breakpoint in malloc_error_break to debug
It’s coming from “msgDest = dest;” in ofxPd::startList function.
Sounds like a cross threading issue. Try adding a check if a message is in progress or maybe add your own mutex … All of the operations should be wrapped by the ofxPd mutex, perhaps it’s doing something in a bad order …
Hmm ok. Yeah, I’m currently wrapping things around a “isMsgInProgress” and that helped a little, but on occasion it would still conflict. It was even sending messages to the wrong place sometimes. I printed the output from PD and some of the list receivers were receiving values that were supposed to be sent somewhere else. Right now I’m avoiding it by not sending lists/messages from the touch events and sending direct values (sendFloat) instead. I’m not too familiar with mutex/threading so i’ll have to look into that. I assumed since the list/messages are already wrapped in mutex it shouldn’t be a problem but apparently it is in some circumstances.
For anyone else, the conflict is when sending messages/lists from the audioRequested callback in conjunction with sending messages/lists from touch events (down, up, move).