Skip to content

Commit

Permalink
Don't reuse transceiver in one round negotiation
Browse files Browse the repository at this point in the history
Should not reuse transceiver (remove & add track)
in one round negotiation, it cause the transceiver
changes ssrc/id without transit to inactive and the
remote peer connection can't fire track close and
OnTrack event.
  • Loading branch information
cnderrauber committed Sep 6, 2024
1 parent 9a71f69 commit bc4bdaf
Show file tree
Hide file tree
Showing 2 changed files with 58 additions and 8 deletions.
22 changes: 15 additions & 7 deletions peerconnection.go
Original file line number Diff line number Diff line change
Expand Up @@ -1118,10 +1118,18 @@ func (pc *PeerConnection) SetRemoteDescription(desc SessionDescription) error {
case direction == RTPTransceiverDirectionRecvonly:
if t.Direction() == RTPTransceiverDirectionSendrecv {
t.setDirection(RTPTransceiverDirectionSendonly)
} else if t.Direction() == RTPTransceiverDirectionRecvonly {
t.setDirection(RTPTransceiverDirectionInactive)
}
case direction == RTPTransceiverDirectionSendrecv:
if t.Direction() == RTPTransceiverDirectionSendonly {
t.setDirection(RTPTransceiverDirectionSendrecv)
} else if t.Direction() == RTPTransceiverDirectionInactive {
t.setDirection(RTPTransceiverDirectionRecvonly)
}
case direction == RTPTransceiverDirectionSendonly:
if t.Direction() == RTPTransceiverDirectionInactive {
t.setDirection(RTPTransceiverDirectionRecvonly)

Check warning on line 1132 in peerconnection.go

View check run for this annotation

Codecov / codecov/patch

peerconnection.go#L1132

Added line #L1132 was not covered by tests
}
}

Expand Down Expand Up @@ -1288,7 +1296,7 @@ func setRTPTransceiverCurrentDirection(answer *SessionDescription, currentTransc
// If a transceiver is created by applying a remote description that has recvonly transceiver,
// it will have no sender. In this case, the transceiver's current direction is set to inactive so
// that the transceiver can be reused by next AddTrack.
if direction == RTPTransceiverDirectionSendonly && t.Sender() == nil {
if !weOffer && direction == RTPTransceiverDirectionSendonly && t.Sender() == nil {
direction = RTPTransceiverDirectionInactive
}

Expand Down Expand Up @@ -1340,28 +1348,28 @@ func (pc *PeerConnection) configureRTPReceivers(isRenegotiation bool, remoteDesc

mid := t.Mid()
receiverNeedsStopped := false
func() {
for _, t := range tracks {
for _, track := range tracks {
func(t *TrackRemote) {
t.mu.Lock()
defer t.mu.Unlock()

if t.rid != "" {
if details := trackDetailsForRID(incomingTracks, mid, t.rid); details != nil {
t.id = details.id
t.streamID = details.streamID
continue
return

Check warning on line 1360 in peerconnection.go

View check run for this annotation

Codecov / codecov/patch

peerconnection.go#L1360

Added line #L1360 was not covered by tests
}
} else if t.ssrc != 0 {
if details := trackDetailsForSSRC(incomingTracks, t.ssrc); details != nil {
t.id = details.id
t.streamID = details.streamID
continue
return
}
}

receiverNeedsStopped = true
}
}()
}(track)
}

if !receiverNeedsStopped {
continue
Expand Down
44 changes: 43 additions & 1 deletion peerconnection_renegotiation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1160,12 +1160,15 @@ func TestPeerConnection_Regegotiation_ReuseTransceiver(t *testing.T) {
t.Fatal(err)
}

vp8Track, err := NewTrackLocalStaticSample(RTPCodecCapability{MimeType: MimeTypeVP8}, "foo", "bar")
vp8Track, err := NewTrackLocalStaticRTP(RTPCodecCapability{MimeType: MimeTypeVP8}, "foo", "bar")
assert.NoError(t, err)
sender, err := pcOffer.AddTrack(vp8Track)
assert.NoError(t, err)
assert.NoError(t, signalPair(pcOffer, pcAnswer))

peerConnectionConnected := untilConnectionState(PeerConnectionStateConnected, pcOffer, pcAnswer)
peerConnectionConnected.Wait()

assert.Equal(t, len(pcOffer.GetTransceivers()), 1)
assert.Equal(t, pcOffer.GetTransceivers()[0].getCurrentDirection(), RTPTransceiverDirectionSendonly)
assert.NoError(t, pcOffer.RemoveTrack(sender))
Expand All @@ -1185,6 +1188,45 @@ func TestPeerConnection_Regegotiation_ReuseTransceiver(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, len(pcOffer.GetTransceivers()), 2)
assert.True(t, sender.rtpTransceiver == pcOffer.GetTransceivers()[0])
assert.NoError(t, signalPair(pcOffer, pcAnswer))

tracksCh := make(chan *TrackRemote, 2)
pcAnswer.OnTrack(func(tr *TrackRemote, _ *RTPReceiver) {
tracksCh <- tr
})

ssrcReuse := sender.GetParameters().Encodings[0].SSRC
for i := 0; i < 10; i++ {
assert.NoError(t, vp8Track.WriteRTP(&rtp.Packet{Header: rtp.Header{Version: 2}, Payload: []byte{0, 1, 2, 3, 4, 5}}))
time.Sleep(20 * time.Millisecond)
}

// shold not reuse tranceiver between two CreateOffer
offer, err := pcOffer.CreateOffer(nil)
assert.NoError(t, err)
assert.NoError(t, pcOffer.RemoveTrack(sender))
assert.NoError(t, pcOffer.SetLocalDescription(offer))
assert.NoError(t, pcAnswer.SetRemoteDescription(offer))
answer, err := pcAnswer.CreateAnswer(nil)
assert.NoError(t, pcAnswer.SetLocalDescription(answer))
assert.NoError(t, err)
assert.NoError(t, pcOffer.SetRemoteDescription(answer))
sender3, err := pcOffer.AddTrack(vp8Track)
ssrcNotReuse := sender3.GetParameters().Encodings[0].SSRC
assert.NoError(t, err)
assert.Equal(t, len(pcOffer.GetTransceivers()), 3)
assert.NoError(t, signalPair(pcOffer, pcAnswer))
assert.True(t, sender3.rtpTransceiver == pcOffer.GetTransceivers()[2])

for i := 0; i < 10; i++ {
assert.NoError(t, vp8Track.WriteRTP(&rtp.Packet{Header: rtp.Header{Version: 2}, Payload: []byte{0, 1, 2, 3, 4, 5}}))
time.Sleep(20 * time.Millisecond)
}

tr1 := <-tracksCh
tr2 := <-tracksCh
assert.Equal(t, tr1.SSRC(), ssrcReuse)
assert.Equal(t, tr2.SSRC(), ssrcNotReuse)

closePairNow(t, pcOffer, pcAnswer)
}
Expand Down

0 comments on commit bc4bdaf

Please sign in to comment.