Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add CACAO streams alongside data streams. #264

Draft
wants to merge 5 commits into
base: develop
Choose a base branch
from

Conversation

ehpor
Copy link
Collaborator

@ehpor ehpor commented Nov 7, 2024

This PR makes it possible to use CACAO streams as part of the service, natively distributed alongside catkit2 data streams. This simplifies the interface and makes cacao streams easier to access.

It makes data on CACAO processes visible on catkit2 services with a minimal interface. This allows catkit2 services to control a CACAO RTC. It does not however make catkit2 data streams available to CACAO processes.

There is no impact on the performance of catkit2 streams. @RemiSoummer

Todo:

  • Test.
  • Add error checks upon opening and closing streams.
  • Use pyMilk on the Python proxy instead of bare IMAGE objects.

@ehpor ehpor added the enhancement New feature or request label Nov 7, 2024
@ehpor ehpor self-assigned this Nov 7, 2024
@ehpor ehpor marked this pull request as draft November 7, 2024 21:37
@RemiSoummer
Copy link
Collaborator

Thanks for clarifying no impact on performance of the streams.

@a-sevin
Copy link
Collaborator

a-sevin commented Nov 10, 2024

Do you also want to handle milk's semaphores to wait and publish new data ?
It could be very useful to have the notion of slices (cnt1 for 3D buffer) to avoid data corruption.

@ehpor
Copy link
Collaborator Author

ehpor commented Nov 10, 2024

@a-sevin

Do you also want to handle milk's semaphores to wait and publish new data ?

Right now, this PR aims for very loose coupling: just distributing ImageStreamIO streams alongside proprerties, methods and data streams already in catkit2 services. Interacting with that ImageStreamIO stream would be purely via their library, so via the ImageStreamIO library itself (for C++ compatibility) or via ImageStreamIOWrap or pyMilk (for Python compatibility).

On the C++ side, the ServiceProxy returns an std::shared_ptr<IMAGE> object. On the Python side, it will be a IMAGE object via ImageStreamIOWrap, though as of yet, I'm unsure how to link that together without needing to compile ImageStreamIOWrap.cpp inside the library too. Chatgpt told me about some very interestring constructions there, that I want to explore.

All of this to say: CACAO would still own its image streams. Catkit2 would just simplify connecting to them inside a service, since you only need to write the filenames inside the service itself, and that attribute would be available to all of catkit2, already opened and ready.

I'm not burning my hands yet on full integration of CACAO streams into catkit2 via the same interface as DataStreams. It feels like they are too dissimilar to make that easy.

It could be very useful to have the notion of slices (cnt1 for 3D buffer) to avoid data corruption.

Yes. I'm still confused by what CACAO is using. I was under the impression that CACAO used ring buffers for all image streams too. But after talking to Vincent and Olivier, it seems that is not the case? While that gives obvious advantages in cache usage (the reason I was told/remember from our conversation), data corruption seems a real issue with that imo. Catkit2 uses ring buffers by default to avoid data corruption (unless the reader is so slow that the writer catches up to it in the ring buffer).

@ehpor ehpor force-pushed the feature/cacao_streams branch from 7fc0fac to ae80cfb Compare November 12, 2024 21:37
@a-sevin
Copy link
Collaborator

a-sevin commented Nov 13, 2024

OK ! Thanks for the clarification

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants