-
Notifications
You must be signed in to change notification settings - Fork 313
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
feat: refactor, remove hardcoded youtube rtmp url #508
base: master
Are you sure you want to change the base?
Conversation
Hi, thanks for your contribution! |
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## master #508 +/- ##
============================================
+ Coverage 48.05% 48.17% +0.11%
Complexity 174 174
============================================
Files 73 73
Lines 2110 2105 -5
Branches 202 199 -3
============================================
Hits 1014 1014
+ Misses 1026 1021 -5
Partials 70 70
Continue to review full report in Codecov by Sentry.
|
Others probably have a better memory of this than I do, but there's actually existing code that I believe is trying to support exactly this. If you look at this chunk you removed in your PR: val rtmpUrl = if (startIq.streamId.isRtmpUrl()) {
startIq.streamId
} else {
"$YOUTUBE_URL/${startIq.streamId}"
} That's allowing for the value passed as the |
Yep, so I moved this part in |
@roanta2 We'll need to modify Jitsi Meet too to use the full rtmp URL format. |
LGTM at a glance, but we need to wait to have Jitsi Meet ready before we can apply this. Can you start working on that @roanta2 ? |
@saghul Thanks. I'm mostly backend but it seems like a minor change in Jisti Meet, so I'll give it a try. |
Refactoring on livestream part to not treat youtube as a special case. So now you need to provide the entire rtmp url ( stream url + key).