## Summary
Fixed inconsistent test duration and occational `panic timeout` in
Test_PeerConnection_RTX_E2E
```
panic: timeout
goroutine 4309 [running]:
github.com/pion/webrtc/v4.Test_PeerConnection_RTX_E2E.TimeOut.func5()
/Users/ytakeda/go/pkg/mod/github.com/pion/transport/v3@v3.1.1/test/util.go:27 +0xb4
created by time.goFunc
/Users/ytakeda/sdk/go1.25.4/src/time/sleep.go:215 +0x44
```
### Problem
The test was timing out intermittently (0.2s-18.6s variance) when run
with the race detector due to:
* Random 20% packet loss causing unpredictable RTX detection timing
* Missing goroutine cleanup causing hangs
* Race detector overhead amplifying timing issues
```
go test -v -race -run Test_PeerConnection_RTX_E2E -count 20
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (7.46s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (1.42s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (0.72s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (18.62s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (1.22s)
:
```
### Solution
1. Deterministic packet dropping - Changed from random 20% loss to dropping every 5th packet, ensuring predictable NACK/RTX triggering
2. Dynamic payload type detection - Uses negotiated payload type instead of hardcoded 96, making the test robust to codec configuration changes
3. Proper goroutine cleanup - Added context-based cleanup for RTCP reader and OnTrack callback
4. Fail-fast validation - Checks assertion return values and exits immediately on failure
5. Correct cleanup order - Closes peer connections before stopping the virtual network
### Result
* Consistent ~0.21s execution time (tested 50 runs with race detector)
* No more timeouts
* Still validates NACK/RTX behavior as intended by PR #2919
```
go test -v -race -run Test_PeerConnection_RTX_E2E -count 10
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (0.21s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (0.21s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (0.21s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (0.21s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (0.21s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (0.21s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (0.21s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (0.21s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (0.21s)
=== RUN Test_PeerConnection_RTX_E2E
--- PASS: Test_PeerConnection_RTX_E2E (0.21s)
PASS
ok github.com/pion/webrtc/v4 3.373s
```
The code currently ignores the first packet when reading Simulcast IDs
from a new SSRC, and probes only subsequent packets. This commit makes
it so that we consider the first packet as well (which we already have
read). Helps if the publisher only sends Simulcast IDs on the first
packet.
Done when creating a transceiver from remote description to respect
codec order preference of remote peer.
There was a recent change to include partial matches which overwrote
same codecs and also rtx was getting magled.
Change it by removing codecs from search space as matches are found so
that a codec match is applied only once.
Also, move RTX matching to separate block to ensure proper RTXes ar
matched.
A follow on to https://github.com/pion/webrtc/pull/3200.
This removes the setting engine flag and uses knowledge
of remote direction to decide if a transceiver can be
re-used for sending.
Refactored the code a bit and moved the check into
RTPTransceiver.isSendAllowed.
Re-did the UT to check for re-use cases.
SetDisableTransceiverReuseInRecvonly controls if a
transceiver is re-used when its current direction is `recvonly`.
This is useful for the following scenario
- Remote side sends `offer` with `sendonly` media section.
- Local side creates transceiver in `SetRemoteDescription`
and sets direction to `recvonly.
- Local side calls `AddTrack`.
- As the current direction is `recvonly`, the transceiver added
above will be re-used. That will set the direction to `sendrecv`
and the generated `answer` will have `sendrecv` for that
media section.
- That answer becomes incompatible as the offerer is using
`sendonly`.
Note that local transceiver will be in `recvonly` for both `sendrecv`
and `sendonly` directions in the media section. If the `offer` did use
`sendrecv`, it is possible to re-use that transceiver for sending.
So, disabling re-use will prohibit re-use in the `sendrecv` case also
and hence is slightly wasteful.
- Remove custom atomicBool implementation
- Replace all atomicBool usages with standard library sync/atomic.Bool
Signed-off-by: Xiaobo Liu <cppcoffee@gmail.com>
Introduces a fallback mechanism to handle undeclared SSRCs from multiple
sections using the RTP stream's payload type. For legacy clients
without MID extension support, it documents the existing behavior for
handling undeclared SSRCs in single media sections.
The way currently DTLS fingerprints and ICE credentials
are picked is causing interop issues as described in #2621
Peers which don't use Bundle can use different fingerprints
and credentials in each media section. Even though is
not (yet) supported by Pion, receiving an SDP offer from
such a peer is valid.
Additionally if Bundle is being used the group attribute
determines which media section is the master bundle section,
which establishes the transport. Currently Pion always
just uses the first credentials/fingerprint it can find
in the SDP, which results in not spec compliant behavior.
This PR attempts to fix the above issues and make
Pion more spec compliant and interoperable.
Fixes#2621
If the MediaEngine contains support for them a SSRC will be generated
appropriately
Co-authored-by: aggresss <aggresss@163.com>
Co-authored-by: Kevin Wang <kevmo314@gmail.com>
Resolves#1989Resolves#1675
In Go 1.22 and earlier, a ticker needs to be explicitly stopped
when it's no longer useful in order to avoid a resource leak.
In Go 1.23 and later, an orphaned ticker will eventually be
garbage collected, but it's still more thrifty to stop it early.
libwebrtc has started sending media probes on an unannounced SSRC(0).
Currently Pion will ignore this as the SSRC hasn't been declared
explicitly and no RID/MID RTP Headers.
This adds a special case to accept SSRC 0 and Read the RTP packets. This
allows the TWCC reports to properly be generated.
Firefox would send updated header extension
in renegotiation, e.g. publish a track without
simucalst then renegotiate second track with
simucalst, the two media secontions will have
different rtp header extensions in offer. Need
to match remote header extentions for each
media sections to avoid second track publish
failed.
Fix#2751, updates remote track's rtx ssrc for
simulcast track doesn't contain rtx ssrc in sdp
since readRTX relies on rtx ssrc to determine if
it has a rtx stream.
In the case where a remote track is sending PCMU with payload type 0
checkAndUpdateTrack will fail to update the track codec and params
(because t.PayloadType() is already 0). Add an extra check to handle
this case.
Everytime we receieve a new SSRC we probe it and try to determine the
proper way to handle it. In most cases a Track explicitly declares a
SSRC and a OnTrack is fired. In two cases we don't know the SSRC
ahead of time
* Undeclared SSRC in a single media section (pion/webrtc#880)
* Simulcast
The Undeclared SSRC processing code would run before Simulcast.
If a Simulcast Offer/Answer only contained one Media Section we
would never fire the OnTrack. We would assume it was a failed
Undeclared SSRC processing. This commit fixes the behavior.
This change adapts pion/ice to use a new interface for most network
related operations. The interface was formerly a simple struct vnet.Net
which was originally intended to facilicate testing. By replacing it
with an interface we have greater flexibility and allow users to hook
into the networking stack by providing their own implementation of
the interface.
Introduces AddEncoding method in RTP sender to add simulcast encodings.
Added UTs for AddEncoding.
Also modified the Simulcast send test to use the new API.
If a transceiver is created by remote sdp, then set
prefer codec same as offer peer.
For pion's codec match, it will use exact match
first, and then partial match. If patial match
only, the partial match codecs will become
negotiated codes. So it will be set prefer codec
when only exist partial match. And has same payload.
Add test cast for this.
refer to https://www.w3.org/TR/webrtc/#bib-rfc8829
Problem:
--------
Firefox (testing with Firefox 93) sends an `offer`
with simulcast track which includes both RIDs and SSRCs.
When track details are gathered, RIDs are fetched
using `getRids` which returns a map. A `range` is
done on the returned map and RIDs are appended to
the `rids` array in `trackDetails`.
And then SSRCs are read from SDP and set into the
`ssrcs` array of `trackDetails`.
As map range order is not guaranteed, some times
the RID index and SSRC index in their respective arrays
do not match.
Due to the mismatch, services like ion-sfu which rely
on RID to find the correct spatial layer get confused
and end up forwarding the wrong layer.
Solution(s):
------------
There are three possible solutions I could think of
1. The simplest is to not populate SSRCs in `trackDetails`
if RIDs are available. Let `handleIncomingSSRC` do the
SSRC resolution based on RID.
According to RFC 8853 (https://www.ietf.org/rfc/rfc8853.pdf),
the binding to SSRC should happen using RID.
This is the change I have made in this PR. See testing
below for browsers tested.
2. Look for `simulcast` attribute in SDP and take
the RID ordering from that line in `getRids` if that
attribute is available. If not fall back to `rid` attribute.
Also, change `getRids` to return an array.
But, I cannot find an RFC which defines behaviour when
both RID and SSRC are used in `offer`. The question is,
"Will the `ssrc` attribute ordering match the `rid`
ordering?". If that is not guaranteed, this will run
into trouble.
This should be easy to do though if we want to go down this
route.
3. The hardest option is to change the receiver SSRC based
on RID. But, that makes it too complicated (for example
if we have to change SSRC binding of a receiver based on
received RID, there will be two receivers with the same SSRC
binding unless there is some way to swap bindings of receivers)
Testing:
--------
Tested on Firefox, Firefox 78.15.0esr, Chrome, Ssafari and
ensured that Simulcast sender and receiver are in sync for
rid/ssrc/spatial layer resolution.
Read + Discard packets from the Simulcast repair stream. When a
Simulcast stream is enabled the remote will send packets via the repair
stream for probing. We can't ignore these packets anymore because it
will cause gaps in the feedback reports
Resolves#1957
(*SCTPTransport).generateAndSetDataChannelID performed a double loop to
find the next available data channel ID. This changes that behavior to
generate a lookup map with the taken IDs first, so generating a data
channel ID takes much less time.
Before, it would take more than 1000ms to generate the next data channel
ID once you had roughly 50k of them. Now it only takes 4ms at that same
point.
Fixes#1945
