-
Notifications
You must be signed in to change notification settings - Fork 39
Fix bitstream dependency and netmap in new build system #1107
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
base: master
Are you sure you want to change the base?
Conversation
nto
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thank you for fixing this. Perhaps netmap should be added to the CI to avoid any such breakage in the future?
| have_upipe_id3v2_decaps = $(have_bitstream) | ||
| have_upipe_rtcp = $(have_bitstream) | ||
| have_upipe_rtpd = $(have_bitstream) | ||
| have_upipe_rtp_anc_unpack = $(have_bitstream) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Can you change the other lines to keep the "=" alignment?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Do you want me to amend or make a whitespace only commit?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
As for adding netmap to the CI: you, we, I can consider it. I think it only needs 4 headers in an include path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sure. Do you want me to amend or make a whitespace only commit?
You can amend your current commit. BTW, can you rephrase your commit messages in imperative?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
You can use a more direct style, as commit titles should avoid taking much more than 50 characters. Eg:
upipe_rtp_anc_unpack: add missing bitstream dependency
upipe-netmap: fix netmap config check
upipe-netmap: add missing bitstream dependency
I experimented with the new build system recently and found a few problems
I fixed them in the way that seemed right considering other parts of the new system.