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
58.89k stars 7.84k forks source link

WebRTC WHIP - Auto gathering disabled in OBS 30.2.0-beta4 #10910

Closed arite closed 2 months ago

arite commented 2 months ago

Operating System Info

Other

Other OS

Windows 10, Debian

OBS Studio Version

30.2.0-beta4

OBS Studio Version (Other)

No response

OBS Studio Log URL

N/A

OBS Studio Crash Log URL

No response

Expected Behavior

TL;DR: OBS 30.1.2 gathers local ICE candidates prior sending an SDP offer via WHIP. In 30.2.0-beta4 this is disabled, meaning that the WHIP endpoint/Media Server must gather candidates prior to answering in order to establish a connection.

If this is by design (i.e. the WHIP endpoint/Media Server is responsible for gathering ICE candidates) then feel free to close this issue - I just wanted to highlight the change in behavior between OBS 30.1.2 and 30.2.4-beta4.


When establishing a WHIP connection with a WHIP endpoint/Media Server that does not gather ICE candidates before responding with an answer, a WebRTC session is established.

In OBS 30.1.2, the SDP offer in the WHIP HTTP POST request contained candidate attributes (a=candidate.... This meant that it was possible to establish a WebRTC session with a WHIP endpoint/Media Server without needing to perform/complete ICE gathering prior to responding.

In 30.2.0-beta4 this is no longer the case as disableAutoGathering is set to false (if libdatachannel is > 0.20.x): https://github.com/obsproject/obs-studio/blob/30.2.0-beta4/plugins/obs-webrtc/whip-output.cpp#L229-L231

This change appears to have been introduced in PR #10524.

Current Behavior

When establishing a WHIP connection with a WHIP endpoint/Media Server that does not gather ICE candidates before responding, no WebRTC session is established as no candidates are shared.

In OBS it appears to have successfully started streaming, however no connection is made due to lack of candidates.

Steps to Reproduce

  1. Configure a connection with a WHIP service in OBS with a WHIP endpoint that does not perform ICE gathering
  2. Start streaming to the endpoint
  3. Observe that when using OBS 30.1.2 a WebRTC connection is established, and when using 30.2.0-beta4 and no connection is established.

One way to setup a WHIP endpoint that does not perform ICE gathering would be to build broadcast-box from source and comment out the call to webrtc.GatheringCompletePromise() here.

And then remove/comment out the receive operation <-gatherComplete here.

i.e.:

        // TRIMMED
    //gatherComplete := webrtc.GatheringCompletePromise(peerConnection)
    answer, err := peerConnection.CreateAnswer(nil)

    if err != nil {
        return "", err
    } else if err = peerConnection.SetLocalDescription(answer); err != nil {
        return "", err
    }

    //<-gatherComplete
    return peerConnection.LocalDescription().SDP, nil

As a git diff:

diff --git a/internal/webrtc/whip.go b/internal/webrtc/whip.go
index d3c3bea..6780fa1 100644
--- a/internal/webrtc/whip.go
+++ b/internal/webrtc/whip.go
@@ -175,7 +175,7 @@ func WHIP(offer, streamKey string) (string, error) {
                return "", err
        }

-       gatherComplete := webrtc.GatheringCompletePromise(peerConnection)
+//     gatherComplete := webrtc.GatheringCompletePromise(peerConnection)
        answer, err := peerConnection.CreateAnswer(nil)

        if err != nil {
@@ -184,6 +184,6 @@ func WHIP(offer, streamKey string) (string, error) {
                return "", err
        }

-       <-gatherComplete
+//     <-gatherComplete
        return peerConnection.LocalDescription().SDP, nil
 }

After applying the patch, broadcast-box can be be built with:

# Build frontend
pushd web
npm install && npm run build
popd

# Build backend
go run .

Anything else we should know?

As noted above, this may be by design. However as it was possible to establish a WHIP session without gathering on the WHIP endpoint/Media Server-side in 30.1.2, I wanted to raise this observation.

RytoEX commented 2 months ago

Given the changes in the linked diff, I'm fairly certain that was intentional, but only @Sean-Der could say.

Sean-Der commented 2 months ago

@arite it was done on purpose to support TURN/STUN.

libdatachannel doesn’t support trickle ICE. The ICE servers come after the offer, so it’s an either/or situation.

With the change in your media server does everything work again? We can send candidates via PATCH if you are blocked

arite commented 2 months ago

@Sean-Der - Great, thanks for clarifying!

FYI after gathering ICE candidates on the media server-side before sending the answer it works fine with OBS v30.2.0-beta4. I was mainly raising this to note the change in behavior etc.

Thanks again - I'll close this ticket now.

Sean-Der commented 2 months ago

@arite Thanks for pointing it out! I really appreciate people having eyes on these things :)

I have made plenty of mistakes along the away. I am also in the Discord linked in the broadcast box repo if you wanna chat anytime :)