diff --git a/pkg/sip/inbound.go b/pkg/sip/inbound.go index 0c3f74e5..95fded36 100644 --- a/pkg/sip/inbound.go +++ b/pkg/sip/inbound.go @@ -697,36 +697,35 @@ func (s *Server) onNotify(log *slog.Logger, req *sip.Request, tx sip.ServerTrans } type inboundCall struct { - s *Server - tid traceid.ID - logPtr atomic.Pointer[logger.Logger] - cc *sipInbound - mon *stats.CallMonitor - state *CallState - callStart time.Time - extraAttrs map[string]string - attrsToHdr map[string]string - ctx context.Context - cancel func() - closeReason atomic.Pointer[ReasonHeader] - call *rpc.SIPCall - mmu sync.Mutex - 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 - audioInProcessor msdk.PCM16Processor + s *Server + tid traceid.ID + logPtr atomic.Pointer[logger.Logger] + cc *sipInbound + mon *stats.CallMonitor + state *CallState + callStart time.Time + extraAttrs map[string]string + attrsToHdr map[string]string + ctx context.Context + cancel func() + closeReason atomic.Pointer[ReasonHeader] + call *rpc.SIPCall + mmu sync.Mutex + 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( @@ -911,7 +910,7 @@ func (c *inboundCall) handleInvite(ctx context.Context, tid traceid.ID, req *sip } rawSDP := req.Body() tmedia := c.mon.StageDurTimer("start-media") - answerData, err := c.runMediaConn(tid, rawSDP, m, conf, disp.EnabledFeatures, disp.FeatureFlags) + answerData, err := c.runMediaConn(tid, rawSDP, m, conf, disp.FeatureFlags) tmedia() if err != nil { sipReason := sip.StatusInternalServerError @@ -1036,7 +1035,7 @@ func (c *inboundCall) handleInvite(ctx context.Context, tid traceid.ID, req *sip 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) @@ -1136,7 +1135,7 @@ func (w *dtmfEventWriter) WriteSample(sample *livekit.SipDTMF) error { return nil } -func (c *inboundCall) runMediaConn(tid traceid.ID, offerData []byte, mconf *sipMediaConfig, conf *config.Config, features []livekit.SIPFeature, featureFlags map[string]string) ([]byte, error) { +func (c *inboundCall) runMediaConn(tid traceid.ID, offerData []byte, mconf *sipMediaConfig, conf *config.Config, featureFlags map[string]string) ([]byte, error) { c.mmu.Lock() defer c.mmu.Unlock() c.mon.SDPSize(len(offerData), true) @@ -1174,7 +1173,6 @@ func (c *inboundCall) runMediaConn(tid traceid.ID, offerData []byte, mconf *sipM c.mon.SDPSize(len(answerData), false) c.log().Debugw("SDP answer", "sdp", string(answerData)) - c.audioInProcessor = c.s.handler.GetMediaProcessor(features, featureFlags, string(c.cc.ID()), MediaProcessorOpts{InputSampleRate: RoomSampleRate}) mp.WriteDTMFTo(&dtmfEventWriter{handler: c.handleDTMF}) // Must be set earlier to send the pin prompts. @@ -1192,6 +1190,7 @@ func (c *inboundCall) runMediaConn(tid traceid.ID, offerData []byte, mconf *sipM }) return answerData, nil } + func (c *inboundCall) waitMedia(ctx context.Context) (bool, error) { defer c.mon.StageDurTimer("wait-media")() ctx, span := Tracer.Start(ctx, "sip.inbound.waitMedia") @@ -1606,7 +1605,7 @@ 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) if err != nil { @@ -1614,8 +1613,8 @@ func (c *inboundCall) publishTrack() error { return err } - if c.audioInProcessor != nil { - local = c.audioInProcessor(local) + if audioInProcessor := c.s.handler.GetMediaProcessor(features, featureFlags, string(c.cc.ID()), MediaProcessorOpts{InputSampleRate: RoomSampleRate}); audioInProcessor != nil { + local = audioInProcessor(local) } c.media.WriteAudioTo(local) return nil diff --git a/pkg/sip/media_pipeline.go b/pkg/sip/media_pipeline.go index 7762221d..0c3892fd 100644 --- a/pkg/sip/media_pipeline.go +++ b/pkg/sip/media_pipeline.go @@ -129,9 +129,6 @@ func (p *mediaPortPipeline) init( if p.conf.opts.IgnoreLocalAddrInSDP && mc.Remote.Addr().IsPrivate() { port.SetSymmetric(true) // Already initialized with opts, turn on for edge case } - - // stopDiscarding() must be done ahead of SRTP session creation, or risk dead read without read deadlines - port.stopDiscarding() p.lastDTMFTimestamp.Store(math.MaxUint32) var err error diff --git a/pkg/sip/media_pipeline_test.go b/pkg/sip/media_pipeline_test.go index 848e3205..b8a65835 100644 --- a/pkg/sip/media_pipeline_test.go +++ b/pkg/sip/media_pipeline_test.go @@ -15,12 +15,8 @@ package sip import ( - "context" - "io" - "net" - "net/netip" - "os" - "slices" + "fmt" + "strings" "sync" "sync/atomic" "testing" @@ -31,140 +27,65 @@ import ( "github.com/stretchr/testify/require" msdk "github.com/livekit/media-sdk" + "github.com/livekit/media-sdk/amrwb" "github.com/livekit/media-sdk/dtmf" "github.com/livekit/media-sdk/g711" + "github.com/livekit/media-sdk/g722" + "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 ( - testAudioPT = byte(0) // PCMU - testDTMFPT = byte(101) - testCodecRate = 8000 -) - -// In-memory UDP pipe for pipeline tests (same shape as media_port_test's testUDPConn). -type pipelineUDPConn struct { - addr netip.AddrPort - closed chan struct{} - buf chan []byte - peer atomic.Pointer[pipelineUDPConn] - deadline chan time.Time -} - -func (c *pipelineUDPConn) Read(b []byte) (int, error) { - n, _, err := c.ReadFromUDPAddrPort(b) - return n, err -} - -func (c *pipelineUDPConn) Write(b []byte) (int, error) { - return c.WriteToUDPAddrPort(b, netip.AddrPort{}) -} - -func (c *pipelineUDPConn) RemoteAddr() net.Addr { - p := c.peer.Load() - if p == nil { - return &net.UDPAddr{} - } - return p.LocalAddr() -} +const testDTMFPT = byte(101) -func (c *pipelineUDPConn) SetDeadline(t time.Time) error { - return c.SetReadDeadline(t) -} - -func (c *pipelineUDPConn) SetReadDeadline(t time.Time) error { - select { - case c.deadline <- t: - default: +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 } -func (c *pipelineUDPConn) SetWriteDeadline(time.Time) error { return nil } - -func (c *pipelineUDPConn) ReadFromUDPAddrPort(buf []byte) (int, netip.AddrPort, error) { - peer := c.peer.Load() - if peer == nil { - return 0, netip.AddrPort{}, io.ErrClosedPipe - } - var curDeadline time.Time - for { - var deadlineCh <-chan time.Time - if !curDeadline.IsZero() { - deadlineCh = time.After(time.Until(curDeadline)) - } - select { - case <-c.closed: - 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 +// 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) } - continue - case data := <-c.buf: - n := copy(buf, data) - var err error - if n < len(data) { - err = io.ErrShortBuffer + return d + }, + func(w msdk.WriteCloser[opus.Sample]) msdk.PCM16Writer { + e, err := opus.Encode(w, 1, log) + if err != nil { + panic(err) } - return n, peer.addr, err - } - } -} - -func (c *pipelineUDPConn) WriteToUDPAddrPort(buf []byte, addr netip.AddrPort) (int, error) { - peer := c.peer.Load() - if peer == nil { - return 0, io.ErrClosedPipe - } else if peer.addr.String() != addr.String() { - panic("unexpected address") - } - buf = slices.Clone(buf) - select { - default: - return 0, io.ErrShortWrite - case <-peer.closed: - return 0, io.ErrClosedPipe - case peer.buf <- buf: - return len(buf), nil - } + return e + }, + ) } -func (c *pipelineUDPConn) LocalAddr() net.Addr { - return &net.UDPAddr{ - IP: c.addr.Addr().AsSlice(), - Port: int(c.addr.Port()), +func testAudioPT(c msdk.AudioCodec) byte { + info := c.Info() + if info.RTPIsStatic { + return info.RTPDefType } -} - -func (c *pipelineUDPConn) Close() error { - if c.peer.Swap(nil) != nil { - close(c.closed) - } - return nil -} - -func newPipelineUDPPipe() (c1, c2 *pipelineUDPConn) { - c1 = &pipelineUDPConn{ - addr: netip.AddrPortFrom(netip.AddrFrom4([4]byte{1, 1, 1, 1}), 10000), - buf: make(chan []byte, 256), - closed: make(chan struct{}), - deadline: make(chan time.Time, 1), - } - c2 = &pipelineUDPConn{ - addr: netip.AddrPortFrom(netip.AddrFrom4([4]byte{2, 2, 2, 2}), 20000), - buf: make(chan []byte, 256), - closed: make(chan struct{}), - deadline: make(chan time.Time, 1), - } - c1.peer.Store(c2) - c2.peer.Store(c1) - return c1, c2 + return 96 } type dtmfCollector struct { @@ -193,50 +114,50 @@ func (c *dtmfCollector) snapshot() []*livekit.SipDTMF { return out } +// 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 *pipelineUDPConn - remote *pipelineUDPConn + local *testUDPConn + remote *testUDPConn port *udpConn - audioIn msdk.WriteCloserSwitch[msdk.PCM16Sample] - audioOut msdk.WriteCloserSwitch[msdk.PCM16Sample] - dtmfIn msdk.WriteCloserSwitch[*livekit.SipDTMF] - dtmfOut msdk.WriteCloserSwitch[*livekit.SipDTMF] + conf *MediaPortPipelineConfig + audioIn *msdk.WriteCloserSwitch[msdk.PCM16Sample] + audioOut *msdk.WriteCloserSwitch[msdk.PCM16Sample] + dtmfIn *msdk.WriteCloserSwitch[*livekit.SipDTMF] + dtmfOut *msdk.WriteCloserSwitch[*livekit.SipDTMF] roomAudio *msdk.PCM16Sample roomDTMF *dtmfCollector pipeline *mediaPortPipeline ssrcCount atomic.Uint64 packetCount atomic.Uint64 + codec msdk.AudioCodec audioPT byte dtmfPT byte } -func newPipelineHarness(t *testing.T, dtmfType byte, dtmfAudio bool) *pipelineHarness { +func newPipelineHarness(t *testing.T, sampleRate int) *pipelineHarness { t.Helper() - local, remote := newPipelineUDPPipe() - - log := logger.GetLogger() - port := newUDPConn(log, local, false) + local, remote := newUDPPipe() + log := logger.NewTestLogger(t) h := &pipelineHarness{ t: t, local: local, remote: remote, - port: port, + 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: new(msdk.PCM16Sample), roomDTMF: &dtmfCollector{}, - audioPT: testAudioPT, - dtmfPT: dtmfType, } - - codec, ok := sdp.CodecByName(g711.ULawSDPNameAndRate).(msdk.AudioCodec) - require.True(t, ok) - - h.audioIn.Swap(msdk.NewPCM16BufferWriter(h.roomAudio, RoomSampleRate)) + h.audioIn.Swap(msdk.NewPCM16BufferWriter(h.roomAudio, sampleRate)) h.dtmfIn.Swap(h.roomDTMF) - - pipelineConfig := &MediaPortPipelineConfig{ + h.conf = &MediaPortPipelineConfig{ log: log, - opts: &MediaOptions{DTMFAudio: dtmfAudio}, + opts: &MediaOptions{}, stats: &PortStats{}, onNewSSRC: func() bool { h.ssrcCount.Add(1) @@ -246,32 +167,78 @@ func newPipelineHarness(t *testing.T, dtmfType byte, dtmfAudio bool) *pipelineHa h.packetCount.Add(1) }, } - mc := &sdp.MediaConfig{ - Local: local.addr, - Remote: remote.addr, + 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: codec, - Type: testAudioPT, - DTMFType: dtmfType, + Codec: h.codec, + Type: h.audioPT, + DTMFType: h.dtmfPT, }, } - pipe, err := NewMediaPortPipeline(pipelineConfig, mc, port, &h.audioIn, &h.dtmfIn, RoomSampleRate) - require.NoError(t, err) +} + +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() } - t.Cleanup(func() { - _ = pipe.Close() - _ = local.Close() - _ = remote.Close() - }) - return h +} + +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) { @@ -296,28 +263,148 @@ func (h *pipelineHarness) injectRTP(pkt *rtp.Packet) { func (h *pipelineHarness) injectAudio(ssrc uint32, seq uint16, ts uint32, pcm msdk.PCM16Sample) { h.t.Helper() - var ulaw g711.ULawSample - ulaw.Encode(pcm) + 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: h.audioPT, - SequenceNumber: seq, + PayloadType: pt, + SequenceNumber: 1, Timestamp: ts, SSRC: ssrc, + Marker: true, }, - Payload: []byte(ulaw), + Payload: payload[:n], }) } -func (h *pipelineHarness) injectDTMFDigit(ssrc uint32, digit string, ts uint32) { - h.t.Helper() - var buf msrtp.Buffer - w := msrtp.NewSeqWriter(&buf).NewStream(h.dtmfPT, dtmf.SampleRate) - require.NoError(h.t, dtmf.Write(context.Background(), nil, w, ts, digit)) - for _, pkt := range buf { - pkt.Header.SSRC = ssrc - h.injectRTP(pkt) +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 := len(*h.roomAudio) + 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 len(*h.roomAudio) > 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(), + len(*h.roomAudio), + ) + require.Greater(t, pcmEnergy((*h.roomAudio)[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) } } @@ -345,136 +432,55 @@ func pcmEnergy(s msdk.PCM16Sample) int64 { return sum } -func TestMediaPipelinePermutations(t *testing.T) { - cases := []struct { - name string - dtmfType byte - dtmfAudio bool - }{ - {name: "dtmf_disabled", dtmfType: 0, dtmfAudio: false}, - {name: "dtmf_enabled", dtmfType: testDTMFPT, dtmfAudio: false}, - {name: "dtmf_enabled_with_audio", dtmfType: testDTMFPT, dtmfAudio: true}, - } +type testCodecSpec struct { + name string + sdp string +} - frame := testCodecRate / int(time.Second/msrtp.DefFrameDur) - - for _, tc := range cases { - t.Run(tc.name, func(t *testing.T) { - h := newPipelineHarness(t, tc.dtmfType, tc.dtmfAudio) - - t.Run("audio_to_port", func(t *testing.T) { - sample := tonePCM(testCodecRate, frame, 10000) - require.NoError(t, h.audioOut.WriteSample(sample)) - deadline := time.Now().Add(time.Second) - foundAudio := false - for time.Now().Before(deadline) && !foundAudio { - pkt, ok := h.readRemotePacket(50 * time.Millisecond) - if !ok { - continue - } - if pkt.PayloadType == h.audioPT && len(pkt.Payload) > 0 { - foundAudio = true - } - } - require.True(t, foundAudio, "expected RTP audio toward the peer") - }) - - t.Run("audio_to_room", func(t *testing.T) { - before := len(*h.roomAudio) - sample := tonePCM(testCodecRate, frame, 12000) - for i := uint16(0); i < 5; i++ { - h.injectAudio(0xA11CE, 1+i, 160+uint32(i)*160, sample) - } - require.Eventually(t, func() bool { - return h.packetCount.Load() >= 5 - }, time.Second, 5*time.Millisecond, "RTP should be accepted") - require.Eventually(t, func() bool { - return len(*h.roomAudio) > 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(), - len(*h.roomAudio), - ) - got := (*h.roomAudio)[before:] - require.Greater(t, pcmEnergy(got), int64(0), "decoded room audio should carry energy") - }) - - t.Run("dtmf_to_port", func(t *testing.T) { - // Drain while DTMF write runs (in-band audio can emit many RTP frames). - var ( - mu sync.Mutex - dtmfPkts int - stop = make(chan struct{}) - wg sync.WaitGroup - ) - wg.Add(1) - go func() { - defer wg.Done() - for { - select { - case <-stop: - return - default: - } - pkt, ok := h.readRemotePacket(20 * time.Millisecond) - if !ok { - continue - } - if tc.dtmfType != 0 && pkt.PayloadType == tc.dtmfType { - mu.Lock() - dtmfPkts++ - mu.Unlock() - } - } - }() - - err := h.dtmfOut.WriteSample(&livekit.SipDTMF{Digit: "5", Code: 5}) - require.NoError(t, err, "DTMF write must not error when disabled or enabled") - time.Sleep(50 * time.Millisecond) // allow final packets to flush - close(stop) - wg.Wait() - - mu.Lock() - n := dtmfPkts - mu.Unlock() - if tc.dtmfType == 0 { - require.Zero(t, n, "DTMF disabled: no telephone-event RTP") - } else { - require.NotZero(t, n, "DTMF enabled: expected telephone-event RTP") - } - }) - - t.Run("dtmf_to_room", func(t *testing.T) { - before := len(h.roomDTMF.snapshot()) - if tc.dtmfType == 0 { - // Inject telephone-event anyway; mux should drop without error. - h.dtmfPT = testDTMFPT - h.injectDTMFDigit(0xD7DF, "7", 8000) - h.dtmfPT = 0 - time.Sleep(50 * time.Millisecond) - require.Equal(t, before, len(h.roomDTMF.snapshot()), "DTMF disabled: must not reach room") - return - } +type testDTMFSpec struct { + name string + pt byte + audio bool +} + +var ( + pipelineTestCodecs = []testCodecSpec{ + {name: "PCMU", sdp: g711.ULawSDPNameAndRate}, + {name: "PCMA", sdp: g711.ALawSDPNameAndRate}, + {name: "G722", sdp: g722.SDPNameAndRate}, + {name: "AMR-WB", sdp: amrwb.SDPNameAndRate}, + } + 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}, + } +) - h.injectDTMFDigit(0xD7DF, "7", 8000) - 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 TestMediaPipelinePermutations(t *testing.T) { + for _, spec := range pipelineTestCodecs { + t.Run(spec.name, func(t *testing.T) { + codec := audioCodecByName(t, spec.sdp) + 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) { - h := newPipelineHarness(t, testDTMFPT, false) - frame := testCodecRate / int(time.Second/msrtp.DefFrameDur) - sample := tonePCM(testCodecRate, frame, 8000) + 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) @@ -482,7 +488,7 @@ func TestMediaPipelineTeardownMultiSSRC(t *testing.T) { require.Eventually(t, func() bool { return h.ssrcCount.Load() >= 2 }, time.Second, 5*time.Millisecond, "expected AcceptStream for two SSRCs") - assert.GreaterOrEqual(t, h.packetCount.Load(), uint64(2)) + assert.Equal(t, h.packetCount.Load(), uint64(2)) done := make(chan error, 1) go func() { @@ -497,77 +503,22 @@ func TestMediaPipelineTeardownMultiSSRC(t *testing.T) { } func TestMediaPipelineReuseUDPConn(t *testing.T) { - local, remote := newPipelineUDPPipe() - log := logger.GetLogger() - port := newUDPConn(log, local, false) - - codec, ok := sdp.CodecByName(g711.ULawSDPNameAndRate).(msdk.AudioCodec) - require.True(t, ok) - frame := testCodecRate / int(time.Second/msrtp.DefFrameDur) - sample := tonePCM(testCodecRate, frame, 9000) - - build := func(t *testing.T) (*mediaPortPipeline, *msdk.WriteCloserSwitch[msdk.PCM16Sample]) { - t.Helper() - var audioIn msdk.WriteCloserSwitch[msdk.PCM16Sample] - var audioOut msdk.WriteCloserSwitch[msdk.PCM16Sample] - var dtmfIn msdk.WriteCloserSwitch[*livekit.SipDTMF] - roomBuf := new(msdk.PCM16Sample) - audioIn.Swap(msdk.NewPCM16BufferWriter(roomBuf, RoomSampleRate)) - - pipelineConfig := &MediaPortPipelineConfig{ - log: log, - opts: &MediaOptions{}, - stats: &PortStats{}, - } - mc := &sdp.MediaConfig{ - Local: local.addr, - Remote: remote.addr, - Audio: sdp.AudioConfig{ - Codec: codec, - Type: testAudioPT, - DTMFType: testDTMFPT, - }, - } - pipe, err := NewMediaPortPipeline(pipelineConfig, mc, port, &audioIn, &dtmfIn, RoomSampleRate) - require.NoError(t, err) - audioToPort, dtmfToPort := pipe.GetConnectors() - _ = audioOut.Swap(audioToPort) - _ = dtmfToPort // unused in this test - return pipe, &audioOut - } - - // Generation 1 - pipe1, out1 := build(t) - require.NoError(t, out1.WriteSample(sample)) - select { - case raw := <-remote.buf: - var pkt rtp.Packet - require.NoError(t, pkt.Unmarshal(raw)) - assert.Equal(t, testAudioPT, pkt.PayloadType) - case <-time.After(time.Second): - t.Fatal("first pipeline produced no RTP") - } - require.NoError(t, pipe1.Close()) - if w := out1.Swap(nil); w != nil { - _ = w.Close() - } - - // Soft-closed port must be reopened before the next session. - port.Reopen() - - // Generation 2 on the same udpConn / test pipe - pipe2, out2 := build(t) - require.NoError(t, out2.WriteSample(sample)) - select { - case raw := <-remote.buf: - var pkt rtp.Packet - require.NoError(t, pkt.Unmarshal(raw)) - assert.Equal(t, testAudioPT, pkt.PayloadType) - case <-time.After(time.Second): - t.Fatal("second pipeline produced no RTP after Reopen") + const rate = 48000 + d := pipelineTestDTMF[1] // event-only + + for _, from := range pipelineTestCodecs { + t.Run("from_"+from.name, func(t *testing.T) { + for _, to := range pipelineTestCodecs { + t.Run("to_"+to.name, func(t *testing.T) { + c1 := audioCodecByName(t, from.sdp) + c2 := audioCodecByName(t, to.sdp) + 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) + }) + } + }) } - require.NoError(t, pipe2.Close()) - - _ = local.Close() - _ = remote.Close() } diff --git a/pkg/sip/media_port.go b/pkg/sip/media_port.go index c0681c8a..64c6dee9 100644 --- a/pkg/sip/media_port.go +++ b/pkg/sip/media_port.go @@ -35,6 +35,7 @@ import ( "github.com/livekit/protocol/livekit" "github.com/livekit/protocol/logger" + "github.com/livekit/sip/pkg/config" "github.com/livekit/sip/pkg/stats" ) @@ -381,6 +382,19 @@ func (o *MediaOptions) ApplyDefaults() { 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 + } +} + +type MediaSegment interface { + GetAudioWriter() msdk.PCM16Writer + WriteAudioTo(w msdk.PCM16Writer) msdk.PCM16Writer + GetDTMFWriter() msdk.WriteCloser[*livekit.SipDTMF] + WriteDTMFTo(w msdk.WriteCloser[*livekit.SipDTMF]) msdk.WriteCloser[*livekit.SipDTMF] } // MediaPort is the insulated media-plane API: UDP/RTP to the wire, SDP negotiation, @@ -389,10 +403,7 @@ type MediaPort interface { Close() CloseWait() - GetAudioWriter() msdk.PCM16Writer // To Port - WriteAudioTo(w msdk.PCM16Writer) msdk.PCM16Writer // From Port - GetDTMFWriter() msdk.WriteCloser[*livekit.SipDTMF] // To Port - WriteDTMFTo(w msdk.WriteCloser[*livekit.SipDTMF]) msdk.WriteCloser[*livekit.SipDTMF] // From Port + MediaSegment // If there is no offer, this generates an offer. // If there is an offer, this simply returns the SDP of that offer. @@ -475,7 +486,7 @@ func NewMediaPortWith(log logger.Logger, mon *stats.CallMonitor, conn UDPConn, o p.port.startDiscarding() p.timeoutInitial.Store(&opts.MediaTimeoutInitial) p.timeoutGeneral.Store(&opts.MediaTimeout) - go p.timeoutLoop() + p.wg.Go(p.mediaTimeoutLoop) p.log.Debugw("listening for media on UDP", "port", p.Port()) return p, nil } @@ -483,6 +494,7 @@ func NewMediaPortWith(log logger.Logger, mon *stats.CallMonitor, conn UDPConn, o // mediaPort is the concrete MediaPort implementation. type mediaPort struct { log logger.Logger + wg sync.WaitGroup opts *MediaOptions mon *stats.CallMonitor externalIP netip.Addr @@ -551,7 +563,7 @@ func (p *mediaPort) SetTimeoutForDelayedAck(initial, general time.Duration) { p.enableTimeout(initial, general) } -func (p *mediaPort) timeoutLoop() { +func (p *mediaPort) mediaTimeoutLoop() { defer p.log.Infow("media timeout loop stopped") const disabledPark = time.Hour @@ -683,12 +695,17 @@ func (p *mediaPort) Close() { } else { _ = conn.Close() } + p.audioIn.Close() // Propagate Close() to onwards to room + p.dtmfIn.Close() // Propagate Close() to onwards to room + p.audioOut.Close() // Pipeline insulated, but close switch + p.dtmfOut.Close() // Pipeline insulated, but close switch }) } func (p *mediaPort) CloseWait() { p.Close() <-p.closed.Watch() + p.wg.Wait() } func (p *mediaPort) Port() int { @@ -771,12 +788,10 @@ func (p *mediaPort) GenerateAnswer(offerData []byte) ([]byte, error) { if err != nil { return nil, SDPError{Err: err} } - answerData, err := answer.SDP.Marshal() if err != nil { return nil, err } - return answerData, p.configure(mc, answerData) } @@ -851,7 +866,8 @@ func (p *mediaPort) configure(c *sdp.MediaConfig, localSDP []byte) error { // TODO: Avoid reconfiguring if mc unchanged; maybe only adjust direction p.closePipelineLocked() - p.port.Reopen() // Allow reads from socket again + 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, diff --git a/pkg/sip/media_port_negotiation_test.go b/pkg/sip/media_port_negotiation_test.go index e0530eff..abdda6cf 100644 --- a/pkg/sip/media_port_negotiation_test.go +++ b/pkg/sip/media_port_negotiation_test.go @@ -115,7 +115,7 @@ func answerCodec(t testing.TB, answerData []byte) string { // 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.GetLogger(), nil, &MediaOptions{ + return newTestPort(t, logger.NewTestLogger(t), newTestConn(1), &MediaOptions{ IP: newIP("127.0.0.1"), Codecs: testCodecSet(names...), }, RoomSampleRate) @@ -164,8 +164,9 @@ func TestMediaPortCodecSet(t *testing.T) { // 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) + m1, m2 := newMediaPair(t, nil, nil, "") recv2 := &recvBuffer{} m2.WriteAudioTo(recv2) @@ -186,7 +187,7 @@ func TestMediaPortRenegotiation(t *testing.T) { t.Run("codec change", func(t *testing.T) { c1, c2 := newUDPPipe() - log := logger.GetLogger() + log := logger.NewTestLogger(t) m1 := newTestPort(t, log.WithName("one"), c1, &MediaOptions{ IP: newIP("1.1.1.1"), @@ -242,7 +243,7 @@ func TestMediaPortHold(t *testing.T) { } for _, tc := range cases { t.Run(tc.name, func(t *testing.T) { - m1, m2 := newMediaPair(t, nil, nil) + m1, m2 := newMediaPair(t, nil, nil, "") recv1 := &recvBuffer{} m1.WriteAudioTo(recv1) diff --git a/pkg/sip/media_port_pending_test.go b/pkg/sip/media_port_pending_test.go index e9f59ebf..50c006ea 100644 --- a/pkg/sip/media_port_pending_test.go +++ b/pkg/sip/media_port_pending_test.go @@ -17,7 +17,6 @@ // Tests parked by the mediaPort/mediaPortPipeline split, kept verbatim for review. // Each one still references API that the refactor removed: // -// PrintAudioInWriter mediaPort.audioInHandler -> mediaPortPipeline // TestMediaPortUpdateRemote MediaPort.UpdateRemote (re-INVITE now goes through GenerateAnswer) // TestMediaPort NewOffer/SetOffer/SetAnswer/SetConfig/Config, MediaOptions.NoInputResample. // The expected pipeline chain strings also predate the always-on @@ -51,37 +50,42 @@ import ( "github.com/livekit/protocol/logger" ) -func PrintAudioInWriter(p *MediaPort) string { - return p.audioInHandler.(fmt.Stringer).String() +func PrintAudioInWriter(p *mediaPort) string { + return p.pipeline.audioToRoom.String() } func TestMediaPortUpdateRemote(t *testing.T) { - log := logger.GetLogger() + log := logger.NewTestLogger(t) mon := newTestCallMonitor(t) // newUDPPipe wires two in-memory testUDPConn together. c1, _ := newUDPPipe() - mp, err := NewMediaPortWith(1, log, mon, c1, &MediaOptions{ + mp, err := NewMediaPortWith(log, mon, c1, &MediaOptions{ IP: netip.MustParseAddr("127.0.0.1"), }, 8000) 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") + require.False(t, getMediaPortRemoteAddr(t, mp).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") + require.Equal(t, addr, getMediaPortRemoteAddr(t, mp), "RemoteAddr should reflect the updated address") // 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") + require.Equal(t, addr, getMediaPortRemoteAddr(t, mp), "UpdateRemote with invalid addr should not change RemoteAddr") // 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") + require.Equal(t, addr, getMediaPortRemoteAddr(t, mp), "UpdateRemote with unspecified addr should not change RemoteAddr") + + // Test successful updte to new address + addr = netip.MustParseAddrPort("10.10.10.10:54321") + mp.UpdateRemote(addr) + require.Equal(t, addr, getMediaPortRemoteAddr(t, mp), "UpdateRemote with new address should change RemoteAddr") } func TestMediaPort(t *testing.T) { @@ -129,7 +133,7 @@ func TestMediaPort(t *testing.T) { t.Run(fmt.Sprintf("%d%s", tconf.Rate, suff), func(t *testing.T) { c1, c2 := newUDPPipe() - log := logger.GetLogger() + log := logger.NewTestLogger(t) const ( ip1 = "1.1.1.1" @@ -408,7 +412,7 @@ func TestMediaPortDTMF(t *testing.T) { 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 := &mediaPort{} p.lastDTMFTimestamp.Store(math.MaxUint32) got := "" p.HandleDTMF(func(ev dtmf.Event) { diff --git a/pkg/sip/media_port_test.go b/pkg/sip/media_port_test.go index 97d68d0b..0317fc80 100644 --- a/pkg/sip/media_port_test.go +++ b/pkg/sip/media_port_test.go @@ -53,7 +53,7 @@ func newTestMediaPort(t testing.TB, provider string) MediaPort { t.Helper() mon := newTestCallMonitor(t) mon.SetProvider(provider) - mp, err := NewMediaPortWith(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) @@ -174,7 +174,7 @@ 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), + buf: make(chan []byte, 256), closed: make(chan struct{}), deadline: make(chan time.Time, 1), } @@ -206,6 +206,45 @@ func newTestPort(t testing.TB, log logger.Logger, conn UDPConn, opts *MediaOptio return mp.(*mediaPort) } +func offerAt(t testing.TB, addr netip.AddrPort) []byte { + t.Helper() + offer, err := sdp.NewOfferWith(defaultCodecs, addr.Addr(), int(addr.Port()), sdp.EncryptionNone) + require.NoError(t, err) + data, err := offer.SDP.Marshal() + require.NoError(t, err) + return data +} + +func TestMediaPortUpdateRemote(t *testing.T) { + c1, _ := newUDPPipe() + mp := newTestPort(t, logger.NewTestLogger(t), c1, &MediaOptions{ + IP: netip.MustParseAddr("127.0.0.1"), + }, RoomSampleRate) + + require.False(t, mp.RemoteAddr().IsValid(), "RemoteAddr should be invalid before any offer") + + 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") + + // 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") + + // 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") + + // 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") +} + // 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() @@ -219,11 +258,11 @@ func negotiate(t testing.TB, m1, m2 *mediaPort) []byte { return answerData } -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 newMediaPair(t testing.TB, opt1, opt2 *MediaOptions, codec string) (m1, m2 *mediaPort) { + return newMediaPairWithAddr(t, newIP("1.1.1.1"), newIP("2.2.2.2"), opt1, opt2, codec) } -func newMediaPairWithAddr(t testing.TB, ip1, ip2 netip.Addr, opt1, opt2 *MediaOptions) (m1, m2 *mediaPort) { +func newMediaPairWithAddr(t testing.TB, ip1, ip2 netip.Addr, opt1, opt2 *MediaOptions, codec string) (m1, m2 *mediaPort) { if opt1 == nil { opt1 = &MediaOptions{} } @@ -234,19 +273,27 @@ func newMediaPairWithAddr(t testing.TB, ip1, ip2 netip.Addr, opt1, opt2 *MediaOp opt1.IP = ip1 opt1.Ports = rtcconfig.PortRange{Start: 10000} + rate1 := RoomSampleRate + if codec != "" { + opt1.Codecs = testCodecSet(codec) + rate1 = opt1.Codecs.ListEnabled()[0].Info().SampleRate + } // TODO(port-refactor): MediaOptions.NoInputResample is gone, the pipeline always // resamples the receive side to RoomSampleRate. // opt1.NoInputResample = true opt2.IP = ip2 opt2.Ports = rtcconfig.PortRange{Start: 20000} + rate2 := RoomSampleRate + if codec != "" { + opt2.Codecs = testCodecSet(codec) + rate2 = opt2.Codecs.ListEnabled()[0].Info().SampleRate + } - const rate = 16000 - - log := logger.GetLogger() + log := logger.NewTestLogger(t) - m1 = newTestPort(t, log.WithName("one"), c1, opt1, rate) - m2 = newTestPort(t, log.WithName("two"), c2, opt2, rate) + m1 = newTestPort(t, log.WithName("one"), c1, opt1, rate1) + m2 = newTestPort(t, log.WithName("two"), c2, opt2, rate2) negotiate(t, m1, m2) @@ -260,6 +307,7 @@ func newMediaPairWithAddr(t testing.TB, ip1, ip2 netip.Addr, opt1, opt2 *MediaOp func TestMediaTimeout(t *testing.T) { const ( + codec = "G722/8000" timeout = time.Second / 4 initial = timeout * 2 dt = timeout / 4 @@ -269,9 +317,7 @@ func TestMediaTimeout(t *testing.T) { m1, _ := newMediaPair(t, &MediaOptions{ MediaTimeoutInitial: initial, MediaTimeout: timeout, - }, nil) - - m1.EnableTimeout(true) + }, nil, codec) targ := time.Now().Add(initial) select { @@ -291,8 +337,7 @@ func TestMediaTimeout(t *testing.T) { m1, m2 := newMediaPair(t, &MediaOptions{ MediaTimeoutInitial: initial, MediaTimeout: timeout, - }, nil) - m1.EnableTimeout(true) + }, nil, codec) w2 := m2.GetAudioWriter() err := w2.WriteSample(msdk.PCM16Sample{0, 0}) @@ -315,8 +360,7 @@ func TestMediaTimeout(t *testing.T) { m1, m2 := newMediaPair(t, &MediaOptions{ MediaTimeoutInitial: initial, MediaTimeout: timeout, - }, nil) - m1.EnableTimeout(true) + }, nil, codec) w2 := m2.GetAudioWriter() @@ -336,8 +380,7 @@ func TestMediaTimeout(t *testing.T) { m1, m2 := newMediaPair(t, &MediaOptions{ MediaTimeoutInitial: initial, MediaTimeout: timeout, - }, nil) - m1.EnableTimeout(true) + }, nil, codec) w2 := m2.GetAudioWriter() @@ -356,7 +399,7 @@ func TestMediaTimeout(t *testing.T) { // the general timeout applies relative to the last received RTP packet. // Last packet arrived at most timeout/2 ago, so the timeout should fire // within ~timeout from now, well before initial would elapse. - m1.SetTimeout(initial, timeout) + m1.SetTimeoutForDelayedAck(initial, timeout) select { case <-time.After(timeout + dt): @@ -369,14 +412,13 @@ func TestMediaTimeout(t *testing.T) { m1, _ := newMediaPair(t, &MediaOptions{ MediaTimeoutInitial: initial, MediaTimeout: timeout, - }, nil) - m1.EnableTimeout(true) + }, nil, codec) // 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 // the moment of the SetTimeout call. time.Sleep(initial / 2) - m1.SetTimeout(initial, timeout) + m1.SetTimeoutForDelayedAck(initial, timeout) targ := time.Now().Add(initial) select { @@ -396,8 +438,7 @@ func TestMediaTimeout(t *testing.T) { m1, m2 := newMediaPair(t, &MediaOptions{ MediaTimeoutInitial: initial, MediaTimeout: timeout, - }, nil) - m1.EnableTimeout(true) + }, nil, codec) w2 := m2.GetAudioWriter() @@ -412,8 +453,6 @@ func TestMediaTimeout(t *testing.T) { } } - m1.SetTimeout(initial, timeout) - for i := 0; i < 5; i++ { err := w2.WriteSample(msdk.PCM16Sample{0, 0}) require.NoError(t, err) @@ -428,8 +467,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) dstPtr := m1.port.dst.Load() require.NotNil(t, dstPtr) dst := *dstPtr @@ -454,7 +495,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) dstPtr := m1.port.dst.Load() require.NotNil(t, dstPtr) require.True(t, dstPtr.IsValid()) @@ -481,6 +522,7 @@ func TestSymmetricRTP(t *testing.T) { m1, m2 := newMediaPairWithAddr(t, newIP("1.1.1.1"), newIP("10.10.10.10"), &MediaOptions{IgnoreLocalAddrInSDP: true}, nil, + codec, ) dstPtr := m1.port.dst.Load() require.NotNil(t, dstPtr) diff --git a/pkg/sip/outbound_utilities_test.go b/pkg/sip/outbound_utilities_test.go index 78550807..999ff086 100644 --- a/pkg/sip/outbound_utilities_test.go +++ b/pkg/sip/outbound_utilities_test.go @@ -33,7 +33,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" @@ -216,7 +215,7 @@ func (r *testRoom) CloseOutput() error { return r.room.CloseOutput() } -func (r *testRoom) SetDTMFOutput(w dtmf.Writer) { +func (r *testRoom) SetDTMFOutput(w msdk.WriteCloser[*livekit.SipDTMF]) { r.room.SetDTMFOutput(w) } diff --git a/pkg/sip/signal_logger_test.go b/pkg/sip/signal_logger_test.go index 84f6d027..45714519 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 8744f46f..8bc9531c 100644 --- a/pkg/sip/signaling_test.go +++ b/pkg/sip/signaling_test.go @@ -637,6 +637,21 @@ 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 +} + func TestReinvite(t *testing.T) { t.Run("inbound", func(t *testing.T) { t.Run("normal", func(t *testing.T) { @@ -663,7 +678,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) { @@ -691,21 +706,21 @@ 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("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 := getMediaPortRemoteAddr(t, oc.media) require.NotEqual(t, call.localSDP, serverLocalSDP, "local and remote SDP should be different") // Re-INVITE @@ -727,27 +742,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) diff --git a/pkg/sip/silence_filler_test.go b/pkg/sip/silence_filler_test.go index 371217a7..1c45ba5a 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)