obsproject / obs-studio

OBS Studio - Free and open source software for live streaming and screen recording
https://obsproject.com
GNU General Public License v2.0
60.39k stars 7.99k forks source link

rtmp-services: Update sheeta service URL #11471

Closed KOUCHANGdw closed 6 days ago

KOUCHANGdw commented 2 weeks ago

Description

I changed the RTMP URL for the streaming service "sheeta".

Motivation and Context

In the sheeta "service", we are adding a query string to detect the use of the "service" on the server side.

"sheeta" is a service described at the following link: https://www.sheeta-info.com/ . It is a fan club management service that utilizes OBS for its broadcasting features. We Dwango Inc.(KADOKAWA Group) provide this service.

The addition of the "sheeta" service was made in the following PR: https://github.com/obsproject/obs-studio/pull/10591

How Has This Been Tested?

I did "Local testing" as described in Service Submission Guidelines.

Types of changes

Checklist:

Fenrirthviti commented 2 weeks ago

What is the purpose of this change? It seems to just be adding metadata for identifying/collecting telemetry for when a client is OBS, which is something we should already be sending as part of the RTMP connection anyway.

I'm a bit wary of adding this kind of thing in as part of the RTMP URL, without sufficient justification.

KOUCHANGdw commented 2 weeks ago

What is the purpose of this change? It seems to just be adding metadata for identifying/collecting telemetry for when a client is OBS, which is something we should already be sending as part of the RTMP connection anyway.

I'm a bit wary of adding this kind of thing in as part of the RTMP URL, without sufficient justification.

@Fenrirthviti

For context, it appears that our service (in a general sense) may not be utilizing the "OBS service" as expected. This is suggested by signs like the keyframe interval not being set to 1, which has led to issues with live stream archiving. Additionally, the RTMP URL we display as a streaming destination in our service does not typically include a query string.

This modification—adding a query string to the OBS service RTMP URL—is intended to verify that customers are indeed using the OBS service correctly. Your caution here is appreciated. If necessary, I can provide verification that I’m associated with the service operating company; ideally, my email domain alone could serve as sufficient confirmation.

Also, if I may have misunderstood any details, I apologize. Could you clarify the following? When you mentioned, "which is something we should already be sending as part of the RTMP connection anyway," does this mean that it’s already possible to see from the RTMP connection data whether OBS is being used? Furthermore, would that information confirm the use of the “OBS service” specifically? (In this case, we want to verify use of the OBS “service” itself.) I’d appreciate any clarification you can provide.

I apologize if my English is a bit rough; please feel free to ask for clarification on any unclear parts. Thank you!

Fenrirthviti commented 2 weeks ago

Also, if I may have misunderstood any details, I apologize. Could you clarify the following? When you mentioned, "which is something we should already be sending as part of the RTMP connection anyway," does this mean that it’s already possible to see from the RTMP connection data whether OBS is being used? Furthermore, would that information confirm the use of the “OBS service” specifically? (In this case, we want to verify use of the OBS “service” itself.) I’d appreciate any clarification you can provide.

Yes, you can just read the RTMP request and the Encoder metadata will show OBS Studio is being used. The string will be something similar to:

obs-output module (libobs version 31.0.0-beta2)

Fenrirthviti commented 2 weeks ago

As an additional note, there is no OBS "service" here. OBS Studio is a standalone desktop application, we do not provide or host any kind of service. This may just be a translation quirk, apologies if I am misunderstanding!

KOUCHANGdw commented 2 weeks ago

Yes, you can just read the RTMP request and the Encoder metadata will show OBS Studio is being used. The string will be something similar to:

obs-output module (libobs version 31.0.0-beta2)

It seems that this was simply removed somewhere in the system, and I was unable to confirm it. I’ll discuss it with other engineers and check further. However, since I also want to confirm that sheeta is being used—and it’s likely this won’t be possible without further action—I would like this fix to be merged. (I'll provide more details in the next sentence about what we specifically want to confirm.)

As an additional note, there is no OBS "service" here. OBS Studio is a standalone desktop application, we do not provide or host any kind of service. This may just be a translation quirk, apologies if I am misunderstanding!

By “OBS service,” I’m referring to the “service” selection shown in the referenced image (the feature that allows selecting service-specific settings for platforms like YouTube, Twitch, or sheeta). My previous English wording might have caused some confusion here. (Though it’s off-topic, I’d appreciate any suggestions for how to better phrase this for English-speaking audiences to avoid confusion. I realize "OBS service" could indeed be misleading, lol.)

image

To clarify, we want to encourage the use of this “service feature” shown in the image to ensure the “correct parameters” are set. This is why we’re adding the query parameter—to enable server-side confirmation that this “service feature” is in use and to help us identify telemetry accurately.

WizardCM commented 2 weeks ago

As an additional note, there is no OBS "service" here. OBS Studio is a standalone desktop application, we do not provide or host any kind of service. This may just be a translation quirk, apologies if I am misunderstanding!

By "service" in this context they mean the "sheeta" entry in the Service dropdown vs using the Custom option on the service dropdown. I don't believe there's currently a way to differentiate that via the RTMP headers.

KOUCHANGdw commented 2 weeks ago

Thank you for the approval!

The structural constraints are exactly as you mentioned. Taking into account the nature of the users and the minimal benefits of malicious modifications, we decided to proceed with this approach.

Thank you for your continued support.

RytoEX commented 1 week ago

To clarify, we want to encourage the use of this “service feature” shown in the image to ensure the “correct parameters” are set. This is why we’re adding the query parameter—to enable server-side confirmation that this “service feature” is in use and to help us identify telemetry accurately.

To add to others' comments, if a user checks "Ignore streaming service recommendations", they will of course be using whatever streaming settings they specified in Settings > Output. That caveat aside, this otherwise seems fine.

KOUCHANGdw commented 1 week ago

To add to others' comments, if a user checks "Ignore streaming service recommendations", they will of course be using whatever streaming settings they specified in Settings > Output. That caveat aside, this otherwise seems fine.

Thank you for your valuable feedback. I hadn’t fully considered the points you raised, so I’ll be sure to keep them in mind. (Given the nature of the service, this aspect is within an acceptable range, so I’d like to proceed with this fix as it is.)

KOUCHANGdw commented 1 week ago

Thank you in advance for your continued support until the merge is complete.

PatTheMav commented 6 days ago

If I understand the purpose of this PR correctly, the service's need is to be able to recognise if a stream generated by OBS Studio might have incorrect settings (e.g. a wrong keyframe setting).

The suggested solution to this is to add a query parameter to the RTMP URL in the service's config file, which would be used for any stream generated by OBS Studio when the service was selected in the service dropdown menu.

If that is all correct, then this PR does not solve the problem:

There are only 2 ways in which a stream might end up having wrong settings:

a. A user manually set up the service as a "custom RTMP" service and thus also manually misconfigured the stream settings b. A user selected the service from the drop down, but checked the "Ignore streaming service recommendations" checkbox

For scenario a) you would not get the query parameter, but that fact cannot tell you whether the stream is configured correctly or not.

For scenario b) you would get the query parameter, but - again - that fact will not tell you whether the stream was configured correctly (because of the "Ignore streaming service recommendations" checkbox).

If I'm not missing some detail here, this PR will add an additional parameter that has no qualitative value, as it cannot tell you whether a stream is set up correctly or not.

Fenrirthviti commented 6 days ago

Given the above and the general consensus that we don't feel this is a meaningful solution, I am opting to close this.

It is our recommendation that if users are using settings outside your recommended parameters, to close the connection and determine a way to notify the user through your service.

KOUCHANGdw commented 3 days ago

@PatTheMav @Fenrirthviti

Thank you for your feedback.

to close the connection and determine a way to notify the user through your service

I think this is a valid point as well.

However, implementing a feature to display information on the service front or disconnecting the stream when there is an issue could lead to unnecessary interruptions, even when they aren’t needed. We would like to avoid this risk. Also, fully addressing this area would likely introduce bugs and increase maintenance costs.

Currently, we can already check the stream analysis information from the logs, but to improve efficiency, we want to start by confirming whether "sheeta" is being used within the service.

As for "ignore streaming service recommendations," given the characteristics of our service users, we believe that only those who are aware of the implications would leave it unchecked. Those who do check it are likely to have a thorough understanding of the manual, so we consider the risk of issues arising to be low (although, as you rightly pointed out, it’s not "absolute"). Since this adjustment mainly targets "light users" who only skim the manual, we believe this change will effectively address their needs.

I apologize for the lengthy response, but I believe it's essential to convey this thoroughly to ensure a full understanding of the service. We plan to continue considering backend adjustments to address the areas you mentioned in your feedback. However, this current PR only involves settings related to "sheeta" and does not impact other OBS users. Therefore, I would appreciate it if you could merge it as is. If possible, could you reconsider your evaluation?