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

RFC 39: Service overhaul #15

Draft
wants to merge 71 commits into
base: master
Choose a base branch
from
Draft

RFC 39: Service overhaul #15

wants to merge 71 commits into from

Conversation

tytan652
Copy link
Owner

@tytan652 tytan652 commented Jul 19, 2023

This is made to allow people that want to view the implementation through the PR review system. The draft state has no meaning besides preventing the merge button to exist.

Description

This PR is related to obsproject/rfcs#39, it does not include:

  • changes about output settings which replace network settings
  • CI replacement for the services JSON checks

This PR overhaul services to replace the unique rtmp-common/rtmp-custom combo by a service per streaming services and one for custom (and also internal service per protocol).

This PR provides:

  • Move dock state to profile (required for service integrations plugin)
  • Add to services the API to indicate if it use "1 audio track"/"1 track + 1 archive track"/"multiple audio track"
  • Move service integration outside of the UI as plugins (obs-restream, obs-twitch and obs-youtube)
    • The UUID system in those plugins is mostly for future-proofing
    • Note that Browser docks were already quite crash-prone on Linux if you test on it
  • Add API in Services API to enabled multi-protocol per service support and avoid "H264 only" API.
  • Deprecated APIs that are too much "H264 only"
  • Add bandwidth test related API in the Services API
  • Add a broadcast flow API in the frontend API for service like YouTube and third-parties
  • Add obs-browser related function in the frontend API
  • Migrate old rtmp-common/rtmp-custom to the new services in a new file
  • Add CI to keep obs-services JSON parser and its JSON schema aligned

If I forgot to mention something that you think is necessary, tell me.

Twitch with integration:
Screenshot from 2023-07-08 21-02-53

YouTube with integration:
Screenshot from 2023-07-08 21-03-34

Custom service:
Screenshot from 2023-07-08 21-03-24
Screenshot from 2023-07-08 21-03-11
Screenshot from 2023-07-08 21-03-07

Motivation and Context

How Has This Been Tested?

Types of changes

  • New feature (non-breaking change which adds functionality)
  • Code cleanup (non-breaking change which makes code smaller or more readable)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Documentation (a change to documentation pages)

Checklist:

  • My code has been run through clang-format.
  • I have read the contributing document.
  • My code is not on the master branch.
  • The code has been tested but it needs to be tested by more people than myself because it's big.
  • All commit messages are properly formatted and commits squashed where appropriate. (Tell me if think there is more to squash)
  • I have included updates to all appropriate documentation.

@tytan652 tytan652 marked this pull request as draft July 19, 2023 05:43
@DatCaptainHorse
Copy link

DatCaptainHorse commented Jul 24, 2023

Thought I'd do some quick tests as this looks interesting 🙂
I can take a look at the new Service API and try making a plugin for Neighy, if that's more helpful as this is quite short 😅

Clean install from CI, ran in portable mode.
UI seems to work without issues from my usage, below is a small table of streaming protocols I tried
✅ - Didn't spot issues
➖ - Protocol not supported by service
❌ - Something broke

Streaming WHIP RTMP RTMPS Notes
Neighy (Custom...) None
Twitch Twitch WHIP URL (1)

Note references:

  1. WHIP ingest for Twitch

Update: Found a tiny bug, with "Override Service Settings" enabled, opening settings brings up the warning popup each time.

Update 2: Did a quick Neighy service using the Service API, no real issues encountered, below is a screenshot of added service 🙂

  • Only "issue" was the "maximum bitrate" used in Twitch service, I referenced that service and found out it just forces the encoder bitrate, I feel that should limit the upper value instead of setting the bitrate forcefully?

Update2Image

@tytan652 tytan652 force-pushed the service_overhaul branch 3 times, most recently from 4cf8412 to 7641a1e Compare July 24, 2023 06:34
@tytan652
Copy link
Owner Author

  • Only "issue" was the "maximum bitrate" used in Twitch service, I referenced that service and found out it just forces the encoder bitrate, I feel that should limit the upper value instead of setting the bitrate forcefully?

Thanks for the update, note if you edit a message I'm not notified.

This behavior is not introduce by neither the RFC or the implementation.
Around this feature, I only replaced "H264 only" functions.

For the "Update 2", I will fix that when I can.

@tytan652 tytan652 force-pushed the service_overhaul branch 3 times, most recently from e83828f to daec592 Compare August 3, 2023 06:36
@tytan652 tytan652 force-pushed the service_overhaul branch 3 times, most recently from 9779eee to c141c73 Compare August 11, 2023 08:11
Allow to apply encoder settings per encoder and codec.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants