Skip to content

Commit

Permalink
Fix error handling in RTPReceiver.Receive
Browse files Browse the repository at this point in the history
If we failed to startReceive we would still make the Receiver as ready
to start reading.
    
Fixes #2929
  • Loading branch information
LeeTeng2001 authored Feb 12, 2025
1 parent 1c45355 commit 306dc37
Show file tree
Hide file tree
Showing 3 changed files with 50 additions and 5 deletions.
9 changes: 6 additions & 3 deletions peerconnection_renegotiation_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1127,10 +1127,13 @@ func TestPeerConnection_Renegotiation_Simulcast(t *testing.T) {
defer trackMapLock.Unlock()

for _, track := range trackMap {
_, _, err := track.ReadRTP() // Ignore first Read, this is our peeked data
assert.Nil(t, err)
_, _, err := track.ReadRTP()

// Ignore first Read, this was our peeked data
if err == nil {
_, _, err = track.ReadRTP()
}

_, _, err = track.ReadRTP()
assert.Equal(t, err, io.EOF)
}
}
Expand Down
10 changes: 8 additions & 2 deletions rtpreceiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -193,7 +193,6 @@ func (r *RTPReceiver) startReceive(parameters RTPReceiveParameters) error { //no
return errRTPReceiverReceiveAlreadyCalled
default:
}
defer close(r.received)

globalParams := r.getParameters()
codec := RTPCodecCapability{}
Expand Down Expand Up @@ -257,6 +256,8 @@ func (r *RTPReceiver) startReceive(parameters RTPReceiveParameters) error { //no
}
}

close(r.received)

return nil
}

Expand Down Expand Up @@ -404,7 +405,12 @@ func (r *RTPReceiver) streamsForTrack(t *TrackRemote) *trackStreams {

// readRTP should only be called by a track, this only exists so we can keep state in one place.
func (r *RTPReceiver) readRTP(b []byte, reader *TrackRemote) (n int, a interceptor.Attributes, err error) {
<-r.received
select {
case <-r.received:
case <-r.closed:
return 0, nil, io.EOF
}

if t := r.streamsForTrack(reader); t != nil {
return t.rtpInterceptor.Read(b, a)
}
Expand Down
36 changes: 36 additions & 0 deletions rtpreceiver_go_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ package webrtc

import (
"context"
"io"
"testing"
"time"

Expand Down Expand Up @@ -74,3 +75,38 @@ func TestSetRTPParameters(t *testing.T) {
assert.NoError(t, wan.Stop())
closePairNow(t, sender, receiver)
}

func TestReceiveError(t *testing.T) {
api := NewAPI()

dtlsTransport, err := api.NewDTLSTransport(nil, nil)
assert.NoError(t, err)

rtpReceiver, err := api.NewRTPReceiver(RTPCodecTypeVideo, dtlsTransport)
assert.NoError(t, err)

rtpParameters := RTPReceiveParameters{
Encodings: []RTPDecodingParameters{
{
RTPCodingParameters: RTPCodingParameters{
SSRC: 1000,
},
},
},
}

assert.Error(t, rtpReceiver.Receive(rtpParameters))

chanErrs := make(chan error)
go func() {
_, _, chanErr := rtpReceiver.Read(nil)
chanErrs <- chanErr

_, _, chanErr = rtpReceiver.Track().ReadRTP()
chanErrs <- chanErr
}()

assert.NoError(t, rtpReceiver.Stop())
assert.Error(t, io.ErrClosedPipe, <-chanErrs)
assert.Error(t, io.ErrClosedPipe, <-chanErrs)
}

0 comments on commit 306dc37

Please sign in to comment.