xdf-modules / xdf-Matlab

Matlab code for working with xdf files.
BSD 2-Clause "Simplified" License
11 stars 14 forks source link

Stream times are not synchronized #13

Closed dmedine closed 2 years ago

dmedine commented 2 years ago

Original text from @krigolson:

Hi, I have loaded some EEG data that has event markers in it - and the time stamps appear to be not synchronized so when I load the data into EEGLAB the marker latencies are all the same and 1. I am using the latest version of all the software and note sure what is going wrong here. HELP!

The first stream was EEG data from a Muse Headband and the second stream was from MATLAB as a marker stream.

moved from: https://github.com/sccn/xdf/issues/59

cboulay commented 2 years ago

@krigolson, based on your description (in the original issue), I would guess that the Matlab script that pushes the marker events is misusing the API. If you can share that portion of the script with us then we can probably identify the problem quickly.

krigolson commented 2 years ago

Will send the script - the team we are supporting is sending the code. I have sample data but cannot upload it here.

krigolson commented 2 years ago

Hi,

Here is how we initialize the stream:

lib = lsl_loadlib(); info = lsl_streaminfo(lib,'Markers','Markers',1,0,'cf_string', 'myuniquesourceid23443'); outlet = lsl_outlet(info);

And how we send markers:

outlet.push_sample({num2str(marker)},0);

This used to work for us, but now seems not too which is weird.

Thank you for your assistance.

On Thu, Jun 2, 2022 at 5:23 PM Chadwick Boulay @.***> wrote:

@krigolson https://github.com/krigolson, based on your description (in the original issue), I would guess that the Matlab script that pushes the marker events is misusing the API. If you can share that portion of the script with us then we can probably identify the problem quickly.

— Reply to this email directly, view it on GitHub https://github.com/xdf-modules/xdf-Matlab/issues/13#issuecomment-1145467710, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKRSBS4B6W3ANXYBTSBYYYDVNFF7ZANCNFSM5XWXXTOQ . You are receiving this because you were mentioned.Message ID: @.***>

cboulay commented 2 years ago

That looks OK to me. You can choose to not provide the ,0 but that's the default so no change there.

So the problem is either in the Muse integration (i.e. it is providing incorrect timestamps) or in EEGLAB's importer. I think we'll need to take a look at a problematic xdf file to be sure.

krigolson commented 2 years ago

Here is the sample file.

On Fri, Jun 3, 2022 at 12:27 PM Chadwick Boulay @.***> wrote:

That looks OK to me. You can choose to not provide the ,0 but that's the default so no change there.

So the problem is either in the Muse integration (i.e. it is providing incorrect timestamps) or in EEGLAB's importer. I think we'll need to take a look at a problematic xdf file to be sure.

— Reply to this email directly, view it on GitHub https://github.com/xdf-modules/xdf-Matlab/issues/13#issuecomment-1146291940, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKRSBSYHLWCWDXPIHTPAZULVNJMCNANCNFSM5XWXXTOQ . You are receiving this because you were mentioned.Message ID: @.***>

dmedine commented 2 years ago

Is it possible that a new Matlab release has futzed the type marshalling and that providing a 0 timestamp argument is not behaving as expected? This should be the same as providing no second argument, which I recommend you try. I don't have time to test today, but I will try soon. I haven't updated Matlab for a while so I am still on 2020b. Which version are you using? To be clear, providing a 0 as the second argument in push_sample should tell the outlet to give the sample the default timestamp. Honestly, though, I would be surprised if this is the issue.

There doesn't appear to be any xdf file---attaching it to an email reply to this list won't work, by the way. Maybe host it on a website/github/google drive and provide a link?

dmedine commented 2 years ago

I just tested @krigolson's code for sending markers on a fresh build of liblsl-Matlab with the latest version of lsl (1.16.0) and the timestamps looked to be correct. Certainly (no surprise) the type-marshalling theory is out.

A sample faulty xdf file that can be downloaded would be most helpful here.

krigolson commented 2 years ago

Here you go.

On Mon, Jun 6, 2022 at 5:23 PM David Medine @.***> wrote:

I just tested @krigolson https://github.com/krigolson's code for sending markers on a fresh build of liblsl-Matlab with the latest version of lsl (1.16.0) and the timestamps looked to be correct. Certainly (no surprise) the type-marshalling theory is out.

A sample faulty xdf file that can be downloaded would be most helpful here.

— Reply to this email directly, view it on GitHub https://github.com/xdf-modules/xdf-Matlab/issues/13#issuecomment-1148062806, or unsubscribe https://github.com/notifications/unsubscribe-auth/AKRSBS3QYEKSNWBZWADAPFLVN2I6ZANCNFSM5XWXXTOQ . You are receiving this because you were mentioned.Message ID: @.***>

andraderenew commented 2 years ago

I have this error while trying to run 'build_mex.m'. I don't know if this is related to this issue because I still have lsl 1.16.0 installed with Homebrew and I still have all the markers at the beginning.

error: /Applications/Xcode.app/Contents/Developer/Toolchains/XcodeDefault.xctoolchain/usr/bin/install_name_tool: for: lslloadlib.mexmaci64 (for architecture x86_64) option "-add_rpath @loader_path/" would duplicate path, file already has LC_RPATH for: @loader_path/

cboulay commented 2 years ago

@andraderenew , can you please comment out those lines 40-42 in build_mex.m and try again?

Also, if you're up to it, it would be good to get a Mac user with Matlab to test modifying this line: https://github.com/labstreaminglayer/liblsl-Matlab/blob/master/lsl_get_dll.m#L55 Basically we need it to find the liblsl.dylib that gets installed with hombrew.

dmedine commented 2 years ago

So this issue is still in the wrong place.

The error from install_name_tool, as it says, comes when the path specified is already decorated on the library. You can see what these paths are by hitting

> otool -L <path_to/liblsl-Matlab/bin>/lsl_loadlib_.mexmaci64

If the install_name_tool is making this complaint, it means that Matlab has now (since 202??) updated its mex function for OSX to decorate the mex files with @loader_path automatically---at least that my deduction from what @andraderenew reports. That would mean that the line https://github.com/labstreaminglayer/liblsl-Matlab/blob/8fe8431b30c12bdde67a2277b5ac27a2fea4931c/build_mex.m#L41 needs to be conditional somehow.

However, this decoration is used to help Matlab load lsl_loadlib, not how lsl_loadlib loads LSL and therefor I don't believe that your error has anything to do with Homebrew or lsl.dylib.

Whether or not any of this has anything to do with the incorrect marker timestamps is another question entirely. However, I find it quite telling that now 2 people are having basically the same problem with liblsl-Matlab on Mac. Unfortunately I don't have the hardware or software to diagnose this, but it looks like an update broke something. It is working just fine on Windows at the moment (at least on Matlab 2020b). I'd be interested to know if any liblsl-Matlab users on Linux are also experiencing this issue. In the meantime, I will update my Matlab installation and see if it still works on Windows.

andraderenew commented 2 years ago

I can try in linux matlab 2020b. Also I will try with windows.

Sent from my iPhone

On 24 Jun 2022, at 02:53, David Medine @.***> wrote:

 So this issue is still in the wrong place.

The error from install_name_tool, as it says, comes when the path specified is already decorated on the library. You can see what these paths are by hitting

otool -L <path_to/liblsl-Matlab/bin>/lslloadlib.mexmaci64

If the install_name_tool is making this complaint, it means that Matlab has now (since 202??) updated its mex function for OSX to decorate the mex files with @loader_path automatically---at least that my deduction from what @andraderenew reports. That would mean that the line https://github.com/labstreaminglayer/liblsl-Matlab/blob/8fe8431b30c12bdde67a2277b5ac27a2fea4931c/build_mex.m#L41 needs to be conditional somehow.

However, this decoration is used to help Matlab load lsl_loadlib, not how lsl_loadlib loads LSL and therefor I don't believe that your error has anything to do with Homebrew or lsl.dylib.

Whether or not any of this has anything to do with the incorrect marker timestamps is another question entirely. However, I find it quite telling that now 2 people are having basically the same problem with liblsl-Matlab on Mac. Unfortunately I don't have the hardware or software to diagnose this, but it looks like an update broke something. It is working just fine on Windows at the moment (at least on Matlab 2020b). I'd be interested to know if any liblsl-Matlab users on Linux are also experiencing this issue.

— Reply to this email directly, view it on GitHub, or unsubscribe. You are receiving this because you were mentioned.

dmedine commented 2 years ago

Well, I am sure that Windows is working for 2020b. I still would like to try the newest Matlab, though. By the way, I reopened this on the liblsl-Matlab issues page: https://github.com/labstreaminglayer/liblsl-Matlab/issues/34. It's not over yet, but it I think it is clear that load_xdf.m is not the culprit here.

andraderenew commented 2 years ago

@andraderenew , can you please comment out those lines 40-42 in build_mex.m and try again?

Also, if you're up to it, it would be good to get a Mac user with Matlab to test modifying this line: https://github.com/labstreaminglayer/liblsl-Matlab/blob/master/lsl_get_dll.m#L55 Basically we need it to find the liblsl.dylib that gets installed with hombrew.

I can comment L40-42 and build_mex runs without a problem but error of marker at zero timepoint persists.