Read all channels of multi-channel string streams#58
Open
abcsds wants to merge 1 commit into
Open
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>
Author
|
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. |
Collaborator
|
I can take a look in a couple of days, thanks for the PR! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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.