Skip to content

RFC 45-2: Add protocol in Services API and enum functions in Outputs API #8087

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.

Sign up for GitHub

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

Jump to bottom
Closed
tytan652 wants to merge 6 commits into from
Closed

RFC 45-2: Add protocol in Services API and enum functions in Outputs API #8087

tytan652 wants to merge 6 commits into from

Conversation

@tytan652
Copy link
Collaborator

@tytan652 tytan652 commented Jan 19, 2023 •
edited
Loading

Description

This PR is about the RFC 45.

Depends on:

Additions (those will be used in other parts of RFC 45 implementation):

  • Adds get_protocol to services
  • Add three enumeration functions:
    • obs_enum_output_protocols to enumerate protocols
    • obs_enum_output_types_with_protocol to enumerate outputs with the given protocol.

Motivation and Context

Part of the implementation of the RFC 45.

How Has This Been Tested?

Testing by logging enumeration functions outputs and through other part of RFC 45 implementation.

Types of changes

  • New feature (non-breaking change which adds functionality)

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.
  • All commit messages are properly formatted and commits squashed where appropriate.
  • I have included updates to all appropriate documentation.

@tytan652 tytan652 added the New Feature New feature or plugin label Jan 19, 2023
@tytan652 tytan652 added this to the OBS Studio 29.1 milestone Jan 19, 2023
This was referenced Jan 19, 2023
Merged
Merged
Merged
Merged
@tytan652 tytan652 force-pushed the more_protocol_apis branch 3 times, most recently from d71e9d9 to b025487 Compare January 31, 2023 07:50
@tytan652 tytan652 force-pushed the more_protocol_apis branch 4 times, most recently from 4806f09 to d14112c Compare February 16, 2023 13:20
@tytan652 tytan652 force-pushed the more_protocol_apis branch 4 times, most recently from be0678d to 3da30a1 Compare February 24, 2023 20:48
@tytan652 tytan652 force-pushed the more_protocol_apis branch 2 times, most recently from ad42091 to 5401209 Compare March 1, 2023 09:46
@tytan652 tytan652 force-pushed the more_protocol_apis branch 2 times, most recently from b0600e3 to 4973495 Compare March 11, 2023 21:13
@tytan652
Copy link
Collaborator Author

tytan652 commented Mar 11, 2023

Remove prefix related changes as it was done in the RFC.

@tytan652 tytan652 force-pushed the more_protocol_apis branch from 4973495 to 2b5743a Compare March 12, 2023 11:21
@jp9000
Copy link
Member

jp9000 commented Mar 12, 2023

I generally approve of this, although one note is that twitch uses an API that can generate a new server list whenever the program is run. when that happens, it might switch it internally to RTMPS rather than RTMP.

In general, RTMP/RTMPS are fairly interchangeable. So I'm not sure what to do about that. What do you think?

@jp9000
Copy link
Member

jp9000 commented Mar 12, 2023

Another thing Rodney pointed out is that it might have been nice to make it default to RTMP/RTMPS if a service doesn't have that item. Not sure how you feel about that one though.

@jp9000
Copy link
Member

jp9000 commented Mar 12, 2023

Actually exclude the last message, seems that was an intentional decision.

@tytan652 tytan652 force-pushed the more_protocol_apis branch from 2b5743a to 61cbcdd Compare March 13, 2023 10:00
@tytan652
Copy link
Collaborator Author

tytan652 commented Mar 13, 2023

Added change since it no longer enforce the "protocol" field in the JSON if the server URL is RTMP(S).

@tytan652 tytan652 requested a review from jp9000 March 13, 2023 10:01
@tytan652 tytan652 force-pushed the more_protocol_apis branch 5 times, most recently from df3e884 to 72fd7ec Compare March 16, 2023 09:43
@tytan652
Copy link
Collaborator Author

tytan652 commented Mar 16, 2023 •
edited
Loading

  • Renamed obs_enum_output_service_types to obs_enum_output_types_with_protocol with a refactor to avoid strlist allocation, based on @derrod ideas.
  • Refactored obs_enum_output_protocols based on RFC 45-1 recent changes.

@tytan652 tytan652 force-pushed the more_protocol_apis branch 3 times, most recently from 54faabe to a345045 Compare March 18, 2023 20:34
tytan652 added 6 commits March 19, 2023 15:29
@tytan652
libobs,docs: Add protocol in Outputs API
201bcda
@tytan652
obs-outputs,obs-ffmpeg: Add protocol to service outputs
855956f
@tytan652
rtmp-services: Add protocols to services JSON
513c6bc 
If the server URL is not an RTMP(S) URL, the protocol field becomes
required.

The output field becomes required on non-RTMP(S) services to keep
backward compatibility.

Also skip service if the protocol is not available.
@tytan652
libobs,docs: Add protocol to Services API
813b3b2
@tytan652
libobs,docs: Add protocol enumeration functions
56411ed
@tytan652
rtmp-services: Add protocol getter to services
fa58a38
@tytan652 tytan652 force-pushed the more_protocol_apis branch from a345045 to fa58a38 Compare March 19, 2023 15:31
@tytan652
Copy link
Collaborator Author

tytan652 commented Mar 19, 2023

Refactor of obs_enum_output_types_with_protocol based on #8095 review.

@jp9000 jp9000 closed this in #8095 Mar 20, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@jp9000 jp9000 Awaiting requested review from jp9000

Assignees

No one assigned

Labels

New Feature New feature or plugin

Projects

None yet

Milestone

OBS Studio 29.1

Development

Successfully merging this pull request may close these issues.

2 participants

@tytan652 @jp9000