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

[librtmp] Fix debug build #2537

Merged
merged 1 commit into from
Jan 18, 2018
Merged

[librtmp] Fix debug build #2537

merged 1 commit into from
Jan 18, 2018

Conversation

TheStormN
Copy link
Contributor

Hello,

Sorry for that but it seems that the debug build is a bit broken - it is built successfully but it is not able to connect to an RTMP server. This PR is for the fix.

Sorry that I didn't saw that yesterday before submitting #2532

@TheStormN
Copy link
Contributor Author

TheStormN commented Jan 9, 2018

@ras0219-msft One more thing. I've tried several times to submit patches to the librtmp maintainers but no luck there, nobody looks at it anymore. If I create a fork here in GitHub will it be OK to update the port here with my repo?

@TheStormN TheStormN changed the title librtmp: Fix debug build [librtmp] Fix debug build Jan 9, 2018
@ras0219-msft
Copy link
Contributor

Sorry for the delay and thanks for the PR!

Here's my thinking about forking it: We want to use "maintained", "official" sources as much as possible to maximize the chance that if a user experiences a problem, they can report it/submit a PR/etc.

If you're not interested in actually "forking" librtmp and maintaining it over a longer term, then I think we should just check the patch in here (you might still want the github fork just for your own use, but we won't pull from it).

On the other hand, if you do want to maintain it over the long term, I think the right approach is still to check in the patch for now AND you maintain your GitHub fork. After the fork builds a history of maintenance, then I'd feel better about using it directly.

The problem that occurs when we use a non-official repo is if it goes down, there's no way we can recover -- we have no idea what patches were previously applied and it's very unlikely that there are any mirrors of the branch since it's unofficial.

Given this, is this PR ready to go?

@TheStormN
Copy link
Contributor Author

@ras0219-msft I understand your concerns and I'm completely agree with the terms about the fork.

About this PR, it's ready to go. :)

@ras0219-msft ras0219-msft merged commit 1b5ec0b into microsoft:master Jan 18, 2018
@ras0219-msft
Copy link
Contributor

Thanks!

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.

2 participants