LabRecorder: Use std::thread instead of Boost.Thread, QFile instead of Boost.Filesystem

sccn / lsl_archived

Multi-modal time-synched data transmission over local network

242 stars 134 forks source link

LabRecorder: Use std::thread instead of Boost.Thread, QFile instead of Boost.Filesystem #305

Closed tstenner closed 5 years ago

tstenner commented 6 years ago

Finding and linking to Boost is a major hassle on Windows, so this PR uses Qt (which is required anyway) and std::thread ~so only Boost headers have to be installed~.

In case anyone wants to test it without compiling it first, CI-builds are available for OS X, Windows and Linux.

Things to be done:

[X] port Boost.Thread to std::thread
[X] port Boost.PropertyTree to QSettings
[X] port Boost.Filesystem to QFile
[x] move big-endian / non-iec-559-support to a separate header
[x] detect big-endian / non-iec-559 architectures at build time
[x] add a testcase for binary file writing code

mgrivich commented 6 years ago

I approve of reducing dependence on boost and moving to standard C++ whenever possible. I'm not a huge fan of QT either, but as we are already using QT, there is nothing lost there.

dmedine commented 6 years ago

I agree with Matthew. Standard C++ is always best whenever possible. I don't object to Qt, but if we still need the property tree libraries, we still need boost.

I'll test this later this week and then we can merge it.

On 06/18/2018 07:03 PM, Matthew Grivich wrote:

I approve of reducing dependence on boost and moving to standard C++ whenever possible. I'm not a huge fan of QT either, but as we are already using QT, there is nothing lost there.

— You are receiving this because your review was requested. Reply to this email directly, view it on GitHub https://github.com/sccn/labstreaminglayer/pull/305#issuecomment-398125234, or mute the thread https://github.com/notifications/unsubscribe-auth/ADch7j6A6JuaGs9EV8J_5juEt5RBn3O1ks5t9913gaJpZM4Ura52.

tstenner commented 6 years ago

The property tree usage can be replaced almost perfectly (the comment char changes from # to ;), so the only remaining boost parts are fpclassify (which is in standard C++) and endian conversion (which should only be necessary on weird non-iec559-architectures).

tstenner commented 6 years ago

I've taken another look. We don't need Boost if the target architecture is little-endian, uses IEC559-floats and gzip support isn't needed. I've added a checklist to the PR for things to be done.

tstenner commented 5 years ago

I've factored out the XDF writer code and added a testcase for it (e025451b60c1974ee8582162fa9072acf897f61d).

I've also uploaded the Windows build and the generated sample XDF file.