Skip to content
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 14 additions & 1 deletion peerconnection.go
Original file line number Diff line number Diff line change
Expand Up @@ -1784,7 +1784,20 @@
return err
}

// try to read simulcast IDs from the packet we already have
var mid, rid, rsid string
if _, err = handleUnknownRTPPacket(
b[:i], uint8(midExtensionID), //nolint:gosec // G115
uint8(streamIDExtensionID), //nolint:gosec // G115
uint8(repairStreamIDExtensionID), //nolint:gosec // G115
&mid,
&rid,
&rsid,
); err != nil {
return err
}

Check warning on line 1798 in peerconnection.go

View check run for this annotation

Codecov / codecov/patch

peerconnection.go#L1797-L1798

Added lines #L1797 - L1798 were not covered by tests

// if the first packet didn't contain simuilcast IDs, then probe more packets
var paddingOnly bool
for readCount := 0; readCount <= simulcastProbeCount; readCount++ {
if mid == "" || (rid == "" && rsid == "") {
Expand All @@ -1798,7 +1811,7 @@
return err
}

if _, paddingOnly, err = handleUnknownRTPPacket(
if paddingOnly, err = handleUnknownRTPPacket(
Copy link
Author

@kvasilye kvasilye Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint complained that handleUnknownRTPPacket had a return value which the callers did not consume. Fixed.

b[:i], uint8(midExtensionID), //nolint:gosec // G115
uint8(streamIDExtensionID), //nolint:gosec // G115
uint8(repairStreamIDExtensionID), //nolint:gosec // G115
Expand Down
108 changes: 108 additions & 0 deletions peerconnection_media_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1072,7 +1072,7 @@
closePairNow(t, pcOffer, pcAnswer)
}

func TestPeerConnection_Simulcast_Probe(t *testing.T) { //nolint:cyclop

Check failure on line 1075 in peerconnection_media_test.go

View workflow job for this annotation

GitHub Actions / lint / Go

Function name: TestPeerConnection_Simulcast_Probe, Cyclomatic Complexity: 24, Halstead Volume: 7473.70, Maintainability Index: 16 (maintidx)
lim := test.TimeOut(time.Second * 30) //nolint
defer lim.Stop()

Expand Down Expand Up @@ -1132,6 +1132,114 @@
close(testFinished)
})

// Assert that we can send just one packet with Simulcast IDs (using extensions) and they will be properly received
t.Run("ExtractIDs", func(t *testing.T) {
offerer, answerer, err := newPair()
assert.NoError(t, err)

rids := []string{"layer_1", "layer_2", "layer_3"}
ridSelected := rids[0]

onTrackCalled := &atomic.Bool{}
answerer.OnTrack(func(remote *TrackRemote, receiver *RTPReceiver) {
assert.Equal(t, remote.rid, ridSelected)
onTrackCalled.Store(true)
})

vp8WriterA, err := NewTrackLocalStaticRTP(
RTPCodecCapability{MimeType: MimeTypeVP8}, "video", "pion1", WithRTPStreamID(rids[0]),
)
assert.NoError(t, err)

vp8WriterB, err := NewTrackLocalStaticRTP(
RTPCodecCapability{MimeType: MimeTypeVP8}, "video", "pion1", WithRTPStreamID(rids[1]),
)
assert.NoError(t, err)

vp8WriterC, err := NewTrackLocalStaticRTP(
RTPCodecCapability{MimeType: MimeTypeVP8}, "video", "pion1", WithRTPStreamID(rids[2]),
)
assert.NoError(t, err)

sender, err := offerer.AddTrack(vp8WriterA)
assert.NoError(t, err)
assert.NotNil(t, sender)

assert.NoError(t, sender.AddEncoding(vp8WriterB))
assert.NoError(t, sender.AddEncoding(vp8WriterC))

assert.NoError(t, signalPair(offerer, answerer))

peerConnectionConnected := untilConnectionState(PeerConnectionStateConnected, offerer, answerer)
peerConnectionConnected.Wait()

parameters := sender.GetParameters()

var midID, ridID uint8
for _, extension := range parameters.HeaderExtensions {
switch extension.URI {
case sdp.SDESMidURI:
midID = uint8(extension.ID) //nolint:gosec // G115
case sdp.SDESRTPStreamIDURI:
ridID = uint8(extension.ID) //nolint:gosec // G115
}
}
assert.NotZero(t, midID)
assert.NotZero(t, ridID)

ticker := time.NewTicker(time.Millisecond * 20)
defer ticker.Stop()
testFinished := make(chan struct{})
seenOneStream, seenOneStreamCancel := context.WithCancel(context.Background())

go func() {
sentOnePacket := false

senderTrack := vp8WriterA

for {
select {
case <-testFinished:
return
case <-ticker.C:
answerer.dtlsTransport.lock.Lock()
if len(answerer.dtlsTransport.simulcastStreams) >= 1 {
seenOneStreamCancel()
}
answerer.dtlsTransport.lock.Unlock()

senderTrack.mu.Lock()

// We send just one packet with the RID, that's the point of this test
if !sentOnePacket && len(senderTrack.bindings) > 0 {
sentOnePacket = true

header := &rtp.Header{
Version: 2,
SSRC: util.RandUint32(),
}
header.Extension = true
header.ExtensionProfile = 0x1000
assert.NoError(t, header.SetExtension(midID, []byte("0")))
assert.NoError(t, header.SetExtension(ridID, []byte(ridSelected)))

_, err = senderTrack.bindings[0].writeStream.WriteRTP(header, []byte{0, 1, 2, 3, 4, 5})
assert.NoError(t, err)
}

senderTrack.mu.Unlock()
}
}
}()

<-seenOneStream.Done()

assert.Equal(t, true, onTrackCalled.Load())

closePairNow(t, offerer, answerer)
close(testFinished)
})

// Assert that NonSimulcast Traffic isn't incorrectly broken by the probe
t.Run("Break NonSimulcast", func(t *testing.T) {
unhandledSimulcastError := make(chan struct{})
Expand Down
9 changes: 4 additions & 5 deletions rtptransceiver.go
Original file line number Diff line number Diff line change
Expand Up @@ -291,21 +291,20 @@
streamIDExtensionID,
repairStreamIDExtensionID uint8,
mid, rid, rsid *string,
) (payloadType PayloadType, paddingOnly bool, err error) {
) (paddingOnly bool, err error) {
rp := &rtp.Packet{}
if err = rp.Unmarshal(buf); err != nil {
return 0, false, err
return false, err

Check warning on line 297 in rtptransceiver.go

View check run for this annotation

Codecov / codecov/patch

rtptransceiver.go#L297

Added line #L297 was not covered by tests
}

if rp.Padding && len(rp.Payload) == 0 {
paddingOnly = true
}

if !rp.Header.Extension {
return payloadType, paddingOnly, nil
return paddingOnly, nil
}

payloadType = PayloadType(rp.PayloadType)
if payload := rp.GetExtension(midExtensionID); payload != nil {
*mid = string(payload)
}
Expand All @@ -318,5 +317,5 @@
*rsid = string(payload)
}

return payloadType, paddingOnly, nil
return paddingOnly, nil
Copy link
Author

@kvasilye kvasilye Jun 25, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Lint complained that handleUnknownRTPPacket had a return value which the callers did not consume. Fixed.

}
Loading