RTX support for v3 #2666
RTX support for v3 #2666
Conversation
Fix data race of RTX packet
Always handle header extensions from packet read from interceptor, let interceptor has consistent chance to process headers Fix rtx is not negotiated when there is multiple codecs has same mime but different profile (H264) Fix rtx stream info missed when SSRC group attr shows after base track's ssrc attr.
cnderrauber
requested review from
davidzhao,
Sean-Der,
boks1971 and
adriancable
|
This looks great (and of course I am a fan of RTX support, having done the original implementation in v4 😃). But I am wondering, in general what is our approach regarding backporting stuff to v3 vs. focussing on v4? My own view (not strongly held) is that we should get v4 out of beta so it becomes the 'de facto' base for new users, and basically freeze v3 except maybe for important bug fixes. I think this is what happened when we went from v2 to v3. But we should probably (as maintainers) decide the approach collectively because there is a time/effort cost in backporting things to v3. |
|
@adriancable I need RTX support for Sender to add probing to improve for pion/interceptor#71 support. Am I right that RTX for sender is supported in Pion and your are adding RTX for receiving ? |
|
@alexpokotilo - RTX receiving over a distinct SSRC is supported in v4 and, with this PR, is backported to v3. RTX sending over a distinct SSRC is currently not supported as far as I know. RTX sending/receiving over the same track/SSRC as the main media i.e. via the NACK mechanism has been supported for a while by Pion's NACK generator/sender interceptor. |
|
@adriancable NACK over the main works fine, you are right! The problem with not supporting RTX in Pion is that we cannot implement Bandwidth Estimation as we need to add bandwidth probing. Bandwidth probing from another hand cannot be added using main SSRC since sending the same RTP packet (with the same payload) would lead browsers(Chrome and Firefox) think we are doing Replay Attack. I tried to send some RTP packets twice changing TWCC header, but browsers reject my duplicates since payload is the same as SSRC and Sequence Number of RTP packet is the same.Thanks to @lminiero I was able to find why browsers reject these RTPs. So we need to send probing packets over distinct SSRC. |
|
@alexpokotilo - for video streams with same-track RTX via NACK feedback, BWE probes are implemented as empty-payload (i.e. padding only) RTP packets. You do not need distinct-SSRC RTX for this. Distinct-SSRC RTX has an advantage for BWE in that empty-payload probes are generally not needed because you can use repeat RTP packets as probes instead. But you do not need distinct-SSRC for BWE probes. See: https://chromium.googlesource.com/external/webrtc/+/master/pc/g3doc/rtp.md |
|
@adriancable thanks a million ! I will try. But as you said RTX is preferred as RTX packets act as redundancy. |
|
@adriancable Agree to focus on release v4 instead of backporting features to v3. We also have a plan to upgrade to v4 in our product. I add the RTX to v3 because it is important to us now, after evaluating the effort and risk between upgrading to v4 and supporting it in v3, I chose the latter. It also contains bug fixes merged to master first and cherry-pick to v3 so it is focusing on v4 too. Will re-evaluate v4 later and discuss with other developers about the plan to release v4. |
|
@cnderrauber - that sounds very reasonable. We are running v4 in a volume commercial product (which uses RTX) with no issues so far, which is an encouraging indicator. |
Description
Reference issue
Fixes #...