Skip to content

fix: recording time threaded#613

Open
StevenJacobs61 wants to merge 1 commit into
mainfrom
fix/recording-time-threaded
Open

fix: recording time threaded#613
StevenJacobs61 wants to merge 1 commit into
mainfrom
fix/recording-time-threaded

Conversation

@StevenJacobs61
Copy link
Copy Markdown
Contributor

@StevenJacobs61 StevenJacobs61 commented May 12, 2026

Bugfixes

  • Fixes recording shutdown so each stream captures a precise stop cutoff sequence before teardown, avoiding data being recorded past the intended stop boundary.
  • Updates the threaded producer/shared-slot transport flow to honor that cutoff during async handoff and drain, improving stop-recording reliability for shared-memory video/data streams.
  • Cleans up producer/shared transport exports and sequence tracking so socket-sent, handed-off, and in-flight payload state stay consistent during recording stop/cleanup.

@StevenJacobs61 StevenJacobs61 added the version:patch non-breaking bug fixes; no input/output changes (except defaults) label May 12, 2026
@StevenJacobs61 StevenJacobs61 marked this pull request as ready for review May 12, 2026 14:05
@StevenJacobs61 StevenJacobs61 force-pushed the fix/recording-time-threaded branch from 65cf043 to c97fcdc Compare May 12, 2026 14:24
CougarTasker
CougarTasker reviewed May 12, 2026
View reviewed changes
Comment thread neuracore/data_daemon/communications_management/producer/producer_channel.py Outdated
"open_fixed_shared_slots": payload.model_dump(exclude_none=True),
},
)

Copy link
Copy Markdown
Member

@CougarTasker CougarTasker May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

nit: added these new lines, nice to keep diffs smaller if possible

Comment thread neuracore/data_daemon/communications_management/producer/producer_channel_message_sender.py Outdated
Comment thread neuracore/data_daemon/communications_management/producer/producer_channel_message_sender.py Outdated
Comment thread neuracore/data_daemon/communications_management/producer/producer_channel_message_sender.py Outdated
Comment on lines +197 to +198
if sequence_number >= self._next_sequence_number:
self._next_sequence_number = sequence_number + 1
Copy link
Copy Markdown
Member

@CougarTasker CougarTasker May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Question: how is this possible that you get a sequence number above the next sequence number?

Copy link
Copy Markdown
Contributor Author

@StevenJacobs61 StevenJacobs61 May 12, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This was defensive as there were two registries for the next sequence number, i have refactored to create one source of truth for this in the ProducerChannel so thast ownership is clear and not spread or implied.

Copy link
Copy Markdown
Member

@CougarTasker CougarTasker May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yeah the code it very defensive but in the worse way, if there is an issue it drops it silently. this is an inverant we should assert and fail loudly if this is incorrect

Copy link
Copy Markdown
Contributor Author

@StevenJacobs61 StevenJacobs61 May 13, 2026

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

"i have refactored to create one source of truth for this in the ProducerChannel so that ownership is clear and not spread or implied." as above

Comment thread neuracore/data_daemon/communications_management/shared_transport/registry.py Outdated
Comment thread neuracore/data_daemon/communications_management/shared_transport/registry.py Outdated
Comment thread neuracore/data_daemon/communications_management/shared_transport/registry.py Outdated
Comment thread neuracore/data_daemon/communications_management/shared_transport/shared_slot_transport.py Outdated
Comment thread neuracore/data_daemon/runtime.py Outdated
@StevenJacobs61 StevenJacobs61 force-pushed the fix/recording-time-threaded branch from c97fcdc to cd67bb8 Compare May 12, 2026 15:57
@StevenJacobs61 StevenJacobs61 force-pushed the fix/recording-time-threaded branch 2 times, most recently from 3a5beb1 to e866429 Compare May 12, 2026 16:48
@StevenJacobs61 StevenJacobs61 force-pushed the fix/recording-time-threaded branch from e866429 to f326ee0 Compare May 13, 2026 09:37
CougarTasker
CougarTasker approved these changes May 13, 2026
View reviewed changes
Copy link
Copy Markdown
Member

@CougarTasker CougarTasker left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 👍

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@CougarTasker CougarTasker CougarTasker approved these changes

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

version:patch non-breaking bug fixes; no input/output changes (except defaults)

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

2 participants

@StevenJacobs61 @CougarTasker