Read all channels of multi-channel string streams#58
Conversation
The [Samples] chunk handler reads strings one length-prefixed value per sample, but a string sample actually contains channel_count values, just like every numeric branch (which already loops `for v < channel_count`). For a string stream with more than one channel this reads only the first channel and leaves the remaining channels' bytes unconsumed, so the file cursor is stranded mid-chunk. libxdf then interprets those leftover bytes as the next chunk header, producing a garbage length and breaking the import (crash or corrupt read). Fix: loop over channel_count and read one length-prefixed string per channel, emplacing each into eventMap at the same time stamp. This both restores correct byte alignment and captures every channel's marker. Closes xdf-modules#19. This is the C++/libxdf manifestation of the same root cause we fixed in the Julia reader XDF.jl (cbrnr/XDF.jl#23): the string-reading path was missing the per-channel inner loop present in the numeric paths, so a two-channel string marker stream (e.g. the twochannel_string_marker.xdf example file with "Marker 0A"/"Marker 0B") was misread. pyxdf already handles this correctly; this brings libxdf to parity. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
|
Hi I'm not familiar with the codebase: I'd be thankful for a code review. We had a similar issue in XDF.jl, and my before and after local tests confirm a correct reading of test/data/twochannel_string_marker.xdf. |
|
I can take a look in a couple of days, thanks for the PR! |
cbrnr
left a comment
There was a problem hiding this comment.
This looks good to me! However, I would like to test if this works in SigViewer. Can you try to open the problematic file with the current version and with a version which includes this patched libxdf?
|
Hi @cbrnr, |
|
Thanks @abcsds, I will merge this PR, make a new libxdf release, and then make a new SigViewer release. |
The [Samples] chunk handler reads strings one length-prefixed value per sample, but a string sample actually contains channel_count values, just like every numeric branch (which already loops
for v < channel_count). For a string stream with more than one channel this reads only the first channel and leaves the remaining channels' bytes unconsumed, so the file cursor is stranded mid-chunk. libxdf then interprets those leftover bytes as the next chunk header, producing a garbage length and breaking the import (crash or corrupt read).Fix: loop over channel_count and read one length-prefixed string per channel, emplacing each into eventMap at the same time stamp. This both restores correct byte alignment and captures every channel's marker.
Closes #19.
This is the C++/libxdf manifestation of the same root cause we fixed in the Julia reader XDF.jl (cbrnr/XDF.jl#23): the string-reading path was missing the per-channel inner loop present in the numeric paths, so a two-channel string marker stream (e.g. the twochannel_string_marker.xdf example file with "Marker 0A"/"Marker 0B") was misread. pyxdf already handles this correctly; this brings libxdf to parity.