-
Notifications
You must be signed in to change notification settings - Fork 34
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
A simple C(++) library and examples are needed. #3
Comments
As far as I know, XDF was created primarily to be a way to get data into MATLAB. Of course, it was also developed with generality in mind so that other languages (e.g. python) could handle the format. But, I don't think that anyone else has yet attempted to do this (answer to question 1). I see your reasoning behind conglomerating xdf read and write into a single C++ library that could be wrapped by python/matlab/whatever. So, I would say the answer to question 2 is 'yes but there is also a 'but' which is that to do this would require a lot of work and restructuring of the two projects. To be honest, I've never been totally happy with LabRecorder and I don't actually know why it is in Python and not C++. I believe this was done with cross-platform ndeployment in mind, but now that I have added build scripts for compiling LSL apps for both Linux and OSX, I don't see any reason for it to be a python app. Again, it works (on Windows and Linux, but not OSX) so it is difficult to find the motivation to rewrite it. ps sorry for the delayed contribution to the discussion, I was out of town for a few days. |
I think I should mention that XDF was created to complement LSL as a A C or C++ library for xdf import/export would be very nice indeed, in fact The only reason why the LabRecorder is in Python because it was the Christian On Wed, Sep 30, 2015 at 12:47 PM, David Medine [email protected]
|
To answer some more q's from the original email:
Christian On Wed, Sep 30, 2015 at 1:37 PM, Christian Kothe [email protected]
|
fyi: Unrelated to this thread I've spent a bit of time today to clean up On Wed, Sep 30, 2015 at 1:46 PM, Christian Kothe [email protected]
|
Just wrote a simple C program to dump xdf structure/metadata and extract a streams from an XDF file to a new XDF file. The goal was to work around the fact that EEGLAB plugin to only imports the first EEG stream in a file and also to understand/explore the XDF file format. |
Is there any progress on this topic ? |
Christian added an XDF import function that is implemented in python recently https://github.com/sccn/xdf/tree/master/Python. Does this help? |
Why is it not possible to integrate LabRecorder? |
@alexandrebarachant , I don't have a need for this anymore because I can now read xdf in Python so I haven't worked on it at all. Can you tell us a bit about your software platform so we can think about how the recording would need to be exposed in a lib? |
Importing is not the issue (I actually use the python code of this repository) For now, we use LabRecorder to record our data, but this is not satisfactory. Ideally we want to have our software (in python) to handle the recording process, and we really want to avoid third party software that can't be run in the background. Unless i missed something, we can only interact with LabRecorder through the GUI (more specifically, starting the recording can not be automated) I took a look at the Code from the RecorderLib and that would be exactly what we need, a function that take a bunch of LSL stream as input and write them in a file. |
I would be!
…On 08.03.2017 12:35, Clemens Brunner wrote:
We have added XDF support to SigViewer
<https://github.com/cbrnr/sigviewer> - the new release 0.6.0 will be
available by Friday. For this purpose, @Yida-Lin
<https://github.com/Yida-Lin> has written a small C++ library libxdf
<https://github.com/Yida-Lin/libxdf> that provides basic functionality
to read XDF files. Would you guys be interested in moving libxdf to
this repo?
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#3 (comment)>, or mute
the thread
<https://github.com/notifications/unsubscribe-auth/ADch7rMp35uxVAJ_G-EiShhXIpGa4F80ks5rjpJngaJpZM4GEBK4>.
|
Great! How should we proceed? @Yida-Lin and I need write permission for this repo, would that be OK? I'd be interested in that anyway since I'm planning to contribute more in the future. Before we move libxdf, would anyone of you guys be interested in a quick code review first? |
The normal GitHub way to do it would be to fork this repo, make your changes in your own fork, then submit a pull request. The pull request interface has some code review features. Even people with write permissions usually make their changes via pull request so others may approve it before it gets merged in. So fork in GitHub. |
Sure, but it would be nice to retain some control over libxdf after the PR gets merged. What I mean is that ideally we would be in the collaborators group for this repo so that the original author @Yida-Lin decides which new PRs related to libxdf to merge, don't you think? |
I don't know of any formal rules, but in my experience people aren't invited to become a collaborator on a repo without first contributing. This serves to dissuade trolls from claiming to have a great contribution only to do severe damage to the repo once given access. However, I recognize your name and -- assuming the account belongs to the person with the same name -- I think you've already met this barrier to entry. But I'm not in charge. While I'm sure no one wants extra responsibility and we wouldn't be interested in taking over libxdf unnecessarily, I can imagine a situation where e.g. a bug is found or someone wants to change the build system to make it easier to make a Python wrapper around libxdf, but @Yida-Lin has since moved on and is unresponsive. What do we do then? If you want to retain sole responsibility of libxdf then adding it as a submodule is an option. |
Yes, it's me 😄. I'm not saying we want exclusive control over libxdf. Once it's in there, all collaborators should be able to perform merges in that folder. It's just that it might turn out to be frustrating if we can't merge tiny changes ourselves, but I guess this is up to @Yida-Lin to decide (I wonder why he hasn't been added to this conversation since we mentioned him a couple of times). I'll write him an email and ask him to comment. |
Hi Guys!! Thanks so much for discussing the options. I mostly agree with Clemens' opinion, but if it's too complicated we also have another option to simply put a hyperlink in README directing to libxdf. |
Any update on this? The C++ library is now fully integrated in SigViewer and working nicely. @Yida-Lin would contribute it to this repo. Should he go ahead and make a PR? The main repo would remain at the current location, and whenever @Yida-Lin has accumulated enough updates he could make another PR to update this repo. What do you think? |
@tstenner and I looked at @Yida-Lin 's PR and we agreed that it doesn't quite fit with what we thought our requirements were, but this comment alone isn't very helpful. Unfortunately we haven't had time to dive in and provide the detailed feedback it deserves. We also thought it wasn't our place to approve/deny unilaterally this kind of contribution when there are other stakeholders. We have an ongoing discussion of what a RecorderLib should look like. The resolution of this discussion will determine somewhat what the xdf-writing requirements of libxdf would look like. I invite any interested parties to please hop over to that discussion and give their input. |
Even though I also don't have the time for an extensive review, I'd like to list some of the points I'd like in an "official" xdf reader library:
Everything else could then be built on top of that library (transparent conversion to either float/string samples, resampling, timestamp interpolation, ...). |
Indeed, libxdf by @Yida-Lin is tailored towards SigViewer, which means that it might not be ideal for a generic XDF reader library. However, I'm not even sure if a C library is that important, since (1) everything @tstenner mentioned is already implemented in the Python reader, and (2) deploying C libraries with e.g. Python is always a pain (by which I mean it's of course doable, but requires some effort in comparison to a pure Python package). It would be interesting to learn which program languages people are using to work with XDF files. |
LSL currently has a LabRecorder Python application that depends on a C library called
RecorderLib
. RecorderLib is not much more than a wrapper around recording.h that does all the heavy lifting in creating and writing to an XDF file, including making extensive use of Boost string handling and threading. Also worth mentioning, RecorderLib has proven to be a bit of a headache to maintain in its current form and there have been several mentions of moving it entirely to Python. After all, LabRecorder (a Python application) is the only program that actually uses RecorderLib.It might be better to create a C(++) library for XDF (xdflib or libxdf), and some example applications that link to
xdflib.(lib|dll|dylib|so)
. To this end, it might be useful to look at how EDF achieved this objective.With a C library available, it could then be possible to make a relatively simple Python Ctypes wrapper, and then LabRecorder would import this wrapper. This would eliminate the need for LSL's RecorderLib. Even better, it will make it easier for us to import XDF files into Python for data analysis.
I am not sure, but it is possible that the Matlab interface that exists in this repository and is duplicated in LSL might also benefit from having a generic C library, and this library might also facilitate adding XDF writing capability to Matlab.
One question worth considering before work on this gets started is whether such a C library for XDF should exist here or in the LSL repository. One benefit to keeping it here is that anyone stumbling upon XDF will have easy access to some examples on how to read/write data files without the overhead of trying to understand LSL. One benefit to keeping it with LSL is that, because LSL already depends on Boost, it will be trivial to reuse much of
recording.h
and no extra dependencies will be added.I'm willing to contribute toward this effort either way, but I would like a small discussion from @chkothe , @dmedine and @sccn first so I know which approach will be most likely to have the greatest benefit.
Some questions I have:
If the answer to the above two questions are both 'no', then XDF-write might be able to stay with LSL, and XDF-read can stay here and be made without Boost.
Edit: Update link to recording.h
The text was updated successfully, but these errors were encountered: