1611957 - Implement RTCRtpEncodingParameters.maxFramerate

Status: RESOLVED FIXED

(Core :: WebRTC: Signaling, enhancement, P2)

Description

[Byron Campen [:bwc]]

This is being moved to an extension spec now.

Comment 1

[Byron Campen [:bwc]]

I think I might be able to sneak this in after bug 1616937, instead of having to wait until bug 1401592 lands.

Comment 2

[Byron Campen [:bwc]]

Try looks good.

https://treeherder.mozilla.org/jobs?repo=try&revision=1b8d1ea84e250387a61fcc46dd62eb3adeb11a0a
https://treeherder.mozilla.org/jobs?repo=try&revision=87556d24fddb98c6c76b315088b1269098364712
https://treeherder.mozilla.org/jobs?repo=try&revision=0cfacdef6f1a159e531059afe9d4391d894206bd

Comment 3

[Byron Campen [:bwc]]

Attachment: Bug 1611957: Test-cases for RTCRtpEncodingParameters.maxFramerate r?ng

Depends on D144007

Comment 4

[Byron Campen [:bwc]]

Attachment: Bug 1611957: Make EncodingConstraints.maxFps a Maybe instead of having 0 represent no limit. r?ng

0 is a valid limit in setParameters according to the spec. For now, we treat
max-fr=0 in SDP as no limit, since that is what we have been doing.

Depends on D144008

Comment 5

[Byron Campen [:bwc]]

Attachment: Bug 1611957: Add support for RTCRtpEncodingParameters.maxFramerate r?ng

Depends on D144009

Comment 6

[Byron Campen [:bwc]]

Try looks good after incorporating some review feedback:

https://treeherder.mozilla.org/jobs?repo=try&revision=23968d215fb50682e8ab9db5e99e5b010fc64893

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/5451bd3f5a86
Test-cases for RTCRtpEncodingParameters.maxFramerate r=ng
https://hg.mozilla.org/integration/autoland/rev/7cd1d16fd378
Make EncodingConstraints.maxFps a Maybe instead of having 0 represent no limit. r=ng
https://hg.mozilla.org/integration/autoland/rev/288cc1e05748
Add support for RTCRtpEncodingParameters.maxFramerate r=ng,webidl,smaug

Comment 8

[Marian-Vasile Laza]

https://hg.mozilla.org/mozilla-central/rev/5451bd3f5a86
https://hg.mozilla.org/mozilla-central/rev/7cd1d16fd378
https://hg.mozilla.org/mozilla-central/rev/288cc1e05748

Comment 9

[Hamish Willee]

FF101 docs work for this can be tracked in https://github.com/mdn/content/issues/15470

A question. The spec indicates that maxFramerate value of 0 should stop the video if set to zero by RTCRtpSender.setParameters(), and doesn't say what happens for RTCPeerConnection.addTransceiver() if the rate is set to zero.
It looks like you have allowed 0 to indicate "no limit".

1. Is that correct?
2. Isn't that a spec violation?

Comment 10

[Byron Campen [:bwc]]

(In reply to Hamish Willee from comment #9)

FF101 docs work for this can be tracked in https://github.com/mdn/content/issues/15470

A question. The spec indicates that maxFramerate value of 0 should stop the video if set to zero by RTCRtpSender.setParameters(), and doesn't say what happens for RTCPeerConnection.addTransceiver() if the rate is set to zero.
It looks like you have allowed 0 to indicate "no limit".

1. Is that correct?

Nope! (You are correct, the behavior is not, not sure which question you were asking)

2. Isn't that a spec violation?

Yep! Sadly, this is a limitation in libwebrtc upstream; it crashes if you set a framerate of 0. There's nothing we can do about this on our end at the moment; upstream will need to fix this, and then we'll need to update our import.

Comment 11

[Hamish Willee]

Thanks Byron! I've updated the compatibility data and release note to add a note about issue.