room.LocalParticipant.UnpublishTrack(sid) does not completely unpublish a simulcast track

[neilyoung]

Details here. There is a significant behavioural difference, if just one track is unpublished or a simulcast track. On the other hand I haven't found any other unpublish function for simulcast

https://livekit-users.slack.com/archives/C01KVTJH6BX/p1706698776125179

[neilyoung]

Hack which "fixes" it here:

https://livekit-users.slack.com/archives/C01KVTJH6BX/p1706782394301419?thread_ts=1706698776.125179&cid=C01KVTJH6BX

[neilyoung]

Some comments? Solution is available in Slack

[davidzhao]

this is on our backlog. However, if you'd like to submit a PR for it, we'd welcome it.

[neilyoung]

Neither my GO nor my deeper understanding of the purpose of the failing code allows me to do that :)

I just know, that this patch works for both, but I guess, it's. not good enough:

	var err error
	//if localTrack, ok := pub.track.(webrtc.TrackLocal); ok {
		for _, sender := range p.engine.publisher.pc.GetSenders() {
			//if sender.Track() == localTrack {
				err = p.engine.publisher.pc.RemoveTrack(sender)
				//break
			//}
		}
		p.engine.publisher.Negotiate()
	//}

Also - VSCode's debugging support is not sufficient to have a full understanding for what happens here, I'm sorry

[patstar123]

1. The root cause of the issue is that when "LocalParticipant.PublishSimulcastTrack.NewLocalTrackPublication" is called, the main track is not passed, resulting in "UnpublishTrack" not successfully invoking "p.engine.publisher.pc.RemoveTrack". pub := NewLocalTrackPublication(KindFromRTPType(mainTrack.Kind()), nil, *opts, p.engine.client)

2. The minimal change solution is as follows:

        // [origin code in v1.1.2]
	var err error
	if localTrack, ok := pub.track.(webrtc.TrackLocal); ok {
		for _, sender := range p.engine.publisher.pc.GetSenders() {
			if sender.Track() == localTrack {
				err = p.engine.publisher.pc.RemoveTrack(sender)
				break
			}
		}
		p.engine.publisher.Negotiate()
	}



        // [fixed code in v1.1.2]
	var localTracks []webrtc.TrackLocal
	if localTrack, ok := pub.track.(webrtc.TrackLocal); ok {
		localTracks = append(localTracks, localTrack)
	}
	if pub.simulcastTracks != nil {
		for _, track := range pub.simulcastTracks {
			localTracks = append(localTracks, track)
		}
	}

	var err error
	if localTracks != nil && len(localTracks) > 0 {
		for _, localTrack := range localTracks {
			for _, sender := range p.engine.publisher.pc.GetSenders() {
				if sender.Track() == localTrack {
					e := p.engine.publisher.pc.RemoveTrack(sender)
					if e != nil {
						err = e
					}
					break
				}
			}
		}
		p.engine.publisher.Negotiate()
	}

3. I found that when publishing with simulcast tracks, the trackPublicationBase.track was not set. This not only affects the UnpublishTrack function but may also impact the following logic. However, I have not deeply understood it, so I have not made any changes.

func (r *Room) sendSyncState() {
        ...

	var publishedTracks []*livekit.TrackPublishedResponse
	for _, t := range r.LocalParticipant.Tracks() {
		if t.Track() != nil {
			publishedTracks = append(publishedTracks, &livekit.TrackPublishedResponse{
				Cid:   t.Track().ID(),
				Track: t.TrackInfo(),
			})
		}
	}

        ...
}