Skip to content

Remove libsrt dependency and integrate compilation into build process #1

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

Open
wants to merge 13 commits into
base: master
Choose a base branch
from

Conversation

aaron-jencks
Copy link
Collaborator

Uses golang c bindings to integrate the srt library directly into the srtgo library so that libsrt is no longer required.

@aaron-jencks aaron-jencks requested a review from a team July 17, 2023 20:57
@aaron-jencks aaron-jencks self-assigned this Jul 17, 2023
Copy link

@gkeiser gkeiser left a comment

Choose a reason for hiding this comment

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

I see a lot of C++, is there somewhere in particular I should look to see where you integrate it into build process?

@aaron-jencks
Copy link
Collaborator Author

aaron-jencks commented Jul 17, 2023

I see a lot of C++, is there somewhere in particular I should look to see where you integrate it into build process?

you have to look in the .go files, you can see the #cgo lines are modified/added and the import paths are modified

most of the C++ and header files are just copied directly from the srt repo

Co-authored-by: Grant Keiser <[email protected]>
@aaron-jencks aaron-jencks requested a review from gkeiser July 17, 2023 21:59
Copy link

@gkeiser gkeiser left a comment

Choose a reason for hiding this comment

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

My machine is missing srt.h so I cannot build. Should there be something in the readme about where/how to get set up for development/building?

@aaron-jencks
Copy link
Collaborator Author

you shouldn't need anything in order to build this, except libssl and libcrypto

@aaron-jencks aaron-jencks requested a review from gkeiser July 17, 2023 22:08
@bdudley-syncbak
Copy link

I'm having trouble seeing the advantage of this. The last commit states a libssl-dev and libcrypto-dev dependency. Why not add an srt dependency and eliminate the need for all of this? We did this initially in circleci and on the syncbox and catcher.

One of the motivations for switching from UDT to SRT was UDT was no longer maintained and building it was a PITA. How will updates to srtgo be managed? Won't this just become another PITA too?

@gkeiser
Copy link

gkeiser commented Jul 18, 2023

This does seem like it sets us up for an extremely difficult upgrade path.

@aaron-jencks
Copy link
Collaborator Author

I discussed with Sam and he still sees the worth of doing this, as opposed to having everybody install/build libsrt

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

Successfully merging this pull request may close these issues.

4 participants