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

[depends] remove rtmp as it's included through ffmpeg or as binary addon #11139

Merged
merged 1 commit into from Dec 12, 2016

Conversation

@MartijnKaijser
Copy link
Member

commented Dec 11, 2016

Untested and needs reviewing if something was missed or removed to much

@FernetMenta

This comment has been minimized.

Copy link
Member

commented Dec 11, 2016

last info from @notspiff was that the addon currently does not bring librtmp. but I might be wrong

@akva2

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2016

Has since been remedied

@stefansaraev

This comment has been minimized.

Copy link
Contributor

commented Dec 11, 2016

@MartijnKaijser MartijnKaijser force-pushed the MartijnKaijser:rtmp_cleanup branch from de29433 to 1e8d05e Dec 11, 2016

@MartijnKaijser

This comment has been minimized.

Copy link
Member Author

commented Dec 11, 2016

updated.
Is the change in the curl makefile correct?

@wsnipex

This comment has been minimized.

Copy link
Member

commented Dec 11, 2016

looks good. jenkins build this please

@MartijnKaijser MartijnKaijser force-pushed the MartijnKaijser:rtmp_cleanup branch from 1e8d05e to 5c8e532 Dec 12, 2016

@MartijnKaijser MartijnKaijser added this to the L 18.0-alpha1 milestone Dec 12, 2016

@MartijnKaijser

This comment has been minimized.

Copy link
Member Author

commented Dec 12, 2016

@wsnipex jenkins build was fine. Rebased it due to conflict on other merge.
Your button

@wsnipex wsnipex merged commit 7ba602c into xbmc:master Dec 12, 2016

2 checks passed

continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details

@MartijnKaijser MartijnKaijser deleted the MartijnKaijser:rtmp_cleanup branch Dec 29, 2016

@FernetMenta

This comment has been minimized.

Copy link
Member

commented on tools/depends/target/curl/Makefile in 5c8e532 Dec 30, 2016

@MartijnKaijser I don't think this was your intent, right? this PR is about removing rtmp, not enabling it.

This comment has been minimized.

Copy link
Member Author

replied Dec 30, 2016

@FernetMenta it was for removing it. Since I wasn't sure I asked in the PR if this was indeed correct for removing it.
All we need is internal ffmpeg rtmp or the Addon.

edit:
see #11139 (comment)

This comment has been minimized.

Copy link
Member

replied Dec 30, 2016

right, so we should disable rtmp here with the --without-rtmp option. otherwise curl's automake will enable it if present

This comment has been minimized.

Copy link
Contributor

replied Dec 30, 2016

but it is not present anymore. or does kodi depend builds somehow use system libs / pkg-config files outside of depends sysroot ?

This comment has been minimized.

Copy link
Member

replied Dec 30, 2016

Depends if you completely clear your builds. On my OSX dev system librtmp is still present where depends were installed. As a result curl was built with rtmp.
IMO the simplest approach is always the best: We don't want rtmp, so explicitly disable it.

This comment has been minimized.

Copy link
Member Author

replied Dec 30, 2016

See #11314

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.