From 3ecd03fa3156372de77b96a07c59d354f75913a4 Mon Sep 17 00:00:00 2001 From: cnderrauber Date: Thu, 19 Sep 2024 01:17:12 +0000 Subject: [PATCH] Use new track id for republishing (#3020) * Use new track id for republishing Always generates new track id for track publishing to make the behavior more consistent. A republishing of clien track will be considered as a new track publication and always trigger trackUnpublished & trackPublished event on subscriber side. * remove test --- pkg/rtc/participant.go | 37 ++------------- pkg/rtc/participant_internal_test.go | 71 ---------------------------- 2 files changed, 3 insertions(+), 105 deletions(-) diff --git a/pkg/rtc/participant.go b/pkg/rtc/participant.go index acd59cb45..9286cd96e 100644 --- a/pkg/rtc/participant.go +++ b/pkg/rtc/participant.go @@ -207,9 +207,6 @@ type ParticipantImpl struct { icQueue [2]atomic.Pointer[webrtc.ICECandidate] - // keeps track of unpublished tracks in order to reuse trackID - unpublishedTracks []*livekit.TrackInfo - requireBroadcast bool // queued participant updates before join response is sent // guarded by updateLock @@ -1913,7 +1910,7 @@ func (p *ParticipantImpl) addPendingTrackLocked(req *livekit.AddTrackRequest) *l if ti.Stream == "" { ti.Stream = StreamFromTrackSource(ti.Source) } - p.setStableTrackID(req.Cid, ti) + p.setTrackID(req.Cid, ti) if len(req.SimulcastCodecs) == 0 { if req.Type == livekit.TrackType_VIDEO { @@ -2268,8 +2265,6 @@ func (p *ParticipantImpl) addMediaTrack(signalCid string, sdpCid string, ti *liv if pti := p.pendingTracks[signalCid]; pti != nil { p.sendTrackPublished(signalCid, pti.trackInfos[0]) pti.queued = false - } else { - p.unpublishedTracks = append(p.unpublishedTracks, ti) } p.pendingTracksLock.Unlock() p.handlePendingRemoteTracks() @@ -2368,9 +2363,8 @@ func (p *ParticipantImpl) getPendingTrack(clientId string, kind livekit.TrackTyp return signalCid, pendingInfo.trackInfos[0], pendingInfo.migrated, pendingInfo.createdAt } -// setStableTrackID either generates a new TrackID or reuses a previously used one -// for -func (p *ParticipantImpl) setStableTrackID(cid string, info *livekit.TrackInfo) { +// setTrackID either generates a new TrackID for an AddTrackRequest +func (p *ParticipantImpl) setTrackID(cid string, info *livekit.TrackInfo) { var trackID string // if already pending, use the same SID // should not happen as this means multiple `AddTrack` requests have been called, but check anyway @@ -2378,31 +2372,6 @@ func (p *ParticipantImpl) setStableTrackID(cid string, info *livekit.TrackInfo) trackID = pti.trackInfos[0].Sid } - // check against published tracks as re-publish could be happening - if trackID == "" { - if pt := p.getPublishedTrackBySignalCid(cid); pt != nil { - ti := pt.ToProto() - if ti.Type == info.Type && ti.Source == info.Source && ti.Name == info.Name { - trackID = ti.Sid - } - } - } - - if trackID == "" { - // check a previously published matching track - for i, ti := range p.unpublishedTracks { - if ti.Type == info.Type && ti.Source == info.Source && ti.Name == info.Name { - trackID = ti.Sid - if i < len(p.unpublishedTracks)-1 { - p.unpublishedTracks = append(p.unpublishedTracks[:i], p.unpublishedTracks[i+1:]...) - } else { - p.unpublishedTracks = p.unpublishedTracks[:i] - } - break - } - } - } - // otherwise generate if trackID == "" { trackPrefix := utils.TrackPrefix diff --git a/pkg/rtc/participant_internal_test.go b/pkg/rtc/participant_internal_test.go index 74fcc6090..660897039 100644 --- a/pkg/rtc/participant_internal_test.go +++ b/pkg/rtc/participant_internal_test.go @@ -332,77 +332,6 @@ func TestSubscriberAsPrimary(t *testing.T) { }) } -func TestSetStableTrackID(t *testing.T) { - testCases := []struct { - name string - trackInfo *livekit.TrackInfo - unpublished []*livekit.TrackInfo - cid string - prefix string - remainingUnpublished int - }{ - { - name: "first track, generates new ID", - trackInfo: &livekit.TrackInfo{ - Type: livekit.TrackType_VIDEO, - Source: livekit.TrackSource_CAMERA, - }, - prefix: "TR_VC", - }, - { - name: "re-using existing ID", - trackInfo: &livekit.TrackInfo{ - Type: livekit.TrackType_VIDEO, - Source: livekit.TrackSource_CAMERA, - }, - unpublished: []*livekit.TrackInfo{ - { - Type: livekit.TrackType_VIDEO, - Source: livekit.TrackSource_SCREEN_SHARE, - Sid: "TR_VC1234", - }, - { - Type: livekit.TrackType_VIDEO, - Source: livekit.TrackSource_CAMERA, - Sid: "TR_VC1235", - }, - }, - cid: "TR_VC1235", - prefix: "TR_VC1235", - remainingUnpublished: 1, - }, - { - name: "mismatch name for reuse", - trackInfo: &livekit.TrackInfo{ - Type: livekit.TrackType_VIDEO, - Source: livekit.TrackSource_CAMERA, - Name: "new_name", - }, - unpublished: []*livekit.TrackInfo{ - { - Type: livekit.TrackType_VIDEO, - Source: livekit.TrackSource_CAMERA, - Sid: "TR_NotUsed", - }, - }, - prefix: "TR_VC", - remainingUnpublished: 1, - }, - } - - for _, tc := range testCases { - t.Run(tc.name, func(t *testing.T) { - p := newParticipantForTest("test") - p.unpublishedTracks = tc.unpublished - - ti := tc.trackInfo - p.setStableTrackID(tc.cid, ti) - require.Contains(t, ti.Sid, tc.prefix) - require.Len(t, p.unpublishedTracks, tc.remainingUnpublished) - }) - } -} - func TestDisableCodecs(t *testing.T) { participant := newParticipantForTestWithOpts("123", &participantOpts{ publisher: false,