Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Unable to handle NACK that uses distinct SSRC #1675

Open
ghost opened this issue Feb 13, 2021 · 11 comments
Open

Unable to handle NACK that uses distinct SSRC #1675

ghost opened this issue Feb 13, 2021 · 11 comments

Comments

@ghost
Copy link

ghost commented Feb 13, 2021

Your environment.

  • Version: 3.0.7
  • Browser: Chrome 88.0.4324.150 (Official Build) (x86_64)

What did you do?

A PeerConnection created with NewPeerConnection (not api) logs an error pc ERROR: 2021/02/13 07:35:09 Incoming unhandled RTP ssrc(3537335204), OnTrack will not be fired. mid RTP Extensions required for Simulcast after the browser sends first retransmitted packet. Looks like pion/webrtc confuses the retransmitted packet for another stream/track.

After recreating the code from NewPeerConnection manually, but registering only some codecs showed that adding the codec below triggers the error.

	m.RegisterCodec(webrtc.RTPCodecParameters{
		RTPCodecCapability: webrtc.RTPCodecCapability{"video/rtx", 90000, 0, "apt=96", nil},
		PayloadType:        97,
	}, webrtc.RTPCodecTypeVideo)

The rest of the code:

func NewPeerConnection() (*webrtc.PeerConnection, error) {
	// pc, _ := webrtc.NewPeerConnection(config)
	m := webrtc.MediaEngine{}

	videoRTCPFeedback := []webrtc.RTCPFeedback{{"goog-remb", ""}, {"ccm", "fir"}, {"nack", ""}, {"nack", "pli"}}

	m.RegisterCodec(webrtc.RTPCodecParameters{
		RTPCodecCapability: webrtc.RTPCodecCapability{"video/VP8", 90000, 0, "", videoRTCPFeedback},
		PayloadType: 96,
	}, webrtc.RTPCodecTypeVideo)

	m.RegisterCodec(webrtc.RTPCodecParameters{
		RTPCodecCapability: webrtc.RTPCodecCapability{"video/rtx", 90000, 0, "apt=96", nil},
		PayloadType:        97,
	}, webrtc.RTPCodecTypeVideo)

	m.RegisterCodec(webrtc.RTPCodecParameters{
		RTPCodecCapability: webrtc.RTPCodecCapability{"audio/opus", 48000, 2, "minptime=10;useinbandfec=1", nil},
		PayloadType:        111,
	}, webrtc.RTPCodecTypeAudio)

	// m.RegisterDefaultCodecs()
	i := &interceptor.Registry{}

	generator, _ := nack.NewGeneratorInterceptor()
	responder, _ := nack.NewResponderInterceptor()
	m.RegisterFeedback(webrtc.RTCPFeedback{Type: "nack"}, webrtc.RTPCodecTypeVideo)
	m.RegisterFeedback(webrtc.RTCPFeedback{Type: "nack", Parameter: "pli"}, webrtc.RTPCodecTypeVideo)
	i.Add(generator)
	i.Add(responder)

	api := webrtc.NewAPI(webrtc.WithMediaEngine(&m), webrtc.WithInterceptorRegistry(i))
	pc, _ := api.NewPeerConnection(config)
	return pc, nil
}

What did you expect?

Expected to receive retransmitted packets from the browser

What happened?

Adding the codec above triggers the error and prevents any retransmitted packet from being received.

@Sean-Der Sean-Der changed the title ERROR: Incoming unhandled RTP ssrc(), OnTrack will not be fired. mid RTP Extensions required for Simulcast Unable to handle NACK that uses distinct SSRC Feb 13, 2021
@Sean-Der
Copy link
Member

Yea this is an issue :/

when I am at my desk I will type up some ideas we can do to fix! Thanks for reporting @slavazhil

@Sean-Der
Copy link
Member

Currently Pion doesn't support RTX with a distinct SSRC. When we interact with a remote
client that uses a distinct SSRC for RTX we drop those packets. These are the goals/things
that are important when implementing.

  • RTX should be returned to the user via Read and ReadRTP
    Most users don't care if a packet is a RTX, they just want to process packets as they arrive

  • RTX should be distinguishable by advanced users
    Users who do care should be able to distinguish a packet that was an RTX

  • We need to support RTX for Simulcast and non-Simulcast

Implementation

Allow srtp.ReadStreamSRTP to pull multiple SSRCes

Currently a stream is created for each incoming SSRC. These streams are what Read and ReadRTP
is called upon. I propose we add an additional method to allow multiple SSRCes to a stream.

  • Add AddSSRC to ReadStreamSRTP
  • Change GetSSRC -> GetSSRCes on srtp.ReadStreamSRTP
  • BufferFactory needs to instead take events instead of constructor arguments. We will fire two AddSSRC one for the original, one for the SSRC.

Open RTX Stream in pion/webrtc

Update streamsForSSRC to optionally support opening the RTX Stream as well.

Add a RTCRtpRtxParameters to RTPCodingParameters

Update TrackRemote Read/RTP to detect SSRC

TrackRemote.readRtp will inspect the incoming SSRC. If it is the RTX SSRC it will remove the head off of the buffer to give you the original.

This method will also add isRTX to the Attributes as needed.

Simulcast Support

Simulcast SSRC probing needs to also look for repaired-stream-id. We need to support adding the RTX stream async to the RTP Receiver.

Testing

  • Assert E2E that a NACK can be sent and received successfully
  • Assert that a bad RTP packet doesn't break the 'RTX Unwrap' in TrackRemote.readRTP
  • Assert E2E that a Simulcast NACK can be sent and received successfully

@jech
Copy link
Contributor

jech commented Mar 12, 2021

Most users don't care if a packet is a RTX, they just want to process packets as they arrive

In my understanding, the whole point of having a separate RTX track is to distinguish between original and resent packets. If there is no need to make the distinction, then there is no need for the overhead of a separate RTX track — resent packets can be sent over the main media track. In the words of RFC 4588 Section 3:

   One approach may be to retransmit the RTP packet with its original
   sequence number and send original and retransmission packets in the
   same RTP stream.  The retransmission packet would then be identical
   to the original RTP packet, i.e., the same header (and thus same
   sequence number) and the same payload.  However, such an approach is
   not acceptable because it would corrupt the RTCP statistics.  As a
   consequence, requirement 1 would not be met.  Correct RTCP statistics
   require that for every RTP packet within the RTP stream, the sequence
   number be increased by one.

Note that retransmitting the packets in the main RTP stream is well supported by current browsers. Note further that it is not too difficult to compensate for the retransmissions and get accurate RTP statistics, although I don't think that browsers do that correctly (Galène does).

@OrlandoCo
Copy link
Contributor

@jech @Sean-Der So I was trying to find if there was an advantage for getting RTX stream besides better RTC stats, but I could not find any and as Julius said that is something it can be done without RTX stream. So not sure now if It worth the overhead of getting another stream.

@jech
Copy link
Contributor

jech commented Mar 13, 2021

The same mechanism is used for RED. Unilke RTX, which is nice to have but not difficult to work around, RED is what allows you to do FEC with audio, which is the secret sauce behind Zoom's audio quality over lossy WiFi.

So while it might not be that critical to implement ancillary tracks for RTX, the work will be reused when somebody implements RED.

@jech
Copy link
Contributor

jech commented Mar 13, 2021

Let me explain some more why a separate track is essential for FEC but not strictly required for retransmissions.

Suppose you are doing retransmissions on the same track. You receive

P1 P2 P4 P5

You NACK P3, so you later receive

P1 P2 P4 P5 P6 P3 P7

In order to maintain proper statistics, you need to determine that P3 is a reemission, which is not too difficult: P3 was NACKed and it arrived out of order, that's a pretty good indication that it was a retransmission. Having a separate RTX track makes this simpler and more reliable, but it's not essential.

Suppose now you're doing FEC on a single track. The sender sends

P1 FEC2 P3

and you receive

P1 P3

I don't see a way for the receiver to determine whether the missed packet was FEC or data. If the receiver is an SFU, for example, if the missed packet was FEC it must renumber P3 before sending it out, while it must not do so if the missed packet was data.

In conclusion, support for ancillary tracks is required:

  • it's absolutely required if we want to implement FEC; and
  • it's a nice but nonessential simplification if we implement NACKs.

Sean, I'm therefore enthusiastic about you willing to do the work, just keep in mind that any work you do for RTX must be easy to extend to RED.

@OrlandoCo
Copy link
Contributor

@jech Do you find any advantage on using FEC with RED? On my statics gathered, when network gets congested packets gets dropped in big batches (5+ packets), also the complexity of RED since not all browser supports it makes the SFU to terminate it for some users.

I think that for audio inband-fec offers good performance and you can set a fixed packet loss rate to control the % of FEC.

@jech
Copy link
Contributor

jech commented Mar 13, 2021

Do you find any advantage on using FEC with RED?

I haven't done any experiments yet.

On my statics gathered, when network gets congested packets gets dropped in big batches (5+ packets)

That happens if your bottleneck router does tail drop. Modern routers use AQM and should not drop packets in bursts. (Or at least a bunch of people much smarter than me are working very hard on getting modern AQMs widely deployed.)

However, the main problem I'm trying to solve is people lecturing behind a poor quality WiFi connection. This happens a lot in my world, since a lot of lecturers have small children, and they need to move to an isolated room in order to give their lectures.

makes the SFU to terminate it

Exactly. Opus FEC is great for end-to-end FEC, while ulpfec is the right thing if you want to terminate FEC at the server. There's no point to negotiate out-of-band FEC if you're not going to reconstruct it at the server, Opus FEC is fine in that case.

Terminating FEC at the server has a number of advantages. Since the signal is reconstructed at the server, it reduces the amount of loss in the case where loss occurs on both legs of the flow. It allows you to use different amounts of FEC for different clients, which reduces both overhead and latency. And, finally, it allows you to use higher FEC rates than what the browsers do, which means that audio can survive on extremely lossy links.

(Opus FEC is limited to 100% FEC. Given that an Opus flow is on the order of 30kbps, it is quite reasonable to use 200% FEC, i.e. to multiply the amount of traffic by 3. With suitable interleaving, this should allow acceptable audio quality on even the worst links.)

@http600
Copy link

http600 commented Aug 22, 2021

wait, I need to clarify pc ERROR: 2021/02/13 07:35:09 Incoming unhandled RTP ssrc(3537335204), OnTrack will not be fired. mid RTP Extensions required for Simulcast

OnTrack will not be fired here, means OnTrack will not be fired at all, or only not be fired for FEC track, but can be fired for regular video/audio track, I am confused.

@jech
Copy link
Contributor

jech commented Aug 22, 2021

I believe that's correct.

@alexpokotilo
Copy link

alexpokotilo commented Jul 8, 2022

Hi all,
maybe we need to remove "video/rtx" tracks from func (m *MediaEngine) RegisterDefaultCodecs() since rtx doesn't work in anyway ?
Now when we use webrtc.NewPeerConnection to create "default" peerConnection packet re-transmission doesn't work with chrome as chrome offer RTX and pion confirms it supports RTX but it doesn't.

I also don't understand why video/ulpfec declared in func (m *MediaEngine) RegisterDefaultCodecs()
{
RTPCodecCapability: RTPCodecCapability{"video/ulpfec", 90000, 0, "", nil},
PayloadType: 116,
},

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants