Open Bug 1676879 Opened 4 years ago Updated 4 years ago

WebRTC SDP TIAS parameter is not properly handled for a simulcast peerConnection

Categories

(Core :: WebRTC: Audio/Video, defect, P3)

Firefox 82
defect

Tracking

()

People

(Reporter: jan, Unassigned)

Details

Attachments

(1 file)

Attached image grafik.png

User Agent: Mozilla/5.0 (Windows NT 10.0; Win64; x64; rv:82.0) Gecko/20100101 Firefox/82.0

Steps to reproduce:

Create a simulcast using peerConnection using the API firefox provides for it

  • retrieve the video sender on the peerConnection
  • getParameters
  • add encodings:
    public simulcastEncodings = [
    { rid: "h", maxBitrate: 1200000 },
    { rid: "m", maxBitrate: 500000, scaleResolutionDownBy: 2 },
    { rid: "l", maxBitrate: 150000, scaleResolutionDownBy: 4 }
    ];

send the offer to the server.
retrieve the answer from the server that contains:
b=TIAS:150000

Actual results:

Firefox sends data on all three simulcast layers, each using 150kbit. So in total 450kBit

Expected results:

Janus stops sending the simulcast layers high and mid and just sends data on the lowest layer

Safari does handle an SDP AS appendumg properly in dropping the higher quality streams and just sending with 150kBit on the lowest layer.
Did not check with chrome as the active flag in the setParameters is properly handled and allows a direct switching of layers without the need to touch the SDP.

Bugbug thinks this bug should belong to this component, but please revert this change in case of error.

Component: Untriaged → WebRTC: Audio/Video
Product: Firefox → Core
Severity: -- → S3
Status: UNCONFIRMED → NEW
Ever confirmed: true
Priority: -- → P3

I would argue that using both maxBitrate (in sender parameters) and b=TIAS is a bad idea, and that you simply shouldn't do it. Here's why: there's tons of corner cases lurking here, and it is not super clear how to handle these. Some examples:

b=TIAS:200000
"low" max br: 100000
"mid" max br: 200000

Should we drop "mid" and encode "low" at 100000, or should we drop "low" and encode "mid" at 200000?

b=TIAS:1000000
"low" max br: 150000
"mid" max br: 300000
"high" max br: 850000

We could do low/mid/high at 150000/300000/550000, or we could do low/high at 150000/850000, or we could do mid/high at 300000/700000. Which is appropriate?

That said, what we're doing is not right. Here's where we're messing up:

https://searchfox.org/mozilla-central/rev/c37038c592a352eda0f5e77dfb58c4929bf8bcd3/dom/media/webrtc/libwebrtcglue/VideoStreamFactory.cpp#217-219

We are giving each simulcast stream the full budget, instead of dividing it up. Instead of passing |mNegotiatedMaxBitrate| here, we need to have a local variable that represents the remaining budget, and pass that instead. We also need to do something sensible when there is not enough budget remaining to encode a simulcast stream at a sufficiently higher quality than the others we are already planning to encode (ie; if the budget in the description were 200000 instead of 150000, it would be silly to encode "low" at 150000, and "mid" as 50000). As for handling corner cases like I mention above, I think it is not worth the effort.

You need to log in before you can comment on or make changes to this bug.

Attachment

General

Creator:
Created:
Updated:
Size: