From 5ad9b2215337cae1fe55257eb4db633d2ccb3037 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 21 Aug 2026 13:59:36 -0700 Subject: [PATCH 1/2] Squashing work thus far --- go.mod | 2 +- go.sum | 4 +- pkg/sip/inbound.go | 376 +++++++---- pkg/sip/media.go | 1 - pkg/sip/media_pipeline.go | 440 +++++++----- pkg/sip/media_pipeline_test.go | 635 ++++++++++++++++++ pkg/sip/media_port.go | 676 +++++++++++++------ pkg/sip/media_port_negotiation_test.go | 430 ++++++++++++ pkg/sip/media_port_test.go | 810 +++++++++++------------ pkg/sip/outbound.go | 140 ++-- pkg/sip/outbound_test.go | 12 +- pkg/sip/outbound_utilities_test.go | 38 +- pkg/sip/room.go | 135 ++-- pkg/sip/room_test.go | 2 +- pkg/sip/service_test.go | 117 ++-- pkg/sip/signal_logger_test.go | 6 +- pkg/sip/signaling_test.go | 126 +++- pkg/sip/silence_filler_test.go | 13 +- pkg/sip/tls_test.go | 2 +- pkg/stats/monitor.go | 10 +- test/cloud/integration_test.go | 2 +- test/cloud/service.go | 11 +- test/integration/docker_leftover_test.go | 73 ++ test/integration/docker_mac_test.go | 1 + test/integration/docker_test.go | 1 + test/integration/livekit_test.go | 125 +++- test/integration/sip_test.go | 6 +- test/lktest/livekit.go | 4 +- 28 files changed, 2970 insertions(+), 1228 deletions(-) create mode 100644 pkg/sip/media_pipeline_test.go create mode 100644 pkg/sip/media_port_negotiation_test.go create mode 100644 test/integration/docker_leftover_test.go diff --git a/go.mod b/go.mod index fb3f1edd4..0baa4fb51 100644 --- a/go.mod +++ b/go.mod @@ -9,7 +9,7 @@ require ( github.com/icholy/digest v1.1.0 github.com/jfreymuth/oggvorbis v1.0.5 github.com/livekit/mageutil v0.0.0-20250511045019-0f1ff63f7731 - github.com/livekit/media-sdk v0.0.0-20260812193843-5a5218b19550 + github.com/livekit/media-sdk v0.0.0-20260819185502-8cd8492db728 github.com/livekit/mediatransportutil v0.0.0-20260608063931-a3417d38cda0 github.com/livekit/protocol v1.50.5-0.20260818023635-24e126c61d3e github.com/livekit/psrpc v0.7.3 diff --git a/go.sum b/go.sum index 0884fe385..cd83c5807 100644 --- a/go.sum +++ b/go.sum @@ -128,8 +128,8 @@ github.com/livekit/amrwb-cgo v0.0.0-20260612153743-6d4b69dc1470 h1:pYml12Ue55jDh github.com/livekit/amrwb-cgo v0.0.0-20260612153743-6d4b69dc1470/go.mod h1:nGBFrVVLyO0RlyM2pWnP/VKaZO6W3rzecL/YlkTPkW8= github.com/livekit/mageutil v0.0.0-20250511045019-0f1ff63f7731 h1:9x+U2HGLrSw5ATTo469PQPkqzdoU7be46ryiCDO3boc= github.com/livekit/mageutil v0.0.0-20250511045019-0f1ff63f7731/go.mod h1:Rs3MhFwutWhGwmY1VQsygw28z5bWcnEYmS1OG9OxjOQ= -github.com/livekit/media-sdk v0.0.0-20260812193843-5a5218b19550 h1:aqaMkSNcx2GqCPsmGpkBN6MT5mlVyPZTECw5CxTAxOc= -github.com/livekit/media-sdk v0.0.0-20260812193843-5a5218b19550/go.mod h1:TuYRjSepaakL6ATsM9V2VMuksewW1PlhA32BG7Pxty0= +github.com/livekit/media-sdk v0.0.0-20260819185502-8cd8492db728 h1:reI1PPZE3kZhCLfVT4x869uRjulePiwchJ741GHeryE= +github.com/livekit/media-sdk v0.0.0-20260819185502-8cd8492db728/go.mod h1:TuYRjSepaakL6ATsM9V2VMuksewW1PlhA32BG7Pxty0= github.com/livekit/mediatransportutil v0.0.0-20260608063931-a3417d38cda0 h1:XHNNzebIKZRkLimla/hFGrAIX5EMWHctrgt3hLw7s+I= github.com/livekit/mediatransportutil v0.0.0-20260608063931-a3417d38cda0/go.mod h1:o8CFmAdrVwzJNOCsQCLUzXRjokkufNshnQHOe4fRaqU= github.com/livekit/protocol v1.50.5-0.20260818023635-24e126c61d3e h1:DHSagaJXDSrnqfOuwNLby2r+VIknFJxvJzAazm9Tdh8= diff --git a/pkg/sip/inbound.go b/pkg/sip/inbound.go index 8b7f4f77e..ded3ce6f0 100644 --- a/pkg/sip/inbound.go +++ b/pkg/sip/inbound.go @@ -45,7 +45,6 @@ import ( lksip "github.com/livekit/protocol/sip" "github.com/livekit/protocol/utils/traceid" "github.com/livekit/psrpc" - lksdk "github.com/livekit/server-sdk-go/v2" "github.com/livekit/sipgo/sip" "github.com/livekit/sip/pkg/config" @@ -74,6 +73,9 @@ var allowHeader = sip.NewHeader("Allow", "INVITE, ACK, CANCEL, BYE, NOTIFY, REFE var errNoACK = errors.New("no ACK received for 200 OK") +// RFC 3261 §21.4.27 / §14.2 — glare: INVITE received while an INVITE we sent is in progress. +const statusRequestPending sip.StatusCode = 491 + // hashPassword creates a SHA256 hash of the password for logging purposes func hashPassword(password string) string { if password == "" { @@ -337,18 +339,6 @@ func providerLabel(p *livekit.ProviderInfo) string { } } -func updateRemoteFromSDP(media *MediaPort, log logger.Logger, codecs *msdk.CodecSet, body []byte) { - if len(body) == 0 || media == nil { - return - } - desc, err := sdp.ParseWith(codecs, body) - if err != nil { - log.Warnw("failed to parse re-INVITE SDP, RTP destination not updated", err) - return - } - media.UpdateRemote(desc.Addr) -} - func (s *Server) onInvite(log *slog.Logger, req *sip.Request, tx sip.ServerTransaction) { // Error processed in defer _ = s.processInvite(req, tx) @@ -418,7 +408,16 @@ func (s *Server) processInvite(req *sip.Request, tx sip.ServerTransaction) (retE s.cmu.RUnlock() if existing != nil && existing.cc.InviteCSeq() < cc.InviteCSeq() { existing.log().Infow("reinvite", "content-length", req.ContentLength(), "cseq", cc.InviteCSeq()) - existing.updateRemoteFromSDP(sdpBodyFromRequest(req)) + if err := existing.updateRemoteFromSDP(sdpBodyFromRequest(req)); err != nil { + log.Errorw("failed to update inbound call SDP", err) + if ok := errors.As(err, &SDPError{}); ok { + cc.RejectAsKeepAlive(sip.StatusBadRequest, "Bad Request") + } else { + cc.RejectAsKeepAlive(sip.StatusInternalServerError, "Internal Server Error") + } + return nil + } + // TODO(alexfish): Reply with the new SDP. cc.AcceptAsKeepAlive(existing.cc.OwnSDP()) return nil } @@ -426,15 +425,33 @@ func (s *Server) processInvite(req *sip.Request, tx sip.ServerTransaction) (retE oc := s.cli.getActiveCall(cc.ID()) newCSeq := cc.InviteCSeq() if oc != nil && oc.cc != nil && oc.cc.InviteCSeq() < newCSeq { - localSDP := oc.cc.LocalSDP() - if len(localSDP) != 0 { - oc.log.Infow("accepting reinvite", "content-length", req.ContentLength(), "cseq", cc.InviteCSeq()) - oc.updateRemoteFromSDP(sdpBodyFromRequest(req)) - oc.cc.RecordInvite(newCSeq) - cc.AcceptAsKeepAlive(localSDP) + if oc.media == nil { + oc.log.Errorw("outbound call media has not been negotiated", nil) + cc.RejectAsKeepAlive(statusRequestPending, "Request Pending") + return nil + } + localSDP, err := oc.media.GetLocalSDP() + if err != nil || len(localSDP) == 0 { + oc.log.Errorw("outbound call does not have an SDP", err) + cc.RejectAsKeepAlive(statusRequestPending, "Request Pending") return nil } + oc.log.Infow("accepting reinvite", "content-length", req.ContentLength(), "cseq", cc.InviteCSeq()) + if err := oc.updateRemoteFromSDP(sdpBodyFromRequest(req)); err != nil { + log.Errorw("failed to update outbound call SDP", err) + if ok := errors.As(err, &SDPError{}); ok { + cc.RejectAsKeepAlive(sip.StatusBadRequest, "Bad Request") + } else { + cc.RejectAsKeepAlive(sip.StatusInternalServerError, "Internal Server Error") + } + return nil + } + oc.cc.RecordInvite(newCSeq) + // TODO(alexfish): Reply with the new SDP. + cc.AcceptAsKeepAlive(localSDP) + return nil } + } from, to := cc.From(), cc.To() @@ -707,20 +724,20 @@ type inboundCall struct { closeReason atomic.Pointer[ReasonHeader] call *rpc.SIPCall mmu sync.Mutex - media *MediaPort + media MediaPort mediaCodecs *msdk.CodecSet dtmf chan dtmf.Event // buffered endCall chan EndCall // buffered lkRoom RoomInterface // LiveKit room; only active after correct pin is entered callDur func() time.Duration joinDur func() time.Duration - forwardDTMF atomic.Bool done atomic.Bool started core.Fuse stats Stats sigTs SignalingTimestamps jitterBuf bool projectID string + audioOut *msdk.WriteCloserSwitch[msdk.PCM16Sample] // inner writer owned by MediaPort } func (s *Server) newInboundCall( @@ -750,6 +767,7 @@ func (s *Server) newInboundCall( endCall: make(chan EndCall, 1), jitterBuf: SelectValueBool(s.conf.EnableJitterBuffer, s.conf.EnableJitterBufferProb), projectID: "", // Will be set in handleInvite when available + audioOut: msdk.NewWriteCloserSwitch[msdk.PCM16Sample](RoomSampleRate), } c.stats.Update() c.setLog(log.WithValues("jitterBuf", c.jitterBuf)) @@ -887,52 +905,47 @@ func (c *inboundCall) handleInvite(ctx context.Context, tid traceid.ID, req *sip pinPrompt = true } - runMedia := func(m *sipMediaConfig) ([]byte, error) { - log := c.log() - if h := req.ContentLength(); h != nil { - log = log.WithValues("contentLength", int(*h)) + rawSDP := req.Body() + log := c.log() + if h := req.ContentLength(); h != nil { + log = log.WithValues("contentLength", int(*h)) + } + if h := req.ContentType(); h != nil { + log = log.WithValues("contentType", h.Value()) + switch h.Value() { + default: + log.Infow("unsupported offer type") + case "application/sdp": } - if h := req.ContentType(); h != nil { - log = log.WithValues("contentType", h.Value()) - switch h.Value() { - default: - log.Infow("unsupported offer type") - case "application/sdp": - } - } else { - log.Infow("no offer type specified") + } else { + log.Infow("no offer type specified") + } + + rejectMedia := func(err error) error { + sipReason := sip.StatusInternalServerError + log := log.WithValues("sdp", string(rawSDP)) + status, term := callDropped, stats.ServerError("media-failed") + if errors.Is(err, sdp.ErrNoCommonMedia) { + status, term = callMediaFailed, stats.ClientError("no-common-codec") + sipReason = sip.StatusBadRequest + } else if errors.Is(err, sdp.ErrNoCommonCrypto) { + status, term = callMediaFailed, stats.ClientError("no-common-crypto") + sipReason = sip.StatusBadRequest + } else if e := (SDPError{}); errors.As(err, &e) { + status, term = callMediaFailed, stats.ClientError("sdp-error") + sipReason = sip.StatusBadRequest } - rawSDP := req.Body() - tmedia := c.mon.StageDurTimer("start-media") - answerData, err := c.runMediaConn(tid, rawSDP, m, conf, disp.EnabledFeatures, disp.FeatureFlags) - tmedia() - if err != nil { - sipReason := sip.StatusInternalServerError - log = log.WithValues("sdp", string(rawSDP)) - status, term := callDropped, stats.ServerError("media-failed") - if errors.Is(err, sdp.ErrNoCommonMedia) { - status, term = callMediaFailed, stats.ClientError("no-common-codec") - sipReason = sip.StatusBadRequest - } else if errors.Is(err, sdp.ErrNoCommonCrypto) { - status, term = callMediaFailed, stats.ClientError("no-common-crypto") - sipReason = sip.StatusBadRequest - } else if e := (SDPError{}); errors.As(err, &e) { - status, term = callMediaFailed, stats.ClientError("sdp-error") - sipReason = sip.StatusBadRequest - } - if sipReason >= 500 { - log.Errorw("Cannot start media", err) - } else { - log.Warnw("Cannot start media", err) - } - c.cc.RespondAndDrop(sipReason, "") - c.close(ctx, EndCall{ - Status: status, - Term: term, - }) - return nil, err + if sipReason >= 500 { + log.Errorw("Cannot start media", err) + } else { + log.Warnw("Cannot start media", err) } - return answerData, nil + c.cc.RespondAndDrop(sipReason, "") + c.close(ctx, EndCall{ + Status: status, + Term: term, + }) + return err } // If we do not wait for ACK during Accept, we could wait for it later. @@ -942,7 +955,6 @@ func (c *inboundCall) handleInvite(ctx context.Context, tid traceid.ID, req *sip ackTimeout <-chan time.Time ) - // We need to start media first, otherwise we won't be able to send audio prompts to the caller, or receive DTMF. acceptCall := func(answerData []byte) (bool, error) { defer c.mon.StageDurTimer("call-accept")() headers := disp.Headers @@ -967,13 +979,16 @@ func (c *inboundCall) handleInvite(ctx context.Context, tid traceid.ID, req *sip }) return false, err } + c.media.SetTimeout(c.s.conf.MediaTimeoutInitial, mconf.MediaTimeout) // Only enable media timeout once we send back SDP. if !c.s.conf.Experimental.InboundWaitACK { ackReceived = c.cc.InviteACK() // Start this timer right after the Accept. ackTimeout = time.After(inviteOkAckLateTimeout) } - c.media.EnableTimeout(true) - c.media.EnableOut() + if old := c.audioOut.Swap(c.media.GetOutboundAudioWriter()); old != nil { + c.log().Warnw("unexpected audio out writer", nil) + old.Close() + } if ok, err := c.waitMedia(ctx); !ok { return false, err } @@ -981,17 +996,20 @@ func (c *inboundCall) handleInvite(ctx context.Context, tid traceid.ID, req *sip return true, nil } + if err := c.createMediaPort(mconf, conf, disp.FeatureFlags); err != nil { + return rejectMedia(err) + } + ok := false var answerData []byte if pinPrompt { - var err error - // Accept the call first on the SIP side, so that we can send audio prompts. - // This also means we have to pick encryption setting early, before room is selected. - // Backend must explicitly enable encryption for pin prompts. - answerData, err = runMedia(mconf) + // Negotiate before Accept so pin prompts and DTMF have a live pipeline. + // Encryption is picked here, before the room is selected. + answerData, err = c.negotiateMedia(rawSDP) if err != nil { - return err // already sent a response + return rejectMedia(err) } + c.connectPinDTMF() if ok, err = acceptCall(answerData); !ok { return err // could be success if the caller hung up } @@ -999,14 +1017,8 @@ func (c *inboundCall) handleInvite(ctx context.Context, tid traceid.ID, req *sip if !ok { return err // already sent a response. Could be success if user hung up } - } else { - // Start media with given encryption settings. - var err error - answerData, err = runMedia(mconf) - if err != nil { - return err // already sent a response - } } + p := &disp.Room.Participant p.Attributes = HeadersToAttrs(p.Attributes, disp.HeadersToAttributes, disp.IncludeHeaders, c.cc, nil) if disp.MaxCallDuration <= 0 || disp.MaxCallDuration > maxCallDuration { @@ -1023,11 +1035,15 @@ func (c *inboundCall) handleInvite(ctx context.Context, tid traceid.ID, req *sip if pinPrompt { status = CallActive } + answerData, err = c.negotiateMedia(rawSDP) + if err != nil { + return rejectMedia(err) + } if err := c.joinRoom(ctx, disp.Room, status); err != nil { return fmt.Errorf("failed joining room: %w", err) } // Publish our own track. - if err := c.publishTrack(); err != nil { + if err := c.publishTrack(disp.EnabledFeatures, disp.FeatureFlags); err != nil { c.log().Errorw("Cannot publish track", err) c.closeWithTerm(ctx, stats.ServerError("publish-failed")) return fmt.Errorf("publishing track to room failed: %w", err) @@ -1091,7 +1107,7 @@ func (c *inboundCall) waitForCallEnd(ctx context.Context, ackReceived <-chan str }) c.closeWithTerm(ctx, terminationFromRoomDisconnect(roomReason)) return nil - case <-c.media.Timeout(): + case <-c.media.MediaTimeout(): return c.mediaTimeout(ctx) case <-ackReceived: ackTimeout = nil // all good, disable timeout @@ -1099,21 +1115,60 @@ func (c *inboundCall) waitForCallEnd(ctx context.Context, ackReceived <-chan str case <-ackTimeout: // Only warn, the other side still thinks the call is active, media may be flowing. c.log().Warnw("Call accepted, but no ACK received", errNoACK) - // We don't need to wait for a full media timeout initially, we already know something is not quite right. + + // Today we seek to enforce all calls to be ACKed or dropped. + // Sometimes, though, we do not see ACKs for invites (e.g due to possible + // issues with load balancing). + // To accommodate this issue, instead of ending the call right here, we instead + // set an aggressive timeout as a softer fallback. + // If the issue really is a dropped ACK, media is expected to flow shortly, + // allowing us to accommodate this eventuality. If, however, there is no media + // observed, the call still ends quickly. + // Once ACKs are certain to be reliable, we will end the call here. c.media.SetTimeout(min(inviteOkAckLateTimeout, c.s.conf.MediaTimeoutInitial), mediaTimeout) } } } -func (c *inboundCall) runMediaConn(tid traceid.ID, offerData []byte, mconf *sipMediaConfig, conf *config.Config, features []livekit.SIPFeature, featureFlags map[string]string) (answerData []byte, _ error) { +type pinDTMFWriter struct { + dtmfEvents chan<- dtmf.Event +} + +func (w *pinDTMFWriter) String() string { + return "pinDTMFWriter" +} + +func (w *pinDTMFWriter) SampleRate() int { + return dtmf.SampleRate +} + +func (w *pinDTMFWriter) Close() error { + return nil +} + +func (w *pinDTMFWriter) WriteSample(msg *livekit.SipDTMF) error { + if msg == nil { + return nil + } + + event := dtmfEventFromSipDTMF(msg) + // We should have enough buffer here. + select { + case w.dtmfEvents <- event: + default: + } + return nil +} + +func (c *inboundCall) createMediaPort(mconf *sipMediaConfig, conf *config.Config, featureFlags map[string]string) error { c.mmu.Lock() defer c.mmu.Unlock() - c.mon.SDPSize(len(offerData), true) - c.log().Debugw("SDP offer", "sdp", string(offerData)) + if c.media != nil { + return nil + } - logSignalChanges := false - logSignalChanges, _ = strconv.ParseBool(featureFlags[signalLoggingFeatureFlag]) - mp, err := NewMediaPort(tid, c.log(), c.mon, &MediaOptions{ + logSignalChanges, _ := strconv.ParseBool(featureFlags[signalLoggingFeatureFlag]) + mp, err := NewMediaPort(c.log(), c.mon, &MediaOptions{ IP: c.s.sconf.MediaIP, Ports: conf.RTPPort, MediaTimeoutInitial: c.s.conf.MediaTimeoutInitial, @@ -1123,47 +1178,65 @@ func (c *inboundCall) runMediaConn(tid traceid.ID, offerData []byte, mconf *sipM EnableJitterBuffer: c.jitterBuf, LogSignalChanges: logSignalChanges, Stats: &c.stats.Port, - NoInputResample: !RoomResample, DrainingIdleTimeout: conf.RTPDrainingIdleTimeout, DrainingDuration: conf.RTPDrainingDuration, + Codecs: mconf.Codecs, + Encryption: mconf.Encryption, + DTMFAudio: conf.AudioDTMF, }, RoomSampleRate) if err != nil { - return nil, err + return err } c.media = mp c.mediaCodecs = mconf.Codecs - mp.EnableTimeout(false) // enabled once we accept the call - mp.DisableOut() // disabled until we send 200 - mp.SetDTMFAudio(conf.AudioDTMF) - answer, mc, err := mp.SetOffer(offerData, mconf.Codecs, mconf.Encryption) - if err != nil { - return nil, err + // Mixer is created with the room; attach it now so pin prompts can play. + if old := c.lkRoom.WriteOutboundAudioTo(c.audioOut); old != nil { + c.log().Warnw("room has unexpected outbound audio writer", nil) + old.Close() } - answerData, err = answer.SDP.Marshal() - if err != nil { - return nil, err + if old := c.lkRoom.WriteOutboundDTMFTo(c.media.GetOutboundDTMFWriter()); old != nil { + c.log().Warnw("room has unexpected outbound audio DTMF writer", nil) + old.Close() } - c.mon.SDPSize(len(answerData), false) - c.log().Debugw("SDP answer", "sdp", string(answerData)) + return nil +} - if err = mp.SetConfig(mc); err != nil { - return nil, err +func (c *inboundCall) connectPinDTMF() { + if old := c.media.WriteInboundDTMFTo(&pinDTMFWriter{c.dtmf}); old != nil { + c.log().Warnw("media port has unexpected inbound DTMF writer", nil) + old.Close() + } +} + +func (c *inboundCall) negotiateMedia(offerData []byte) ([]byte, error) { + c.mmu.Lock() + defer c.mmu.Unlock() + if c.media == nil { + return nil, errors.New("media port not created") } - mc.Processor = c.s.handler.GetMediaProcessor(features, featureFlags, string(c.cc.ID()), MediaProcessorOpts{InputSampleRate: c.media.InputSampleRate()}) - if mc.Audio.DTMFType != 0 { - mp.HandleDTMF(c.handleDTMF) + if c.media.NegotiatedAudio() != nil { + return c.media.GetLocalSDP() } - // Must be set earlier to send the pin prompts. - if w := c.lkRoom.SwapOutput(mp.GetAudioWriter()); w != nil { - _ = w.Close() + defer c.mon.StageDurTimer("start-media")() + c.mon.SDPSize(len(offerData), true) + c.log().Debugw("SDP offer", "sdp", string(offerData)) + + answerData, err := c.media.GenerateAnswer(offerData) + if err != nil { + return nil, err } - if mc.Audio.DTMFType != 0 { - c.lkRoom.SetDTMFOutput(mp) + + c.mon.SDPSize(len(answerData), false) + c.log().Debugw("SDP answer", "sdp", string(answerData)) + + audio := c.media.NegotiatedAudio() + if audio == nil { + return nil, fmt.Errorf("media does not have negotiated audio") } c.state.DeferUpdate(func(info *livekit.SIPCallInfo) { - info.AudioCodec = mc.Audio.Codec.Info().SDPName + info.AudioCodec = audio.Codec.Info().SDPName }) return answerData, nil } @@ -1193,7 +1266,7 @@ func (c *inboundCall) waitMedia(ctx context.Context) (bool, error) { case <-c.lkRoom.Closed(): c.closeWithHangup(ctx) return false, psrpc.NewErrorf(psrpc.Canceled, "room closed") - case <-c.media.Timeout(): + case <-c.media.MediaTimeout(): return false, c.mediaTimeout(ctx) case end := <-c.endCall: c.close(ctx, end) @@ -1220,7 +1293,7 @@ func (c *inboundCall) waitSubscribe(ctx context.Context, timeout time.Duration) case <-c.lkRoom.Closed(): c.closeWithHangup(ctx) return false, psrpc.NewErrorf(psrpc.Canceled, "room closed") - case <-c.media.Timeout(): + case <-c.media.MediaTimeout(): return false, c.mediaTimeout(ctx) case end := <-c.endCall: c.close(ctx, end) @@ -1249,7 +1322,7 @@ func (c *inboundCall) pinPrompt(ctx context.Context, trunkID string) (disp CallD case <-ctx.Done(): c.closeWithHangup(ctx) return disp, false, nil - case <-c.media.Timeout(): + case <-c.media.MediaTimeout(): return disp, false, c.mediaTimeout(ctx) case b, ok := <-c.dtmf: if !ok { @@ -1501,10 +1574,18 @@ func (c *inboundCall) Shutdown(ctx context.Context) { c.closeWithTerm(ctx, stats.ServerError("shutdown")) } -func (c *inboundCall) updateRemoteFromSDP(body []byte) { +func (c *inboundCall) updateRemoteFromSDP(body []byte) error { + var mp MediaPort + c.mmu.Lock() - defer c.mmu.Unlock() - updateRemoteFromSDP(c.media, c.log(), c.mediaCodecs, body) + mp = c.media + c.mmu.Unlock() + + if mp == nil { + return nil + } + _, err := mp.GenerateAnswer(body) + return err } func (c *inboundCall) closeMedia() { @@ -1545,7 +1626,6 @@ func (c *inboundCall) createLiveKitParticipant(ctx context.Context, rconf RoomCo partConf.Attributes[k] = v } partConf.Attributes[livekit.AttrSIPCallStatus] = status.Attribute() - c.forwardDTMF.Store(true) select { case <-ctx.Done(): return ctx.Err() @@ -1574,14 +1654,24 @@ func (c *inboundCall) createLiveKitParticipant(ctx context.Context, rconf RoomCo return nil } -func (c *inboundCall) publishTrack() error { +func (c *inboundCall) publishTrack(features []livekit.SIPFeature, featureFlags map[string]string) error { defer c.mon.StageDurTimer("track-publish")() - local, err := c.lkRoom.NewParticipantTrack(RoomSampleRate) + inboundAudio, err := c.lkRoom.GetInboundAudioWriter() if err != nil { _ = c.lkRoom.Close() return err } - c.media.WriteAudioTo(local) + + if audioInProcessor := c.s.handler.GetMediaProcessor(features, featureFlags, string(c.cc.ID()), MediaProcessorOpts{InputSampleRate: RoomSampleRate}); audioInProcessor != nil { + inboundAudio = audioInProcessor(inboundAudio) + } + if old := c.media.WriteInboundAudioTo(inboundAudio); old != nil { + c.log().Warnw("media port has unexpected inbound audio writer", nil) + old.Close() + } + if old := c.media.WriteInboundDTMFTo(c.lkRoom.GetInboundDTMFWriter()); old != nil { + old.Close() // Can be pinDTMFWriter + } return nil } @@ -1622,18 +1712,17 @@ func (c *inboundCall) playAudio(ctx context.Context, frames []msdk.PCM16Sample) _ = msdk.PlayAudio[msdk.PCM16Sample](ctx, t, rtp.DefFrameDur, frames) } -func (c *inboundCall) handleDTMF(tone dtmf.Event) { - if c.forwardDTMF.Load() { - _ = c.lkRoom.SendData(&livekit.SipDTMF{ - Code: uint32(tone.Code), - Digit: string([]byte{tone.Digit}), - }, lksdk.WithDataPublishReliable(true)) - return +func dtmfEventFromSipDTMF(msg *livekit.SipDTMF) dtmf.Event { + code := byte(msg.Code) + digit := byte(0) + if len(msg.Digit) == 1 { + digit = msg.Digit[0] + } else { + digit = dtmf.CodeToChar(code) } - // We should have enough buffer here. - select { - case c.dtmf <- tone: - default: + return dtmf.Event{ + Code: code, + Digit: digit, } } @@ -1650,21 +1739,18 @@ func (c *inboundCall) transferCall(ctx context.Context, transferTo string, heade rctx, rcancel := context.WithCancel(ctx) defer rcancel() - // mute the room audio to the SIP participant - w := c.lkRoom.SwapOutput(nil) + // Mute the room audio to the SIP participant. + // Skip closing the existing writer, which is c.audioOut. + _ = c.lkRoom.WriteOutboundAudioTo(nil) defer func() { if retErr != nil && !c.done.Load() { - c.lkRoom.SwapOutput(w) - } else if w != nil { - w.Close() + c.lkRoom.WriteOutboundAudioTo(c.audioOut) } }() go func() { - aw := c.media.GetAudioWriter() - - err := tones.Play(rctx, aw, ringVolume, tones.ETSIRinging) + err := tones.Play(rctx, c.audioOut, ringVolume, tones.ETSIRinging) if err != nil && !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) { c.log().Infow("cannot play dial tone", "error", err) } @@ -2001,6 +2087,10 @@ func (c *sipInbound) AcceptAsKeepAlive(sdp []byte) { c.respondWithData(sip.StatusOK, "OK", "application/sdp", sdp) } +func (c *sipInbound) RejectAsKeepAlive(status sip.StatusCode, reason string) { + c.respond(status, reason) +} + func (c *sipInbound) OwnSDP() []byte { c.mu.RLock() defer c.mu.RUnlock() diff --git a/pkg/sip/media.go b/pkg/sip/media.go index 33f6ee8c0..632b552d1 100644 --- a/pkg/sip/media.go +++ b/pkg/sip/media.go @@ -140,7 +140,6 @@ func ratePPM(rate float64, expected int) float64 { const ( channels = 1 RoomSampleRate = 48000 - RoomResample = false ) var staticPayloadTypes = map[uint8]string{ diff --git a/pkg/sip/media_pipeline.go b/pkg/sip/media_pipeline.go index 23b4c6320..2cfc0bfc3 100644 --- a/pkg/sip/media_pipeline.go +++ b/pkg/sip/media_pipeline.go @@ -1,4 +1,4 @@ -// Copyright 2024 LiveKit, Inc. +// Copyright 2026 LiveKit, Inc. // // Licensed under the Apache License, Version 2.0 (the "License"); // you may not use this file except in compliance with the License. @@ -17,137 +17,200 @@ package sip import ( "context" "errors" + "fmt" "io" + "math" "os" "strings" + "sync" "sync/atomic" - "time" msdk "github.com/livekit/media-sdk" "github.com/livekit/media-sdk/dtmf" "github.com/livekit/media-sdk/jitter" "github.com/livekit/media-sdk/mixer" "github.com/livekit/media-sdk/rtp" + "github.com/livekit/media-sdk/sdp" "github.com/livekit/media-sdk/srtp" + "github.com/livekit/protocol/livekit" "github.com/livekit/protocol/logger" - "github.com/livekit/protocol/utils/traceid" + "github.com/livekit/sip/pkg/stats" ) -func (p *MediaPort) SetConfig(c *MediaConf) error { - if p.closed.IsBroken() { - return errors.New("media is already closed") +type MediaPortPipelineConfig struct { + log logger.Logger + opts *MediaOptions + mon *stats.CallMonitor + stats *PortStats + onNewSSRC func() bool + onPacket func() +} + +func NewMediaPortPipeline( + conf *MediaPortPipelineConfig, + mc *sdp.MediaConfig, + port *udpConn, + audioToRoom msdk.PCM16Writer, + dtmfToRoom msdk.WriteCloser[*livekit.SipDTMF], + incomingSampleRate int, +) (*mediaPortPipeline, error) { + p := &mediaPortPipeline{ + conf: conf, + } + err := p.init(mc, port, audioToRoom, dtmfToRoom, incomingSampleRate) + if err != nil { + p.Close() + return nil, err + } + return p, nil +} + +// A data structure owning the implementation of everything between a udpConn and two output Switches +// Has two directions, with both audio and optionally DTMF for each. +// Constructed once per negotiation, possibly N times in the lifetime of udpConn/Switch anchors. +type mediaPortPipeline struct { + conf *MediaPortPipelineConfig // Expected to be owned by caller, not managed + + // Owned by pipeline + ctx context.Context + cancel context.CancelFunc + sess rtp.Session + rtpLoopWG sync.WaitGroup + muxToRoom atomic.Pointer[rtp.HandlerCloser] + dtmfMixer *mixer.Mixer + audioToRoom rtp.HandlerCloser + dtmfToRoom rtp.HandlerCloser + dtmfHandler msdk.WriteCloser[*livekit.SipDTMF] // Reference, not closed + audioToPort msdk.PCM16Writer // post-mixer chain towards port + mixerToPort msdk.PCM16Writer // Reference, not closed + dtmfToPort msdk.WriteCloser[*livekit.SipDTMF] + lastDTMFTimestamp atomic.Uint32 // rtp timestamp of last DTMF packet seen +} + +// Returns insulated (nopCloser) connectors, preventing anchor close from closing pipeline. +func (p *mediaPortPipeline) GetConnectors() (msdk.PCM16Writer, msdk.WriteCloser[*livekit.SipDTMF]) { + if p.audioToPort == nil { + return nil, nil + } + if p.dtmfToPort == nil { + return msdk.NopCloser(p.mixerToPort), nil } + return msdk.NopCloser(p.mixerToPort), msdk.NopCloser(p.dtmfToPort) +} + +// Build pipeline between a udpConn and two output Switches. +// Requires fields to be set: +// - log +// - opts +// - mon +// - stats +// - onNewSSRC +// - onPacket +func (p *mediaPortPipeline) init( + mc *sdp.MediaConfig, + port *udpConn, + audioToRoom msdk.PCM16Writer, + dtmfToRoom msdk.WriteCloser[*livekit.SipDTMF], + incomingSampleRate int, +) error { + p.ctx, p.cancel = context.WithCancel(context.Background()) + var crypto string - if c.Crypto != nil { - crypto = c.Crypto.Profile.String() + if mc.Crypto != nil { + crypto = mc.Crypto.Profile.String() } - p.log.Infow("using codecs", - "audio-codec", c.Audio.Codec.Info().SDPName, "audio-rtp", c.Audio.Type, - "dtmf-rtp", c.Audio.DTMFType, + p.conf.log.Infow("using codecs", + "audio-codec", mc.Audio.Codec.Info().SDPName, "audio-rtp", mc.Audio.Type, + "dtmf-rtp", mc.Audio.DTMFType, "srtp", crypto, ) - symmetric := p.opts.SymmetricRTP || (p.opts.IgnoreLocalAddrInSDP && c.Remote.Addr().IsPrivate()) - p.port.SetDst(c.Remote) - if symmetric { - p.port.SetSymmetric(true) + port.SetDst(mc.Remote) + if p.conf.opts.IgnoreLocalAddrInSDP && mc.Remote.Addr().IsPrivate() { + port.SetSymmetric(true) // Already initialized with opts, turn on for edge case } - if p.opts.IgnorePreanswerData { - // this needs to happen before the SRTP session is created, otherwise the read deadline will be - // overwritten and we may get stuck in the discard loop - p.port.stopDiscarding() - } - var ( - sess rtp.Session - err error - ) - if c.Crypto != nil { - sess, err = srtp.NewSession(p.log, p.port, c.Crypto) + p.lastDTMFTimestamp.Store(math.MaxUint32) + + var err error + if mc.Crypto != nil { + p.sess, err = srtp.NewSession(p.conf.log, port, mc.Crypto) } else { - sess = rtp.NewSession(p.log, p.port) + p.sess = rtp.NewSession(p.conf.log, port) } if err != nil { - return err + return fmt.Errorf("failed to setup pipeline session: %w", err) } - p.mu.Lock() - defer p.mu.Unlock() - p.conf = c - p.sess = sess - - if err = p.setupOutput(p.tid); err != nil { - return err + err = p.setupInput(mc, audioToRoom, dtmfToRoom) + if err != nil { + return fmt.Errorf("failed to setup pipeline input: %w", err) + } + err = p.setupOutput(mc, incomingSampleRate) + if err != nil { + return fmt.Errorf("failed to setup pipeline output: %w", err) } - p.setupInput() return nil } -func (p *MediaPort) setupInput() { - // Decoding pipeline (SIP RTP -> LK PCM) - codec := p.conf.Audio.Codec - codecInfo := codec.Info() - if p.opts.NoInputResample { - p.audioIn.SetSampleRate(codecInfo.SampleRate) - } - - // Latency measurement: shared timestamp between entry (RTP handler) and exit (PCM writer). +// Construct the Audio and optionally DTMF pipeline from SIP RTP to LK PCM, in reverse order. +func (p *mediaPortPipeline) setupInput(mc *sdp.MediaConfig, audioToRoom msdk.PCM16Writer, dtmfToRoom msdk.WriteCloser[*livekit.SipDTMF]) error { + var err error var inboundLatencyEntry atomic.Int64 + sink := msdk.NopCloser(audioToRoom) // Prevent pipeline close from closing room + sink = newLatencyPCMExit(sink, &inboundLatencyEntry, &p.conf.stats.LatencyInE2E) + codecInfo := mc.Audio.Codec.Info() + sink = msdk.ResampleWriter(sink, codecInfo.SampleRate) + sink = newMediaWriterCount(sink, &p.conf.stats.AudioInFrames, &p.conf.stats.AudioInSamples) - var audioWriter msdk.PCM16Writer = p.audioIn - audioWriter = newLatencyPCMExit(audioWriter, &inboundLatencyEntry, &p.stats.LatencyInE2E) - if p.stats != nil { - audioWriter = newMediaWriterCount(audioWriter, &p.stats.AudioInFrames, &p.stats.AudioInSamples) - } - if p.logSignalChanges { - signalLogger, err := NewSignalLogger(p.log, "input", audioWriter) + if p.conf.opts.LogSignalChanges { + sink, err = NewSignalLogger(p.conf.log, "input", sink) if err != nil { - p.log.Errorw("failed to create signal logger", err) - } else { - audioWriter = signalLogger + sink.Close() + return err } } - audioHandler := rtp.DecodePCM(audioWriter, p.conf.Audio.Codec, p.conf.Audio.Type) - // Wrap the decoder with silence suppression handler to fill gaps during silence suppression - audioHandler = newSilenceFiller(audioHandler, audioWriter, codecInfo.RTPClockRate, codecInfo.SampleRate, p.log) - p.audioInHandler = audioHandler + + audioHandler := rtp.DecodePCM(sink, mc.Audio.Codec, mc.Audio.Type) + + // SilenceFiller injects silence after decoding, but it needs access to RTP headers + // And these are only available before decoding, hence it wraps both audioHandler & sink + audioHandler = newSilenceFiller(audioHandler, sink, codecInfo.RTPClockRate, codecInfo.SampleRate, p.conf.log) mux := rtp.NewMux(nil) - mux.SetDefault(newRTPStatsHandler(p.mon, "", nil)) - mux.Register( - p.conf.Audio.Type, newRTPHandlerCount( - newRTPStatsHandler(p.mon, codecInfo.SDPName, audioHandler), - &p.stats.AudioPackets, &p.stats.AudioBytes, - ), + mux.SetDefault(newRTPStatsHandler(p.conf.mon, "", nil)) + + audioType := newRTPHandlerCount( + newRTPStatsHandler(p.conf.mon, codecInfo.SDPName, audioHandler), + &p.conf.stats.AudioPackets, &p.conf.stats.AudioBytes, ) - if p.conf.Audio.DTMFType != 0 { - mux.Register( - p.conf.Audio.DTMFType, newRTPHandlerCount( - newRTPStatsHandler(p.mon, dtmf.SDPNameAndRate, rtp.HandlerFunc(p.dtmfHandler)), - &p.stats.DTMFPackets, &p.stats.DTMFBytes, - ), + p.audioToRoom = audioType + mux.Register(mc.Audio.Type, audioType) + + if mc.Audio.DTMFType != 0 { + p.dtmfHandler = dtmfToRoom // Close doesn't propagate through rtp.HandlerFunc + dtmfType := newRTPHandlerCount( + newRTPStatsHandler(p.conf.mon, dtmf.SDPNameAndRate, rtp.HandlerFunc(p.handleEventRTP)), + &p.conf.stats.DTMFPackets, &p.conf.stats.DTMFBytes, ) + p.dtmfToRoom = dtmfType + mux.Register(mc.Audio.DTMFType, dtmfType) } - var hnd rtp.HandlerCloser = newRTPStreamStats(mux, &p.stats.MuxStats) - if p.jitterEnabled { + + var hnd rtp.HandlerCloser = newRTPStreamStats(mux, &p.conf.stats.MuxStats) + if p.conf.opts.EnableJitterBuffer { hnd = rtp.HandleJitter(hnd, jitter.WithPacketLossHandler(func(packetsLost, packetsDropped uint64) { - p.stats.JitterBufferPacketsLost.Store(packetsLost) - p.stats.JitterBufferPacketsDropped.Store(packetsDropped) + p.conf.stats.JitterBufferPacketsLost.Store(packetsLost) + p.conf.stats.JitterBufferPacketsDropped.Store(packetsDropped) })) } - hnd = newLatencyRTPEntry(hnd, &inboundLatencyEntry) - p.hnd.Store(&hnd) + + p.muxToRoom.Store(&hnd) + return nil } -func (p *MediaPort) dtmfHandler(h *rtp.Header, payload []byte) error { - ptr := p.dtmfIn.Load() - if ptr == nil { - return nil - } - fnc := *ptr - if fnc == nil { - return nil - } +// Processes an incoming telephony-event packet, turns into SipDTMF, and forwards it. +func (p *mediaPortPipeline) handleEventRTP(h *rtp.Header, payload []byte) error { // RFC 4733 requires all packets of a given digit to share identical timestamps. // The marker bit could be used instead, but it is prone to occasional loss. if h.Timestamp == p.lastDTMFTimestamp.Load() { @@ -158,47 +221,52 @@ func (p *MediaPort) dtmfHandler(h *rtp.Header, payload []byte) error { return nil } p.lastDTMFTimestamp.Store(h.Timestamp) - fnc(ev) - return nil + return p.dtmfHandler.WriteSample(&livekit.SipDTMF{ + Code: uint32(ev.Code), + Digit: string([]byte{ev.Digit}), + }) } -// Must be called holding the lock -func (p *MediaPort) setupOutput(tid traceid.ID) error { - if p.closed.IsBroken() { - return errors.New("media is already closed") - } - p.rtpLoopWG.Add(1) - go p.rtpLoop(tid, p.sess) +// Construct the Audio and optionally DTMF pipeline from LK PCM to SIP RTP +// Returns the insulated (nopCloser) connectors, and an error. +func (p *mediaPortPipeline) setupOutput(mc *sdp.MediaConfig, incomingSampleRate int) error { + p.rtpLoopWG.Go(p.rtpLoop) w, err := p.sess.OpenWriteStream() if err != nil { - return err + return fmt.Errorf("failed to open write stream: %w", err) } // Latency measurement: shared timestamp between entry (PCM writer) and exit (RTP writer). var outboundLatencyEntry atomic.Int64 - codecInfo := p.conf.Audio.Codec.Info() - w = newLatencyRTPExit(w, &outboundLatencyEntry, &p.stats.LatencyOut) - w = newRTPStatsWriter(p.mon, p.conf.Audio.Type, p.conf.Audio.DTMFType, codecInfo.SDPName, dtmf.SDPName, w) + codecInfo := mc.Audio.Codec.Info() + w = newLatencyRTPExit(w, &outboundLatencyEntry, &p.conf.stats.LatencyOut) + w = newRTPStatsWriter(p.conf.mon, mc.Audio.Type, mc.Audio.DTMFType, codecInfo.SDPName, dtmf.SDPName, w) s := rtp.NewSeqWriter(w) - p.audioOutRTP = s.NewStream(p.conf.Audio.Type, codecInfo.RTPClockRate) + audioOutRTP := s.NewStream(mc.Audio.Type, codecInfo.RTPClockRate) - // Encoding pipeline (LK PCM -> SIP RTP) - audioOut := rtp.EncodePCM(p.audioOutRTP, p.conf.Audio.Codec) - if p.stats != nil { - audioOut = newMediaWriterCount(audioOut, &p.stats.AudioOutFrames, &p.stats.AudioOutSamples) - } - if p.logSignalChanges { - audioOut, err = NewSignalLogger(p.log, "mixed", audioOut) + audioOut := rtp.EncodePCM(audioOutRTP, mc.Audio.Codec) + + audioOut = newMediaWriterCount(audioOut, &p.conf.stats.AudioOutFrames, &p.conf.stats.AudioOutSamples) + + if p.conf.opts.LogSignalChanges { + audioOut, err = NewSignalLogger(p.conf.log, "mixed", audioOut) if err != nil { audioOut.Close() // need to close since it's not linked to the port yet return err } } - if p.conf.Audio.DTMFType != 0 { - p.dtmfOutRTP = s.NewStream(p.conf.Audio.DTMFType, dtmf.SampleRate) - if p.dtmfAudioEnabled { + audioOut = msdk.ResampleWriter(audioOut, incomingSampleRate) + + audioOut = newLatencyPCMEntry(audioOut, &outboundLatencyEntry) + + p.audioToPort = audioOut + p.mixerToPort = audioOut + + if mc.Audio.DTMFType != 0 { + var dtmfAudio msdk.PCM16Writer = nil + if p.conf.opts.DTMFAudio { // Add separate mixer for DTMF audio. // TODO: optimize, if we'll ever need this code path mix, err := mixer.NewMixer(audioOut, rtp.DefFrameDur, 1, mixer.WithOutputChannel()) @@ -206,44 +274,48 @@ func (p *MediaPort) setupOutput(tid traceid.ID) error { return err } audioOut = mix.NewInput() - p.dtmfOutAudio = mix.NewInput() + dtmfAudio = mix.NewInput() + p.dtmfMixer = mix + p.mixerToPort = audioOut } - } - audioOut = newLatencyPCMEntry(audioOut, &outboundLatencyEntry) - - if w := p.audioOut.Swap(audioOut); w != nil { - _ = w.Close() + p.dtmfToPort = &dtmfOutWriter{ + log: p.conf.log, + ctx: p.ctx, + dtmfEvents: s.NewStream(mc.Audio.DTMFType, dtmf.SampleRate), + dtmfAudio: dtmfAudio, + getTimestamp: audioOutRTP.GetCurrentTimestamp, + } } return nil } -func (p *MediaPort) rtpLoop(tid traceid.ID, sess rtp.Session) { - defer p.rtpLoopWG.Done() +func (p *mediaPortPipeline) rtpLoop() { // Need a loop to process all incoming packets. for { - r, ssrc, err := sess.AcceptStream() + r, ssrc, err := p.sess.AcceptStream() if err != nil { if !errors.Is(err, io.EOF) && !errors.Is(err, os.ErrDeadlineExceeded) && !strings.Contains(err.Error(), "closed") { - p.log.Errorw("cannot accept RTP stream", err) + p.conf.log.Errorw("cannot accept RTP stream", err) } return } - p.stats.Streams.Add(1) - p.mediaReceived.Break() - log := p.log.WithValues("ssrc", ssrc) + p.conf.stats.Streams.Add(1) + if p.conf.onNewSSRC != nil { + p.conf.onNewSSRC() + } + log := p.conf.log.WithValues("ssrc", ssrc) log.Debugw("accepting RTP stream") - go p.rtpReadLoop(tid, log, r) + p.rtpLoopWG.Go(func() { p.rtpReadLoop(log, r) }) } } -func (p *MediaPort) rtpReadLoop(tid traceid.ID, log logger.Logger, r rtp.ReadStream) { +func (p *mediaPortPipeline) rtpReadLoop(log logger.Logger, r rtp.ReadStream) { const maxErrors = 50 // 1 sec, given 20 ms frames buf := make([]byte, rtp.MTUSize+1) overflow := false var ( h rtp.Header - pipeline string errorCnt int ) for { @@ -255,74 +327,132 @@ func (p *MediaPort) rtpReadLoop(tid traceid.ID, log logger.Logger, r rtp.ReadStr log.Errorw("read RTP failed", err) return } - p.packetCount.Add(1) - p.lastPacketTime.Store(time.Now().UnixNano()) - p.stats.Packets.Add(1) + if p.conf.onPacket != nil { + p.conf.onPacket() + } + p.conf.stats.Packets.Add(1) if n > rtp.MTUSize { if !overflow { overflow = true log.Errorw("RTP packet is larger than MTU limit", nil, "payloadSize", n) } - p.stats.IgnoredPackets.Add(1) + p.conf.stats.IgnoredPackets.Add(1) continue // ignore partial messages } - ptr := p.hnd.Load() + ptr := p.muxToRoom.Load() if ptr == nil { - p.stats.IgnoredPackets.Add(1) + p.conf.stats.IgnoredPackets.Add(1) continue } hnd := *ptr if hnd == nil { - p.stats.IgnoredPackets.Add(1) + p.conf.stats.IgnoredPackets.Add(1) continue } err = hnd.HandleRTP(&h, buf[:n]) if err != nil { - if pipeline == "" { - pipeline = hnd.String() - } log := log.WithValues( "payloadSize", n, "rtpHeader", h, - "pipeline", pipeline, + "pipeline", hnd.String(), "errorCount", errorCnt, ) log.Debugw("handle RTP failed", "error", err) errorCnt++ - p.stats.FailedPackets.Add(1) + p.conf.stats.FailedPackets.Add(1) if errorCnt >= maxErrors { log.Errorw("killing RTP loop due to persisted errors", err) return } continue } - p.stats.InputPackets.Add(1) + p.conf.stats.InputPackets.Add(1) errorCnt = 0 - pipeline = "" } } -func (p *MediaPort) WriteDTMF(ctx context.Context, digits string) error { - if len(digits) == 0 { - return nil +func (p *mediaPortPipeline) Close() error { + if p.cancel != nil { + p.cancel() // stop active DTMF digit send + } + var errs []error + if p.sess != nil { + errs = append(errs, p.sess.Close()) + p.rtpLoopWG.Wait() + } + if closer := p.muxToRoom.Swap(nil); closer != nil { + (*closer).Close() // Doesn't propagate onwards + } + if p.audioToRoom != nil { + p.audioToRoom.Close() + } + if p.dtmfToRoom != nil { + p.dtmfToRoom.Close() + } + if p.dtmfMixer != nil { + p.dtmfMixer.Stop() } - p.mu.Lock() - dtmfOut := p.dtmfOutRTP - audioOut := p.dtmfOutAudio - audioOutRTP := p.audioOutRTP - p.mu.Unlock() - if !p.dtmfAudioEnabled { - audioOut = nil + if p.audioToPort != nil { + errs = append(errs, p.audioToPort.Close()) } - if dtmfOut == nil && audioOut == nil { + if p.dtmfToPort != nil { + errs = append(errs, p.dtmfToPort.Close()) + } + return errors.Join(errs...) +} + +// dtmfOutWriter sends SipDTMF as RFC 4733 telephone-events (optional in-band audio). +type dtmfOutWriter struct { + log logger.Logger + + mu sync.Mutex + ctx context.Context // canceled by pipeline Close, aborts an in-flight digit train + dtmfEvents *rtp.Stream + dtmfAudio msdk.PCM16Writer + getTimestamp func() uint32 +} + +func (w *dtmfOutWriter) String() string { + return fmt.Sprintf("dtmfOutWriter(dtmfAudio: %v)", w.dtmfAudio != nil) +} + +func (w *dtmfOutWriter) SampleRate() int { + return dtmf.SampleRate +} + +func (w *dtmfOutWriter) Close() error { + if w == nil || w.dtmfAudio == nil { return nil } + return w.dtmfAudio.Close() +} - var rtpTs uint32 - if audioOutRTP != nil { - rtpTs = audioOutRTP.GetCurrentTimestamp() +func (w *dtmfOutWriter) WriteSample(sample *livekit.SipDTMF) error { + if sample == nil || sample.Code >= 0x10 || (len(sample.Digit) == 0 && sample.Code == 0) { + return fmt.Errorf("invalid DTMF sample: %v", sample) + } + digits := sample.Digit + if len(digits) == 0 { + digit := dtmf.CodeToChar(byte(sample.Code)) + if digit == 0 { + return fmt.Errorf("code %d not supported", sample.Code) + } + digits = string([]byte{digit}) + } else if sample.Code > 0 { + // We can't distinguish between a code0 and no code, but better have something here + w.log.Debugw("code payload detected, ignored due to explicit digits", "code", sample.Code, "digits", sample.Digit) } - return dtmf.Write(ctx, audioOut, dtmfOut, rtpTs, digits) + w.mu.Lock() + defer w.mu.Unlock() + var rtpTs uint32 + if w.dtmfEvents != nil { + rtpTs = w.getTimestamp() // TODO: Maybe time to introduce the auto timestamp feature? + } + err := dtmf.Write(w.ctx, w.dtmfAudio, w.dtmfEvents, rtpTs, digits) + if err != nil { + return err + } + return nil } diff --git a/pkg/sip/media_pipeline_test.go b/pkg/sip/media_pipeline_test.go new file mode 100644 index 000000000..ed629b1c4 --- /dev/null +++ b/pkg/sip/media_pipeline_test.go @@ -0,0 +1,635 @@ +// Copyright 2024 LiveKit, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package sip + +import ( + "context" + "fmt" + "math" + "slices" + "strings" + "sync" + "sync/atomic" + "testing" + "time" + + "github.com/pion/rtp" + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + msdk "github.com/livekit/media-sdk" + "github.com/livekit/media-sdk/dtmf" + "github.com/livekit/media-sdk/g711" + "github.com/livekit/media-sdk/opus" + msrtp "github.com/livekit/media-sdk/rtp" + "github.com/livekit/media-sdk/sdp" + "github.com/livekit/protocol/livekit" + "github.com/livekit/protocol/logger" +) + +const testDTMFPT = byte(101) + +func audioCodecByName(t testing.TB, name string) msdk.AudioCodec { + t.Helper() + for _, c := range msdk.Codecs() { + if strings.EqualFold(c.Info().SDPName, name) { + ac, ok := c.(msdk.AudioCodec) + require.True(t, ok, "codec %s is not audio", name) + return ac + } + } + t.Skipf("codec %s is not registered", name) + return nil +} + +// Opus is not a registered SIP SDP codec; wrap media-sdk/opus so the pipeline +// can encode/decode at RoomSampleRate (no resample). +func testOpusCodec(t testing.TB) msdk.AudioCodec { + t.Helper() + log := logger.NewTestLogger(t) + return msdk.NewAudioCodec(msdk.CodecInfo{ + SDPName: "opus/48000", + SampleRate: RoomSampleRate, + RTPClockRate: RoomSampleRate, + }, + func(w msdk.PCM16Writer) msdk.WriteCloser[opus.Sample] { + d, err := opus.Decode(w, 1, log) + if err != nil { + panic(err) + } + return d + }, + func(w msdk.WriteCloser[opus.Sample]) msdk.PCM16Writer { + e, err := opus.Encode(w, 1, log) + if err != nil { + panic(err) + } + return e + }, + ) +} + +func testAudioPT(c msdk.AudioCodec) byte { + info := c.Info() + if info.RTPIsStatic { + return info.RTPDefType + } + return 96 +} + +type dtmfCollector struct { + mu sync.Mutex + events []*livekit.SipDTMF +} + +func (c *dtmfCollector) String() string { + c.mu.Lock() + defer c.mu.Unlock() + res := "" + for _, event := range c.events { + res += event.Digit + } + return res +} + +func (c *dtmfCollector) SampleRate() int { return dtmf.SampleRate } + +func (c *dtmfCollector) Close() error { return nil } + +func (c *dtmfCollector) WriteSample(sample *livekit.SipDTMF) error { + c.mu.Lock() + defer c.mu.Unlock() + c.events = append(c.events, sample) + return nil +} + +func (c *dtmfCollector) snapshot() []*livekit.SipDTMF { + c.mu.Lock() + defer c.mu.Unlock() + out := make([]*livekit.SipDTMF, len(c.events)) + copy(out, c.events) + return out +} + +// pcmCollector accumulates decoded room audio. The pipeline writes from the RTP +// read goroutine while the test reads, so every access is guarded. +type pcmCollector struct { + sampleRate int + + mu sync.Mutex + buf msdk.PCM16Sample +} + +func (c *pcmCollector) String() string { return fmt.Sprintf("pcmCollector(%d)", c.sampleRate) } + +func (c *pcmCollector) SampleRate() int { return c.sampleRate } + +func (c *pcmCollector) Close() error { return nil } + +func (c *pcmCollector) WriteSample(sample msdk.PCM16Sample) error { + c.mu.Lock() + defer c.mu.Unlock() + c.buf = append(c.buf, sample...) + return nil +} + +func (c *pcmCollector) len() int { + c.mu.Lock() + defer c.mu.Unlock() + return len(c.buf) +} + +// since returns a copy of everything written after the first n samples. +func (c *pcmCollector) since(n int) msdk.PCM16Sample { + c.mu.Lock() + defer c.mu.Unlock() + if n >= len(c.buf) { + return nil + } + return slices.Clone(c.buf[n:]) +} + +// pipelineHarness is the durable side of a mediaPort: UDP pipe, pipeline config, +// buffer anchors, and a synthesized MediaConfig. The pipeline itself is swapped +// on configure / reconfigure. +type pipelineHarness struct { + t *testing.T + local *testUDPConn + remote *testUDPConn + port *udpConn + conf *MediaPortPipelineConfig + audioIn *msdk.WriteCloserSwitch[msdk.PCM16Sample] + audioOut *msdk.WriteCloserSwitch[msdk.PCM16Sample] + dtmfIn *msdk.WriteCloserSwitch[*livekit.SipDTMF] + dtmfOut *msdk.WriteCloserSwitch[*livekit.SipDTMF] + roomAudio *pcmCollector + roomDTMF *dtmfCollector + pipeline *mediaPortPipeline + ssrcCount atomic.Uint64 + packetCount atomic.Uint64 + codec msdk.AudioCodec + audioPT byte + dtmfPT byte +} + +func newPipelineHarness(t *testing.T, sampleRate int) *pipelineHarness { + t.Helper() + local, remote := newUDPPipe() + log := logger.NewTestLogger(t) + h := &pipelineHarness{ + t: t, + local: local, + remote: remote, + port: newUDPConn(log, local, false), + audioIn: msdk.NewWriteCloserSwitch[msdk.PCM16Sample](sampleRate), + audioOut: msdk.NewWriteCloserSwitch[msdk.PCM16Sample](sampleRate), + dtmfIn: msdk.NewWriteCloserSwitch[*livekit.SipDTMF](dtmf.SampleRate), + dtmfOut: msdk.NewWriteCloserSwitch[*livekit.SipDTMF](dtmf.SampleRate), + roomAudio: &pcmCollector{sampleRate: sampleRate}, + roomDTMF: &dtmfCollector{}, + } + h.audioIn.Swap(h.roomAudio) + h.dtmfIn.Swap(h.roomDTMF) + h.conf = &MediaPortPipelineConfig{ + log: log, + opts: &MediaOptions{}, + stats: &PortStats{}, + onNewSSRC: func() bool { + h.ssrcCount.Add(1) + return true + }, + onPacket: func() { + h.packetCount.Add(1) + }, + } + t.Cleanup(func() { + if h.pipeline != nil { + _ = h.pipeline.Close() + } + _ = local.Close() + _ = remote.Close() + }) + return h +} + +func (h *pipelineHarness) mediaConfig() *sdp.MediaConfig { + return &sdp.MediaConfig{ + Local: h.local.addr, + Remote: h.remote.addr, + Audio: sdp.AudioConfig{ + Codec: h.codec, + Type: h.audioPT, + DTMFType: h.dtmfPT, + }, + } +} + +func (h *pipelineHarness) configure(codec msdk.AudioCodec, audioPT, dtmfPT byte, dtmfAudio bool) { + h.t.Helper() + h.codec = codec + h.audioPT = audioPT + h.dtmfPT = dtmfPT + h.conf.opts = &MediaOptions{DTMFAudio: dtmfAudio} + + pipe, err := NewMediaPortPipeline(h.conf, h.mediaConfig(), h.port, h.audioIn, h.dtmfIn, h.audioIn.SampleRate()) + require.NoError(h.t, err) + audioToPort, dtmfToPort := pipe.GetConnectors() + h.pipeline = pipe + if old := h.audioOut.Swap(audioToPort); old != nil { + _ = old.Close() + } + if old := h.dtmfOut.Swap(dtmfToPort); old != nil { + _ = old.Close() + } +} + +func (h *pipelineHarness) reconfigure(codec msdk.AudioCodec, audioPT, dtmfPT byte, dtmfAudio bool) { + h.t.Helper() + if h.pipeline != nil { + require.NoError(h.t, h.pipeline.Close()) + } + h.port.Reopen() + h.ssrcCount.Store(0) + h.packetCount.Store(0) + h.configure(codec, audioPT, dtmfPT, dtmfAudio) +} + +func (h *pipelineHarness) drainRemote() { + for { + select { + case <-h.remote.buf: + default: + return + } + } +} + +func (h *pipelineHarness) roomFrame() msdk.PCM16Sample { + sampleRate := h.audioOut.SampleRate() + n := sampleRate / int(time.Second/msrtp.DefFrameDur) + return tonePCM(sampleRate, n, 10000) +} + +func (h *pipelineHarness) codecFrame() msdk.PCM16Sample { + rate := h.codec.Info().SampleRate + n := rate / int(time.Second/msrtp.DefFrameDur) + return tonePCM(rate, n, 12000) +} + +func (h *pipelineHarness) readRemotePacket(timeout time.Duration) (*rtp.Packet, bool) { + h.t.Helper() + select { + case raw := <-h.remote.buf: + var pkt rtp.Packet + require.NoError(h.t, pkt.Unmarshal(raw)) + return &pkt, true + case <-time.After(timeout): + return nil, false + } +} + +func (h *pipelineHarness) injectRTP(pkt *rtp.Packet) { + h.t.Helper() + raw, err := pkt.Marshal() + require.NoError(h.t, err) + _, err = h.remote.WriteToUDPAddrPort(raw, h.local.addr) + require.NoError(h.t, err) +} + +func (h *pipelineHarness) injectAudio(ssrc uint32, seq uint16, ts uint32, pcm msdk.PCM16Sample) { + h.t.Helper() + clock := h.codec.Info().RTPClockRate + if clock == 0 { + clock = h.codec.Info().SampleRate + } + var buf msrtp.Buffer + stream := msrtp.NewSeqWriter(&buf).NewStream(h.audioPT, clock) + enc := msrtp.EncodePCM(stream, h.codec) + require.NoError(h.t, enc.WriteSample(pcm)) + require.NoError(h.t, enc.Close()) + require.NotEmpty(h.t, buf, "codec produced no RTP") + for i, pkt := range buf { + pkt.Header.SSRC = ssrc + pkt.Header.SequenceNumber = seq + uint16(i) + if i == 0 { + pkt.Header.Timestamp = ts + } + h.injectRTP(pkt) + } +} + +func (h *pipelineHarness) injectDTMFDigit(ssrc uint32, digit string, ts uint32) { + h.t.Helper() + require.NotEmpty(h.t, digit) + pt := h.dtmfPT + if pt == 0 { + pt = testDTMFPT + } + var payload [4]byte + n, err := dtmf.Encode(payload[:], dtmf.Event{ + Digit: digit[0], + Volume: 10, + Dur: 800, + End: true, + }) + require.NoError(h.t, err) + h.injectRTP(&rtp.Packet{ + Header: rtp.Header{ + Version: 2, + PayloadType: pt, + SequenceNumber: 1, + Timestamp: ts, + SSRC: ssrc, + Marker: true, + }, + Payload: payload[:n], + }) +} + +func (h *pipelineHarness) runDirections(t *testing.T) { + t.Run("audio_from_room", h.testAudioFromRoom) + t.Run("audio_from_port", h.testAudioFromPort) + t.Run("dtmf_from_room", h.testDTMFFromRoom) + t.Run("dtmf_from_port", h.testDTMFFromPort) +} + +func (h *pipelineHarness) testAudioFromRoom(t *testing.T) { + h.drainRemote() + sample := h.roomFrame() + for range 5 { + require.NoError(t, h.audioOut.WriteSample(sample)) + } + deadline := time.Now().Add(time.Second) + found := false + for time.Now().Before(deadline) && !found { + pkt, ok := h.readRemotePacket(50 * time.Millisecond) + if !ok { + continue + } + if pkt.PayloadType == h.audioPT && len(pkt.Payload) > 0 { + found = true + } + } + require.True(t, found, "expected RTP audio toward the peer") +} + +func (h *pipelineHarness) testAudioFromPort(t *testing.T) { + before := h.roomAudio.len() + packetsBefore := h.packetCount.Load() + clock := h.codec.Info().RTPClockRate + if clock == 0 { + clock = h.codec.Info().SampleRate + } + samplesPerFrame := uint32(clock / int(time.Second/msrtp.DefFrameDur)) + sample := h.codecFrame() + for i := uint16(0); i < 5; i++ { + h.injectAudio(0xA11CE, 1+i, samplesPerFrame+uint32(i)*samplesPerFrame, sample) + } + require.Eventually(t, func() bool { + return h.packetCount.Load() >= packetsBefore+5 + }, time.Second, 5*time.Millisecond, "RTP should be accepted") + require.Eventually(t, func() bool { + return h.roomAudio.len() > before + }, time.Second, 5*time.Millisecond, "decoded PCM should reach room (packets=%d input=%d failed=%d ignored=%d room=%d)", + h.packetCount.Load(), + h.pipeline.conf.stats.InputPackets.Load(), + h.pipeline.conf.stats.FailedPackets.Load(), + h.pipeline.conf.stats.IgnoredPackets.Load(), + h.roomAudio.len(), + ) + require.Greater(t, pcmEnergy(h.roomAudio.since(before)), int64(0), "decoded room audio should carry energy") +} + +func (h *pipelineHarness) testDTMFFromRoom(t *testing.T) { + h.drainRemote() + if h.dtmfPT == 0 { + require.NoError(t, h.dtmfOut.WriteSample(&livekit.SipDTMF{Digit: "5", Code: 5})) + h.drainRemote() + return + } + + // dtmf.Write paces a 250ms tone on a real ticker. Assert the first + // telephone-event and let pipeline Close cancel the rest. + go func() { + _ = h.dtmfOut.WriteSample(&livekit.SipDTMF{Digit: "5", Code: 5}) + }() + deadline := time.Now().Add(time.Second) + for time.Now().Before(deadline) { + pkt, ok := h.readRemotePacket(20 * time.Millisecond) + if ok && pkt.PayloadType == h.dtmfPT { + return + } + } + t.Fatal("DTMF enabled: expected telephone-event RTP") +} + +func (h *pipelineHarness) testDTMFFromPort(t *testing.T) { + before := len(h.roomDTMF.snapshot()) + packetsBefore := h.packetCount.Load() + h.injectDTMFDigit(0xD7DF, "7", 8000) + require.Eventually(t, func() bool { + return h.packetCount.Load() > packetsBefore + }, time.Second, 5*time.Millisecond, "RTP should be accepted") + if h.dtmfPT == 0 { + require.Equal(t, before, len(h.roomDTMF.snapshot()), "DTMF disabled: must not reach room") + return + } + require.Eventually(t, func() bool { + return len(h.roomDTMF.snapshot()) > before + }, time.Second, 5*time.Millisecond) + got := h.roomDTMF.snapshot()[before:] + if assert.NotEmpty(t, got) { + assert.Equal(t, "7", got[0].Digit) + } +} + +func tonePCM(rate, samples int, amp int16) msdk.PCM16Sample { + out := make(msdk.PCM16Sample, samples) + for i := range out { + // Simple square-ish tone so PCMU round-trip keeps energy. + if (i/(rate/400))%2 == 0 { + out[i] = amp + } else { + out[i] = -amp + } + } + return out +} + +func pcmEnergy(s msdk.PCM16Sample) int64 { + var sum int64 + for _, v := range s { + if v < 0 { + v = -v + } + sum += int64(v) + } + return sum +} + +type testCodecSpec struct { + name string + sdp string +} + +type testDTMFSpec struct { + name string + pt byte + audio bool +} + +var ( + pipelineTestCodecs = allAudioCodecs() + pipelineTestRates = []int{8000, 16000, 48000} + pipelineTestDTMF = []testDTMFSpec{ + {name: "dtmf_disabled", pt: 0, audio: false}, + {name: "dtmf_event", pt: testDTMFPT, audio: false}, + {name: "dtmf_event_audio", pt: testDTMFPT, audio: true}, + } +) + +func TestMediaPipelinePermutations(t *testing.T) { + for _, spec := range pipelineTestCodecs { + t.Run(spec.Info().SDPName, func(t *testing.T) { + codec := spec.(msdk.AudioCodec) + pt := testAudioPT(codec) + for _, rate := range pipelineTestRates { + for _, d := range pipelineTestDTMF { + t.Run(fmt.Sprintf("%dHz/%s", rate, d.name), func(t *testing.T) { + h := newPipelineHarness(t, rate) + h.configure(codec, pt, d.pt, d.audio) + h.runDirections(t) + }) + } + } + }) + } +} + +func TestMediaPipelineTeardownMultiSSRC(t *testing.T) { + codec := audioCodecByName(t, g711.ULawSDPNameAndRate) + h := newPipelineHarness(t, RoomSampleRate) + h.configure(codec, testAudioPT(codec), testDTMFPT, false) + sample := h.codecFrame() + + h.injectAudio(0x11111111, 1, 160, sample) + h.injectAudio(0x22222222, 1, 160, sample) + + require.Eventually(t, func() bool { + return h.ssrcCount.Load() >= 2 + }, time.Second, 5*time.Millisecond, "expected AcceptStream for two SSRCs") + assert.Equal(t, h.packetCount.Load(), uint64(2)) + + done := make(chan error, 1) + go func() { + done <- h.pipeline.Close() + }() + select { + case err := <-done: + require.NoError(t, err) + case <-time.After(2 * time.Second): + t.Fatal("pipeline.Close hung with multiple SSRCs") + } +} + +func TestMediaPipelineReuseUDPConn(t *testing.T) { + const rate = 48000 + d := pipelineTestDTMF[1] // event-only + + for _, from := range pipelineTestCodecs { + t.Run("from_"+from.Info().SDPName, func(t *testing.T) { + for _, to := range pipelineTestCodecs { + t.Run("to_"+to.Info().SDPName, func(t *testing.T) { + c1 := from.(msdk.AudioCodec) + c2 := to.(msdk.AudioCodec) + h := newPipelineHarness(t, rate) + h.configure(c1, testAudioPT(c1), d.pt, d.audio) + t.Run("gen1", h.runDirections) + h.reconfigure(c2, testAudioPT(c2), d.pt, d.audio) + t.Run("gen2", h.runDirections) + }) + } + }) + } +} + +func generateDTMFPackets(t *testing.T, digits string) [][]*rtp.Packet { + t.Helper() + var buf msrtp.Buffer + packets := make([][]*rtp.Packet, len(digits)) + last := len(buf) + w := msrtp.NewSeqWriter(&buf).NewStream(101, dtmf.SampleRate) + timestamp := uint32(1000) + for i := range digits { + err := dtmf.Write(context.Background(), nil, w, timestamp, digits[i:i+1]) + require.NoError(t, err) + require.NotEmpty(t, buf) + timestamp += uint32(dtmf.SampleRate / 2) + packets[i] = slices.Clone(buf[last:]) + last = len(buf) + } + return packets +} + +func dropPackets(t *testing.T, dropType string, packets []*rtp.Packet) []*rtp.Packet { + t.Helper() + switch dropType { + case "none": + return packets + case "first": + require.Greater(t, len(packets), 3) + return packets[3:] + case "last": + require.Greater(t, len(packets), 3) + return packets[:len(packets)-3] + case "middle": + require.Greater(t, len(packets), 6) + ret := slices.Clone(packets[:3]) + ret = append(ret, packets[len(packets)-3:]...) + return ret + default: + t.Fatal("unknown drop type: " + dropType) + return nil + } +} + +func TestMediaPipelineDTMF(t *testing.T) { + // Multi-digit test, including correct handling of lost packets + digitCases := []string{"1", "12", "123"} + lossCases := []string{"none", "first", "last", "middle"} + + for _, digits := range digitCases { + packets := generateDTMFPackets(t, digits) + for _, lossPackets := range lossCases { + t.Run(fmt.Sprintf("digits=%s/loss=%s", digits, lossPackets), func(t *testing.T) { + got := &dtmfCollector{} + p := &mediaPortPipeline{dtmfHandler: got} + p.lastDTMFTimestamp.Store(math.MaxUint32) + for _, digitPackets := range packets { + sendPackets := dropPackets(t, lossPackets, digitPackets) + t.Logf("sending %d/%d packets", len(sendPackets), len(digitPackets)) + for _, pkt := range sendPackets { + h := pkt.Header + t.Logf("sending packet: seq=%d, ts=%d, marker=%t", h.SequenceNumber, h.Timestamp, h.Marker) + require.NoError(t, p.handleEventRTP(&h, pkt.Payload)) + } + } + t.Logf("sent: %s", digits) + t.Logf("got: %s", got.String()) + require.Equal(t, digits, got.String()) + }) + } + } +} diff --git a/pkg/sip/media_port.go b/pkg/sip/media_port.go index f854de962..ff0cc665a 100644 --- a/pkg/sip/media_port.go +++ b/pkg/sip/media_port.go @@ -15,6 +15,7 @@ package sip import ( + "bytes" "errors" "io" "math" @@ -26,15 +27,17 @@ import ( "time" "github.com/frostbyte73/core" + psdp "github.com/pion/sdp/v3" msdk "github.com/livekit/media-sdk" - "github.com/livekit/media-sdk/dtmf" "github.com/livekit/media-sdk/rtp" "github.com/livekit/media-sdk/sdp" + "github.com/livekit/media-sdk/srtp" "github.com/livekit/mediatransportutil/pkg/rtcconfig" + "github.com/livekit/protocol/livekit" "github.com/livekit/protocol/logger" - "github.com/livekit/protocol/utils/traceid" + "github.com/livekit/sip/pkg/config" "github.com/livekit/sip/pkg/stats" ) @@ -43,6 +46,7 @@ const ( defaultMediaTimeoutInitial = 30 * time.Second dstChangePrintInterval = 10 * 1000 * 1000 * 1000 // 10 seconds, in nanoseconds srcChangePrintInterval = dstChangePrintInterval + holdEnabled = false // Disabled in current code ) type PortStatsSnapshot struct { @@ -270,7 +274,9 @@ func (c *udpConn) Read(b []byte) (n int, err error) { } if c.symmetric.Load() { dst := c.dst.Load() - if dst == nil || !dst.IsValid() || *dst != addr { + if dst != nil && dst.Addr().IsUnspecified() { + // On hold: the peer may keep sending, but it doesn't want our media. + } else if dst == nil || !dst.IsValid() || *dst != addr { c.SetDst(addr) } } @@ -279,7 +285,7 @@ func (c *udpConn) Read(b []byte) (n int, err error) { func (c *udpConn) Write(b []byte) (n int, err error) { dst := c.dst.Load() - if dst == nil { + if dst == nil || dst.Addr().IsUnspecified() { // No remote or on hold return len(b), nil // ignore } return c.WriteToUDPAddrPort(b, *dst) @@ -337,6 +343,11 @@ func (c *udpConn) unwrap() UDPConn { return c.UDPConn } +func (c *udpConn) Reopen() { + c.closed.Store(false) + c.UDPConn.SetReadDeadline(time.Time{}) // Clear deadline, if set +} + func (c *udpConn) Close() error { c.stopDiscarding() c.closed.Store(true) @@ -344,11 +355,6 @@ func (c *udpConn) Close() error { return nil } -type MediaConf struct { - sdp.MediaConfig - Processor msdk.PCM16Processor -} - type MediaOptions struct { IP netip.Addr Ports rtcconfig.PortRange @@ -358,30 +364,95 @@ type MediaOptions struct { IgnoreLocalAddrInSDP bool // enable symmetric RTP if local IP is specified in SDP Stats *PortStats EnableJitterBuffer bool - NoInputResample bool - IgnorePreanswerData bool LogSignalChanges bool DrainingIdleTimeout time.Duration DrainingDuration time.Duration + Codecs *msdk.CodecSet + Encryption sdp.Encryption + DTMFAudio bool } -func NewMediaPort(tid traceid.ID, log logger.Logger, mon *stats.CallMonitor, opts *MediaOptions, sampleRate int) (*MediaPort, error) { - return NewMediaPortWith(tid, log, mon, nil, opts, sampleRate) +func (o *MediaOptions) ApplyDefaults() { + if o.MediaTimeoutInitial <= 0 { + o.MediaTimeoutInitial = defaultMediaTimeoutInitial + } + if o.MediaTimeout <= 0 { + o.MediaTimeout = defaultMediaTimeout + } + if o.Stats == nil { + o.Stats = &PortStats{} + } + if o.Codecs == nil { + o.Codecs = defaultCodecs + } + if o.Ports.Start == 0 { + o.Ports.Start = config.DefaultRTPPortRange.Start + } + if o.Ports.End == 0 { + o.Ports.End = config.DefaultRTPPortRange.End + } } -func NewMediaPortWith(tid traceid.ID, log logger.Logger, mon *stats.CallMonitor, conn UDPConn, opts *MediaOptions, sampleRate int) (*MediaPort, error) { +// MediaPort is the insulated media-plane API: UDP/RTP to the wire, SDP negotiation, +// and audio/DTMF endpoints. It does not know about calls, rooms, or SIP dialogs. +type MediaPort interface { + Close() + CloseWait() + + // GetOutboundAudioWriter returns the LK room -> SIP writer. + GetOutboundAudioWriter() msdk.PCM16Writer + // GetOutboundDTMFWriter returns the LK room -> SIP DTMF writer. + GetOutboundDTMFWriter() msdk.WriteCloser[*livekit.SipDTMF] + + // WriteInboundAudioTo tells port where to write inbound SIP audio. + WriteInboundAudioTo(w msdk.PCM16Writer) msdk.PCM16Writer + // WriteInboundDTMFTo tells port where to write inbound SIP DTMF. + WriteInboundDTMFTo(w msdk.WriteCloser[*livekit.SipDTMF]) msdk.WriteCloser[*livekit.SipDTMF] + + // If there is no offer, this generates an offer. + // If there is an offer, this simply returns the SDP of that offer. + // An offer is cleared once a negotiation is successful. + GenerateOffer() ([]byte, error) + + // GenerateAnswer returns an encoded SDP answer for the given offer. + // This does not arm the media timeout, use SetTimeout to do so. + // + // SIDE EFFECT: May cause a rebuild of the pipeline. + GenerateAnswer(offer []byte) ([]byte, error) + + // ProcessAnswer processes an encoded SDP answer from the remote client. Returns an + // error if the answer is invalid, the offer has not yet been generated, or + // if media has already been negotiated. + // + // SIDE EFFECT: May cause a rebuild of the pipeline. + ProcessAnswer(answer []byte) error + + GetLocalSDP() ([]byte, error) + + // NegotiatedAudio returns the audio configuration chosen by SDP negotiation. + // Returns nil if media has not been negotiated yet. + // + // REQUIRES: The caller should not mutate the returned audio config. + NegotiatedAudio() *sdp.AudioConfig + + // SetTimeout resets the media timeout with the given values. + // + // NOTE: This method is likely to go through additional changes. + SetTimeout(initial, general time.Duration) + + Received() <-chan struct{} + MediaTimeout() <-chan struct{} +} + +func NewMediaPort(log logger.Logger, mon *stats.CallMonitor, opts *MediaOptions, targetSampleRate int) (MediaPort, error) { + return NewMediaPortWith(log, mon, nil, opts, targetSampleRate) +} + +func NewMediaPortWith(log logger.Logger, mon *stats.CallMonitor, conn UDPConn, opts *MediaOptions, targetSampleRate int) (MediaPort, error) { if opts == nil { opts = &MediaOptions{} } - if opts.MediaTimeoutInitial <= 0 { - opts.MediaTimeoutInitial = defaultMediaTimeoutInitial - } - if opts.MediaTimeout <= 0 { - opts.MediaTimeout = defaultMediaTimeout - } - if opts.Stats == nil { - opts.Stats = &PortStats{} - } + opts.ApplyDefaults() if conn == nil { // use an even RTP port (RFC 3550); some gateways misroute media when offered an odd one c, err := rtp.ListenUDPEvenPortRange(opts.Ports.Start, opts.Ports.End, netip.AddrFrom4([4]byte{0, 0, 0, 0})) @@ -390,106 +461,84 @@ func NewMediaPortWith(tid traceid.ID, log logger.Logger, mon *stats.CallMonitor, } conn = c } - mediaTimeout := make(chan struct{}) - inSampleRate := sampleRate - if opts.NoInputResample { - inSampleRate = -1 // set only after SDP is accepted - } - p := &MediaPort{ - tid: tid, - log: log, - opts: opts, - mon: mon, - externalIP: opts.IP, - mediaTimeout: mediaTimeout, - timeoutKick: make(chan struct{}, 1), - jitterEnabled: opts.EnableJitterBuffer, - logSignalChanges: opts.LogSignalChanges, - port: newUDPConn(log, conn, opts.SymmetricRTP), - audioOut: msdk.NewSwitchWriter(sampleRate), - audioIn: msdk.NewSwitchWriter(inSampleRate), - stats: opts.Stats, - } - p.lastDTMFTimestamp.Store(math.MaxUint32) - if p.opts.IgnorePreanswerData { - p.port.startDiscarding() + var localCrypto []srtp.Profile + if opts.Encryption != sdp.EncryptionNone { + var err error + localCrypto, err = srtp.DefaultProfiles() + if err != nil { + return nil, err + } } + p := &mediaPort{ + log: log, + opts: opts, + mon: mon, + externalIP: opts.IP, + timeoutKick: make(chan struct{}, 1), + port: newUDPConn(log, conn, opts.SymmetricRTP), + stats: opts.Stats, + codecs: opts.Codecs, + encryption: opts.Encryption, + localCrypto: localCrypto, + } + // Explicitly set sample rate. We manually create resamplers to include in latency + p.audioOut = msdk.NewWriteCloserSwitch[msdk.PCM16Sample](targetSampleRate) + p.audioIn = msdk.NewWriteCloserSwitch[msdk.PCM16Sample](targetSampleRate) + p.dtmfIn = msdk.NewWriteCloserSwitch[*livekit.SipDTMF](0) + p.dtmfOut = msdk.NewWriteCloserSwitch[*livekit.SipDTMF](0) + + p.port.startDiscarding() p.timeoutInitial.Store(&opts.MediaTimeoutInitial) p.timeoutGeneral.Store(&opts.MediaTimeout) - go p.timeoutLoop(func() { - close(mediaTimeout) - }) + p.wg.Go(p.mediaTimeoutLoop) p.log.Debugw("listening for media on UDP", "port", p.Port()) return p, nil } -// MediaPort combines all functionality related to sending and accepting SIP media. -type MediaPort struct { - tid traceid.ID - log logger.Logger - opts *MediaOptions - mon *stats.CallMonitor - externalIP netip.Addr - port *udpConn - mediaReceived core.Fuse - packetCount atomic.Uint64 - lastPacketTime atomic.Int64 // UnixNano of last RTP packet, 0 if none - mediaTimeout <-chan struct{} - timeoutKick chan struct{} // wakes timeoutLoop when the deadline may have changed - timeoutStart atomic.Pointer[time.Time] - timeoutInitial atomic.Pointer[time.Duration] - timeoutGeneral atomic.Pointer[time.Duration] - closed core.Fuse - stats *PortStats - dtmfAudioEnabled bool - jitterEnabled bool - logSignalChanges bool - rtpLoopWG sync.WaitGroup - - mu sync.Mutex - conf *MediaConf - sess rtp.Session - hnd atomic.Pointer[rtp.HandlerCloser] - dtmfOutRTP *rtp.Stream - dtmfOutAudio msdk.PCM16Writer - - audioOutRTP *rtp.Stream - audioOut *msdk.SwitchWriter // LK PCM -> SIP RTP - audioIn *msdk.SwitchWriter // SIP RTP -> LK PCM - audioInHandler rtp.Handler // for debug only - dtmfIn atomic.Pointer[func(ev dtmf.Event)] - lastDTMFTimestamp atomic.Uint32 // rtp timestamp of last DTMF packet seen -} - -func (p *MediaPort) DisableOut() { - p.audioOut.Disable() -} - -func (p *MediaPort) EnableOut() { - p.audioOut.Enable() -} - -func (p *MediaPort) kickTimeoutLoop() { - select { - case p.timeoutKick <- struct{}{}: - default: // already pending - } -} - -func (p *MediaPort) disableTimeout() { - p.log.Debugw("media timeout disabled") - p.timeoutStart.Store(nil) - p.kickTimeoutLoop() -} - -func (p *MediaPort) enableTimeout(initial, general time.Duration) { +// mediaPort is the concrete MediaPort implementation. +type mediaPort struct { + log logger.Logger + wg sync.WaitGroup + opts *MediaOptions + mon *stats.CallMonitor + externalIP netip.Addr + port *udpConn + mediaReceived core.Fuse + packetCount atomic.Uint64 + lastPacketTime atomic.Int64 // UnixNano of last RTP packet, 0 if none + mediaTimeout core.Fuse + timeoutKick chan struct{} // wakes timeoutLoop when the deadline may have changed + timeoutStart atomic.Pointer[time.Time] + timeoutInitial atomic.Pointer[time.Duration] + timeoutGeneral atomic.Pointer[time.Duration] + closed core.Fuse + stats *PortStats + + targetSampleRate int + codecs *msdk.CodecSet + encryption sdp.Encryption + localCrypto []srtp.Profile // our SRTP material, generated once per port + + mu sync.RWMutex + pipeline *mediaPortPipeline + localSDP []byte + offer *sdp.Offer + negotiated *sdp.MediaConfig + + audioIn *msdk.WriteCloserSwitch[msdk.PCM16Sample] // SIP RTP -> LK PCM + audioOut *msdk.WriteCloserSwitch[msdk.PCM16Sample] // LK PCM -> SIP RTP + dtmfIn *msdk.WriteCloserSwitch[*livekit.SipDTMF] // SIP DTMF -> LK DTMF + dtmfOut *msdk.WriteCloserSwitch[*livekit.SipDTMF] // LK DTMF -> SIP DTMF +} + +func (p *mediaPort) SetTimeout(initial, general time.Duration) { if initial <= 0 || general <= 0 { - p.log.Warnw("attempting to set zero media timeout", nil, "initial", initial, "timeout", general) + p.log.Debugw("attempting to set zero media timeout", "initial", initial, "timeout", general, "fallbackInitial", p.opts.MediaTimeoutInitial, "fallbackTimeout", p.opts.MediaTimeout) if initial <= 0 { - initial = defaultMediaTimeoutInitial + initial = p.opts.MediaTimeoutInitial } if general <= 0 { - general = defaultMediaTimeout + general = p.opts.MediaTimeout } } p.timeoutInitial.Store(&initial) @@ -501,22 +550,13 @@ func (p *MediaPort) enableTimeout(initial, general time.Duration) { "initial", initial, "timeout", general, ) - p.kickTimeoutLoop() -} - -func (p *MediaPort) EnableTimeout(enabled bool) { - if !enabled { - p.disableTimeout() - return + select { + case p.timeoutKick <- struct{}{}: + default: // already pending } - p.enableTimeout(p.opts.MediaTimeoutInitial, p.opts.MediaTimeout) } -func (p *MediaPort) SetTimeout(initial, general time.Duration) { - p.enableTimeout(initial, general) -} - -func (p *MediaPort) timeoutLoop(timeoutCallback func()) { +func (p *mediaPort) mediaTimeoutLoop() { defer p.log.Infow("media timeout loop stopped") const disabledPark = time.Hour @@ -607,7 +647,7 @@ func (p *MediaPort) timeoutLoop(timeoutCallback func()) { "timeout", timeout, "isInitial", isInitial, ) - timeoutCallback() + p.mediaTimeout.Break() return } // Cap the wake-up at the general timeout so packet arrivals during a long @@ -617,50 +657,62 @@ func (p *MediaPort) timeoutLoop(timeoutCallback func()) { } } -func (p *MediaPort) Close() { +func (p *mediaPort) closePipelineLocked() { + // Lock must already be held + + // Close switch -> port + if closer := p.audioOut.Swap(nil); closer != nil { + _ = closer.Close() + } + if closer := p.dtmfOut.Swap(nil); closer != nil { + _ = closer.Close() + } + // Close port -> switch + if p.pipeline != nil { + _ = p.pipeline.Close() // Waits until session terminates + p.pipeline = nil + } +} + +func (p *mediaPort) Close() { p.closed.Once(func() { defer p.stats.Closed.Store(true) + logError := func(comp string, err error) { + if err != nil { + p.log.Errorw("error closing media port", err, "component", comp) + } + } + p.mu.Lock() defer p.mu.Unlock() - if w := p.audioOut.Swap(nil); w != nil { - _ = w.Close() - } - if w := p.audioIn.Swap(nil); w != nil { - _ = w.Close() - } - p.audioOutRTP = nil - p.audioInHandler = nil - p.dtmfOutRTP = nil - if p.dtmfOutAudio != nil { - p.dtmfOutAudio.Close() - p.dtmfOutAudio = nil - } - p.dtmfIn.Store(nil) - if p.sess != nil { - _ = p.sess.Close() - } - p.port.Close() - p.rtpLoopWG.Wait() + p.closePipelineLocked() + logError("port", p.port.Close()) conn := p.port.unwrap() if uc, ok := conn.(*net.UDPConn); ok { go DrainPort(p.log, uc, p.opts.DrainingIdleTimeout, p.opts.DrainingDuration, nil) } else { - _ = conn.Close() + logError("conn", conn.Close()) } - hnd := p.hnd.Load() - if hnd != nil { - (*hnd).Close() - } + logError("audioIn", p.audioIn.Close()) // Propagate Close() to onwards to room + logError("dtmfIn", p.dtmfIn.Close()) // Propagate Close() to onwards to room + logError("audioOut", p.audioOut.Close()) // No-op, but do anyway + logError("dtmfOut", p.dtmfOut.Close()) // No-op, but do anyway }) } -func (p *MediaPort) Port() int { +func (p *mediaPort) CloseWait() { + p.Close() + <-p.closed.Watch() + p.wg.Wait() +} + +func (p *mediaPort) Port() int { return p.port.LocalAddr().(*net.UDPAddr).Port } -func (p *MediaPort) RemoteAddr() netip.AddrPort { +func (p *mediaPort) RemoteAddr() netip.AddrPort { dst := p.port.dst.Load() if dst == nil { return netip.AddrPort{} @@ -668,103 +720,299 @@ func (p *MediaPort) RemoteAddr() netip.AddrPort { return *dst } -func (p *MediaPort) UpdateRemote(addr netip.AddrPort) { - if addr.IsValid() && !addr.Addr().IsUnspecified() { - p.port.SetDst(addr) +// Reported for inbound (SetOffer) only since outbound (SetAnswer) only contains the +// codec picked by the end user, and not what they actually support +func (p *mediaPort) reportPeerCodecs(d sdp.MediaDesc, reinvite bool) { + if p.mon == nil { + return } + p.mon.PeerSDP(peerCodecNames(d), reinvite) +} + +// Plumbing + +func (p *mediaPort) GetOutboundAudioWriter() msdk.PCM16Writer { + return p.audioOut +} + +// WriteInboundAudioTo sets audio writer that will receive decoded PCM from incoming RTP packets. +func (p *mediaPort) WriteInboundAudioTo(w msdk.PCM16Writer) msdk.PCM16Writer { + return p.audioIn.Swap(w) +} + +func (p *mediaPort) GetOutboundDTMFWriter() msdk.WriteCloser[*livekit.SipDTMF] { + return p.dtmfOut +} + +func (p *mediaPort) WriteInboundDTMFTo(w msdk.WriteCloser[*livekit.SipDTMF]) msdk.WriteCloser[*livekit.SipDTMF] { + return p.dtmfIn.Swap(w) } -func (p *MediaPort) Received() <-chan struct{} { +func (p *mediaPort) Received() <-chan struct{} { return p.mediaReceived.Watch() } -func (p *MediaPort) Timeout() <-chan struct{} { - return p.mediaTimeout +func (p *mediaPort) MediaTimeout() <-chan struct{} { + return p.mediaTimeout.Watch() } -func (p *MediaPort) Config() *MediaConf { +// SDP + +func (p *mediaPort) GenerateOffer() ([]byte, error) { p.mu.Lock() defer p.mu.Unlock() - return p.conf -} + if p.offer != nil { + return p.offer.SDP.Marshal() + } -// InputSampleRate returns the expected sample rate for incoming audio from SIP. -// Must be called after SetConfig, which sets the audioIn sample rate. -func (p *MediaPort) InputSampleRate() int { - return p.audioIn.SampleRate() + offer, err := sdp.NewOfferWith(p.codecs, p.externalIP, p.Port(), p.encryption, sdp.WithLocalProfiles(p.localCrypto)) + if err != nil { + return nil, err + } + p.offer = offer + return offer.SDP.Marshal() } -// WriteAudioTo sets audio writer that will receive decoded PCM from incoming RTP packets. -func (p *MediaPort) WriteAudioTo(w msdk.PCM16Writer) { - if processor := p.conf.Processor; processor != nil { - w = processor(w) +func (p *mediaPort) GenerateAnswer(offerData []byte) ([]byte, error) { + if len(offerData) == 0 { + return p.GetLocalSDP() } - if pw := p.audioIn.Swap(w); pw != nil { - _ = pw.Close() + + offer, err := sdp.ParseOfferWith(p.codecs, offerData) + if err != nil { + return nil, SDPError{Err: err} + } + p.mu.RLock() + isReinvite := p.negotiated != nil + p.mu.RUnlock() + p.reportPeerCodecs(offer.MediaDesc, isReinvite) + answer, mc, err := offer.Answer(p.externalIP, p.Port(), p.encryption, sdp.WithLocalProfiles(p.localCrypto)) + if err != nil { + return nil, SDPError{Err: err} } -} -// GetAudioWriter returns audio writer that will send PCM to the destination via RTP. -func (p *MediaPort) GetAudioWriter() msdk.PCM16Writer { - return p.audioOut + answerData, err := answer.SDP.Marshal() + if err != nil { + return nil, err + } + err = p.configure(mc, answerData) + if err != nil { + return nil, err + } + return answerData, nil } -// NewOffer generates an SDP offer for the media. -func (p *MediaPort) NewOffer(codecs *msdk.CodecSet, encrypted sdp.Encryption) (*sdp.Offer, error) { - return sdp.NewOfferWith(codecs, p.externalIP, p.Port(), encrypted) -} +func (p *mediaPort) ProcessAnswer(answerData []byte) error { + if len(answerData) == 0 { + return errors.New("no answer provided") + } -// SetAnswer decodes and applies SDP answer for offer from NewOffer. -// SetConfig must be called with the decoded configuration. -func (p *MediaPort) SetAnswer(offer *sdp.Offer, answerData []byte, codecs *msdk.CodecSet, enc sdp.Encryption) (*MediaConf, []byte, error) { - answer, err := sdp.ParseAnswerWith(codecs, answerData) + p.mu.RLock() + offer := p.offer + p.mu.RUnlock() + + if offer == nil { + return errors.New("no offer generated") + } + + answer, err := sdp.ParseAnswerWith(p.codecs, answerData) if err != nil { - return nil, nil, SDPError{Err: err} + return SDPError{Err: err} } - mc, localSDP, err := answer.ApplyWithLocal(offer, enc) + mc, localSDP, err := answer.ApplyWithLocal(offer, p.encryption) if err != nil { - return nil, nil, SDPError{Err: err} + return SDPError{Err: err} } + localSDPBytes, err := localSDP.Marshal() if err != nil { - return nil, nil, err + return err } - return &MediaConf{MediaConfig: *mc}, localSDPBytes, nil -} -// SetOffer decodes the offer from another party and returns encoded answer. To accept the offer, call SetConfig. -func (p *MediaPort) SetOffer(offerData []byte, codecs *msdk.CodecSet, enc sdp.Encryption) (*sdp.Answer, *MediaConf, error) { - offer, err := sdp.ParseOfferWith(codecs, offerData) + err = p.configure(mc, localSDPBytes) if err != nil { - return nil, nil, SDPError{Err: err} + return err } - p.reportPeerCodecs(offer.MediaDesc) - answer, mc, err := offer.Answer(p.externalIP, p.Port(), enc) - if err != nil { - return nil, nil, SDPError{Err: err} + p.SetTimeout(p.opts.MediaTimeoutInitial, p.opts.MediaTimeout) + return nil +} + +func (p *mediaPort) GetLocalSDP() ([]byte, error) { + p.mu.RLock() + defer p.mu.RUnlock() + if p.pipeline == nil || len(p.localSDP) == 0 { + return nil, errors.New("no SDP provided, no local SDP available") } - return answer, &MediaConf{MediaConfig: *mc}, nil + return p.localSDP, nil } -// Reported for inbound (SetOffer) only since outbound (SetAnswer) only contains the -// codec picked by the end user, and not what they actually support -func (p *MediaPort) reportPeerCodecs(d sdp.MediaDesc) { - if p.mon == nil { - return +func (p *mediaPort) NegotiatedAudio() *sdp.AudioConfig { + p.mu.RLock() + defer p.mu.RUnlock() + if p.negotiated == nil { + return nil } - p.mon.PeerSDP(peerCodecNames(d)) + return &p.negotiated.Audio } -// SetDTMFAudio forces SIP to generate audio dTMF tones in addition to digital signals. -func (p *MediaPort) SetDTMFAudio(enabled bool) { - p.dtmfAudioEnabled = enabled +// Building pipeline + +func (p *mediaPort) configure(c *sdp.MediaConfig, localSDP []byte) error { + // Map the durable udpConn + WriteCloserSwitch anchors onto a fresh mediaPortPipeline. + // Rebuild from scratch under mu: closePipelineLocked (soft-closes the session via udpConn), + // Reopen the port, then Configure a new generation and Swap TX leaves into the anchors. + + if c.Audio.Codec == nil { + return SDPError{Err: errors.New("no audio codec selected")} + } + + p.mu.Lock() // No concurrent rebuilding of the pipeline + defer p.mu.Unlock() + + p.offer = nil + + if p.closed.IsBroken() { + return errors.New("media is already closed") + } + + changeSetSummary := NewChangeSetSummary(p.negotiated, c) + + if changeSetSummary.includes(changeSetLocalAddr) { + return errors.New("unexpected local address change") + } + + audioToPort := p.audioOut.Swap(nil) // either nil or no-op closer + defer func() { p.audioOut.Swap(audioToPort) }() + dtmfToPort := p.dtmfOut.Swap(nil) // either nil or no-op closer + defer func() { p.dtmfOut.Swap(dtmfToPort) }() + + hold := false + + if changeSetSummary.includes(changeSetRemoteAddr) { + if c.Remote.Addr().IsUnspecified() { + // Older hold semantics: c=0.0.0.0 + hold = true + } else { + p.port.SetDst(netip.AddrPortFrom(c.Remote.Addr(), c.Remote.Port())) + p.negotiated.Remote = c.Remote + } + } + if changeSetSummary.includes(changeSetPeerDirection) { + // Newer hold semantics: a=sendonly + // TODO: Support a=recvonly/inactive; requires toggling media timeout; + // maybe gate these on timers being active on the session to prevent dud calls + hold = c.PeerDirection == psdp.DirectionSendOnly + } + if holdEnabled && hold { + audioToPort = nil + dtmfToPort = nil + zero := netip.IPv4Unspecified() + if !c.Remote.Addr().Is4() { + zero = netip.IPv6Unspecified() + } + p.port.SetDst(netip.AddrPortFrom(zero, c.Remote.Port())) + p.log.Infow("peer requested hold", "direction", c.PeerDirection.String(), "remote", c.Remote.String()) + } + if changeSetSummary.shouldReconfigure() { + if changeSetSummary != changeSetNew { + // Explicitly disable renegotiation for now + // Compatibility to today's behavior: return 200 OK, but don't reconfigure the pipeline + return nil + } + + p.closePipelineLocked() + audioToPort = nil + dtmfToPort = nil + p.port.stopDiscarding() // Needs readDeadline. Must be ahead of Reopen() and NewMediaPortPipeline() + p.port.Reopen() // Allow reads from socket again + + pipelineConfig := &MediaPortPipelineConfig{ + log: p.log, + opts: p.opts, + mon: p.mon, + stats: p.stats, + onNewSSRC: p.mediaReceived.Break, + onPacket: p.onNewMediaPacket, + } + newPipeline, err := NewMediaPortPipeline( + pipelineConfig, + c, + p.port, + p.audioIn, + p.dtmfIn, + p.audioOut.SampleRate(), + ) + if err != nil { + return err + } + + audioToPort, dtmfToPort = newPipeline.GetConnectors() // These are not propagating Close() + p.pipeline = newPipeline + + p.localSDP = localSDP // TODO: Move to end of function when reconfiguring is supported + } + p.negotiated = c + return nil +} + +func (p *mediaPort) onNewMediaPacket() { + p.packetCount.Add(1) + p.lastPacketTime.Store(time.Now().UnixNano()) } -// HandleDTMF sets an incoming DTMF handler. -func (p *MediaPort) HandleDTMF(h func(ev dtmf.Event)) { - if h == nil { - p.dtmfIn.Store(nil) - } else { - p.dtmfIn.Store(&h) +type changeSetSummary uint + +const ( + changeSetNew changeSetSummary = 1 << iota // 1 << 0 = 1 + changeSetAudioCodec + changeSetDTMF + changeSetCrypto + changeSetLocalAddr + changeSetRemoteAddr + changeSetPeerDirection +) + +func NewChangeSetSummary(current, new *sdp.MediaConfig) changeSetSummary { + if current == nil { + return changeSetNew + } + var changeSetSummary changeSetSummary + if current.Audio.Codec.Info().SDPName != new.Audio.Codec.Info().SDPName || current.Audio.Type != new.Audio.Type { + changeSetSummary |= changeSetAudioCodec } + if current.Audio.DTMFType != new.Audio.DTMFType { + changeSetSummary |= changeSetDTMF + } + a, b := current.Crypto, new.Crypto + if a == nil || b == nil { + if a != b { + changeSetSummary |= changeSetCrypto + } + } else { // Profile exists on both + if a.Profile != b.Profile || + !bytes.Equal(a.Keys.LocalMasterKey, b.Keys.LocalMasterKey) || + !bytes.Equal(a.Keys.LocalMasterSalt, b.Keys.LocalMasterSalt) || + !bytes.Equal(a.Keys.RemoteMasterKey, b.Keys.RemoteMasterKey) || + !bytes.Equal(a.Keys.RemoteMasterSalt, b.Keys.RemoteMasterSalt) { + changeSetSummary |= changeSetCrypto + } + } + if current.Local != new.Local { + changeSetSummary |= changeSetLocalAddr + } + if current.Remote != new.Remote { + changeSetSummary |= changeSetRemoteAddr + } + if current.PeerDirection != new.PeerDirection { + changeSetSummary |= changeSetPeerDirection + } + return changeSetSummary +} + +func (c changeSetSummary) shouldReconfigure() bool { + return c&(changeSetNew|changeSetAudioCodec|changeSetDTMF|changeSetCrypto) != 0 +} + +func (c changeSetSummary) includes(feature changeSetSummary) bool { + return c&feature != 0 } diff --git a/pkg/sip/media_port_negotiation_test.go b/pkg/sip/media_port_negotiation_test.go new file mode 100644 index 000000000..a33292e18 --- /dev/null +++ b/pkg/sip/media_port_negotiation_test.go @@ -0,0 +1,430 @@ +// Copyright 2024 LiveKit, Inc. +// +// Licensed under the Apache License, Version 2.0 (the "License"); +// you may not use this file except in compliance with the License. +// You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, software +// distributed under the License is distributed on an "AS IS" BASIS, +// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +// See the License for the specific language governing permissions and +// limitations under the License. + +package sip + +import ( + "errors" + "net/netip" + "strings" + "sync" + "testing" + "time" + + "github.com/stretchr/testify/assert" + "github.com/stretchr/testify/require" + + msdk "github.com/livekit/media-sdk" + "github.com/livekit/media-sdk/dtmf" + "github.com/livekit/media-sdk/g711" + "github.com/livekit/media-sdk/g722" + "github.com/livekit/media-sdk/rtp" + "github.com/livekit/media-sdk/sdp" + "github.com/livekit/mediatransportutil/pkg/rtcconfig" + "github.com/livekit/protocol/logger" +) + +// recvBuffer counts samples arriving from the RTP read loop. It only records the count, +// so tests can poll it while the pipeline is still writing. +type recvBuffer struct { + mu sync.Mutex + samples int +} + +func (b *recvBuffer) String() string { return "recvBuffer" } +func (b *recvBuffer) SampleRate() int { return RoomSampleRate } +func (b *recvBuffer) Close() error { return nil } + +func (b *recvBuffer) WriteSample(sample msdk.PCM16Sample) error { + b.mu.Lock() + defer b.mu.Unlock() + b.samples += len(sample) + return nil +} + +func (b *recvBuffer) count() int { + b.mu.Lock() + defer b.mu.Unlock() + return b.samples +} + +func roomFrame() msdk.PCM16Sample { + frame := make(msdk.PCM16Sample, RoomSampleRate/int(time.Second/rtp.DefFrameDur)) + for i := range frame { + if (i/40)%2 == 0 { + frame[i] = 8000 + } else { + frame[i] = -8000 + } + } + return frame +} + +// writeFrames pushes room audio into the port. Write errors are ignored: the in-memory +// UDP pipe is bounded, and a peer that is mid-renegotiation may not be draining it. +func writeFrames(m *mediaPort, frames int) { + w := m.GetOutboundAudioWriter() + frame := roomFrame() + for range frames { + _ = w.WriteSample(frame) + } +} + +// requireAudioFlows asserts that audio written to src is observed by dst's room writer. +func requireAudioFlows(t testing.TB, src *mediaPort, dst *recvBuffer) { + t.Helper() + before := dst.count() + require.Eventually(t, func() bool { + writeFrames(src, 5) + return dst.count() > before + }, 5*time.Second, 50*time.Millisecond, "no audio received") +} + +func testCodecSet(names ...string) *msdk.CodecSet { + set := msdk.NewCodecSet() + set.SetEnabled(dtmf.SDPNameAndRate, true) + for _, name := range names { + set.SetEnabled(name, true) + } + return set +} + +func enabledAudioCodecs() []msdk.Codec { + var audio []msdk.Codec + for _, c := range msdk.GlobalCodecs().ListEnabled() { + if _, ok := c.(msdk.AudioCodec); !ok { + continue // telephone-event and other non-audio codecs + } + audio = append(audio, c) + } + return audio +} + +func allAudioCodecs() []msdk.Codec { + var audio []msdk.Codec + for _, c := range msdk.Codecs() { + if _, ok := c.(msdk.AudioCodec); !ok { + continue // telephone-event and other non-audio codecs + } + audio = append(audio, c) + } + return audio +} + +func answerCodec(t testing.TB, answerData []byte) string { + t.Helper() + answer, err := sdp.ParseAnswerWith(defaultCodecs, answerData) + require.NoError(t, err) + for _, c := range answer.Codecs { + if c.Codec == nil || (answer.DTMFType != 0 && c.Type == answer.DTMFType) { + continue + } + return c.Codec.Info().SDPName + } + t.Fatal("no audio codec in answer") + return "" +} + +// A port only offers and accepts the codecs it was configured with. +func TestMediaPortCodecSet(t *testing.T) { + newLocked := func(t *testing.T, names ...string) *mediaPort { + return newTestPort(t, logger.NewTestLogger(t), newTestConn(1), &MediaOptions{ + IP: newIP("127.0.0.1"), + Codecs: testCodecSet(names...), + }, RoomSampleRate) + } + + t.Run("offer lists only enabled codecs", func(t *testing.T) { + m := newLocked(t, g711.ALawSDPNameAndRate) + + offerData, err := m.GenerateOffer() + require.NoError(t, err) + + offer, err := sdp.ParseOfferWith(defaultCodecs, offerData) + require.NoError(t, err) + + var names []string + for _, c := range offer.Codecs { + if c.Codec == nil || (offer.DTMFType != 0 && c.Type == offer.DTMFType) { + continue + } + names = append(names, c.Codec.Info().SDPName) + } + assert.Equal(t, []string{g711.ALawSDPNameAndRate}, names) + assert.NotZero(t, offer.DTMFType, "DTMF should still be offered") + }) + + t.Run("answer picks an enabled codec", func(t *testing.T) { + m := newLocked(t, g711.ALawSDPNameAndRate) + + // Peer offers both, only PCMA is enabled here. + offer := sdpWithMedia("m=audio 5004 RTP/AVP 0 8", + "a=rtpmap:0 PCMU/8000", "a=rtpmap:8 PCMA/8000") + answerData, err := m.GenerateAnswer(offer) + require.NoError(t, err) + assert.Equal(t, g711.ALawSDPNameAndRate, answerCodec(t, answerData)) + }) + + t.Run("offer without an enabled codec is rejected", func(t *testing.T) { + m := newLocked(t, g711.ALawSDPNameAndRate) + + offer := sdpWithMedia("m=audio 5004 RTP/AVP 0", "a=rtpmap:0 PCMU/8000") + _, err := m.GenerateAnswer(offer) + require.ErrorIs(t, err, sdp.ErrNoCommonMedia) + }) +} + +func TestMediaPortRejectsDifferentCodecOffer(t *testing.T) { + t.Skip("renegotiation is disabled: GenerateAnswer returns the prior answer when one already exists") + // TODO: change this test to confirm renegotiation when it's enabled + m := newTestPort(t, logger.NewTestLogger(t), newTestConn(1), &MediaOptions{ + IP: newIP("127.0.0.1"), + Codecs: testCodecSet(g711.ULawSDPNameAndRate, g722.SDPNameAndRate), + }, RoomSampleRate) + + sdpA := sdpWithMedia("m=audio 5004 RTP/AVP 0", "a=rtpmap:0 PCMU/8000") + sdpB := sdpWithMedia("m=audio 5004 RTP/AVP 9", "a=rtpmap:9 G722/8000") + + // Offer codec A + answer, err := m.GenerateAnswer(sdpA) + require.NoError(t, err) + require.Equal(t, g711.ULawSDPNameAndRate, answerCodec(t, answer)) + + // Attempt to offer only codec B, expect failure + answer, err = m.GenerateAnswer(sdpB) + require.ErrorIs(t, err, sdp.ErrNoCommonMedia) + + // Offer codec A again, expect success + answer, err = m.GenerateAnswer(sdpA) + require.NoError(t, err) + require.Equal(t, g711.ULawSDPNameAndRate, answerCodec(t, answer)) +} + +// Renegotiation rebuilds the pipeline under the same port and keeps audio flowing, +// including across a codec change that moves the encoder's sample rate. +func TestMediaPortRenegotiation(t *testing.T) { + t.Skip("renegotiation is disabled: GenerateAnswer returns the prior answer when one already exists") + t.Run("repeated", func(t *testing.T) { + m1, m2 := newMediaPair(t, nil, nil, "", RoomSampleRate) + + recv2 := &recvBuffer{} + m2.WriteInboundAudioTo(recv2) + requireAudioFlows(t, m1, recv2) + + for range 3 { + negotiate(t, m1, m2) + + local, err := m1.GetLocalSDP() + require.NoError(t, err) + assert.NotEmpty(t, local) + + // The room-facing writers survive the rebuild, and the new pipeline sends. + assert.NotNil(t, m1.audioOut.Get()) + requireAudioFlows(t, m1, recv2) + } + }) + + t.Run("codec change", func(t *testing.T) { + c1, c2 := newUDPPipe() + log := logger.NewTestLogger(t) + + m1 := newTestPort(t, log.WithName("one"), c1, &MediaOptions{ + IP: newIP("1.1.1.1"), + Ports: rtcconfig.PortRange{Start: 10000}, + }, RoomSampleRate) + m2 := newTestPort(t, log.WithName("two"), c2, &MediaOptions{ + IP: newIP("2.2.2.2"), + Ports: rtcconfig.PortRange{Start: 20000}, + Codecs: testCodecSet(g711.ULawSDPNameAndRate), + }, RoomSampleRate) + + answerData := negotiate(t, m1, m2) + require.Equal(t, g711.ULawSDPNameAndRate, answerCodec(t, answerData)) + + recv2 := &recvBuffer{} + m2.WriteInboundAudioTo(recv2) + requireAudioFlows(t, m1, recv2) + + // G722 samples at 16k, so the encode leaf changes sample rate under the same + // room-facing switch. + m2.codecs = testCodecSet(g722.SDPNameAndRate) + + answerData = negotiate(t, m1, m2) + require.Equal(t, g722.SDPNameAndRate, answerCodec(t, answerData)) + + requireAudioFlows(t, m1, recv2) + }) +} + +// A peer that will not receive (RFC 3264 a=sendonly, or the legacy c=0.0.0.0) stops our +// media without stopping theirs, and resumes on the next offer. +func TestMediaPortHold(t *testing.T) { + t.Skip("hold requires renegotiation: configure returns without rebuilding when a pipeline already exists") + cases := []struct { + name string + hold func(t *testing.T, offer string) string + }{ + { + name: "sendonly", + hold: func(t *testing.T, offer string) string { + held := strings.Replace(offer, "a=sendrecv", "a=sendonly", 1) + require.NotEqual(t, offer, held) + return held + }, + }, + { + name: "zero connection address", + hold: func(t *testing.T, offer string) string { + held := strings.ReplaceAll(offer, "c=IN IP4 2.2.2.2", "c=IN IP4 0.0.0.0") + require.NotEqual(t, offer, held) + return held + }, + }, + } + for _, tc := range cases { + t.Run(tc.name, func(t *testing.T) { + m1, m2 := newMediaPair(t, nil, nil, "", RoomSampleRate) + + recv1 := &recvBuffer{} + m1.WriteInboundAudioTo(recv1) + recv2 := &recvBuffer{} + m2.WriteInboundAudioTo(recv2) + + // Baseline: m1 sends to m2. + require.NotNil(t, m1.audioOut.Get()) + requireAudioFlows(t, m1, recv2) + + // m2 re-INVITEs with the hold form of its offer. + base, err := m2.GenerateOffer() + require.NoError(t, err) + _, err = m1.GenerateAnswer([]byte(tc.hold(t, string(base)))) + require.NoError(t, err) + + // m1 no longer sends: no destination to write to, and the room-facing + // writer is detached. Room audio is dropped rather than erroring. + dst := m1.port.dst.Load() + if assert.NotNil(t, dst) { + assert.True(t, dst.Addr().IsUnspecified(), "held port kept a destination: %v", dst) + } + assert.Nil(t, m1.audioOut.Get(), "held port still accepts room audio") + assert.NoError(t, m1.GetOutboundAudioWriter().WriteSample(roomFrame())) + + sent := recv2.count() + writeFrames(m1, 10) + time.Sleep(100 * time.Millisecond) + assert.Equal(t, sent, recv2.count(), "held port kept sending") + + // ...while m2's media still reaches us. + requireAudioFlows(t, m2, recv1) + + // Resume with the original offer. + _, err = m1.GenerateAnswer(base) + require.NoError(t, err) + + dst = m1.port.dst.Load() + if assert.NotNil(t, dst) { + assert.False(t, dst.Addr().IsUnspecified(), "destination not restored") + assert.Equal(t, m2.Port(), int(dst.Port())) + } + assert.NotNil(t, m1.audioOut.Get()) + + requireAudioFlows(t, m1, recv2) + }) + } +} + +var policyToString = map[sdp.Encryption]string{ + sdp.EncryptionNone: "none", + sdp.EncryptionAllow: "allow", + sdp.EncryptionRequire: "require", +} + +func TestMediaPortEncryptionPolicy(t *testing.T) { + encryptionPolicies := []sdp.Encryption{ + sdp.EncryptionNone, + sdp.EncryptionAllow, + sdp.EncryptionRequire, + } + + forEach := func(t *testing.T, negotiate func(t *testing.T, mp *mediaPort, policy sdp.Encryption) (*sdp.MediaConfig, error)) { + for _, portEncryptionPolicy := range encryptionPolicies { + name := "port=" + policyToString[portEncryptionPolicy] + t.Run(name, func(t *testing.T) { + for _, remoteEncryptionPolicy := range encryptionPolicies { + name := "remote=" + policyToString[remoteEncryptionPolicy] + t.Run(name, func(t *testing.T) { + opts := &MediaOptions{ + IP: netip.MustParseAddr("1.1.1.1"), + Ports: rtcconfig.PortRange{Start: 10000}, + Encryption: portEncryptionPolicy, + } + conn := newTestConn(1) + mp := newTestPort(t, logger.NewTestLogger(t), conn, opts, RoomSampleRate) + + mc, err := negotiate(t, mp, remoteEncryptionPolicy) + if portEncryptionPolicy != sdp.EncryptionAllow && remoteEncryptionPolicy != sdp.EncryptionAllow && portEncryptionPolicy != remoteEncryptionPolicy { + // Expect failue + assert.Error(t, err) + assert.ErrorIs(t, err, sdp.ErrNoCommonCrypto) + return + } + // Expect success + assert.NoError(t, err) + if portEncryptionPolicy == sdp.EncryptionNone || remoteEncryptionPolicy == sdp.EncryptionNone { + assert.Nil(t, mc.Crypto) + } else { + assert.NotNil(t, mc.Crypto) + } + }) + } + }) + } + } + + t.Run("inbound", func(t *testing.T) { // Receive offer + negotiate := func(t *testing.T, mp *mediaPort, policy sdp.Encryption) (*sdp.MediaConfig, error) { + offer, err := sdp.NewOfferWith(defaultCodecs, newIP("127.0.0.1"), 5004, policy) + require.NoError(t, err) + offerData, err := offer.SDP.Marshal() + require.NoError(t, err) + answerData, err := mp.GenerateAnswer(offerData) + if err != nil { + return nil, err + } + answer, err := sdp.ParseAnswerWith(defaultCodecs, answerData) + require.NoError(t, err) + mc, _, err := answer.ApplyWithLocal(offer, policy) + return mc, err + } + forEach(t, negotiate) + }) + + t.Run("outbound", func(t *testing.T) { // Send offer, receive answer + negotiate := func(t *testing.T, mp *mediaPort, policy sdp.Encryption) (*sdp.MediaConfig, error) { + offerData, err := mp.GenerateOffer() + require.NoError(t, err) + offer, err := sdp.ParseOfferWith(defaultCodecs, offerData) + require.NoError(t, err) + answer, mc, err := offer.Answer(newIP("127.0.0.1"), 5004, policy) + if errors.Is(err, sdp.ErrNoCommonCrypto) { + return mc, err + } + require.NoError(t, err) + answerData, err := answer.SDP.Marshal() + require.NoError(t, err) + return mc, mp.ProcessAnswer(answerData) + } + forEach(t, negotiate) + }) +} diff --git a/pkg/sip/media_port_test.go b/pkg/sip/media_port_test.go index 64b693b4a..1cfea02b9 100644 --- a/pkg/sip/media_port_test.go +++ b/pkg/sip/media_port_test.go @@ -15,7 +15,6 @@ package sip import ( - "context" "fmt" "io" "math" @@ -23,7 +22,6 @@ import ( "net/netip" "os" "slices" - "strconv" "strings" "sync" "sync/atomic" @@ -31,12 +29,13 @@ import ( "time" "github.com/prometheus/client_golang/prometheus" + "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" msdk "github.com/livekit/media-sdk" - "github.com/livekit/media-sdk/dtmf" "github.com/livekit/media-sdk/rtp" "github.com/livekit/media-sdk/sdp" + "github.com/livekit/media-sdk/srtp" "github.com/livekit/mediatransportutil/pkg/rtcconfig" "github.com/livekit/protocol/logger" @@ -57,11 +56,11 @@ func newTestCallMonitor(t testing.TB) *stats.CallMonitor { return mon.NewCall(stats.Inbound, "test", "test") } -func newTestMediaPort(t testing.TB, provider string) *MediaPort { +func newTestMediaPort(t testing.TB, provider string) MediaPort { t.Helper() mon := newTestCallMonitor(t) mon.SetProvider(provider) - mp, err := NewMediaPortWith(1, logger.GetLogger(), mon, nil, &MediaOptions{ + mp, err := NewMediaPortWith(logger.NewTestLogger(t), mon, nil, &MediaOptions{ IP: netip.MustParseAddr("127.0.0.1"), }, 8000) require.NoError(t, err) @@ -70,11 +69,17 @@ func newTestMediaPort(t testing.TB, provider string) *MediaPort { } type testUDPConn struct { - addr netip.AddrPort - closed chan struct{} - buf chan []byte - peer atomic.Pointer[testUDPConn] - deadline chan time.Time + addr netip.AddrPort + closed chan struct{} + buf chan []byte + peer atomic.Pointer[testUDPConn] + + // Deadlines follow net.Conn: the latest value wins, zero clears it. The value + // lives in the guarded field and kick only wakes a parked reader, so coalescing + // a wakeup can never drop a deadline - which would strand Close() forever. + dmu sync.Mutex + deadline time.Time + kick chan struct{} } func (c *testUDPConn) Read(b []byte) (int, error) { @@ -100,13 +105,22 @@ func (c *testUDPConn) SetDeadline(t time.Time) error { } func (c *testUDPConn) SetReadDeadline(t time.Time) error { + c.dmu.Lock() + c.deadline = t + c.dmu.Unlock() select { - case c.deadline <- t: + case c.kick <- struct{}{}: default: } return nil } +func (c *testUDPConn) readDeadline() time.Time { + c.dmu.Lock() + defer c.dmu.Unlock() + return c.deadline +} + func (c *testUDPConn) SetWriteDeadline(t time.Time) error { return nil } @@ -116,24 +130,28 @@ func (c *testUDPConn) ReadFromUDPAddrPort(buf []byte) (int, netip.AddrPort, erro if peer == nil { return 0, netip.AddrPort{}, io.ErrClosedPipe } - var curDeadline time.Time for { - var deadlineCh <-chan time.Time = nil - if !curDeadline.IsZero() { - deadlineCh = time.After(time.Until(curDeadline)) + var ( + deadlineCh <-chan time.Time + timer *time.Timer + ) + if dl := c.readDeadline(); !dl.IsZero() { + timer = time.NewTimer(time.Until(dl)) + deadlineCh = timer.C } + select { case <-c.closed: + stopTimer(timer) return 0, netip.AddrPort{}, io.ErrClosedPipe case <-deadlineCh: return 0, netip.AddrPort{}, os.ErrDeadlineExceeded - case newDeadline := <-c.deadline: - if !newDeadline.IsZero() && (newDeadline.Before(curDeadline) || curDeadline.IsZero()) { - curDeadline = newDeadline - } + case <-c.kick: + stopTimer(timer) // deadline changed, re-arm continue case data := <-c.buf: + stopTimer(timer) n := copy(buf, data) var err error if n < len(data) { @@ -144,6 +162,12 @@ func (c *testUDPConn) ReadFromUDPAddrPort(buf []byte) (int, netip.AddrPort, erro } } +func stopTimer(t *time.Timer) { + if t != nil { + t.Stop() + } +} + func (c *testUDPConn) WriteToUDPAddrPort(buf []byte, addr netip.AddrPort) (int, error) { peer := c.peer.Load() if peer == nil { @@ -182,9 +206,9 @@ func newTestConn(i int) *testUDPConn { netip.AddrFrom4([4]byte{byte(i), byte(i), byte(i), byte(i)}), uint16(10000*i), ), - buf: make(chan []byte, 10), - closed: make(chan struct{}), - deadline: make(chan time.Time, 1), + buf: make(chan []byte, 256), + closed: make(chan struct{}), + kick: make(chan struct{}, 1), } } @@ -196,10 +220,6 @@ func newUDPPipe() (c1, c2 *testUDPConn) { return } -func PrintAudioInWriter(p *MediaPort) string { - return p.audioInHandler.(fmt.Stringer).String() -} - func newIP(v string) netip.Addr { ip, err := netip.ParseAddr(v) if err != nil { @@ -208,271 +228,292 @@ func newIP(v string) netip.Addr { return ip } -func TestMediaPortUpdateRemote(t *testing.T) { - log := logger.GetLogger() - mon := newTestCallMonitor(t) - - // newUDPPipe wires two in-memory testUDPConn together. - c1, _ := newUDPPipe() - mp, err := NewMediaPortWith(1, log, mon, c1, &MediaOptions{ - IP: netip.MustParseAddr("127.0.0.1"), - }, 8000) +// newTestPort is NewMediaPortWith for tests: it keeps the concrete type, so tests can reach +// the udpConn and the timeout controls that are not part of the MediaPort interface. +func newTestPort(t testing.TB, log logger.Logger, conn UDPConn, opts *MediaOptions, rate int) *mediaPort { + t.Helper() + mp, err := NewMediaPortWith(log, newTestCallMonitor(t), conn, opts, rate) require.NoError(t, err) - defer mp.Close() - - // Initially no destination is set. - require.False(t, mp.RemoteAddr().IsValid(), "RemoteAddr should be invalid before any update") - - // Update to a valid address. - addr := netip.MustParseAddrPort("9.8.7.6:12345") - mp.UpdateRemote(addr) - require.Equal(t, addr, mp.RemoteAddr(), "RemoteAddr should reflect the updated address") + t.Cleanup(mp.Close) + return mp.(*mediaPort) +} - // UpdateRemote with invalid addr should be a no-op. - mp.UpdateRemote(netip.AddrPort{}) - require.Equal(t, addr, mp.RemoteAddr(), "UpdateRemote with invalid addr should not change RemoteAddr") +func offerAt(t testing.TB, addr netip.AddrPort) []byte { + t.Helper() + return offerAtEnc(t, addr, sdp.EncryptionNone) +} - // UpdateRemote with unspecified address (c=0.0.0.0 hold form) should be a no-op. - mp.UpdateRemote(netip.MustParseAddrPort("0.0.0.0:12345")) - require.Equal(t, addr, mp.RemoteAddr(), "UpdateRemote with unspecified addr should not change RemoteAddr") +func offerAtEnc(t testing.TB, addr netip.AddrPort, enc sdp.Encryption) []byte { + t.Helper() + offer, err := sdp.NewOfferWith(defaultCodecs, addr.Addr(), int(addr.Port()), enc) + require.NoError(t, err) + data, err := offer.SDP.Marshal() + require.NoError(t, err) + return data } -func TestMediaPort(t *testing.T) { - // Main resampler has unpredictable (although tiny) output delay - // and other randomness in the generated samples. - // Enable a predictable resampler to avoid flaky tests. - prevOpts := msdk.DefaultResampleOptions - msdk.DefaultResampleOptions = []msdk.ResampleOption{ - msdk.WithPredictableResample(true), - } - defer func() { - msdk.DefaultResampleOptions = prevOpts - }() - codecList := msdk.Codecs() - for _, codec := range codecList { - info := codec.Info() - tname := strings.ReplaceAll(info.SDPName, "/", "-") - t.Run(tname, func(t *testing.T) { - codecs := msdk.NewCodecSet() - codecs.SetEnabled(info.SDPName, true) - - sub := strings.SplitN(info.SDPName, "/", 2) - codecName := sub[0] - nativeRateSDP, err := strconv.Atoi(sub[1]) - nativeRate := nativeRateSDP - require.NoError(t, err) - switch codecName { - case "telephone-event": - t.SkipNow() - case "G722": - nativeRate *= 2 // error in RFC - } +func TestMediaPortUpdateRemote(t *testing.T) { + c1, _ := newUDPPipe() + mp := newTestPort(t, logger.NewTestLogger(t), c1, &MediaOptions{ + IP: netip.MustParseAddr("127.0.0.1"), + }, RoomSampleRate) - for _, tconf := range []struct { - Rate int - Encrypted sdp.Encryption - }{ - {nativeRate, sdp.EncryptionNone}, - {48000, sdp.EncryptionRequire}, - } { - suff := "" - if tconf.Encrypted != sdp.EncryptionNone { - suff = " srtp" - } - t.Run(fmt.Sprintf("%d%s", tconf.Rate, suff), func(t *testing.T) { - c1, c2 := newUDPPipe() + require.False(t, mp.RemoteAddr().IsValid(), "RemoteAddr should be invalid before any offer") - log := logger.GetLogger() + addr := netip.MustParseAddrPort("9.8.7.6:12345") + _, err := mp.GenerateAnswer(offerAt(t, addr)) + require.NoError(t, err) + require.Equal(t, addr, mp.RemoteAddr(), "GenerateAnswer should set RemoteAddr from the offer") - const ( - ip1 = "1.1.1.1" - ip2 = "2.2.2.2" - port1 = 10000 - port2 = 20000 - ) - testRate := tconf.Rate - bobToAliceNoResample := testRate == 8000 + // Body-less re-INVITE: empty offer returns the local SDP and must not change dest. + _, err = mp.GenerateAnswer(nil) + require.NoError(t, err) + require.Equal(t, addr, mp.RemoteAddr(), "empty offer should not change RemoteAddr") - alicePort, err := NewMediaPortWith(1, log.WithName("Alice"), newTestCallMonitor(t), c1, &MediaOptions{ - IP: newIP(ip1), - Ports: rtcconfig.PortRange{Start: port1}, - NoInputResample: bobToAliceNoResample, - }, testRate) - require.NoError(t, err) - defer alicePort.Close() + // Hold form c=0.0.0.0 must not clobber dest once media is established. + _, err = mp.GenerateAnswer(offerAt(t, netip.MustParseAddrPort("0.0.0.0:12345"))) + require.NoError(t, err) + require.Equal(t, addr, mp.RemoteAddr(), "offer with unspecified addr should not change RemoteAddr") - bobPort, err := NewMediaPortWith(2, log.WithName("Bob"), newTestCallMonitor(t), c2, &MediaOptions{ - IP: newIP(ip2), - Ports: rtcconfig.PortRange{Start: port2}, - }, testRate) - require.NoError(t, err) - defer bobPort.Close() + // successful re-INVITE update + addr = netip.MustParseAddrPort("10.10.10.10:54321") + _, err = mp.GenerateAnswer(offerAt(t, addr)) + require.NoError(t, err) + require.Equal(t, addr, mp.RemoteAddr(), "re-INVITE offer should update RemoteAddr") +} - // Alice sends an offer to Bob +// Re-INVITE with the original offer SDP and crypto material must result in +// re-use of already-negotiated keys. +func TestMediaPortReinviteSameCrypto(t *testing.T) { + c1, _ := newUDPPipe() + mp := newTestPort(t, logger.NewTestLogger(t), c1, &MediaOptions{ + IP: netip.MustParseAddr("127.0.0.1"), + Encryption: sdp.EncryptionRequire, + }, RoomSampleRate) - offer, err := alicePort.NewOffer(codecs, tconf.Encrypted) - require.NoError(t, err) - offerData, err := offer.SDP.Marshal() - require.NoError(t, err) + addr := netip.MustParseAddrPort("9.8.7.6:12345") + offer := offerAtEnc(t, addr, sdp.EncryptionRequire) - t.Logf("SDP offer:\n%s", string(offerData)) + _, err := mp.GenerateAnswer(offer) + require.NoError(t, err) + require.Equal(t, addr, mp.RemoteAddr()) + + require.NotNil(t, mp.negotiated) + require.NotNil(t, mp.negotiated.Crypto) + localKey := slices.Clone(mp.negotiated.Crypto.Keys.LocalMasterKey) + localSalt := slices.Clone(mp.negotiated.Crypto.Keys.LocalMasterSalt) + require.NotEmpty(t, localKey) + require.NotEmpty(t, localSalt) + localSDP, err := mp.GetLocalSDP() + require.NoError(t, err) + require.NotEmpty(t, localSDP) + + // Same offer bytes: NewOfferWith would generate a new peer key. + _, err = mp.GenerateAnswer(offer) + require.NoError(t, err, "re-INVITE with the same offer must be accepted") + require.Equal(t, addr, mp.RemoteAddr(), "same offer must not change dest") + require.Equal(t, localKey, mp.negotiated.Crypto.Keys.LocalMasterKey, "local master key must not change") + require.Equal(t, localSalt, mp.negotiated.Crypto.Keys.LocalMasterSalt, "local master salt must not change") + gotSDP, err := mp.GetLocalSDP() + require.NoError(t, err) + require.Equal(t, localSDP, gotSDP, "local SDP (including a=crypto) must not change") +} - answer, bobConf, err := bobPort.SetOffer(offerData, codecs, tconf.Encrypted) - require.NoError(t, err) - answerData, err := answer.SDP.Marshal() - require.NoError(t, err) +func TestMediaPortReofferSameCrypto(t *testing.T) { + c1, _ := newUDPPipe() + mp := newTestPort(t, logger.NewTestLogger(t), c1, &MediaOptions{ + IP: netip.MustParseAddr("127.0.0.1"), + Encryption: sdp.EncryptionRequire, + }, RoomSampleRate) + + newOffer := func(t testing.TB, mp *mediaPort, localCrypto []srtp.Profile) (*sdp.Offer, *sdp.MediaConfig) { + t.Helper() + addr := netip.MustParseAddrPort("9.8.7.6:12345") + offerData, err := mp.GenerateOffer() + require.NoError(t, err) + offer, err := sdp.ParseOfferWith(defaultCodecs, offerData) + require.NoError(t, err) + answer, mc, err := offer.Answer(addr.Addr(), int(addr.Port()), sdp.EncryptionRequire, sdp.WithLocalProfiles(localCrypto)) + require.NoError(t, err) + answerData, err := answer.SDP.Marshal() + require.NoError(t, err) + err = mp.ProcessAnswer(answerData) + require.NoError(t, err) + require.Nil(t, mp.offer) + return offer, mc + } + localCrypto, err := srtp.DefaultProfiles() + require.NoError(t, err) + offer1, mc1 := newOffer(t, mp, localCrypto) + offer2, mc2 := newOffer(t, mp, localCrypto) - t.Logf("SDP answer:\n%s", string(answerData)) + // Offers must not regenerate keys + require.Equal(t, offer1.CryptoProfiles, offer2.CryptoProfiles, "crypto profiles must not change") + require.Equal(t, mc1.Crypto.Keys.RemoteMasterKey, mc2.Crypto.Keys.RemoteMasterKey, "remote master key must not change") + require.Equal(t, mc1.Crypto.Keys.RemoteMasterSalt, mc2.Crypto.Keys.RemoteMasterSalt, "remote master salt must not change") +} - aliceConf, _, err := alicePort.SetAnswer(offer, answerData, codecs, tconf.Encrypted) - require.NoError(t, err) +// negotiate runs a full offer/answer between two ports, m1 offering, and returns the answer. +func negotiate(t testing.TB, m1, m2 *mediaPort) []byte { + t.Helper() + offerData, err := m1.GenerateOffer() + require.NoError(t, err) - err = alicePort.SetConfig(aliceConf) - require.NoError(t, err) + answerData, err := m2.GenerateAnswer(offerData) + require.NoError(t, err) - err = bobPort.SetConfig(bobConf) - require.NoError(t, err) + require.NoError(t, m1.ProcessAnswer(answerData)) - aliceAudio := alicePort.Config().Audio - bobAudio := bobPort.Config().Audio + m2.SetTimeout(m2.opts.MediaTimeoutInitial, m2.opts.MediaTimeout) + return answerData +} - aliceCodec := aliceAudio.Codec - bobCodec := bobAudio.Codec +func newMediaPair(t testing.TB, opt1, opt2 *MediaOptions, codec string, targetRate int) (m1, m2 *mediaPort) { + return newMediaPairWithAddr(t, newIP("1.1.1.1"), newIP("2.2.2.2"), opt1, opt2, codec, targetRate) +} - require.Equal(t, info.SDPName, aliceCodec.Info().SDPName) - require.Equal(t, info.SDPName, bobCodec.Info().SDPName) +func newMediaPairWithAddr(t testing.TB, ip1, ip2 netip.Addr, opt1, opt2 *MediaOptions, codec string, targetRate int) (m1, m2 *mediaPort) { + if opt1 == nil { + opt1 = &MediaOptions{} + } + if opt2 == nil { + opt2 = &MediaOptions{} + } + c1, c2 := newUDPPipe() - // Buffers should match the rate of the samples we write. + if targetRate <= 0 { + targetRate = RoomSampleRate + } - var aliceRecvBuf msdk.PCM16Sample - aliceHandler := msdk.NewPCM16BufferWriter(&aliceRecvBuf, testRate) - alicePort.WriteAudioTo(aliceHandler) + opt1.IP = ip1 + opt1.Ports = rtcconfig.PortRange{Start: 10000} + if codec != "" { + opt1.Codecs = testCodecSet(codec) + } - var bobRecvBuf msdk.PCM16Sample - bobHandler := msdk.NewPCM16BufferWriter(&bobRecvBuf, testRate) - bobPort.WriteAudioTo(bobHandler) + opt2.IP = ip2 + opt2.Ports = rtcconfig.PortRange{Start: 20000} + if codec != "" { + opt2.Codecs = testCodecSet(codec) + } - aliceToBob := alicePort.GetAudioWriter() - bobToAlice := bobPort.GetAudioWriter() + log := logger.NewTestLogger(t) - aliceToBobWriteChain := aliceToBob.String() - bobToAliceWriteChain := bobToAlice.String() + m1 = newTestPort(t, log.WithName("one"), c1, opt1, targetRate) + m2 = newTestPort(t, log.WithName("two"), c2, opt2, targetRate) - bobToAliceHandleChain := PrintAudioInWriter(alicePort) - aliceToBobHandleChain := PrintAudioInWriter(bobPort) + negotiate(t, m1, m2) + return m1, m2 +} - t.Log("A -> B (write)", aliceToBobWriteChain) - t.Log("B -> A (write)", bobToAliceWriteChain) +type codecConfig struct { + rampUpFrames int + offsetSamples int +} - t.Log("B -> A (handle)", bobToAliceHandleChain) - t.Log("A -> B (handle)", aliceToBobHandleChain) +var codecConfigMap = map[string]codecConfig{ + "G722/8000": {rampUpFrames: 1, offsetSamples: 22}, + "AMR-WB/16000": {rampUpFrames: 1, offsetSamples: 14 + 16}, +} - t.Log("resample", !bobToAliceNoResample) +func TestMediaPortAudioRoundTrip(t *testing.T) { + // Production resampler delay is tiny but not deterministic; checkPCM needs a stable delay. + prevOpts := msdk.DefaultResampleOptions + msdk.DefaultResampleOptions = []msdk.ResampleOption{ + msdk.WithPredictableResample(true), + } + defer func() { + msdk.DefaultResampleOptions = prevOpts + }() - packetSize := testRate / int(time.Second/rtp.DefFrameDur) - aliceToBobSamples := make(msdk.PCM16Sample, packetSize) - bobToAliceSamples := make(msdk.PCM16Sample, packetSize) - const ( - amp1 = 10000 - amp2 = 5000 - freq = 10 - ) - for i := range packetSize { - aliceToBobSamples[i] = int16(amp1 * math.Sin(freq*2*math.Pi*float64(i)/float64(packetSize))) - bobToAliceSamples[i] = int16(amp2 * math.Sin(freq*2*math.Pi*float64(i)/float64(packetSize))) + for _, codec := range allAudioCodecs() { + info := codec.Info() + t.Run(strings.ReplaceAll(info.SDPName, "/", "-"), func(t *testing.T) { + for _, resample := range []bool{true, false} { + t.Run(fmt.Sprintf("resample=%t", resample), func(t *testing.T) { + for _, enc := range []sdp.Encryption{sdp.EncryptionNone, sdp.EncryptionRequire} { + t.Run("enc="+policyToString[enc], func(t *testing.T) { + + opts1 := &MediaOptions{Encryption: enc} + opts2 := &MediaOptions{Encryption: enc} + targetRate := RoomSampleRate + if !resample { + targetRate = info.SampleRate + } + m1, m2 := newMediaPair(t, opts1, opts2, info.SDPName, targetRate) + + var recv1, recv2 msdk.PCM16Sample + h1 := msdk.NewPCM16BufferWriter(&recv1, targetRate) + h2 := msdk.NewPCM16BufferWriter(&recv2, targetRate) + m1.WriteInboundAudioTo(h1) + m2.WriteInboundAudioTo(h2) + + w1 := m1.GetOutboundAudioWriter() + w2 := m2.GetOutboundAudioWriter() + + packetSize := targetRate / int(time.Second/rtp.DefFrameDur) + to2 := make(msdk.PCM16Sample, packetSize) + to1 := make(msdk.PCM16Sample, packetSize) + const ( + amp1 = 10000 + amp2 = 5000 + freq = 10 + ) + for i := range packetSize { + to2[i] = int16(amp1 * math.Sin(freq*2*math.Pi*float64(i)/float64(packetSize))) + to1[i] = int16(amp2 * math.Sin(freq*2*math.Pi*float64(i)/float64(packetSize))) + } + + codecConfig := codecConfigMap[info.SDPName] // defaults to 0,0 + + // Ramp-up time for the codec. + // Some codecs have "inertia" and cannot immediately represent the sound exactly. + // This is why we write signal multiple times to give it some time to adapt. + // We will also cut the ramp-up part from the destination buffer before comparing. + // This variable is in full frames, so that we clearly see where frames start to calculate the offset below. + rampUpFrames := codecConfig.rampUpFrames + // Some codecs have an extra buffering internally, and we have to offset the compared sample + // by this number of sampled values. + offsetSamples := codecConfig.offsetSamples + + writes := 1 + rampUpFrames + discard := rampUpFrames * packetSize + resampleMult := targetRate / info.SampleRate + offsetSamples *= resampleMult + + var wg sync.WaitGroup + wg.Add(2) + go func() { + defer wg.Done() + for range writes { + require.NoError(t, w1.WriteSample(to2)) + } + }() + go func() { + defer wg.Done() + for range writes { + require.NoError(t, w2.WriteSample(to1)) + } + }() + wg.Wait() + + time.Sleep(time.Second / 4) + + h1.Close() + h2.Close() + m1.Close() + m2.Close() + + checkPCM(t, "A -> B", to2[:packetSize-offsetSamples], recv2[discard+offsetSamples:]) + checkPCM(t, "B -> A", to1[:packetSize-offsetSamples], recv1[discard+offsetSamples:]) + }) } - aliceToBobWrites := 1 - bobToAliceWrites := 1 - if tconf.Rate == nativeRate { - expChainBase := fmt.Sprintf("Switch(%d) -> LatencyEntry -> %s(encode) -> ByteEncoder(%d) -> StatsWriter(%s/%d) -> LatencyExit", - nativeRate, codecName, nativeRate, codecName, nativeRateSDP) - require.Equal(t, fmt.Sprintf("%s -> RTPWriteStream(%s:%d)", expChainBase, ip2, port2), aliceToBobWriteChain) - require.Equal(t, fmt.Sprintf("%s -> RTPWriteStream(%s:%d)", expChainBase, ip1, port1), bobToAliceWriteChain) - - expChainBase = fmt.Sprintf("SilenceFiller(25) -> RTP(%%d) -> ByteDecoder -> %s(decode) -> LatencyExit -> Switch(%d) -> Buffer(%d)", codecName, nativeRate, nativeRate) - require.Equal(t, fmt.Sprintf(expChainBase, aliceAudio.Type), bobToAliceHandleChain) - require.Equal(t, fmt.Sprintf(expChainBase, bobAudio.Type), aliceToBobHandleChain) - } else { - expChain := fmt.Sprintf("Switch(48000) -> Resample(48000->%d) -> LatencyEntry -> %s(encode) -> ByteEncoder(%d) -> StatsWriter(%s/%d) -> LatencyExit -> SRTPWriteStream", - nativeRate, codecName, nativeRate, codecName, nativeRateSDP) - require.Equal(t, expChain, aliceToBobWriteChain) - require.Equal(t, expChain, bobToAliceWriteChain) - - // This side does not resample the received audio, it uses sample rate of the RTP source. - var expChainAlice string - if bobToAliceNoResample { - expChainAlice = fmt.Sprintf("SilenceFiller(25) -> RTP(%d) -> ByteDecoder -> %s(decode) -> LatencyExit -> Switch(%d) -> Buffer(%d)", aliceAudio.Type, codecName, nativeRate, nativeRate) - } else { - expChainAlice = fmt.Sprintf("SilenceFiller(25) -> RTP(%d) -> ByteDecoder -> %s(decode) -> Resample(%d->48000) -> LatencyExit -> Switch(48000) -> Buffer(48000)", aliceAudio.Type, codecName, nativeRate) - } - - // This side resamples the received audio to the expected sample rate. - expChainBob := fmt.Sprintf("SilenceFiller(25) -> RTP(%d) -> ByteDecoder -> %s(decode) -> Resample(%d->48000) -> LatencyExit -> Switch(48000) -> Buffer(48000)", bobAudio.Type, codecName, nativeRate) - - require.Equal(t, expChainAlice, bobToAliceHandleChain) - require.Equal(t, expChainBob, aliceToBobHandleChain) - } - // Ramp-up time for the codec. - // Some codecs have "inertia" and cannot immediately represent the sound exactly. - // This is shy we write signal multiple times to give it some time to adapt. - // We will also cut the ramp-up part from the destination buffer before comparing. - // This variable is in full frames, so that we clearly see where frames start to calculate the offset below. - rampUpFrames := 0 - // Some codecs have an extra buffering internally, and we have to offset the compared sample - // by this number of sampled values. - offsetSamples := 0 - - switch codecName { - case "G722": - rampUpFrames += 1 - offsetSamples += 22 - case "AMR-WB": - rampUpFrames += 1 - offsetSamples += 14 + 16 - } - aliceToBobWrites += rampUpFrames - bobToAliceWrites += rampUpFrames - discard := rampUpFrames * packetSize - - resampleMult := testRate / nativeRate - offsetSamples *= resampleMult - - var wg sync.WaitGroup - wg.Add(2) - go func() { - defer wg.Done() - for range aliceToBobWrites { - err := aliceToBob.WriteSample(aliceToBobSamples) - require.NoError(t, err) - } - }() - go func() { - defer wg.Done() - for range bobToAliceWrites { - err := bobToAlice.WriteSample(bobToAliceSamples) - require.NoError(t, err) - } - }() - wg.Wait() - - time.Sleep(time.Second / 4) - - // Cut buffers earlier, otherwise we might get extra samples - // that we added to push resampler forward. - aliceHandler.Close() - bobHandler.Close() - - alicePort.Close() - bobPort.Close() - - checkPCM(t, "A -> B", aliceToBobSamples[:packetSize-offsetSamples], bobRecvBuf[discard+offsetSamples:]) - checkPCM(t, "B -> A", bobToAliceSamples[:packetSize-offsetSamples], aliceRecvBuf[discard+offsetSamples:]) }) } + }) } - } func checkPCM(t testing.TB, name string, exp, got msdk.PCM16Sample) { @@ -513,69 +554,59 @@ func checkPCM(t testing.TB, name string, exp, got msdk.PCM16Sample) { ) } -func newMediaPair(t testing.TB, opt1, opt2 *MediaOptions) (m1, m2 *MediaPort) { - return newMediaPairWithAddr(t, newIP("1.1.1.1"), newIP("2.2.2.2"), opt1, opt2) -} - -func newMediaPairWithAddr(t testing.TB, ip1, ip2 netip.Addr, opt1, opt2 *MediaOptions) (m1, m2 *MediaPort) { - if opt1 == nil { - opt1 = &MediaOptions{} - } - if opt2 == nil { - opt2 = &MediaOptions{} - } - c1, c2 := newUDPPipe() - - codecs := defaultCodecs - - opt1.IP = ip1 - opt1.Ports = rtcconfig.PortRange{Start: 10000} - opt1.NoInputResample = true - - opt2.IP = ip2 - opt2.Ports = rtcconfig.PortRange{Start: 20000} - - const rate = 16000 - - log := logger.GetLogger() - - var err error - - m1, err = NewMediaPortWith(1, log.WithName("one"), newTestCallMonitor(t), c1, opt1, rate) - require.NoError(t, err) - t.Cleanup(m1.Close) - - m2, err = NewMediaPortWith(2, log.WithName("two"), newTestCallMonitor(t), c2, opt2, rate) - require.NoError(t, err) - t.Cleanup(m2.Close) - - offer, err := m1.NewOffer(codecs, sdp.EncryptionNone) - require.NoError(t, err) - offerData, err := offer.SDP.Marshal() - require.NoError(t, err) - - answer, mc2, err := m2.SetOffer(offerData, codecs, sdp.EncryptionNone) - require.NoError(t, err) - answerData, err := answer.SDP.Marshal() - require.NoError(t, err) - - mc1, _, err := m1.SetAnswer(offer, answerData, codecs, sdp.EncryptionNone) - require.NoError(t, err) - - err = m1.SetConfig(mc1) - require.NoError(t, err) +func TestPipelineChains(t *testing.T) { + for _, codec := range enabledAudioCodecs() { + t.Run(codec.Info().SDPName, func(t *testing.T) { + // Create new test media port + // Process offer with a specific codec + dtmf + codecs := testCodecSet(codec.Info().SDPName) + opts := &MediaOptions{ + IP: netip.MustParseAddr("1.1.1.1"), + Ports: rtcconfig.PortRange{Start: 10000}, + Codecs: codecs, + } + conn := newTestConn(1) + mp := newTestPort(t, logger.NewTestLogger(t), conn, opts, RoomSampleRate) - err = m2.SetConfig(mc2) - require.NoError(t, err) + info := codec.Info() + offer, err := sdp.NewOfferWith(codecs, netip.MustParseAddr("2.2.2.2"), 20000, sdp.EncryptionNone) + require.NoError(t, err) + answerData, err := offer.SDP.Marshal() + require.NoError(t, err) + _, err = mp.GenerateAnswer(answerData) + require.NoError(t, err) - w2 := m2.GetAudioWriter() - require.Equal(t, "Switch(16000) -> LatencyEntry -> G722(encode) -> ByteEncoder(16000) -> StatsWriter(G722/8000) -> LatencyExit -> RTPWriteStream(1.1.1.1:10000)", w2.String()) + codecName := strings.Split(info.SDPName, "/")[0] + sampleRate := info.SampleRate + clockRate := info.RTPClockRate + payloadType := info.RTPDefType + audioOutChain := fmt.Sprintf("WriteCloserSwitch(%d) -> LatencyEntry -> Resample(%d->%d) -> %s(encode) -> ByteEncoder(%d) -> StatsWriter(%s/%d) -> LatencyExit -> RTPWriteStream(:0)", + RoomSampleRate, RoomSampleRate, sampleRate, codecName, sampleRate, codecName, clockRate) + audioInChain := fmt.Sprintf("StatsHandler(%s/%d) -> SilenceFiller(25) -> RTP(%d) -> ByteDecoder -> %s(decode) -> Resample(%d->%d) -> LatencyExit -> WriteCloserSwitch(nil)", + codecName, clockRate, payloadType, codecName, sampleRate, RoomSampleRate) + dtmfOutChain := fmt.Sprintf("WriteCloserSwitch(%d) -> dtmfOutWriter(dtmfAudio: false)", clockRate) + dtmfInChain := fmt.Sprintf("StatsHandler(telephone-event/%d) -> HandlerFunc", clockRate) + assert.Equal(t, audioOutChain, mp.GetOutboundAudioWriter().String(), "out audio chain mismatch") + assert.Equal(t, audioInChain, mp.pipeline.audioToRoom.String(), "in audio chain mismatch") + assert.Equal(t, dtmfOutChain, mp.GetOutboundDTMFWriter().String(), "out dtmf chain mismatch") + assert.Equal(t, dtmfInChain, mp.pipeline.dtmfToRoom.String(), "in dtmf chain mismatch") + }) + } +} - return m1, m2 +// pushAudio writes two room-rate frames. The outbound resampler keeps a one-frame +// delay (soxr returns a short buffer on the first call), so a single WriteSample +// never produces RTP when the port runs at RoomSampleRate. +func pushAudio(t testing.TB, w msdk.PCM16Writer) { + t.Helper() + frame := roomFrame() + require.NoError(t, w.WriteSample(frame)) + require.NoError(t, w.WriteSample(frame)) } func TestMediaTimeout(t *testing.T) { const ( + codec = "G722/8000" timeout = time.Second / 4 initial = timeout * 2 dt = timeout / 4 @@ -585,13 +616,11 @@ func TestMediaTimeout(t *testing.T) { m1, _ := newMediaPair(t, &MediaOptions{ MediaTimeoutInitial: initial, MediaTimeout: timeout, - }, nil) - - m1.EnableTimeout(true) + }, nil, codec, RoomSampleRate) targ := time.Now().Add(initial) select { - case <-m1.Timeout(): + case <-m1.MediaTimeout(): t.Fatal("initial timeout ignored") case <-time.After(initial / 2): } @@ -599,7 +628,7 @@ func TestMediaTimeout(t *testing.T) { select { case <-time.After(time.Until(targ) + dt): t.Fatal("timeout didn't trigger") - case <-m1.Timeout(): + case <-m1.MediaTimeout(): } }) @@ -607,12 +636,10 @@ func TestMediaTimeout(t *testing.T) { m1, m2 := newMediaPair(t, &MediaOptions{ MediaTimeoutInitial: initial, MediaTimeout: timeout, - }, nil) - m1.EnableTimeout(true) + }, nil, codec, RoomSampleRate) - w2 := m2.GetAudioWriter() - err := w2.WriteSample(msdk.PCM16Sample{0, 0}) - require.NoError(t, err) + w2 := m2.GetOutboundAudioWriter() + pushAudio(t, w2) select { case <-time.After(dt): @@ -623,7 +650,7 @@ func TestMediaTimeout(t *testing.T) { select { case <-time.After(2*timeout + dt): t.Fatal("timeout didn't trigger") - case <-m1.Timeout(): + case <-m1.MediaTimeout(): } }) @@ -631,18 +658,16 @@ func TestMediaTimeout(t *testing.T) { m1, m2 := newMediaPair(t, &MediaOptions{ MediaTimeoutInitial: initial, MediaTimeout: timeout, - }, nil) - m1.EnableTimeout(true) + }, nil, codec, RoomSampleRate) - w2 := m2.GetAudioWriter() + w2 := m2.GetOutboundAudioWriter() for i := 0; i < 10; i++ { - err := w2.WriteSample(msdk.PCM16Sample{0, 0}) - require.NoError(t, err) + pushAudio(t, w2) select { case <-time.After(timeout / 2): - case <-m1.Timeout(): + case <-m1.MediaTimeout(): t.Fatal("timeout") } } @@ -652,18 +677,16 @@ func TestMediaTimeout(t *testing.T) { m1, m2 := newMediaPair(t, &MediaOptions{ MediaTimeoutInitial: initial, MediaTimeout: timeout, - }, nil) - m1.EnableTimeout(true) + }, nil, codec, RoomSampleRate) - w2 := m2.GetAudioWriter() + w2 := m2.GetOutboundAudioWriter() for i := 0; i < 5; i++ { - err := w2.WriteSample(msdk.PCM16Sample{0, 0}) - require.NoError(t, err) + pushAudio(t, w2) select { case <-time.After(timeout / 2): - case <-m1.Timeout(): + case <-m1.MediaTimeout(): t.Fatal("timeout") } } @@ -677,7 +700,7 @@ func TestMediaTimeout(t *testing.T) { select { case <-time.After(timeout + dt): t.Fatal("timeout didn't trigger") - case <-m1.Timeout(): + case <-m1.MediaTimeout(): } }) @@ -685,8 +708,7 @@ func TestMediaTimeout(t *testing.T) { m1, _ := newMediaPair(t, &MediaOptions{ MediaTimeoutInitial: initial, MediaTimeout: timeout, - }, nil) - m1.EnableTimeout(true) + }, nil, codec, RoomSampleRate) // No media has ever arrived. SetTimeout re-arms startTime, and since the // port has never seen an RTP packet, the new initial window applies from @@ -696,7 +718,7 @@ func TestMediaTimeout(t *testing.T) { targ := time.Now().Add(initial) select { - case <-m1.Timeout(): + case <-m1.MediaTimeout(): t.Fatal("initial timeout fired too early") case <-time.After(initial / 2): } @@ -704,7 +726,7 @@ func TestMediaTimeout(t *testing.T) { select { case <-time.After(time.Until(targ) + dt): t.Fatal("timeout didn't trigger") - case <-m1.Timeout(): + case <-m1.MediaTimeout(): } }) @@ -712,31 +734,26 @@ func TestMediaTimeout(t *testing.T) { m1, m2 := newMediaPair(t, &MediaOptions{ MediaTimeoutInitial: initial, MediaTimeout: timeout, - }, nil) - m1.EnableTimeout(true) + }, nil, codec, RoomSampleRate) - w2 := m2.GetAudioWriter() + w2 := m2.GetOutboundAudioWriter() for i := 0; i < 5; i++ { - err := w2.WriteSample(msdk.PCM16Sample{0, 0}) - require.NoError(t, err) + pushAudio(t, w2) select { case <-time.After(timeout / 2): - case <-m1.Timeout(): + case <-m1.MediaTimeout(): t.Fatal("timeout") } } - m1.SetTimeout(initial, timeout) - for i := 0; i < 5; i++ { - err := w2.WriteSample(msdk.PCM16Sample{0, 0}) - require.NoError(t, err) + pushAudio(t, w2) select { case <-time.After(timeout / 2): - case <-m1.Timeout(): + case <-m1.MediaTimeout(): t.Fatal("timeout") } } @@ -744,8 +761,10 @@ func TestMediaTimeout(t *testing.T) { } func TestSymmetricRTP(t *testing.T) { + const codec = "G722/8000" + t.Run("disabled", func(t *testing.T) { - m1, m2 := newMediaPair(t, &MediaOptions{SymmetricRTP: false}, nil) + m1, m2 := newMediaPair(t, &MediaOptions{SymmetricRTP: false}, nil, codec, RoomSampleRate) dstPtr := m1.port.dst.Load() require.NotNil(t, dstPtr) dst := *dstPtr @@ -755,8 +774,7 @@ func TestSymmetricRTP(t *testing.T) { newAddr := netip.AddrPortFrom(newIP("9.9.9.9"), 9999) c2.addr = newAddr - err := m2.GetAudioWriter().WriteSample(msdk.PCM16Sample{0, 0}) - require.NoError(t, err) + pushAudio(t, m2.GetOutboundAudioWriter()) select { case <-m1.Received(): @@ -770,7 +788,7 @@ func TestSymmetricRTP(t *testing.T) { }) t.Run("enabled", func(t *testing.T) { - m1, m2 := newMediaPair(t, &MediaOptions{SymmetricRTP: true}, nil) + m1, m2 := newMediaPair(t, &MediaOptions{SymmetricRTP: true}, nil, codec, RoomSampleRate) dstPtr := m1.port.dst.Load() require.NotNil(t, dstPtr) require.True(t, dstPtr.IsValid()) @@ -779,8 +797,7 @@ func TestSymmetricRTP(t *testing.T) { newAddr := netip.AddrPortFrom(newIP("9.9.9.9"), 9999) c2.addr = newAddr - err := m2.GetAudioWriter().WriteSample(msdk.PCM16Sample{0, 0}) - require.NoError(t, err) + pushAudio(t, m2.GetOutboundAudioWriter()) select { case <-m1.Received(): @@ -797,6 +814,8 @@ func TestSymmetricRTP(t *testing.T) { m1, m2 := newMediaPairWithAddr(t, newIP("1.1.1.1"), newIP("10.10.10.10"), &MediaOptions{IgnoreLocalAddrInSDP: true}, nil, + codec, + RoomSampleRate, ) dstPtr := m1.port.dst.Load() require.NotNil(t, dstPtr) @@ -808,8 +827,7 @@ func TestSymmetricRTP(t *testing.T) { newAddr := netip.AddrPortFrom(newIP("3.3.3.3"), 9999) c2.addr = newAddr - err := m2.GetAudioWriter().WriteSample(msdk.PCM16Sample{0, 0}) - require.NoError(t, err) + pushAudio(t, m2.GetOutboundAudioWriter()) select { case <-m1.Received(): @@ -823,78 +841,6 @@ func TestSymmetricRTP(t *testing.T) { }) } -func generateDTMFPackets(t *testing.T, digits string) [][]*rtp.Packet { - t.Helper() - var buf rtp.Buffer - packets := make([][]*rtp.Packet, len(digits)) - last := len(buf) - w := rtp.NewSeqWriter(&buf).NewStream(101, dtmf.SampleRate) - timestamp := uint32(1000) - for i := range digits { - err := dtmf.Write(context.Background(), nil, w, timestamp, digits[i:i+1]) - require.NoError(t, err) - require.NotEmpty(t, buf) - timestamp += uint32(dtmf.SampleRate / 2) - packets[i] = slices.Clone(buf[last:]) - last = len(buf) - } - return packets -} - -func dropPackets(t *testing.T, dropType string, packets []*rtp.Packet) []*rtp.Packet { - t.Helper() - switch dropType { - case "none": - return packets - case "first": - require.Greater(t, len(packets), 3) - return packets[3:] - case "last": - require.Greater(t, len(packets), 3) - return packets[:len(packets)-3] - case "middle": - require.Greater(t, len(packets), 6) - ret := slices.Clone(packets[:3]) - ret = append(ret, packets[len(packets)-3:]...) - return ret - default: - t.Fatal("unknown drop type: " + dropType) - return nil - } -} - -func TestMediaPortDTMF(t *testing.T) { - digitCases := []string{"1", "12", "123"} - lossCases := []string{"none", "first", "last", "middle"} - - for _, digits := range digitCases { - packets := generateDTMFPackets(t, digits) - for _, lossPackets := range lossCases { - t.Run(fmt.Sprintf("digits=%s/loss=%s", digits, lossPackets), func(t *testing.T) { - p := &MediaPort{} - p.lastDTMFTimestamp.Store(math.MaxUint32) - got := "" - p.HandleDTMF(func(ev dtmf.Event) { - t.Logf("received DTMF event: %+v", ev) - got = fmt.Sprintf("%s%s", got, strconv.Itoa(int(ev.Code))) - }) - for _, digitPackets := range packets { - sendPackets := dropPackets(t, lossPackets, digitPackets) - t.Logf("sending %d/%d packets", len(sendPackets), len(digitPackets)) - for _, pkt := range sendPackets { - h := pkt.Header - t.Logf("sending packet: seq=%d, ts=%d, marker=%t", h.SequenceNumber, h.Timestamp, h.Marker) - require.NoError(t, p.dtmfHandler(&h, pkt.Payload)) - } - } - t.Logf("sent: %s", digits) - t.Logf("got: %s", got) - require.Equal(t, digits, got) - }) - } - } -} - // Test util for incrementing prometheus counter metrics. func gatherCounter(t testing.TB, name string, labels map[string]string) float64 { t.Helper() @@ -937,7 +883,7 @@ func TestSetOfferReportsCodecsBeforeFailing(t *testing.T) { pcmuBefore := gatherCounter(t, offeredMetric, pcmu) offer := sdpWithMedia("m=audio 5004 RTP/AVP 96", "a=rtpmap:96 SPEEX/16000") - _, _, err := mp.SetOffer(offer, defaultCodecs, sdp.EncryptionNone) + _, err := mp.GenerateAnswer(offer) require.ErrorIs(t, err, sdp.ErrNoCommonMedia) // Codecs that are not part of the internal set are classified as "other" @@ -959,7 +905,7 @@ func TestSetOfferReportsCodecsPerProvider(t *testing.T) { offer := sdpWithMedia("m=audio 5004 RTP/AVP 0 9", "a=rtpmap:0 PCMU/8000", "a=rtpmap:9 G722/8000") - _, _, err := mp.SetOffer(offer, defaultCodecs, sdp.EncryptionNone) + _, err := mp.GenerateAnswer(offer) require.NoError(t, err) require.Equal(t, parsedBefore+1, gatherCounter(t, parsedMetric, parsed)) @@ -976,7 +922,7 @@ func TestSetOfferReportsUnknownProvider(t *testing.T) { before := gatherCounter(t, parsedMetric, parsed) offer := sdpWithMedia("m=audio 5004 RTP/AVP 0", "a=rtpmap:0 PCMU/8000") - _, _, err := mp.SetOffer(offer, defaultCodecs, sdp.EncryptionNone) + _, err := mp.GenerateAnswer(offer) require.NoError(t, err) require.Equal(t, before+1, gatherCounter(t, parsedMetric, parsed)) diff --git a/pkg/sip/outbound.go b/pkg/sip/outbound.go index 89d5252cd..2cf4f36a6 100644 --- a/pkg/sip/outbound.go +++ b/pkg/sip/outbound.go @@ -31,14 +31,12 @@ import ( "golang.org/x/exp/maps" msdk "github.com/livekit/media-sdk" - "github.com/livekit/media-sdk/dtmf" "github.com/livekit/media-sdk/tones" "github.com/livekit/protocol/livekit" "github.com/livekit/protocol/logger" "github.com/livekit/protocol/utils/guid" "github.com/livekit/protocol/utils/traceid" "github.com/livekit/psrpc" - lksdk "github.com/livekit/server-sdk-go/v2" "github.com/livekit/sipgo" "github.com/livekit/sipgo/sip" @@ -73,7 +71,7 @@ type outboundCall struct { state *CallState callStart time.Time cc *sipOutbound - media *MediaPort + media MediaPort started core.Fuse stopped core.Fuse closing core.Fuse @@ -87,6 +85,7 @@ type outboundCall struct { lkRoom RoomInterface lkRoomIn msdk.PCM16Writer // output to room; OPUS at 48k sipConf sipOutboundConfig + audioOut *msdk.WriteCloserSwitch[msdk.PCM16Sample] // inner writer owned by MediaPort } func (c *Client) newCall(ctx context.Context, tid traceid.ID, conf *config.Config, log logger.Logger, id LocalTag, room RoomConfig, sipConf sipOutboundConfig, state *CallState, projectID string) (*outboundCall, error) { @@ -115,6 +114,7 @@ func (c *Client) newCall(ctx context.Context, tid traceid.ID, conf *config.Confi sigTs: SignalingTimestamps{APITime: now}, jitterBuf: jitterBuf, projectID: projectID, + audioOut: msdk.NewWriteCloserSwitch[msdk.PCM16Sample](RoomSampleRate), // inner writer owned by MediaPort } call.stats.Update() call.cc = c.newOutbound(log, id, sipConf.uri, sipConf.to, sipConf.from, contact, call.setAttrsToHeaders) @@ -126,7 +126,7 @@ func (c *Client) newCall(ctx context.Context, tid traceid.ID, conf *config.Confi call.mon = c.mon.NewCall(stats.Outbound, sipConf.from.Address.Host, sipConf.to.Address.Host) var err error - call.media, err = NewMediaPort(tid, call.log, call.mon, &MediaOptions{ + call.media, err = NewMediaPort(call.log, call.mon, &MediaOptions{ IP: c.sconf.MediaIP, Ports: conf.RTPPort, MediaTimeoutInitial: c.conf.MediaTimeoutInitial, @@ -136,10 +136,11 @@ func (c *Client) newCall(ctx context.Context, tid traceid.ID, conf *config.Confi EnableJitterBuffer: call.jitterBuf, LogSignalChanges: signalLoggingEnabled, Stats: &call.stats.Port, - NoInputResample: !RoomResample, - IgnorePreanswerData: true, DrainingIdleTimeout: conf.RTPDrainingIdleTimeout, DrainingDuration: conf.RTPDrainingDuration, + DTMFAudio: conf.AudioDTMF, + Codecs: sipConf.mediaConfig.Codecs, + Encryption: sipConf.mediaConfig.Encryption, }, RoomSampleRate) if err != nil { call.close(ctx, EndCall{ @@ -149,9 +150,6 @@ func (c *Client) newCall(ctx context.Context, tid traceid.ID, conf *config.Confi }) return nil, err } - call.media.SetDTMFAudio(conf.AudioDTMF) - call.media.EnableTimeout(false) - call.media.DisableOut() // disabled until we get 200 if err := call.connectToRoom(ctx, room, c.getRoom); err != nil { call.close(ctx, EndCall{ Report: fmt.Errorf("room join failed: %w", err), @@ -262,7 +260,7 @@ func (c *outboundCall) waitClose(ctx context.Context, tid traceid.ID) error { Reason: disconnectReasonFromRoomClose(roomReason), }) return nil - case <-c.media.Timeout(): + case <-c.media.MediaTimeout(): c.closeWithTimeout(ctx) err := psrpc.NewErrorf(psrpc.DeadlineExceeded, "media timeout") c.setErrStatus(ctx, err) @@ -385,9 +383,14 @@ func (c *outboundCall) close(ctx context.Context, end EndCall) bool { } if r := c.lkRoom; r != nil { - _ = r.CloseOutput() _ = r.CloseWithReason(end.Status.DisconnectReason()) } + + if c.lkRoomIn != nil { + if err := c.lkRoomIn.Close(); err != nil { + log.Warnw("error closing livekit room audio input", err) + } + } c.lkRoomIn = nil c.c.cmu.Lock() @@ -513,27 +516,55 @@ func (c *outboundCall) dialSIP(ctx context.Context, tid traceid.ID) error { if digits := c.sipConf.dtmf; digits != "" { c.setStatus(CallAutomation) // Write initial DTMF to SIP - if err := c.media.WriteDTMF(ctx, digits); err != nil { - return err + dtmfWriter := c.media.GetOutboundDTMFWriter() + if err := dtmfWriter.WriteSample(&livekit.SipDTMF{ + Digit: digits, + }); err != nil { + return fmt.Errorf("error writing digits (%s): %w", digits, err) } } c.setStatus(CallActive) - return nil } -func (c *outboundCall) updateRemoteFromSDP(body []byte) { - updateRemoteFromSDP(c.media, c.log, c.sipConf.mediaConfig.Codecs, body) +func (c *outboundCall) updateRemoteFromSDP(body []byte) error { + var mp MediaPort + + c.mu.Lock() + mp = c.media + c.mu.Unlock() + + if mp == nil { + return nil + } + _, err := mp.GenerateAnswer(body) + return err } func (c *outboundCall) connectMedia() { - if w := c.lkRoom.SwapOutput(c.media.GetAudioWriter()); w != nil { - _ = w.Close() + if old := c.lkRoom.WriteOutboundAudioTo(c.audioOut); old != nil { + old.Close() + c.log.Warnw("room has unexpected outbound audio writer", nil) } - c.lkRoom.SetDTMFOutput(c.media) - c.media.WriteAudioTo(c.lkRoomIn) - c.media.HandleDTMF(c.handleDTMF) + if old := c.lkRoom.WriteOutboundDTMFTo(c.media.GetOutboundDTMFWriter()); old != nil { + old.Close() + c.log.Warnw("room has unexpected outbound DTMF writer", nil) + } + + if processor := c.c.handler.GetMediaProcessor(c.sipConf.enabledFeatures, c.sipConf.featureFlags, string(c.cc.ID()), MediaProcessorOpts{InputSampleRate: RoomSampleRate}); processor != nil { + c.lkRoomIn = processor(c.lkRoomIn) + } + + if old := c.media.WriteInboundAudioTo(c.lkRoomIn); old != nil { + old.Close() + c.log.Warnw("media port has unexpected inbound audio writer", nil) + } + + if old := c.media.WriteInboundDTMFTo(c.lkRoom.GetInboundDTMFWriter()); old != nil { + old.Close() + c.log.Warnw("media port has unexpected inbound DTMF writer", nil) + } } type sipRespFunc func(code sip.StatusCode, hdrs Headers) @@ -678,12 +709,7 @@ func (c *outboundCall) sipSignal(ctx context.Context, tid traceid.ID) error { cancel() }() - mconf := c.sipConf.mediaConfig - sdpOffer, err := c.media.NewOffer(mconf.Codecs, mconf.Encryption) - if err != nil { - return err - } - sdpOfferData, err := sdpOffer.SDP.Marshal() + sdpOfferData, err := c.media.GenerateOffer() if err != nil { return err } @@ -741,19 +767,17 @@ func (c *outboundCall) sipSignal(ctx context.Context, tid traceid.ID) error { c.log = LoggerWithHeaders(c.log, c.cc) - mc, localSDP, err := c.media.SetAnswer(sdpOffer, sdpResp, mconf.Codecs, mconf.Encryption) + err = c.media.ProcessAnswer(sdpResp) if err != nil { return err } - if err = c.media.SetConfig(mc); err != nil { - return err - } - mc.Processor = c.c.handler.GetMediaProcessor(c.sipConf.enabledFeatures, c.sipConf.featureFlags, string(c.cc.ID()), MediaProcessorOpts{InputSampleRate: c.media.InputSampleRate()}) - c.cc.SetLocalSDP(localSDP) c.mon.InviteAccept() - c.media.EnableOut() - c.media.EnableTimeout(true) + if old := c.audioOut.Swap(c.media.GetOutboundAudioWriter()); old != nil { + c.log.Warnw("unexpected audio out writer", nil) + old.Close() + } + err = c.cc.AckInviteOK(ctx) if err != nil { c.log.Infow("SIP accept failed", "error", err) @@ -770,23 +794,19 @@ func (c *outboundCall) sipSignal(ctx context.Context, tid traceid.ID) error { } c.setExtraAttrs(c.sipConf.headersToAttrs, c.sipConf.includeHeaders, c.cc, nil) + audio := c.media.NegotiatedAudio() + if audio == nil { + return fmt.Errorf("call media does not have negotiated audio") + } + c.state.DeferUpdate(func(info *livekit.SIPCallInfo) { - info.AudioCodec = mc.Audio.Codec.Info().SDPName + info.AudioCodec = audio.Codec.Info().SDPName if r := c.lkRoom.Room(); r != nil { info.ParticipantAttributes = r.LocalParticipant.Attributes() // clones } }) - return nil -} -func (c *outboundCall) handleDTMF(ev dtmf.Event) { - if c.lkRoom == nil { - return - } - _ = c.lkRoom.SendData(&livekit.SipDTMF{ - Code: uint32(ev.Code), - Digit: string([]byte{ev.Digit}), - }, lksdk.WithDataPublishReliable(true)) + return nil } func (c *outboundCall) transferCall(ctx context.Context, transferTo string, headers map[string]string, dialtone bool) (retErr error) { @@ -804,21 +824,18 @@ func (c *outboundCall) transferCall(ctx context.Context, transferTo string, head rctx, rcancel := context.WithCancel(ctx) defer rcancel() - // mute the room audio to the SIP participant - w := c.lkRoom.SwapOutput(nil) + // Mute the room audio to the SIP participant. + // Skip closing the existing writer, which is c.audioOut. + _ = c.lkRoom.WriteOutboundAudioTo(nil) defer func() { if retErr != nil && !c.stopped.IsBroken() { - c.lkRoom.SwapOutput(w) - } else { - w.Close() + c.lkRoom.WriteOutboundAudioTo(c.audioOut) } }() go func() { - aw := c.media.GetAudioWriter() - - err := tones.Play(rctx, aw, ringVolume, tones.ETSIRinging) + err := tones.Play(rctx, c.audioOut, ringVolume, tones.ETSIRinging) if err != nil && !errors.Is(err, context.Canceled) && !errors.Is(err, context.DeadlineExceeded) { c.log.Infow("cannot play dial tone", "error", err) } @@ -880,7 +897,6 @@ type sipOutbound struct { callID string invite *sip.Request inviteOk *sip.Response - localSDP []byte // SDP Offer, constrained by the answer nextCSeq uint32 getHeaders setHeadersFunc @@ -941,20 +957,6 @@ func (c *sipOutbound) RecordInvite(cseq uint32) { } } -// SetLocalSDP stores the precomputed local SDP for re-INVITE (from ApplyWithLocal). -func (c *sipOutbound) SetLocalSDP(localSDP []byte) { - c.mu.Lock() - defer c.mu.Unlock() - c.localSDP = localSDP -} - -// LocalSDP returns the precomputed local SDP for re-INVITE (from ApplyWithLocal). -func (c *sipOutbound) LocalSDP() []byte { - c.mu.RLock() - defer c.mu.RUnlock() - return c.localSDP -} - // Returns the original SDP offer. func (c *sipOutbound) OwnSDP() []byte { c.mu.RLock() diff --git a/pkg/sip/outbound_test.go b/pkg/sip/outbound_test.go index e01a1953f..038f68c61 100644 --- a/pkg/sip/outbound_test.go +++ b/pkg/sip/outbound_test.go @@ -104,7 +104,7 @@ func TestOutboundRouteHeaderWithRecordRoute(t *testing.T) { return } - fmt.Println("Received INVITE, validating") + t.Log("Received INVITE, validating") require.NotNil(t, tr) require.NotNil(t, tr.req) @@ -298,7 +298,7 @@ func TestOutboundACKDestinationAfterInviteResponse(t *testing.T) { // sipResponse returns immediately on a cancelled context, sending a CANCEL. func TestSIPResponseCancelReturnsImmediately(t *testing.T) { tx := &testSIPClientTransaction{ - log: logger.GetLogger(), + log: logger.NewTestLogger(t), responses: make(chan *sip.Response), cancels: make(chan struct{}, 1), done: make(chan struct{}), @@ -343,12 +343,12 @@ func TestWatchCancelledInvite(t *testing.T) { {"non-2xx final", []*sip.Response{sip.NewResponse(sip.StatusRequestTerminated, "Terminated")}, nil}, } { t.Run(tt.name, func(t *testing.T) { - tx := &testSIPClientTransaction{log: logger.GetLogger(), responses: make(chan *sip.Response, len(tt.resps)), done: make(chan struct{})} + tx := &testSIPClientTransaction{log: logger.NewTestLogger(t), responses: make(chan *sip.Response, len(tt.resps)), done: make(chan struct{})} for _, r := range tt.resps { tx.responses <- r } cli := &recordingSIPClient{} - watchCancelledInvite(logger.GetLogger(), cli, nil, newInvite(), tx) + watchCancelledInvite(logger.NewTestLogger(t), cli, nil, newInvite(), tx) require.Equal(t, tt.want, cli.methods()) }) } @@ -356,9 +356,9 @@ func TestWatchCancelledInvite(t *testing.T) { t.Run("no answer within grace", func(t *testing.T) { defer func(d time.Duration) { cancelResponseGrace = d }(cancelResponseGrace) cancelResponseGrace = 10 * time.Millisecond - tx := &testSIPClientTransaction{log: logger.GetLogger(), responses: make(chan *sip.Response), done: make(chan struct{})} + tx := &testSIPClientTransaction{log: logger.NewTestLogger(t), responses: make(chan *sip.Response), done: make(chan struct{})} cli := &recordingSIPClient{} - watchCancelledInvite(logger.GetLogger(), cli, nil, newInvite(), tx) + watchCancelledInvite(logger.NewTestLogger(t), cli, nil, newInvite(), tx) require.Empty(t, cli.methods()) }) } diff --git a/pkg/sip/outbound_utilities_test.go b/pkg/sip/outbound_utilities_test.go index 785508075..53262b778 100644 --- a/pkg/sip/outbound_utilities_test.go +++ b/pkg/sip/outbound_utilities_test.go @@ -17,7 +17,6 @@ package sip import ( "context" "errors" - "fmt" "sync/atomic" "testing" "time" @@ -33,7 +32,6 @@ import ( "github.com/livekit/sipgo/sip" msdk "github.com/livekit/media-sdk" - "github.com/livekit/media-sdk/dtmf" "github.com/livekit/media-sdk/mixer" "github.com/livekit/media-sdk/rtp" lksdk "github.com/livekit/server-sdk-go/v2" @@ -104,6 +102,8 @@ type testRoom struct { room *Room } +var _ RoomInterface = (*testRoom)(nil) + type testRoomConfig struct { ringForever bool } @@ -124,15 +124,17 @@ func newTestRoomWithConfig(log logger.Logger, st *RoomStats, cfg *testRoomConfig } // Create a Room with all the necessary structure but skip connection room := &Room{ - log: log, - stats: st, - out: msdk.NewSwitchWriter(RoomSampleRate), - subscribe: atomic.Bool{}, + log: log, + stats: st, + outboundAudio: msdk.NewWriteCloserSwitch[msdk.PCM16Sample](RoomSampleRate), + outboundDTMF: msdk.NewWriteCloserSwitch[*livekit.SipDTMF](0), + subscribe: atomic.Bool{}, } + room.inboundDTMF = inboundDTMFWriter{room} // Create mixer var err error - room.mix, err = mixer.NewMixer(room.out, rtp.DefFrameDur, 1, mixer.WithStats(&st.Mixer), mixer.WithOutputChannel()) + room.mix, err = mixer.NewMixer(room.outboundAudio, rtp.DefFrameDur, 1, mixer.WithStats(&st.Mixer), mixer.WithOutputChannel()) if err != nil { panic(err) } @@ -204,20 +206,20 @@ func (r *testRoom) Subscribe() { r.room.Subscribe() } -func (r *testRoom) Output() msdk.Writer[msdk.PCM16Sample] { - return r.room.Output() +func (r *testRoom) WriteOutboundAudioTo(w msdk.PCM16Writer) msdk.PCM16Writer { + return r.room.WriteOutboundAudioTo(w) } -func (r *testRoom) SwapOutput(out msdk.PCM16Writer) msdk.PCM16Writer { - return r.room.SwapOutput(out) +func (r *testRoom) WriteOutboundDTMFTo(w msdk.WriteCloser[*livekit.SipDTMF]) msdk.WriteCloser[*livekit.SipDTMF] { + return r.room.WriteOutboundDTMFTo(w) } -func (r *testRoom) CloseOutput() error { - return r.room.CloseOutput() +func (r *testRoom) GetInboundAudioWriter() (msdk.PCM16Writer, error) { + return r.NewParticipantTrack(RoomSampleRate) } -func (r *testRoom) SetDTMFOutput(w dtmf.Writer) { - r.room.SetDTMFOutput(w) +func (r *testRoom) GetInboundDTMFWriter() msdk.WriteCloser[*livekit.SipDTMF] { + return r.room.GetInboundDTMFWriter() } func (r *testRoom) Close() error { @@ -258,10 +260,6 @@ func (w *noOpWriter) Close() error { return nil } -func (r *testRoom) SendData(data lksdk.DataPacket, opts ...lksdk.DataPublishOption) error { - return r.room.SendData(data, opts...) -} - func (r *testRoom) NewTrack() *mixer.Input { return r.room.NewTrack() } @@ -420,7 +418,7 @@ func (w *testSIPClient) WriteRequest(req *sip.Request, options ...sipgo.ClientRe if len(options) > 0 { panic("options not supported for testSIPClient") } - fmt.Printf("SIP WriteRequest sent on client %v:\n%s\n", w, req.String()) + w.log.Infow("SIP WriteRequest sent on client", "client", w, "request", req.String()) w.FillRequestBlanks(req) w.sequence++ reqReq := &sipRequest{ diff --git a/pkg/sip/room.go b/pkg/sip/room.go index 862a8641a..2d4abb325 100644 --- a/pkg/sip/room.go +++ b/pkg/sip/room.go @@ -185,17 +185,27 @@ type RoomInterface interface { Subscribed() <-chan struct{} Room() *lksdk.Room Subscribe() - Output() msdk.Writer[msdk.PCM16Sample] - SwapOutput(out msdk.PCM16Writer) msdk.PCM16Writer - CloseOutput() error - SetDTMFOutput(w dtmf.Writer) Close() error CloseWithReason(reason livekit.DisconnectReason) error Participant() ParticipantInfo NewParticipantTrack(sampleRate int) (msdk.WriteCloser[msdk.PCM16Sample], error) - SendData(data lksdk.DataPacket, opts ...lksdk.DataPublishOption) error NewTrack() *mixer.Input lksdk.RoomRPCInterface + + // WriteOutboundAudioTo tells the room where to send audio to. + // Returns the previously-set writer (if one exists). + WriteOutboundAudioTo(w msdk.PCM16Writer) msdk.PCM16Writer + + // WriteOutboundDTMFTo tells the room where to send DTMF to. + // Returns the previously-set writer (if one exists). + WriteOutboundDTMFTo(w msdk.WriteCloser[*livekit.SipDTMF]) msdk.WriteCloser[*livekit.SipDTMF] + + // GetInboundAudioWriter returns a writer that, when written to, writes + // audio to the room. + GetInboundAudioWriter() (msdk.PCM16Writer, error) + // GetInboundDTMFWriter returns a writer that, when written to, writes DTMF + // to the room. + GetInboundDTMFWriter() msdk.WriteCloser[*livekit.SipDTMF] } type GetRoomFunc func(log logger.Logger, st *RoomStats) RoomInterface @@ -208,10 +218,13 @@ type Room struct { log logger.Logger roomLog logger.Logger // deferred logger // room is cleared on close while SDK callback goroutines still read it. - room atomic.Pointer[lksdk.Room] - mix *mixer.Mixer - out *msdk.SwitchWriter - outDtmf atomic.Pointer[dtmf.Writer] + room atomic.Pointer[lksdk.Room] + mix *mixer.Mixer + + outboundAudio *msdk.WriteCloserSwitch[msdk.PCM16Sample] + outboundDTMF *msdk.WriteCloserSwitch[*livekit.SipDTMF] + inboundDTMF inboundDTMFWriter + // p is replaced on every reconnect, since the server issues a new // participant SID, and read concurrently by Participant(). p atomic.Pointer[ParticipantInfo] @@ -246,10 +259,17 @@ func NewRoom(log logger.Logger, st *RoomStats) *Room { if st == nil { st = &RoomStats{} } - r := &Room{log: log, stats: st, out: msdk.NewSwitchWriter(RoomSampleRate)} + r := &Room{ + log: log, + stats: st, + + outboundAudio: msdk.NewWriteCloserSwitch[msdk.PCM16Sample](RoomSampleRate), + outboundDTMF: msdk.NewWriteCloserSwitch[*livekit.SipDTMF](0), + } + r.inboundDTMF = inboundDTMFWriter{r} var err error - r.mix, err = mixer.NewMixer(r.out, rtp.DefFrameDur, 1, mixer.WithStats(&st.Mixer), mixer.WithOutputChannel()) + r.mix, err = mixer.NewMixer(r.outboundAudio, rtp.DefFrameDur, 1, mixer.WithStats(&st.Mixer), mixer.WithOutputChannel()) if err != nil { panic(err) } @@ -676,52 +696,10 @@ func (r *Room) subscribeAll(room *lksdk.Room) { } } -func (r *Room) Output() msdk.Writer[msdk.PCM16Sample] { - return r.out.Get() -} - -// SwapOutput sets room audio output and returns the old one. -// Caller is responsible for closing the old writer. -func (r *Room) SwapOutput(out msdk.PCM16Writer) msdk.PCM16Writer { - if r == nil { - return nil - } - if out == nil { - return r.out.Swap(nil) - } - return r.out.Swap(msdk.ResampleWriter(out, r.mix.SampleRate())) -} - -func (r *Room) CloseOutput() error { - w := r.SwapOutput(nil) - if w == nil { - return nil - } - return w.Close() -} - -func (r *Room) SetDTMFOutput(w dtmf.Writer) { - if r == nil { - return - } - if w == nil { - r.outDtmf.Store(nil) - return - } - r.outDtmf.Store(&w) -} - func (r *Room) sendDTMF(ctx context.Context, msg *livekit.SipDTMF) { - outDTMF := r.outDtmf.Load() - if outDTMF == nil { - r.log.Infow("ignoring dtmf", "digit", msg.Digit) - return - } // TODO: Separate goroutine? - ctx, cancel := context.WithCancel(ctx) - defer cancel() r.log.Debugw("forwarding dtmf to sip", "digit", msg.Digit) - _ = (*outDTMF).WriteDTMF(ctx, msg.Digit) + r.outboundDTMF.WriteSample(msg) } func (r *Room) Close() error { @@ -732,13 +710,13 @@ func (r *Room) CloseWithReason(reason livekit.DisconnectReason) error { if r == nil { return nil } - var err error + var errs []error r.closed.Once(func() { defer r.stats.Closed.Store(true) r.subscribe.Store(false) - err = r.CloseOutput() - r.SetDTMFOutput(nil) + errs = append(errs, r.outboundAudio.Close()) + errs = append(errs, r.outboundDTMF.Close()) if room := r.room.Swap(nil); room != nil { room.DisconnectWithReason(reason) } @@ -746,7 +724,7 @@ func (r *Room) CloseWithReason(reason livekit.DisconnectReason) error { r.mix.Stop() } }) - return err + return errors.Join(errs...) } func (r *Room) Participant() ParticipantInfo { @@ -759,6 +737,8 @@ func (r *Room) Participant() ParticipantInfo { return ParticipantInfo{} } +// NewParticipantTrack publishes a local Opus audio track into the LiveKit room. +// TODO(alexfish): Remove this from the public interface. func (r *Room) NewParticipantTrack(sampleRate int) (msdk.WriteCloser[msdk.PCM16Sample], error) { track, err := webrtc.NewTrackLocalStaticSample(webrtc.RTPCodecCapability{MimeType: webrtc.MimeTypeOpus}, "audio", "pion") if err != nil { @@ -800,6 +780,45 @@ func (r *Room) NewTrack() *mixer.Input { return r.mix.NewInput() } +func (r *Room) WriteOutboundAudioTo(w msdk.PCM16Writer) msdk.PCM16Writer { + return r.outboundAudio.Swap(w) +} + +func (r *Room) WriteOutboundDTMFTo(w msdk.WriteCloser[*livekit.SipDTMF]) msdk.WriteCloser[*livekit.SipDTMF] { + return r.outboundDTMF.Swap(w) +} + +func (r *Room) GetInboundAudioWriter() (msdk.PCM16Writer, error) { + return r.NewParticipantTrack(RoomSampleRate) +} + +func (r *Room) GetInboundDTMFWriter() msdk.WriteCloser[*livekit.SipDTMF] { + return &r.inboundDTMF +} + +type inboundDTMFWriter struct { + r *Room +} + +func (w *inboundDTMFWriter) String() string { + return "inboundDTMFWriter" +} + +func (w *inboundDTMFWriter) SampleRate() int { + return dtmf.SampleRate +} + +func (w *inboundDTMFWriter) Close() error { + return nil +} + +func (w *inboundDTMFWriter) WriteSample(sample *livekit.SipDTMF) error { + if sample == nil { + return nil + } + return w.r.SendData(sample, lksdk.WithDataPublishReliable(true)) +} + // roomOverrideLogger converts errors to warnings and ignore debug type roomOverrideLogger struct { logger.Logger diff --git a/pkg/sip/room_test.go b/pkg/sip/room_test.go index 581b54b09..7eaefa5e0 100644 --- a/pkg/sip/room_test.go +++ b/pkg/sip/room_test.go @@ -74,7 +74,7 @@ type reconnectFixture struct { func newReconnectFixture(t *testing.T) *reconnectFixture { t.Helper() - r := NewRoom(logger.GetLogger(), &RoomStats{}) + r := NewRoom(logger.NewTestLogger(t), &RoomStats{}) t.Cleanup(func() { _ = r.Close() }) cb := r.newRoomCallback(&config.Config{}, RoomConfig{}) diff --git a/pkg/sip/service_test.go b/pkg/sip/service_test.go index 7adbffcc7..9256e5926 100644 --- a/pkg/sip/service_test.go +++ b/pkg/sip/service_test.go @@ -5,7 +5,6 @@ import ( "fmt" "log/slog" "math/rand" - "net/netip" "slices" "sync" "sync/atomic" @@ -49,11 +48,29 @@ func getResponseOrFail(t *testing.T, tx sip.ClientTransaction) *sip.Response { return nil } +func getResponseOrFailTimeout(t *testing.T, ctx context.Context, tx sip.ClientTransaction) *sip.Response { + t.Helper() + var ctxDone <-chan struct{} = nil + if ctx != nil { + ctxDone = ctx.Done() + } + select { + // Avoid using t.Context, this helper is used in test cleanup code as well. + case <-ctxDone: + t.Fatal("Context cancelled") + case <-tx.Done(): + t.Fatal("Transaction failed to complete") + case res := <-tx.Responses(): + return res + } + + return nil +} -func getFinalResponseOrFail(t *testing.T, tx sip.ClientTransaction, req *sip.Request) *sip.Response { +func getFinalResponseOrFail(t *testing.T, ctx context.Context, tx sip.ClientTransaction) *sip.Response { var res *sip.Response for { - res = getResponseOrFail(t, tx) + res = getResponseOrFailTimeout(t, ctx, tx) if res.StatusCode >= 200 { break } @@ -139,7 +156,7 @@ func testInvite(t *testing.T, h Handler, hidden bool, from, to string, test func require.NoError(t, err) // Use a no-op logger to avoid panics from async logging after test completion - log := logger.LogRLogger(logr.Discard()) + log := logger.NewTestLogger(t) s, err := NewService("", &config.Config{ HideInboundPort: hidden, SIPPort: sipPort, @@ -324,7 +341,7 @@ func TestService_RejectedInviteCacheReplay(t *testing.T) { tx, err := client.TransactionRequest(req) require.NoError(t, err) t.Cleanup(tx.Terminate) - return getFinalResponseOrFail(t, tx, req) + return getFinalResponseOrFail(t, nil, tx) } // First INVITE: full handler invocation, 404 from DispatchNoRuleReject. @@ -898,46 +915,32 @@ func TestCANCELSendsBothResponses(t *testing.T) { ) st := NewServiceTest(t, &serviceTestConfig{GetRoom: newTestRoomConfig(&testRoomConfig{ringForever: true})}) - loopback := netip.MustParseAddr("127.0.0.1") - sipServerAddress := st.Address() - // Create SIP client using sipgo - sipUserAgent, err := sipgo.NewUA( - sipgo.WithUserAgent(fromUser), - ) + call := newTestCall(st.TestUA, false) + req, localSDP, err := call.Invite(nil) require.NoError(t, err) + call.SetLocalSDP(localSDP) - sipClient, err := sipgo.NewClient(sipUserAgent) - require.NoError(t, err) + ctx, cancel := context.WithTimeout(t.Context(), 1*time.Second) + defer cancel() - // Create SDP offer - offer, err := sdp.NewOfferWith(defaultCodecs, loopback, 0xB0B, sdp.EncryptionNone) - require.NoError(t, err) - offerData, err := offer.SDP.Marshal() + inviteTx, err := st.TestUA.Client.TransactionRequest(req) require.NoError(t, err) - - // Create INVITE request - inviteRecipient := sip.Uri{User: toUser, Host: sipServerAddress} - inviteRequest := sip.NewRequest(sip.INVITE, inviteRecipient) - inviteRequest.SetDestination(sipServerAddress) - inviteRequest.SetBody(offerData) - inviteRequest.AppendHeader(sip.NewHeader("Content-Type", "application/sdp")) - - // Send INVITE - tx, err := sipClient.TransactionRequest(inviteRequest) - require.NoError(t, err) - t.Cleanup(tx.Terminate) + defer inviteTx.Terminate() // Wait for 100 Trying - res100 := getResponseOrFail(t, tx) + res100 := getResponseOrFailTimeout(t, ctx, inviteTx) require.Equal(t, sip.StatusCode(100), res100.StatusCode, "Should receive 100 Trying") // Wait for 180 Ringing (call is now ringing) - res180 := getResponseOrFail(t, tx) + res180 := getResponseOrFailTimeout(t, ctx, inviteTx) require.Equal(t, sip.StatusCode(180), res180.StatusCode, "Should receive 180 Ringing") + remoteTag, ok := res180.To().Params.Get("tag") + require.True(t, ok, "remote tag should be present") + call.SetRemoteTag(LocalTag(remoteTag)) // Now send CANCEL - err = tx.Cancel() + err = inviteTx.Cancel() require.NoError(t, err, "Should be able to send CANCEL") // On-the-wire there should be two responses after CANCEL: @@ -948,51 +951,11 @@ func TestCANCELSendsBothResponses(t *testing.T) { // Sipgo treats both INVITE and CANCEL as the same transaction, and has special handling // to swallow the 200 OK response to CANCEL, so it can't look like the INVITE got the 200. - // Collect responses until we get the final 487 or transaction completes - var responses []*sip.Response - - // Wait for responses with a timeout - timeout := time.After(time.Second) - - // Collect responses until we get 487 or timeout - for { - select { - case res := <-tx.Responses(): - responses = append(responses, res) - cseq := res.CSeq() - - // Debug: log all responses to understand what we're receiving - cseqMethod := "nil" - if cseq != nil { - cseqMethod = string(cseq.MethodName) - } - t.Logf("Received response: StatusCode=%d, CSeq method=%s", res.StatusCode, cseqMethod) - - if res.StatusCode < 200 { - continue - } - require.Equal(t, sip.StatusCode(487), res.StatusCode, "Should have received 487 Request Terminated response to INVITE when CANCEL is sent") - require.NotNil(t, cseq, "487 response should have CSeq header") - require.Equal(t, sip.INVITE, cseq.MethodName, "487 response should be for INVITE method") - return // Success! - - case <-tx.Done(): - t.Fatal("Transaction done without receiving expected 487 response") - - case <-timeout: - // Log all received responses for debugging - t.Logf("Timeout after receiving %d responses", len(responses)) - for i, res := range responses { - cseq := res.CSeq() - cseqMethod := "nil" - if cseq != nil { - cseqMethod = string(cseq.MethodName) - } - t.Logf(" Response %d: StatusCode=%d, CSeq method=%s", i+1, res.StatusCode, cseqMethod) - } - t.Fatal("Timeout waiting for 487 Request Terminated response after CANCEL") - } - } + res := getFinalResponseOrFail(t, ctx, inviteTx) + require.Equal(t, sip.StatusCode(487), res.StatusCode, "Should have received 487 Request Terminated response to INVITE when CANCEL is sent") + cseq := res.CSeq() + require.NotNil(t, cseq, "487 response should have CSeq header") + require.Equal(t, sip.INVITE, cseq.MethodName, "487 response should be for INVITE method") } // TestSameCallIDForAuthFlow verifies that the same LiveKit call ID is assigned to both diff --git a/pkg/sip/signal_logger_test.go b/pkg/sip/signal_logger_test.go index 84f6d027f..457145191 100644 --- a/pkg/sip/signal_logger_test.go +++ b/pkg/sip/signal_logger_test.go @@ -61,10 +61,10 @@ func (m *mockPCM16Writer) WriteSample(sample msdk.PCM16Sample) error { } func TestSignalLogger_initialization(t *testing.T) { - log := logger.GetLogger() next := newMockPCM16Writer(48000) t.Run("default initialization", func(t *testing.T) { + log := logger.NewTestLogger(t) out, err := NewSignalLogger(log, "incoming", next) sl, ok := out.(*SignalLogger) require.True(t, ok) @@ -77,6 +77,7 @@ func TestSignalLogger_initialization(t *testing.T) { }) t.Run("with valid options", func(t *testing.T) { + log := logger.NewTestLogger(t) out, err := NewSignalLogger(log, "incoming", next, WithNoiseFloor(-60), WithHangoverDuration(2*time.Second), WithEnterVoiceOffsetDB(9), WithExitVoiceOffsetDB(4)) sl, ok := out.(*SignalLogger) require.True(t, ok) @@ -89,6 +90,7 @@ func TestSignalLogger_initialization(t *testing.T) { }) t.Run("with invalid options", func(t *testing.T) { + log := logger.NewTestLogger(t) _, err := NewSignalLogger(log, "incoming", next, WithHangoverDuration(-time.Second)) require.Error(t, err) require.Contains(t, err.Error(), "hangover duration must be positive, got -1s") @@ -106,7 +108,7 @@ func TestSignalLogger_initialization(t *testing.T) { func newTestLogger(t *testing.T, opts ...SignalLoggerOption) (*SignalLogger, *mockPCM16Writer) { next := newMockPCM16Writer(48000) - out, err := NewSignalLogger(logger.GetLogger(), "incoming", next, opts...) + out, err := NewSignalLogger(logger.NewTestLogger(t), "incoming", next, opts...) sl, ok := out.(*SignalLogger) require.True(t, ok) require.NoError(t, err) diff --git a/pkg/sip/signaling_test.go b/pkg/sip/signaling_test.go index 8744f46fc..6cebbc2dd 100644 --- a/pkg/sip/signaling_test.go +++ b/pkg/sip/signaling_test.go @@ -18,6 +18,7 @@ import ( "github.com/stretchr/testify/require" "google.golang.org/protobuf/types/known/durationpb" + "github.com/livekit/media-sdk/g711" "github.com/livekit/media-sdk/sdp" "github.com/livekit/mediatransportutil/pkg/rtcconfig" "github.com/livekit/protocol/livekit" @@ -284,7 +285,7 @@ func (s *sipUATest) TransactionRequest(t *testing.T, req *sip.Request, isFromUAC require.NoError(t, err) defer tx.Terminate() - resp := getFinalResponseOrFail(t, tx, req) + resp := getFinalResponseOrFail(t, nil, tx) if req.Method == sip.INVITE && resp.StatusCode < 300 { // Need to send ACK for 2xx INVITE, sipgo already sends ACK for 3xx+ ack := sip.NewAckRequest(req, resp, nil) @@ -556,6 +557,10 @@ func (st *serviceTest) CreateInboundCall(t *testing.T, opts ...createCallTestOpt call.SetRemoteTag(LocalTag(remoteTag)) call.SetRemoteSDP(resp.Body()) call.SetRouteSet(resp, true) + t.Cleanup(func() { + bye := call.NewRequest(sip.BYE) + st.TestUA.TransactionRequest(t, bye, true) + }) st.Server.cmu.Lock() defer st.Server.cmu.Unlock() @@ -627,6 +632,10 @@ func (st *serviceTest) CreateOutboundCall(t *testing.T, opts ...createCallTestOp case <-ctx.Done(): require.Fail(t, "timeout waiting for ACK") } + t.Cleanup(func() { + bye := call.NewRequest(sip.BYE) + st.TestUA.TransactionRequest(t, bye, false) + }) st.Client.cmu.Lock() defer st.Client.cmu.Unlock() @@ -637,6 +646,38 @@ func (st *serviceTest) CreateOutboundCall(t *testing.T, opts ...createCallTestOp return call, oc, ackReq } +func getMediaPort(t *testing.T, m MediaPort) *mediaPort { + t.Helper() + port, ok := m.(*mediaPort) + require.True(t, ok, "media port should be a *mediaPort") + return port +} + +func getMediaPortRemoteAddr(t *testing.T, m MediaPort) netip.AddrPort { + t.Helper() + port := getMediaPort(t, m) + dst := port.port.dst.Load() + require.NotNil(t, dst, "destination should be set") + return *dst +} + +// incompatibleCodecOffer builds an SDP offer whose only audio codec is not the +// one already negotiated on m. +func incompatibleCodecOffer(t *testing.T, addr netip.AddrPort, m MediaPort) []byte { + t.Helper() + codecSet := testCodecSet(g711.ULawSDPNameAndRate) + ngotiated := m.NegotiatedAudio() + require.NotNil(t, ngotiated, "media must already be negotiated") + if ngotiated.Codec.Info().SDPName == g711.ULawSDPNameAndRate { + codecSet = testCodecSet(g711.ALawSDPNameAndRate) + } + sdpOffer, err := sdp.NewOfferWith(codecSet, addr.Addr(), int(addr.Port()), sdp.EncryptionNone) + require.NoError(t, err) + offer, err := sdpOffer.SDP.Marshal() + require.NoError(t, err) + return offer +} + func TestReinvite(t *testing.T) { t.Run("inbound", func(t *testing.T) { t.Run("normal", func(t *testing.T) { @@ -663,7 +704,7 @@ func TestReinvite(t *testing.T) { require.Equal(t, serverLocalSDP, resp.Body(), "reinvite 200 OK should return server local SDP") // After the re-INVITE with new offer, the media port destination must be updated. - require.Equal(t, newOffer.Addr, ic.media.RemoteAddr(), "re-INVITE should redirect RTP to the new remote address") + require.Equal(t, newOffer.Addr, getMediaPortRemoteAddr(t, ic.media), "re-INVITE should redirect RTP to the new remote address") }) t.Run("miss", func(t *testing.T) { @@ -679,7 +720,13 @@ func TestReinvite(t *testing.T) { require.Equal(t, serverLocalSDP, resp.Body(), "reinvite 200 OK should return server local SDP") // re-INVITE with different tag + oldTag := call.remoteTag call.remoteTag = "something-else" + t.Cleanup(func() { + req := call.NewRequest(sip.BYE) + st.TestUA.TransactionRequest(t, req, true) + call.remoteTag = oldTag + }) req, _, err = call.Invite(call.localSDP) require.NoError(t, err) resp = st.TestUA.TransactionRequest(t, req, true) @@ -691,21 +738,50 @@ func TestReinvite(t *testing.T) { st := NewServiceTest(t, nil) call, ic := st.CreateInboundCall(t) serverLocalSDP := call.remoteSDP - initialRemote := ic.media.RemoteAddr() + initialRemote := getMediaPortRemoteAddr(t, ic.media) // Re-INVITE with no SDP body — destination must not change. req := call.NewRequest(sip.INVITE) // no body, no Content-Type resp := st.TestUA.TransactionRequest(t, req, true) require.Equal(t, sip.StatusCode(200), resp.StatusCode, "body-less re-INVITE should still get 200 OK") require.Equal(t, serverLocalSDP, resp.Body(), "body-less re-INVITE should return server local SDP") - require.Equal(t, initialRemote, ic.media.RemoteAddr(), "body-less re-INVITE must not change RTP destination") + require.Equal(t, initialRemote, getMediaPortRemoteAddr(t, ic.media), "body-less re-INVITE must not change RTP destination") + }) + + t.Run("incompatible_codec", func(t *testing.T) { + // TODO: change this test to confirm renegotiation when it's enabled + st := NewServiceTest(t, nil) + call, ic := st.CreateInboundCall(t) + serverLocalSDP := call.remoteSDP + initialRemote := getMediaPortRemoteAddr(t, ic.media) + initialCodec := ic.media.NegotiatedAudio().Codec.Info().SDPName + + // TODO: Change to reflect full negotiation once enabled + updatedRemote := netip.MustParseAddrPort("9.8.7.6:12345") + req, _, err := call.Invite(incompatibleCodecOffer(t, updatedRemote, ic.media)) + require.NoError(t, err) + resp := st.TestUA.TransactionRequest(t, req, true) + require.Equal(t, sip.StatusCode(200), resp.StatusCode, "incompatible re-INVITE should get 200 OK") + require.Equal(t, updatedRemote, getMediaPortRemoteAddr(t, ic.media), "incompatible re-INVITE must still change RTP destination") + require.Equal(t, initialCodec, ic.media.NegotiatedAudio().Codec.Info().SDPName, "Codec must not be updated") + + // Re-INVITE with original codec + req, _, err = call.Invite(call.localSDP) + require.NoError(t, err) + resp = st.TestUA.TransactionRequest(t, req, true) + require.Equal(t, sip.StatusCode(200), resp.StatusCode, "original offer should still be accepted") + require.Equal(t, serverLocalSDP, resp.Body(), "successful re-INVITE should return server local SDP") + require.Equal(t, initialRemote, getMediaPortRemoteAddr(t, ic.media), "original re-INVITE must restore RTP destination") + require.Equal(t, initialCodec, ic.media.NegotiatedAudio().Codec.Info().SDPName, "Codec must not be updated") + }) }) t.Run("outbound", func(t *testing.T) { t.Run("normal", func(t *testing.T) { st := NewServiceTest(t, nil) call, oc, _ := st.CreateOutboundCall(t) - serverLocalSDP := oc.cc.LocalSDP() + serverLocalSDP, err := oc.media.GetLocalSDP() + require.NoError(t, err) require.NotEqual(t, call.localSDP, serverLocalSDP, "local and remote SDP should be different") // Re-INVITE @@ -727,27 +803,29 @@ func TestReinvite(t *testing.T) { require.Equal(t, serverLocalSDP, resp.Body(), "reinvite 200 OK should return server local SDP") // After the re-INVITE with new offer, the media port destination must be updated. - require.Equal(t, newOffer.Addr, oc.media.RemoteAddr(), "re-INVITE should redirect outbound call RTP to the new remote address") + require.Equal(t, newOffer.Addr, getMediaPortRemoteAddr(t, oc.media), "re-INVITE should redirect outbound call RTP to the new remote address") }) t.Run("no_body", func(t *testing.T) { st := NewServiceTest(t, nil) call, oc, _ := st.CreateOutboundCall(t) - serverLocalSDP := oc.cc.LocalSDP() - initialRemote := oc.media.RemoteAddr() + serverLocalSDP, err := getMediaPort(t, oc.media).GetLocalSDP() + require.NoError(t, err) + initialRemote := getMediaPortRemoteAddr(t, oc.media) // Re-INVITE with no SDP body — destination must not change. req := call.NewRequest(sip.INVITE) // no body, no Content-Type resp := st.TestUA.TransactionRequest(t, req, false) require.Equal(t, sip.StatusCode(200), resp.StatusCode, "body-less re-INVITE should still get 200 OK") require.Equal(t, serverLocalSDP, resp.Body(), "body-less re-INVITE should return server local SDP") - require.Equal(t, initialRemote, oc.media.RemoteAddr(), "body-less re-INVITE must not change RTP destination") + require.Equal(t, initialRemote, getMediaPortRemoteAddr(t, oc.media), "body-less re-INVITE must not change RTP destination") }) t.Run("miss", func(t *testing.T) { st := NewServiceTest(t, nil) call, oc, _ := st.CreateOutboundCall(t) - serverLocalSDP := oc.cc.LocalSDP() + serverLocalSDP, err := getMediaPort(t, oc.media).GetLocalSDP() + require.NoError(t, err) // Re-INVITE req, _, err := call.Invite(call.localSDP) @@ -764,6 +842,34 @@ func TestReinvite(t *testing.T) { require.Equal(t, sip.StatusCode(200), resp.StatusCode, "reinvite for outbound call should get 200 OK") require.NotEqual(t, serverLocalSDP, resp.Body(), "reinvite for new call should return new server local SDP") }) + + t.Run("incompatible_codec", func(t *testing.T) { + // TODO: change this test to confirm renegotiation when it's enabled + st := NewServiceTest(t, nil) + call, oc, _ := st.CreateOutboundCall(t) + serverLocalSDP, err := oc.media.GetLocalSDP() + require.NoError(t, err) + initialCodec := oc.media.NegotiatedAudio().Codec.Info().SDPName + initialRemote := getMediaPortRemoteAddr(t, oc.media) + + // TODO: Change to reflect full negotiation once enabled + updatedRemote := netip.MustParseAddrPort("9.8.7.6:12345") + req, _, err := call.Invite(incompatibleCodecOffer(t, updatedRemote, oc.media)) + require.NoError(t, err) + resp := st.TestUA.TransactionRequest(t, req, true) + require.Equal(t, sip.StatusCode(200), resp.StatusCode, "incompatible re-INVITE should get 200 OK") + require.Equal(t, updatedRemote, getMediaPortRemoteAddr(t, oc.media), "incompatible re-INVITE must still change RTP destination") + require.Equal(t, initialCodec, oc.media.NegotiatedAudio().Codec.Info().SDPName, "Codec must not be updated") + + // Re-INVITE with original codec + req, _, err = call.Invite(call.localSDP) + require.NoError(t, err) + resp = st.TestUA.TransactionRequest(t, req, true) + require.Equal(t, sip.StatusCode(200), resp.StatusCode, "original offer should still be accepted") + require.Equal(t, serverLocalSDP, resp.Body(), "successful re-INVITE should return server local SDP") + require.Equal(t, initialRemote, getMediaPortRemoteAddr(t, oc.media), "original re-INVITE must restore RTP destination") + require.Equal(t, initialCodec, oc.media.NegotiatedAudio().Codec.Info().SDPName, "Codec must not be updated") + }) }) } diff --git a/pkg/sip/silence_filler_test.go b/pkg/sip/silence_filler_test.go index 371217a7a..1c45ba5a1 100644 --- a/pkg/sip/silence_filler_test.go +++ b/pkg/sip/silence_filler_test.go @@ -139,9 +139,8 @@ func TestSilenceSuppressionHandling(t *testing.T) { samplesPerFrame = uint32(sampleRate / rtp.DefFramesPerSec) // 160 samples per 20ms frame ) - log := logger.GetLogger() - t.Run("no gap", func(t *testing.T) { + log := logger.NewTestLogger(t) tester := newSilenceSuppressionTester(sampleRate, log) _, _, err := tester.SendSignalFrames(10, 100, 1000) require.NoError(t, err) @@ -150,6 +149,7 @@ func TestSilenceSuppressionHandling(t *testing.T) { }) t.Run("single frame gap", func(t *testing.T) { + log := logger.NewTestLogger(t) tester := newSilenceSuppressionTester(sampleRate, log) nextSeq := uint16(100) @@ -165,6 +165,7 @@ func TestSilenceSuppressionHandling(t *testing.T) { }) t.Run("handful of frames gap", func(t *testing.T) { + log := logger.NewTestLogger(t) tester := newSilenceSuppressionTester(sampleRate, log) nextSeq := uint16(100) @@ -180,6 +181,7 @@ func TestSilenceSuppressionHandling(t *testing.T) { }) t.Run("large gap that's not filled", func(t *testing.T) { + log := logger.NewTestLogger(t) tester := newSilenceSuppressionTester(sampleRate, log) nextSeq := uint16(100) @@ -195,6 +197,7 @@ func TestSilenceSuppressionHandling(t *testing.T) { }) t.Run("timestamp wrap-around no gap", func(t *testing.T) { + log := logger.NewTestLogger(t) tester := newSilenceSuppressionTester(sampleRate, log) // Start near wrap-around @@ -207,6 +210,7 @@ func TestSilenceSuppressionHandling(t *testing.T) { }) t.Run("timestamp wrap-around with gap", func(t *testing.T) { + log := logger.NewTestLogger(t) tester := newSilenceSuppressionTester(sampleRate, log) // 2 signal + 3 silence (across wrap-around) + 2 signal = 7 total @@ -223,6 +227,7 @@ func TestSilenceSuppressionHandling(t *testing.T) { }) t.Run("sequence wrap-around no gap", func(t *testing.T) { + log := logger.NewTestLogger(t) tester := newSilenceSuppressionTester(sampleRate, log) // Start near sequence wrap-around @@ -235,6 +240,7 @@ func TestSilenceSuppressionHandling(t *testing.T) { }) t.Run("sequence wrap-around with gap", func(t *testing.T) { + log := logger.NewTestLogger(t) tester := newSilenceSuppressionTester(sampleRate, log) // Start near sequence wrap-around @@ -251,8 +257,6 @@ func TestSilenceSuppressionHandling(t *testing.T) { }) } func TestSilenceSuppressionDifferentCodecs(t *testing.T) { - log := logger.GetLogger() - testCases := []struct { name string clockRate int @@ -287,6 +291,7 @@ func TestSilenceSuppressionDifferentCodecs(t *testing.T) { for _, tc := range testCases { t.Run(tc.name, func(t *testing.T) { + log := logger.NewTestLogger(t) tester := newSilenceSuppressionTester(tc.sampleRate, log, WithClockRate(tc.clockRate)) tsPerFrame := uint32(tc.clockRate / rtp.DefFramesPerSec) diff --git a/pkg/sip/tls_test.go b/pkg/sip/tls_test.go index d06ec19ff..4d55254a9 100644 --- a/pkg/sip/tls_test.go +++ b/pkg/sip/tls_test.go @@ -9,7 +9,7 @@ import ( ) func TestParseCipherSuites(t *testing.T) { - log := logger.GetLogger() + log := logger.NewTestLogger(t) t.Run("valid cipher suites - secure", func(t *testing.T) { cipherSuites := []string{ diff --git a/pkg/stats/monitor.go b/pkg/stats/monitor.go index d016c1b87..bfeb6a626 100644 --- a/pkg/stats/monitor.go +++ b/pkg/stats/monitor.go @@ -16,6 +16,7 @@ package stats import ( "errors" + "strconv" "sync/atomic" "time" @@ -250,7 +251,7 @@ func (m *Monitor) Start(conf *config.Config) error { Name: "sdp_parsed_total", Help: "Number of SDP bodies parsed successfully during SDP negotiation", ConstLabels: prometheus.Labels{"node_id": conf.NodeID}, - }, []string{"dir", "provider"})) + }, []string{"dir", "provider", "reinvite"})) m.codecOffered = mustRegister(m, prometheus.NewCounterVec(prometheus.CounterOpts{ Namespace: "livekit", @@ -258,7 +259,7 @@ func (m *Monitor) Start(conf *config.Config) error { Name: "codec_offered_total", Help: "Number of SDP bodies that advertised a given audio codec", ConstLabels: prometheus.Labels{"node_id": conf.NodeID}, - }, []string{"dir", "provider", "codec"})) + }, []string{"dir", "provider", "codec", "reinvite"})) m.nodeAvailable = mustRegister(m, prometheus.NewGaugeFunc(prometheus.GaugeOpts{ Namespace: "livekit", @@ -526,14 +527,15 @@ func (c *CallMonitor) StageDurTimer(stage string) func() time.Duration { // PeerSDP increments SDP count and each individual codec from the SDP body. // Should be called before codec selection such that failed negotiations are still counted -func (c *CallMonitor) PeerSDP(names []string) { +func (c *CallMonitor) PeerSDP(names []string, reinvite bool) { provider := c.providerLabel() - c.m.sdpParsed.With(prometheus.Labels{"dir": c.dir, "provider": provider}).Inc() + c.m.sdpParsed.With(prometheus.Labels{"dir": c.dir, "provider": provider, "reinvite": strconv.FormatBool(reinvite)}).Inc() for _, name := range names { c.m.codecOffered.With(prometheus.Labels{ "dir": c.dir, "provider": provider, "codec": name, + "reinvite": strconv.FormatBool(reinvite), }).Inc() } } diff --git a/test/cloud/integration_test.go b/test/cloud/integration_test.go index 7092b2bcc..fc9e4fe4e 100644 --- a/test/cloud/integration_test.go +++ b/test/cloud/integration_test.go @@ -25,7 +25,7 @@ func TestSIP(t *testing.T) { } bus := psrpc.NewLocalMessageBus() - svc, err := NewService(conf, bus) + svc, err := NewService(t, conf, bus) require.NoError(t, err) defer svc.Stop(true) diff --git a/test/cloud/service.go b/test/cloud/service.go index 4d83d6a07..a2558de5c 100644 --- a/test/cloud/service.go +++ b/test/cloud/service.go @@ -1,6 +1,8 @@ package cloud import ( + "testing" + "github.com/livekit/protocol/livekit" "github.com/livekit/protocol/logger" "github.com/livekit/protocol/rpc" @@ -10,19 +12,22 @@ import ( "github.com/livekit/sip/pkg/stats" ) -func NewService(conf *IntegrationConfig, bus psrpc.MessageBus) (*service.Service, error) { +func NewService(t testing.TB, conf *IntegrationConfig, bus psrpc.MessageBus) (*service.Service, error) { psrpcClient := NewIOTestClient(conf) + log := logger.NewTestLogger(t) mon, err := stats.NewMonitor(conf.Config) if err != nil { return nil, err } - sipsrv, err := sip.NewService("", conf.Config, mon, logger.GetLogger(), func(projectID string, _ *rpc.SIPCallObservability, _ *livekit.SIPCallInfo) sip.StateHandler { return sip.NewRPCStateHandler(psrpcClient) }) + sipsrv, err := sip.NewService("", conf.Config, mon, log, func(projectID string, _ *rpc.SIPCallObservability, _ *livekit.SIPCallInfo) sip.StateHandler { + return sip.NewRPCStateHandler(psrpcClient) + }) if err != nil { return nil, err } - svc := service.NewService(conf.Config, logger.GetLogger(), sipsrv, sipsrv.Stop, sipsrv.ActiveCalls, psrpcClient, bus, mon) + svc := service.NewService(conf.Config, log, sipsrv, sipsrv.Stop, sipsrv.ActiveCalls, psrpcClient, bus, mon) sipsrv.SetHandler(svc) if err = sipsrv.Start(); err != nil { diff --git a/test/integration/docker_leftover_test.go b/test/integration/docker_leftover_test.go new file mode 100644 index 000000000..41d801f16 --- /dev/null +++ b/test/integration/docker_leftover_test.go @@ -0,0 +1,73 @@ +package integration + +import ( + "fmt" + "log" + "strings" + + "github.com/ory/dockertest/v3/docker" +) + +func requireNoLeftoverSIPTestDocker() { + containers, networks, err := listSIPTestLeftovers() + if err != nil { + log.Fatalf("Could not list siptest docker resources: %s", err) + } + if len(containers) == 0 && len(networks) == 0 { + return + } + + var b strings.Builder + b.WriteString("siptest docker resources already exist (possible parallel runs):\n") + if len(containers) > 0 { + fmt.Fprintf(&b, " containers: %s\n", strings.Join(containers, ", ")) + } + if len(networks) > 0 { + fmt.Fprintf(&b, " networks: %s\n", strings.Join(networks, ", ")) + } + b.WriteString("\nPurge with:\n") + if len(containers) > 0 { + fmt.Fprintf(&b, " docker rm -f %s\n", strings.Join(containers, " ")) + } + if len(networks) > 0 { + fmt.Fprintf(&b, " docker network rm %s\n", strings.Join(networks, " ")) + } + log.Fatal(b.String()) +} + +func listSIPTestLeftovers() (containers, networks []string, err error) { + listed, err := Docker.Client.ListContainers(docker.ListContainersOptions{All: true}) + if err != nil { + return nil, nil, err + } + for _, c := range listed { + if name, ok := sipTestContainerName(c); ok { + containers = append(containers, name) + } + } + + listedNets, err := Docker.Client.ListNetworks() + if err != nil { + return nil, nil, err + } + for _, n := range listedNets { + if isSIPTestName(n.Name) { + networks = append(networks, n.Name) + } + } + return containers, networks, nil +} + +func sipTestContainerName(c docker.APIContainers) (string, bool) { + for _, name := range c.Names { + name = strings.TrimPrefix(name, "/") + if isSIPTestName(name) { + return name, true + } + } + return "", false +} + +func isSIPTestName(name string) bool { + return strings.HasPrefix(strings.TrimPrefix(name, "/"), dockerPrefix) +} diff --git a/test/integration/docker_mac_test.go b/test/integration/docker_mac_test.go index e302c3269..b8b710fd5 100644 --- a/test/integration/docker_mac_test.go +++ b/test/integration/docker_mac_test.go @@ -32,6 +32,7 @@ func TestMain(m *testing.M) { log.Fatalf("Could not connect to Docker: %s", err) } Docker = pool + requireNoLeftoverSIPTestDocker() code := m.Run() os.Exit(code) diff --git a/test/integration/docker_test.go b/test/integration/docker_test.go index c22c15805..480ed0549 100644 --- a/test/integration/docker_test.go +++ b/test/integration/docker_test.go @@ -25,6 +25,7 @@ func TestMain(m *testing.M) { log.Fatalf("Could not connect to Docker: %s", err) } Docker = pool + requireNoLeftoverSIPTestDocker() code := m.Run() os.Exit(code) diff --git a/test/integration/livekit_test.go b/test/integration/livekit_test.go index 292f861c0..f456d246d 100644 --- a/test/integration/livekit_test.go +++ b/test/integration/livekit_test.go @@ -1,9 +1,11 @@ package integration import ( + "bytes" "context" "fmt" "os" + "strings" "sync/atomic" "testing" @@ -17,16 +19,45 @@ import ( "github.com/livekit/sip/test/lktest" ) +const ( + dockerPrefix = "siptest-" + dockerNetworkPrefix = dockerPrefix + "net" + dockerRedisPrefix = dockerPrefix + "redis" + dockerLivekitPrefix = dockerPrefix + "livekit" +) + var debugLKServer = os.Getenv("DEBUG_LK_SERVER") != "" var redisLast uint32 +func createTestNetwork(t testing.TB, name string) *dockertest.Network { + t.Helper() + existing, err := Docker.NetworksByName(name) + if err != nil { + t.Fatal(err) + } + if len(existing) > 0 { + t.Fatal("network already exists:", name) + } + network, err := Docker.CreateNetwork(name) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { + if info, err := Docker.Client.NetworkInfo(network.Network.ID); err == nil { + network.Network = info + } + if err := Docker.RemoveNetwork(network); err != nil { + t.Log("remove network", name, err) + } + }) + return network +} + func runRedis(t testing.TB, network *dockertest.Network) (*redis.RedisConfig, string) { - name := fmt.Sprintf("siptest-redis-%d", atomic.AddUint32(&redisLast, 1)) - c, ok := Docker.ContainerByName(name) - if ok { - t.Log("Redis container already exists - stopping and removing", name) - Docker.Purge(c) + name := fmt.Sprintf("%s-%d", dockerRedisPrefix, atomic.AddUint32(&redisLast, 1)) + if _, ok := Docker.ContainerByName(name); ok { + t.Fatal("Redis container already exists:", name) } c, err := Docker.RunWithOptions( &dockertest.RunOptions{ @@ -38,7 +69,9 @@ func runRedis(t testing.TB, network *dockertest.Network) (*redis.RedisConfig, st t.Fatal(err) } t.Cleanup(func() { - _ = Docker.Purge(c) + if err := Docker.Purge(c); err != nil { + t.Log("purge", name, err) + } }) addr := c.GetHostPort("6379/tcp") waitTCPPort(t, addr) @@ -60,23 +93,15 @@ func runLiveKit(t testing.TB) *LiveKit { // Shared network so LiveKit reaches Redis by name, avoiding a // container->host round-trip that some CI runners block. - network, err := Docker.CreateNetwork(fmt.Sprintf("siptest-net-%d", id)) - if err != nil { - t.Fatal(err) - } - t.Cleanup(func() { - _ = network.Close() - }) + network := createTestNetwork(t, fmt.Sprintf("%s-%d", dockerNetworkPrefix, id)) redis, redisName := runRedis(t, network) - name := fmt.Sprintf("siptest-livekit-%d", id) - c, ok := Docker.ContainerByName(name) - if ok { - t.Log("Livekit-server container already exists - stopping and removing", name) - Docker.Purge(c) + name := fmt.Sprintf("%s-%d", dockerLivekitPrefix, id) + if _, ok := Docker.ContainerByName(name); ok { + t.Fatal("Livekit-server container already exists:", name) } - c, err = Docker.RunWithOptions( + c, err := Docker.RunWithOptions( &dockertest.RunOptions{ Name: name, Repository: "livekit/livekit-server", Tag: "master", @@ -94,7 +119,12 @@ func runLiveKit(t testing.TB) *LiveKit { lctx, cancel := context.WithCancel(context.Background()) t.Cleanup(func() { cancel() - _ = Docker.Purge(c) + if t.Failed() && debugLKServer { + dumpLivekitServerLogs(t, c.Container.ID) + } + if err := Docker.Purge(c); err != nil { + t.Log("purge", name, err) + } }) if debugLKServer { go Docker.Client.Logs(docker.LogsOptions{ @@ -138,3 +168,58 @@ func runLiveKit(t testing.TB) *LiveKit { return lk } + +func dumpLivekitServerLogs(t testing.TB, containerID string) { + t.Helper() + var logBuffer bytes.Buffer + if err := Docker.Client.Logs(docker.LogsOptions{ + Container: containerID, + OutputStream: &logBuffer, + RawTerminal: true, + }); err != nil { + t.Log("LiveKit logs:", err) + return + } + livekitServerLogs(t, logBuffer.String(), 40) +} + +func livekitServerLogs(t testing.TB, logs string, maxLines int) { + type lineRecord struct { + number int + text string + } + lines := strings.Split(logs, "\n") + fatalLines := []*lineRecord{} + errorLines := []*lineRecord{} + tailLines := lines + truncated := false + if len(lines) > maxLines { + tailLines = lines[len(lines)-maxLines:] + truncated = true + } + for i, line := range lines { + if strings.Contains(line, "fatal") || strings.Contains(line, "panic") { + l := &lineRecord{number: i, text: line} + fatalLines = append(fatalLines, l) + } else if strings.Contains(line, "error") { + l := &lineRecord{number: i, text: line} + errorLines = append(errorLines, l) + } + } + t.Logf("Found %d fatal lines, %d error lines", len(fatalLines), len(errorLines)) + for _, l := range fatalLines { + t.Logf("Fatal line %d: %s", l.number, l.text) + } + for _, l := range errorLines { + t.Logf("Error line %d: %s", l.number, l.text) + } + if len(lines) > 0 { + t.Logf("Tail lines:") + if truncated { + t.Logf("... truncated ...") + } + for _, l := range tailLines { + t.Log(l) + } + } +} diff --git a/test/integration/sip_test.go b/test/integration/sip_test.go index d6da07df6..da458bafe 100644 --- a/test/integration/sip_test.go +++ b/test/integration/sip_test.go @@ -78,7 +78,7 @@ func runSIPServer(t testing.TB, lk *LiveKit) *SIPServer { JaegerURL: os.Getenv("JAEGER_URL"), } _ = conf.InitLogger() - log := logger.GetLogger() + log := logger.NewTestLogger(t) if conf.JaegerURL != "" { jaeger.Configure(t.Context(), conf.JaegerURL, conf.ServiceName) } @@ -95,7 +95,9 @@ func runSIPServer(t testing.TB, lk *LiveKit) *SIPServer { if err != nil { t.Fatal(err) } - sipsrv, err := sip.NewService("", conf, mon, log, func(projectID string, _ *rpc.SIPCallObservability, _ *livekit.SIPCallInfo) sip.StateHandler { return sip.NewRPCStateHandler(psrpcCli) }) + sipsrv, err := sip.NewService("", conf, mon, log, func(projectID string, _ *rpc.SIPCallObservability, _ *livekit.SIPCallInfo) sip.StateHandler { + return sip.NewRPCStateHandler(psrpcCli) + }) if err != nil { t.Fatal(err) } diff --git a/test/lktest/livekit.go b/test/lktest/livekit.go index ecfb1687b..acef032d9 100644 --- a/test/lktest/livekit.go +++ b/test/lktest/livekit.go @@ -201,7 +201,7 @@ func (lk *LiveKit) ConnectParticipant(t TB, room, identity string, cb *RoomParti inp := p.mixIn.NewInput() defer inp.Close() - codec, err := opus.Decode(inp, channels, logger.GetLogger()) + codec, err := opus.Decode(inp, channels, logger.NewTestLogger(t)) if err != nil { return } @@ -294,7 +294,7 @@ func (p *Participant) newAudioTrack() (msdk.Writer[msdk.PCM16Sample], error) { return nil, err } ow := msdk.FromSampleWriter[opus.Sample](track, RoomSampleRate, rtp.DefFrameDur) - pw, err := opus.Encode(ow, channels, logger.GetLogger()) + pw, err := opus.Encode(ow, channels, logger.NewTestLogger(p.t)) if err != nil { return nil, err } From 8f36e8f8ee5adf694c4f2a072ef2aae1e46cbd16 Mon Sep 17 00:00:00 2001 From: Alex Date: Fri, 21 Aug 2026 14:23:58 -0700 Subject: [PATCH 2/2] Fix double close of inbound audio writer (#804) --- pkg/sip/media_port.go | 8 +++++++- pkg/sip/outbound.go | 4 ++++ 2 files changed, 11 insertions(+), 1 deletion(-) diff --git a/pkg/sip/media_port.go b/pkg/sip/media_port.go index ff0cc665a..eb46143e8 100644 --- a/pkg/sip/media_port.go +++ b/pkg/sip/media_port.go @@ -405,8 +405,15 @@ type MediaPort interface { GetOutboundDTMFWriter() msdk.WriteCloser[*livekit.SipDTMF] // WriteInboundAudioTo tells port where to write inbound SIP audio. + // + // MediaPort.Close() will propagate to the argument writer. The caller is + // responsible for closing the returned media writer. WriteInboundAudioTo(w msdk.PCM16Writer) msdk.PCM16Writer + // WriteInboundDTMFTo tells port where to write inbound SIP DTMF. + // + // MediaPort.Close() will propagate to the argument writer. The caller is + // responsible for closing the returned media writer. WriteInboundDTMFTo(w msdk.WriteCloser[*livekit.SipDTMF]) msdk.WriteCloser[*livekit.SipDTMF] // If there is no offer, this generates an offer. @@ -735,7 +742,6 @@ func (p *mediaPort) GetOutboundAudioWriter() msdk.PCM16Writer { return p.audioOut } -// WriteInboundAudioTo sets audio writer that will receive decoded PCM from incoming RTP packets. func (p *mediaPort) WriteInboundAudioTo(w msdk.PCM16Writer) msdk.PCM16Writer { return p.audioIn.Swap(w) } diff --git a/pkg/sip/outbound.go b/pkg/sip/outbound.go index 2cf4f36a6..43d27d485 100644 --- a/pkg/sip/outbound.go +++ b/pkg/sip/outbound.go @@ -561,6 +561,10 @@ func (c *outboundCall) connectMedia() { c.log.Warnw("media port has unexpected inbound audio writer", nil) } + // WriteInboundAudioTo takes ownership of c.lkRoomIn, so nil it out to avoid + // double-closing later. + c.lkRoomIn = nil + if old := c.media.WriteInboundDTMFTo(c.lkRoom.GetInboundDTMFWriter()); old != nil { old.Close() c.log.Warnw("media port has unexpected inbound DTMF writer", nil)