GitHub/olcrtc
1
0
Fork 0
mirror of https://github.com/openlibrecommunity/olcrtc.git synced 2026-06-09 05:44:43 +00:00
Code Issues Packages Projects Releases Wiki Activity

fix(jitsi): skip idle peer connection keepalive

Browse Source
This commit is contained in:
sbramn
2026-06-05 19:49:06 +03:00
parent 760ff2b6c0
commit 275b61480c
2 changed files with 51 additions and 31 deletions
Download Patch File Download Diff File Expand all files Collapse all files

45
internal/engine/jitsi/jitsi.go
View File

@@ -365,8 +365,8 @@ func (s *Session) waitForJingle() {
}
}
// completeJingleSetup opens the bridge and negotiates the PeerConnection after
// receiving session-initiate from Jicofo.
// completeJingleSetup opens the bridge and negotiates a PeerConnection only
// when media or the SCTP bridge fallback needs one.
func (s *Session) completeJingleSetup(ctx context.Context, jSess *j.Session) error {
logger.Infof("jitsi: session-initiate received; colibri-ws=%s", jSess.ColibriWS)
@@ -379,7 +379,7 @@ func (s *Session) completeJingleSetup(ctx context.Context, jSess *j.Session) err
}
}
if s.shouldNegotiatePC() {
if s.shouldNegotiatePC(sctpBridge) {
if err := s.negotiatePC(ctx, jSess, sctpBridge); err != nil {
return err
}
@@ -425,14 +425,8 @@ func (s *Session) openBridgeSCTP(ctx context.Context, jSess *j.Session) error {
return nil
}
func (s *Session) shouldNegotiatePC() bool {
if s.onData != nil {
return true
}
if s.onPeerData != nil {
return true
}
return s.shouldRequestVideo()
func (s *Session) shouldNegotiatePC(sctpBridge bool) bool {
return sctpBridge || s.shouldRequestVideo()
}
func (s *Session) shouldRequestVideo() bool {
@@ -509,6 +503,7 @@ func (s *Session) negotiatePC(ctx context.Context, jSess *j.Session, sctpBridge
s.videoTrackMu.RLock()
hasLocalTracks := len(s.videoTracks) > 0
requestVideo := hasLocalTracks || s.onVideoTrack != nil
for _, track := range s.videoTracks {
if _, addErr := pc.AddTrack(track); addErr != nil {
s.videoTrackMu.RUnlock()
@@ -519,12 +514,10 @@ func (s *Session) negotiatePC(ctx context.Context, jSess *j.Session, sctpBridge
s.videoTrackMu.RUnlock()
// When sending video, AddTrack already creates the video m-line (sendonly).
// When only receiving, an explicit recvonly transceiver is required so the
// SDP answer includes a video m-line - without it JVB does not set up a
// video forwarding path and ICE stalls. Mirrors the j library reference CLI:
// AddTrack and AddTransceiverFromKind(video,recvonly) are mutually exclusive
// in Plan B; using both produces a malformed SDP.
if !hasLocalTracks {
// When only receiving video, an explicit recvonly transceiver is required
// so the SDP answer includes a video m-line. SCTP-only byte streams do not
// need a video m-line, so keep that idle path lean.
if requestVideo && !hasLocalTracks {
if _, err := pc.AddTransceiverFromKind(
webrtc.RTPCodecTypeVideo,
webrtc.RTPTransceiverInit{Direction: webrtc.RTPTransceiverDirectionRecvonly},
@@ -601,7 +594,7 @@ func (s *Session) negotiatePC(ctx context.Context, jSess *j.Session, sctpBridge
}
}
if s.shouldRequestVideo() {
if requestVideo {
// Tell JVB to forward video streams to this endpoint.
if err := jSess.RequestVideo(ctx, 720); err != nil {
logger.Debugf("jitsi: request video: %v", err)
@@ -1627,19 +1620,23 @@ func (s *Session) teardownPC() {
}
}
// reinitiateBridge negotiates a new PeerConnection and opens the bridge channel.
// reinitiateBridge negotiates a new PeerConnection only when required and opens
// the bridge channel.
func (s *Session) reinitiateBridge(ctx context.Context, jSess *j.Session) error {
sctpBridge := jSess.ColibriWS == ""
if err := s.negotiatePC(ctx, jSess, sctpBridge); err != nil {
logger.Warnf("jitsi: negotiate after reinitiate failed: %v - full reconnect", err)
return s.reconnectFull(ctx)
needBridge := s.onData != nil || s.onPeerData != nil
sctpBridge := needBridge && jSess.ColibriWS == ""
if s.shouldNegotiatePC(sctpBridge) {
if err := s.negotiatePC(ctx, jSess, sctpBridge); err != nil {
logger.Warnf("jitsi: negotiate after reinitiate failed: %v - full reconnect", err)
return s.reconnectFull(ctx)
}
}
if sctpBridge {
if err := s.openBridgeSCTP(ctx, jSess); err != nil {
logger.Warnf("jitsi: bridge after reinitiate failed: %v - full reconnect", err)
return s.reconnectFull(ctx)
}
} else {
} else if needBridge {
if err := s.openBridgeWS(ctx, jSess); err != nil {
logger.Warnf("jitsi: bridge after reinitiate failed: %v - full reconnect", err)
return s.reconnectFull(ctx)

37
internal/engine/jitsi/jitsi_test.go
View File

@@ -93,7 +93,7 @@ func TestNewSucceeds(t *testing.T) {
}
}
func TestByteStreamNegotiatesPeerConnectionWithoutRequestingVideo(t *testing.T) {
func TestByteStreamWebSocketSkipsPeerConnectionWithoutRequestingVideo(t *testing.T) {
sess, err := New(context.Background(), engine.Config{
URL: testHost,
Extra: map[string]string{credentialKeyRoom: testRoom},
@@ -108,8 +108,31 @@ func TestByteStreamNegotiatesPeerConnectionWithoutRequestingVideo(t *testing.T)
if !ok {
t.Fatal("sess is not *Session")
}
if !js.shouldNegotiatePC() {
t.Fatal("shouldNegotiatePC() = false for bytestream session")
if js.shouldNegotiatePC(false) {
t.Fatal("shouldNegotiatePC(false) = true for websocket bytestream session")
}
if js.shouldRequestVideo() {
t.Fatal("shouldRequestVideo() = true for bytestream-only session")
}
}
func TestByteStreamSCTPFallbackNegotiatesPeerConnectionWithoutRequestingVideo(t *testing.T) {
sess, err := New(context.Background(), engine.Config{
URL: testHost,
Extra: map[string]string{credentialKeyRoom: testRoom},
OnData: func([]byte) {},
})
if err != nil {
t.Fatalf("New: %v", err)
}
defer func() { _ = sess.Close() }()
js, ok := sess.(*Session)
if !ok {
t.Fatal("sess is not *Session")
}
if !js.shouldNegotiatePC(true) {
t.Fatal("shouldNegotiatePC(true) = false for SCTP bytestream fallback")
}
if js.shouldRequestVideo() {
t.Fatal("shouldRequestVideo() = true for bytestream-only session")
@@ -130,14 +153,14 @@ func TestVideoSessionNegotiatesPeerConnectionAndRequestsVideo(t *testing.T) {
if !ok {
t.Fatal("sess is not *Session")
}
if js.shouldNegotiatePC() {
t.Fatal("shouldNegotiatePC() = true before bytestream/video is configured")
if js.shouldNegotiatePC(false) {
t.Fatal("shouldNegotiatePC(false) = true before bytestream/video is configured")
}
if err := js.AddVideoTrack(nil); err != nil {
t.Fatalf("AddVideoTrack(nil): %v", err)
}
if !js.shouldNegotiatePC() {
t.Fatal("shouldNegotiatePC() = false for video session")
if !js.shouldNegotiatePC(false) {
t.Fatal("shouldNegotiatePC(false) = false for video session")
}
if !js.shouldRequestVideo() {
t.Fatal("shouldRequestVideo() = false for video session")