Open Bug 875922 Opened 7 years ago Updated 7 months ago

Firefox support for ulpfec/red in WebRTC video channels is not enabled

Categories

(Core :: WebRTC: Networking, defect, P2)

defect

Tracking

()

Blocking Flags:

People

(Reporter: gustavogb, Assigned: dminor)

References

Details

User Agent: Mozilla/5.0 (Macintosh; Intel Mac OS X 10_7_5) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/27.0.1453.93 Safari/537.36

Steps to reproduce:

Establish a PeerConnection to a central WebRTC server


Actual results:

After the PeerConnection is established no RTCP PLI packet is sent if the ulpfec/red "codecs" are enabled.


Expected results:

It should send a PLI packet to force the sender to send a Key Frame.   That is what Chrome does.
We don't currently enable ulpfec/red, so not a blocking issue in any case.

Please provide a log from a debug build, with NSPR_LOG_MODULES=signaling:5,webrtc_trace:65535 and WEBRTC_TRACE_FILE=whatever NSPR_LOG_FILE=something -- please include the NSPR LOG file and the webrtc trace log.  Note that the trace log will wrap if left running long enough; this isn't an issue for a startup issue like this.  You may want to compress the logs, especially the webrtc trace log.

Also info on what build you're using for FF and any mods to it to enable those 'codecs'.

Thanks
Component: WebRTC: Signaling → WebRTC
Whiteboard: [WebRTC] [blocking-webrtc-]
I retested it with logs enabled (didn't know how to enable them, thx) and the issue is slightly different.

With ulpfec/red enabled (our server enables it) Chrome will be sending packets with a different payload type and content (typically 116 red instead of 100 VP8) and firefox is discarding those packets because it doesn't understand that content.

This is the log: "IncomingRTPPacket received invalid payloadtype"

We think it is important you enable support for ulpfec/red sooner than later because:
- It improves the quality on unreliable connections
- It simplifies a lot the use of ulpfec/red in scenarios with a server routing the traffic to multiple destinations (otherwise we have to translate the content in the server)
- It should be supported in WebRTC core and probably it is not difficult to enable it
Summary: RTCP PLI not sent when ulpfec/red are enabled → Firefox support for ulpfec/red in WebRTC video channels is not enabled
Is there something I can help with here in order to reproduce the issue?
Status: UNCONFIRMED → NEW
Ever confirmed: true
OS: Mac OS X → All
Hardware: x86 → All
Whiteboard: [WebRTC] [blocking-webrtc-]
Version: 22 Branch → Trunk
A server that does ULPfec/red with one endpoint and not with another (which is always possible!) must translate, per the recent hacks article about Jitsi bridge.

Also: there are two issues.  Support (this bug), and when if ever to offer it (or accept it).
backlog: --- → webRTC+
Rank: 35
Component: WebRTC → WebRTC: Networking
Priority: -- → P3
QA Contact: jsmith
I'm a contributor to Licode, and we recently had issues with this (also note the current FEC handling in licode is simply wrong; I wrote it, and it won't work).  The code in Jitsi actually won't work correctly assuming FEC packets are being sent, and those packets aren't compound (i.e. multiple payloads per packet).  From what I've seen, Chrome never sends RED payloads with multiple packets, or at least I've never observed it.

If Chrome sends FEC packets, it looks sort of like this:

P1 P2 P3 P4 P5...

All packets have a payload type of RED (let's say 116).  Packets P1 through P4 have an internal RED header that specifies the payload type as VP8 (let's say 100).  Packet P5 is a FEC frame, and has an internal payload type of 117. 

For that P5 packet, what I'm finding is there's no way to reconstruct that packet such that Firefox doesn't NACK the packet (presumably because the packet forwarded on has a payload type Firefox doesn't recognize.  This constant NACKing actually causes Chrome's bandwidth estimation to do strange things.

The only option for an SFU/Media Server that I can see is to A) rewrite sequence numbers and maintain a mapping or B) Disable FEC.  The first is complicated, since NACKs from Firefox to Chrome would have the wrong sequence number, and the subsequent retransmission from Chrome to Firefox would also have the wrong sequence number, so there's a constant accounting/rewriting.

Firefox could do two things to make this *way* easier:

1. Fully implement FEC
2. Have a way to expose FEC/NACK as a legitimate payload type, so firefox at least knows what those packets are and does not NACK them.
3. ???  ...someone else may have other ideas.
It's going to be tricky to come up with a good solution for this problem, at least in the current setting. B is always an option, but not a good one. In A, how do you rewrite the sequence numbers in the following two scenarios? :

1. The sender is sending VP8, VP8, VP8 and the server receives VP8, MISS, VP8.
2. The sender is sending VP8, FEC, VP8 and the server receives VP8, MISS, VP8.

In 1. you want to allocate a sequence number for the missed packet, and in 2. you don't, but you can't always distinguish between the two situations.

I think this issue is only partially caused by the lack of support for FEC in FF. It was very surprising to me when I found out that Chrome is sending standalone FEC packets intermixed with standalone VP8 packets in the same sequence number space.

I would have expected FEC packets to either be piggy backed in the RED payload -so that the sequence number space is not polluted by FEC packets and to avoid RTCP calculation distortions-, or to support a different RTP stream for FEC, similarly to RTX. I think there's an issue for the latter option in the webrtc bug tracker.
For the first example, let's say the sequence numbers for the first stream are 1, 2, and 3.  Those get passed through, so NACK/RTX is pretty straight-forward.

For the second example, the MCU has to maintain a mapping.  The input sequence numbers are 1, 2 and 3, but the MCU rewrites outgoing sequence numbers simply to be 1, 2.  Internally, it maintains a mapping:

1 <--> 1
3 <--> 2

...if Firefox NACKs 2, the MCU would have to rewrite the NACK with 3, and then the subsequent RTX (which is 3) gets rewritten as 2.  This would work, it's just super odious.  

I agree Chrome really shouldn't be using FEC the way it does; it should be a separate stream entirely, which would avoid any RED overhead and make translation/NACK/RTX a lot more straightforward.  I believe there's a defect somewhere in the webrtc bug tracker to address that.
Also, it seems odd to argue option B isn't a good one; that strongly implies Firefox should just have ULP/FEC support.  If I can't safely disable FEC because it's not clear what the resulting hit to quality may be, then....Firefox should have this.
Assignee: nobody → rjesup
Rank: 35 → 15
Priority: P3 → P1
Depends on: 1274340
Depends on: 1275360
Depends on: 1279049
Once this is reasonably tested (including interop and landing any bugfixes needed) and the tradeoffs for enabling it are known, we can flip the pref to enable.
-> dminor to test with realworld servers and interop and enable when good (likely after v57)
Assignee: rjesup → dminor
Depends on: 1341285
Mass change P1->P2 to align with new Mozilla triage process
Priority: P1 → P2
Depends on: CVE-2018-6156
You need to log in before you can comment on or make changes to this bug.