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

feat(livestream): add separate fields for stream URL and stream key #13306

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

Conversation

roanta2
Copy link
Contributor

@roanta2 roanta2 commented May 2, 2023

Add separate fields for Stream URL( will appear pre-typed with default yt rtmp) and Stream key, similar to how it is presented in the youtube dashboard. This also makes it more obvious that you can use other stream URLs rather than the youtube one. Should be merged before this PR on jibri: jitsi/jibri#508 that supports these changes.

Screenshot 2023-04-27 at 18 51 29

@jitsi-jenkins
Copy link

Hi, thanks for your contribution!
If you haven't already done so, could you please make sure you sign our CLA (https://jitsi.org/icla for individuals and https://jitsi.org/ccla for corporations)? We would unfortunately be unable to merge your patch unless we have that piece :(.

@roanta2 roanta2 requested a review from saghul May 2, 2023 09:11
if (!key) {
return false;
}

const rtmpURL = base.endsWith('/') ? base + key : `${base}/${key}`;
Copy link
Member

Choose a reason for hiding this comment

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

We should ensure we preserve the functionality of providing a full RTMP URL instead of base + key, for services which may not have the key as the final part of the path.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sure, good point. I made the key field optional, so you can input the full RTMP URL in the first field if needed.

@roanta2 roanta2 force-pushed the roanta/livestream-add-field branch from a4f2852 to 38ab95a Compare May 15, 2023 11:59
@horymury horymury self-requested a review May 15, 2023 12:24
@horymury horymury dismissed their stale review May 15, 2023 12:25

accidental

@github-actions
Copy link

This PR has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@github-actions github-actions bot added the stale label Aug 14, 2023
@saghul saghul added confirmed Confirmed bug, should not go stale and removed stale labels Aug 14, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
confirmed Confirmed bug, should not go stale
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants