roc-streaming / roc-toolkit

Real-time audio streaming over the network.
https://roc-streaming.org
Mozilla Public License 2.0
1.02k stars 203 forks source link

Factories rework #663

Closed nolan-veed closed 1 month ago

nolan-veed commented 6 months ago

Why

For https://github.com/roc-streaming/roc-toolkit/issues/610

This is for the factories rework.

What

Testing

WIP

github-actions[bot] commented 6 months ago

:robot: Upon creation, pull request description does not have a link to an issue. If there is a related issue, please add it to the description using any of the supported formats.

gavv commented 1 month ago

Hi!

I'm currently working on PLC API (#305), and I need to implement #697 for it, which partially overlaps with this PR.

If you don't mind, I'm going to cherry pick & finish changes made here to my branch, and after it's merged you can proceed with this task when/if you wish.

github-actions[bot] commented 1 month ago

:robot: The latest upstream change made this pull request unmergeable. Please resolve the merge conflicts.

gavv commented 1 month ago

Done, see these 3 commits:

(especially the last one)

I think this part of the task is done now:

factories

  • PacketFactory, BufferFactory: accept IPool instead of IArena
  • node::Context: instantiate pools instead of factories
  • roc_pipeline: pass pools instead of factories
  • roc_pipeline: SenderSession, ReceiverSession, etc: instantiate factories from pools where they're needed

Note that BufferFactory was replaced with FrameFactory, see commit messages for details.

In roc_pipeline, factories are currently created in ReceiverSource and SenderSink and passed to lower-level components like ReceiverSession and SenderSession. For implementing per-session limits, we may need to pass pools instead, but that would be an easy change isolated in roc_pipeline.

gavv commented 1 month ago

I think we can close this PR? Did I miss anything?

gavv commented 1 month ago

Closing, feel free to ping me if I missed anything.

nolan-veed commented 1 month ago

Closing, feel free to ping me if I missed anything.

No prob. Thanks @gavv for bearing with me.

gavv commented 1 month ago

No worries!