Open nolan-veed opened 3 months ago
@gavv Can I get some thoughts on approach?
:robot: The latest upstream change made this pull request unmergeable. Please resolve the merge conflicts.
Thanks, will take a look in upcoming days!
I think the conflict is from this commit from here.
Also: d873794b234b49a32a4e19ad2be37354957d90bd
Hi, sorry for delay. I like the approach!
A few minor comments / thoughts:
Instead of a big Configs
struct with all the things it maybe would be better to pass every argument explicitly.
This way it would be clear what variables are used by what functions, and so easier to understand the flow.
All functions are named setup_xxx()
, but actually some of them do different things.
Some build configs, some open devices, some configure endpoints. I suggest to reflect that in their names, again, it should make it easier to understand the flow in main().
It would be nice if we could create all top-level objects in main().
It seems currently the only exception is input_source
, which we create in separate function. We could keep endpoint parsing in a function, but move construction of input source to main().
The piece where we propagate parameters from io_config
to sender_config
can be extracted into a small function too.
This way this step will be reflected in main() more explicitly and main() will be less cluttered.
If you'll decide to proceed with this PR, I think it's good idea to review & merge just one tool (e.g. roc_send) and then work on the rest, because we can find more pitfalls during review.
Why
For https://github.com/roc-streaming/roc-toolkit/issues/656
What
Testing
WIP