App-LabRecorder icon indicating copy to clipboard operation
App-LabRecorder copied to clipboard

LabRecorder from version 1.16.0 not working with Presentation LSL streams

Open akreilinger opened this issue 3 years ago • 2 comments

Hi all,

after a recent upgrade from LabRecorder 1.14.2 to 1.16.2, I can no longer record LSL marker streams from NeuroBS Presentation. Instead, I get the following error message immediately when trying to start a recording that contains an LSL stream from Presentation:

2022-09-16 11:23:30.997 ( 4.704s) [R_Presentation ] data_receiver.cpp:342 ERR| Stream transmission broke off (The byte order conversion requested by the other party is not supported.); re-connecting...

The message just keeps on repeating and no values are actually saved if I send LSL markers. For the error to appear, I do not have to send anything. It’s enough to just have the LSL stream being available in the network.

The problem appears with "LabRecorder-1.16.2-Win_amd64.zip", but I had the same effect with "LabRecorder-1.16.0-Win_amd64.zip"; The 32-bit release "LabRecorder-1.16.2-Win_i386.zip" works, as do versions before 1.16.0, e.g., tested with 1.14.2.

I am using Presentation 23.0, but it also happens with 22.0 and I did not test it with any older versions.

Best regards Alex

akreilinger avatar Sep 16 '22 09:09 akreilinger

Could you create a packetdump during the initial LSL connection with wireshark and the capture filter portrange 16571-16581?

tstenner avatar Sep 26 '22 13:09 tstenner

Hi Tristan,

thank you very much for your response.

I would be happy to send you the packetdump, but I would rather not put it online. Would you mind contacting me via email, so I can send you the file? The address is [email protected]

Best regards Alex

akreilinger avatar Sep 27 '22 14:09 akreilinger

Presentation downgrades the data protocol to the old format (100):

Request:

LSL:streamfeed/110 33a94754-a8ce-44e5-8f1d-54be42ed6366
Native-Byte-Order: 1234
Endian-Performance: 5.64634e+06
Has-IEEE754-Floats: 1
Supports-Subnormals: 0
Value-Size: 32
Data-Protocol-Version: 110
Max-Buffer-Length: 36000
Max-Chunk-Length: 0
Hostname: BP-LP-1010
Source-Id: Presentation on BP-LP-1010
Session-Id: default

Response:

LSL/110 200 OK
UID: 33a94754-a8ce-44e5-8f1d-54be42ed6366
Byte-Order: 0
Suppress-Subnormals: 0
Data-Protocol-Version: 100
...

This is probably because the client reports the Value-Size as 32, which is both architecture + implementation-dependent and not relevant to the protocol at all.

I fixed it in the outlet code slightly over a year ago (a571262fe7c1cb4ead20dec1d9713f01b8db51f2), but unless NeuroBS ship a liblsl 1.16 or you replace the liblsl.dll in Presentation there's not much to fix it on the inlet side. Switching to 32bit LabRecorder works because the string object is the same size the Presentation liblsl expects, but that's mostly coincidentally.

tstenner avatar Oct 30 '22 18:10 tstenner

Alex alerted me to this, and I (LSL programmer at Neurobehavioral Systems) have been investigating. Am I reading this correctly that Data-Protocol-Version: 100 cannot, in general, be mixed with Data-Protocol-Version: 110?

I'm having an issue in which I can update the current version of LSL in all applications to 1.16, but then if anyone is running old code it won't be compatible with our applications.

Specifically LSL 1.13 -> LSL 1.13 fine LSL 1.16 -> LSL 1.16 fine LSL 1.16 -> LSL 1.13 fine LSL 1.13 -> LSL 1.16 not fine

For all of these, it is a string type message that I'm testing, and the sender is 32 bit, while the receiver is 64 bit.

Is there some sort of code change we can do to make this last one work? Or do I need to support two versions of the library going forward?

mgrivich avatar Nov 11 '22 23:11 mgrivich

P.S. Commenting out the "the byte order conversion requested by the other party is not supported" exception seems to work in this one case, but I don't know what the unintended consequences of that would be.

mgrivich avatar Nov 11 '22 23:11 mgrivich

At the moment I have no environment to test it, but it should be a simple fix (branch, binaries in the CI artifacts):

diff --git a/src/data_receiver.cpp b/src/data_receiver.cpp
index 27fdc145..af4ea423 100644
--- a/src/data_receiver.cpp
+++ b/src/data_receiver.cpp
@@ -227,6 +227,8 @@ void data_receiver::data_thread() {
                                                        // get the header information
                                                        if (type == "byte-order") {
                                                                int use_byte_order = std::stoi(rest);
+                                                               // needed for interoperability with liblsl ~1.13 and data protocol 100
+                                                               if(use_byte_order == 0) use_byte_order = LSL_BYTE_ORDER;
                                                                auto value_size = format_sizes[conn_.type_info().channel_format()];
                                                                if (!lsl::can_convert_endian(use_byte_order, value_size))
                                                                        throw std::runtime_error(

On the other end, I have backported the previously mentioned bugfix to the 1.13 branch.

tstenner avatar Dec 03 '22 10:12 tstenner

In my tests, this fixes the LSL 1.13 (without the backport) -> LSL 1.16 issue.

mgrivich avatar Dec 07 '22 01:12 mgrivich

Great, so hopefully 1.16.0 <-> 1.13.2 as well as 1.16.2 <-> 1.13.1 will work. Time to push out two new point releases and wait for everyone to update.

tstenner avatar Dec 07 '22 12:12 tstenner

Hold off a bit on pushing out releases. There is another, unrelated issue that I'm working on.

mgrivich avatar Dec 07 '22 14:12 mgrivich

Here's the new issue: https://github.com/sccn/labstreaminglayer/issues/105

mgrivich avatar Dec 07 '22 23:12 mgrivich

Just confirming that LSL-LabRecorder-1.16.3-Win_amd64 still produces this error with Presentation 23.1. But LSL-LabRecorder-1.14.2-Win_amd64 still works OK.

mcfarla9 avatar Mar 17 '23 18:03 mcfarla9

Another report here: https://github.com/sccn/liblsl/issues/185

cboulay avatar Apr 09 '23 14:04 cboulay

Could you please merge this into master?

mgrivich avatar May 27 '23 00:05 mgrivich

@mgrivich , just to be clear, are you requesting a new release of LabRecorder using the latest liblsl that contains some fixes you were a part of? Or was there a specific PR in LabRecorder related to this issue?

cboulay avatar May 27 '23 01:05 cboulay

@tstenner's comment from Dec 3, 2022 above needs to be pulled to master for liblsl. It should be deployed to all release products as well, but you can do that on your own timeline.

mgrivich avatar May 27 '23 01:05 mgrivich

Should now be fixed in LabRecorder 1.16.4 https://github.com/labstreaminglayer/App-LabRecorder/releases/tag/v1.16.4

Please let me know if the issue persists and we can re-open.

cboulay avatar May 28 '23 16:05 cboulay