Skip to content

Conversation

@tytan652
Copy link
Collaborator

@tytan652 tytan652 commented Jan 19, 2023

Description

This PR is about the RFC 45.

Depends on:

Or if merge as is, contains:

Replace and deprecate url, key, username and password functions to replace with only one obs_service_get_connect_info().

This allow to ask the service for various connection information like:

  • Server URL (Commonly used in various protocol)
  • Stream key (Usually used with RTMP/FTL/HLS)
  • Username (Can be use with RMTP/FTL/RIST)
  • Password (Can be use with RMTP/FTL/RIST)
  • Stream ID (Can be used for SRT)
  • Encryption passphrase (Can be used for SRT/RIST)
  • It also allow third-party protocol to use custom connection infos when third-party outputs+services will be possible.

Those info type are not bound to protocol.

Adds a function that allow the service to indicate if it has all the connection info that it needs to start streaming: obs_service_can_try_to_connect.

Motivation and Context

Part of the implementation of the RFC 45 recent changes.

How Has This Been Tested?

Only tested trying to stream SRT, might need more testing.

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 Enhancement Improvement to existing functionality Seeking Testers Build artifacts on CI labels Jan 19, 2023
@tytan652 tytan652 added this to the OBS Studio 29.1 milestone Jan 19, 2023
@tytan652 tytan652 force-pushed the service_get_info branch 2 times, most recently from 11e86e9 to a45384c Compare January 27, 2023 11:42
@tytan652 tytan652 changed the title RFC 45-4.5: Enable more connection info in Services API RFC 45-6: Enable more connection info in Services API Jan 30, 2023
@tytan652
Copy link
Collaborator Author

Rebased on #8090 to be able to work on top of all RFC 45 PRs.

@tytan652 tytan652 force-pushed the service_get_info branch 2 times, most recently from 0bafab7 to 4715222 Compare January 31, 2023 10:27
@tytan652 tytan652 force-pushed the service_get_info branch 9 times, most recently from 4e5a563 to ec186bf Compare February 16, 2023 20:22
@tytan652 tytan652 force-pushed the service_get_info branch 6 times, most recently from 57a1900 to 0ab1ee9 Compare February 24, 2023 20:35
@tytan652 tytan652 force-pushed the service_get_info branch 2 times, most recently from 4e5ad21 to 82af445 Compare March 1, 2023 09:49
@tytan652 tytan652 force-pushed the service_get_info branch 2 times, most recently from c276303 to 5acbfbb Compare March 12, 2023 10:40
@tytan652
Copy link
Collaborator Author

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

Ask the service directly rather than checking the presence of a key and
a URL.
Copy link
Member

@jp9000 jp9000 left a comment

Choose a reason for hiding this comment

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

LGTM™

@jp9000 jp9000 marked this pull request as ready for review March 20, 2023 02:13
@jp9000 jp9000 merged commit 4829362 into obsproject:master Mar 20, 2023
@notr1ch
Copy link
Member

notr1ch commented Mar 20, 2023

A crash when using custom output was reported on Discord (https://discord.com/channels/348973006581923840/374636084883095554/1087384798894501939)

	if (!codecs || IsCustomService()) {
		const char *output;
		char **output_codecs;

		obs_enum_output_types_with_protocol(QT_TO_UTF8(protocol),
						    &output, return_first_id);

		output_codecs = strlist_split(
			obs_get_output_supported_video_codecs(output), ';',
			false);

		ret = service_supports_codec((const char **)output_codecs,
					     codec);

obs_enum_output_types_with_protocol fails to find an output, so output remains uninitialized, causing a crash in obs_get_output_supported_audio_codecs. I'm not sure what the intention is here since the code clearly wants to go this path with the IsCustomService condition. Changing it to if (!codecs && !IsCustomService()) might be more correct if we want to allow all codec compatibility for custom outputs.

The same issue exists in ServiceAndACodecCompatible too, perhaps these functions could be merged to avoid logic duplication.

@tytan652
Copy link
Collaborator Author

if we want to allow all codec compatibility for custom outputs.

We don't.

@tytan652
Copy link
Collaborator Author

tytan652 commented Mar 20, 2023

The main problem is why no output were found.

Edit: And this is not the place to discuss about that.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Enhancement Improvement to existing functionality Seeking Testers Build artifacts on CI

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants