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(),
           })
       }
   }
   
       ...
   }