Add scaleResolutionDownTo to RTCRtpEncodingParameters #221

henbos · 2024-08-29T09:28:15Z

Fixes #159.

index.html

henbos · 2024-08-30T08:44:31Z

index.html

+          restricted (portrait mode or landscape mode) by swapping width and
+          height if necessary. This means that it does not matter if 1280x720 or
+          720x1280 is specified, both always result in the exact same scaling
+          factor regardless of the orientation of the frame.


I confirmed that we need to do this and that the C++ implementation already does this.

alvestrand · 2024-08-30T08:52:21Z

index.html

+          not sent if {{scaleResolutionDownTo}} is larger than the frame and
+          there exists another {{RTCRtpEncodingParameters/active}} encoding with
+          a smaller {{scaleResolutionDownTo}} density resulting in the same
+          size.</p>


Does this mean that if we have a 480x360 stream, and there are two encodings:
scaleResolutionDownTo(480x360)
scaleResolutionDownTo(1024x768)
the first one (the "smaller density") is the one that will be sent, no matter which order they occur in?
This makes intuitive sense to me. Be careful of the case where the densities are equal - in that case, the current language says that both will be sent.

Correct! I figured if scaleBy allows any order then scaleTo should too. In practice the libwebrtc bitrate allocation logic probably assumes smaller-to-larger but I'm not aware of anything in the spec that says you have to use this order, so I made this "don't send" logic agnostic towards order too

index.html

jan-ivar · 2024-09-05T14:12:56Z

index.html

+        </dt>
+        <dd>
+          <p>The maximum dimensions at which to restrict this encoding.</p>
+          <p>The default "scale resolution down by" factors (e.g. 4:2:1) are not


please rephrase without air quotes

As per other comment we can remove this sentence because we'll say to ignore it later in the text

The issue with the existing scaleResolutionDownBy values is that they effectively have default values (inserted in prose even though it's not in WebIDL since the defaults are dynamic, e.g. 4,2,1.

Unfortunately, webcompat here is poor since pasting this into web console produces 4,2,1 values in Firefox, 1,1,1, in Safari, and undefined, undefined, undefined in Chrome as defaults for scaleResolutionDownBy:

new RTCPeerConnection().addTransceiver("video", {sendEncodings: [{rid: "a"}, {rid: "b"}]}).sender.getParameters().encodings;

Hopefully this means few websites are actually relying on defaults or they're going to have a hard time.

Long-term we should fix the defaults to be better, but in the short-term we cannot rely on them. Better to ignore them I think

This is crbug 344943229 in Chrome and webkit 279202 in Safari.

index.html

henbos · 2024-09-05T14:50:38Z

index.html

+            </li>
+            <li>
+              <p>{{RTCRtpEncodingParameters/scaleResolutionDownBy}} is not
+              specified on any encoding.</p>


Instead of throwing when this is specified, say to ignore it. This is the way to make get+setParameters compatible with the default factors which today are different on different browsers (due to bugs), i.e. undefined on Chrome, 4:2:1 on Firefox and 1:1:1 on Safari

henbos · 2024-09-06T08:39:55Z

The "Validate and Auto Publish" steps are failing due to #217, i.e. there are no issues in this PR

This CL makes `requested_resolution`, which is the C++ name for what the spec calls scaleResolutionDownTo, align with the latest PR[1]. The PR says to ignore scaleResolutionDownBy when scaleResolutionDownTo is specified as to be backwards compatible with scaleResolutionDownBy's default scaling factors (e.g. 4:2:1). Ignoring is different than what the code does today which is to throw an InvalidModificationError. We don't want to throw or else get+setParameters() would throw by default due to 4:2:1 defaults so the app would have to remember to delete these attributes every time even though it never specified them (Chrome has a bug here but fixing that would expose this problem, see https://crbug.com/344943229). [1] w3c/webrtc-extensions#221 Bug: none Change-Id: I21165c9b9f9ee7259d88b89f9ae58b862ea4521e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362260 Commit-Queue: Henrik Boström <hbos@webrtc.org> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> Cr-Commit-Position: refs/heads/main@{#43002}

index.html

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

index.html

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

index.html

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

jan-ivar · 2024-09-12T14:36:19Z

index.html

+          specified resolution restrictions: frames MUST NOT be upscaled. In simulcast, not
+          upscaling can result in multiple same-sized encodings if the size of
+          the frame is smaller than {{scaleResolutionDownTo}}; an encoding MUST
+          NOT be sent if {{scaleResolutionDownTo}} is larger than the frame and
+          there exists another {{RTCRtpEncodingParameters/active}} encoding with
+          a smaller {{scaleResolutionDownTo}} density resulting in the same


Can we defer this part about simulcast to another issue? This is supposed to be a max value which implies lower resolutions should be sent, but here it says the opposite. Might need a bit more discussion

Yes, I filed #222

jan-ivar

approve with simulcast mention removed

Spec: https://w3c.github.io/webrtc-extensions/#dom-rtcrtpencodingparameters-scaleresolutiondownto Wires up scaleResolutionDownTo[1] to JS behind RuntimeEnabled flag "RTCRtpScaleResolutionDownTo". This is implemented in third_party/webrtc where it is called `requested_resolution`. WPTs are added to test basic functionality, including getting the resolution we expect, changing it on the fly, it being orientation agnostic and throwing on invalid parameters. The tests use small resolutions like 120x60 for fast ramp up even on slow bots (sending HD tends to trigger initial frame dropping and slow BW ramp up). The following test coverage is NOT included yet, but will be added in follow up CL(s): - Simulcast tests: to be written in a separate CL for reviewability. - scaleTo maintaining aspect ratio: blocked on a WebRTC fix. [1] w3c/webrtc-extensions#221 Bug: chromium:363544347 Change-Id: If930ffd686d073d2eb239763e9ea9c1390fbcef1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5828607 Reviewed-by: Dominik Röttsches <drott@chromium.org> Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Harald Alvestrand <hta@chromium.org> Cr-Commit-Position: refs/heads/main@{#1355122}

…ResolutionDownTo., a=testonly Automatic update from web-platform-tests Implement RTCRtpEncodingParameters.scaleResolutionDownTo. Spec: https://w3c.github.io/webrtc-extensions/#dom-rtcrtpencodingparameters-scaleresolutiondownto Wires up scaleResolutionDownTo[1] to JS behind RuntimeEnabled flag "RTCRtpScaleResolutionDownTo". This is implemented in third_party/webrtc where it is called `requested_resolution`. WPTs are added to test basic functionality, including getting the resolution we expect, changing it on the fly, it being orientation agnostic and throwing on invalid parameters. The tests use small resolutions like 120x60 for fast ramp up even on slow bots (sending HD tends to trigger initial frame dropping and slow BW ramp up). The following test coverage is NOT included yet, but will be added in follow up CL(s): - Simulcast tests: to be written in a separate CL for reviewability. - scaleTo maintaining aspect ratio: blocked on a WebRTC fix. [1] w3c/webrtc-extensions#221 Bug: chromium:363544347 Change-Id: If930ffd686d073d2eb239763e9ea9c1390fbcef1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5828607 Reviewed-by: Dominik Röttsches <drott@chromium.org> Commit-Queue: Henrik Boström <hbos@chromium.org> Reviewed-by: Harald Alvestrand <hta@chromium.org> Cr-Commit-Position: refs/heads/main@{#1355122} -- wpt-commits: 85dc128fe5ac6c48d09609411808de0939feb31f wpt-pr: 48155

…ResolutionDownTo., a=testonly Automatic update from web-platform-tests Implement RTCRtpEncodingParameters.scaleResolutionDownTo. Spec: https://w3c.github.io/webrtc-extensions/#dom-rtcrtpencodingparameters-scaleresolutiondownto Wires up scaleResolutionDownTo[1] to JS behind RuntimeEnabled flag "RTCRtpScaleResolutionDownTo". This is implemented in third_party/webrtc where it is called `requested_resolution`. WPTs are added to test basic functionality, including getting the resolution we expect, changing it on the fly, it being orientation agnostic and throwing on invalid parameters. The tests use small resolutions like 120x60 for fast ramp up even on slow bots (sending HD tends to trigger initial frame dropping and slow BW ramp up). The following test coverage is NOT included yet, but will be added in follow up CL(s): - Simulcast tests: to be written in a separate CL for reviewability. - scaleTo maintaining aspect ratio: blocked on a WebRTC fix. [1] w3c/webrtc-extensions#221 Bug: chromium:363544347 Change-Id: If930ffd686d073d2eb239763e9ea9c1390fbcef1 Reviewed-on: https://chromium-review.googlesource.com/c/chromium/src/+/5828607 Reviewed-by: Dominik Röttsches <drottchromium.org> Commit-Queue: Henrik Boström <hboschromium.org> Reviewed-by: Harald Alvestrand <htachromium.org> Cr-Commit-Position: refs/heads/main{#1355122} -- wpt-commits: 85dc128fe5ac6c48d09609411808de0939feb31f wpt-pr: 48155 UltraBlame original commit: 2853f7cfdf43cb7e21a4811e96435bfda1edc9fd

Upstream commit: https://webrtc.googlesource.com/src/+/254bd32188e654b2c03b7d9fc29851d5197200b6 Update when/how `requested_resolution` throws for invalid parameters. This CL makes `requested_resolution`, which is the C++ name for what the spec calls scaleResolutionDownTo, align with the latest PR[1]. The PR says to ignore scaleResolutionDownBy when scaleResolutionDownTo is specified as to be backwards compatible with scaleResolutionDownBy's default scaling factors (e.g. 4:2:1). Ignoring is different than what the code does today which is to throw an InvalidModificationError. We don't want to throw or else get+setParameters() would throw by default due to 4:2:1 defaults so the app would have to remember to delete these attributes every time even though it never specified them (Chrome has a bug here but fixing that would expose this problem, see https://crbug.com/344943229). [1] w3c/webrtc-extensions#221 Bug: none Change-Id: I21165c9b9f9ee7259d88b89f9ae58b862ea4521e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362260 Commit-Queue: Henrik Boström <hbos@webrtc.org> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> Cr-Commit-Position: refs/heads/main@{#43002}

Upstream commit: https://webrtc.googlesource.com/src/+/254bd32188e654b2c03b7d9fc29851d5197200b6 Update when/how `requested_resolution` throws for invalid parameters. This CL makes `requested_resolution`, which is the C++ name for what the spec calls scaleResolutionDownTo, align with the latest PR[1]. The PR says to ignore scaleResolutionDownBy when scaleResolutionDownTo is specified as to be backwards compatible with scaleResolutionDownBy's default scaling factors (e.g. 4:2:1). Ignoring is different than what the code does today which is to throw an InvalidModificationError. We don't want to throw or else get+setParameters() would throw by default due to 4:2:1 defaults so the app would have to remember to delete these attributes every time even though it never specified them (Chrome has a bug here but fixing that would expose this problem, see https://crbug.com/344943229). [1] w3c/webrtc-extensions#221 Bug: none Change-Id: I21165c9b9f9ee7259d88b89f9ae58b862ea4521e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362260 Commit-Queue: Henrik Boström <hboswebrtc.org> Reviewed-by: Ilya Nikolaevskiy <ilnikwebrtc.org> Cr-Commit-Position: refs/heads/main{#43002} UltraBlame original commit: e52d0986d32051a560d60180443c22fefc1efce0

Upstream commit: https://webrtc.googlesource.com/src/+/254bd32188e654b2c03b7d9fc29851d5197200b6 Update when/how `requested_resolution` throws for invalid parameters. This CL makes `requested_resolution`, which is the C++ name for what the spec calls scaleResolutionDownTo, align with the latest PR[1]. The PR says to ignore scaleResolutionDownBy when scaleResolutionDownTo is specified as to be backwards compatible with scaleResolutionDownBy's default scaling factors (e.g. 4:2:1). Ignoring is different than what the code does today which is to throw an InvalidModificationError. We don't want to throw or else get+setParameters() would throw by default due to 4:2:1 defaults so the app would have to remember to delete these attributes every time even though it never specified them (Chrome has a bug here but fixing that would expose this problem, see https://crbug.com/344943229). [1] w3c/webrtc-extensions#221 Bug: none Change-Id: I21165c9b9f9ee7259d88b89f9ae58b862ea4521e Reviewed-on: https://webrtc-review.googlesource.com/c/src/+/362260 Commit-Queue: Henrik Boström <hbos@webrtc.org> Reviewed-by: Ilya Nikolaevskiy <ilnik@webrtc.org> Cr-Commit-Position: refs/heads/main@{#43002}

Add restrictedResolution to encoding parameters.

0d4a28b

henbos requested a review from jan-ivar August 29, 2024 09:28

alvestrand reviewed Aug 29, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

alvestrand reviewed Aug 29, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

alvestrand reviewed Aug 29, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

henbos added 2 commits August 29, 2024 13:56

r/restrictedResolution/resolutionRestriction

b9444fb

Addressed comments

cbc61eb

henbos changed the title ~~Add restrictedResolution (aka scaleResolutionDownTo) to RTCRtpEncodingParameters~~ Add resolutionRestriction (aka scaleResolutionDownTo) to RTCRtpEncodingParameters Aug 29, 2024

scaleResolutionDownTo, remove orientation talk, clarify not sending

046e1ea

henbos changed the title ~~Add resolutionRestriction (aka scaleResolutionDownTo) to RTCRtpEncodingParameters~~ Add scaleResolutionDownTo to RTCRtpEncodingParameters Aug 30, 2024

henbos added 3 commits August 30, 2024 10:32

Orientation agnostic

3adc13b

Orientation agnostic

e3cdde9

Rewording

9e70936

henbos commented Aug 30, 2024

View reviewed changes

alvestrand reviewed Aug 30, 2024

View reviewed changes

r/throw/reject

2685fad

jan-ivar reviewed Sep 5, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

jan-ivar reviewed Sep 5, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

henbos commented Sep 5, 2024

View reviewed changes

Address comments

d8cbcc6

OperationError vs InvalidModification

5846328

jan-ivar reviewed Sep 12, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

jan-ivar reviewed Sep 12, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

jan-ivar reviewed Sep 12, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

jan-ivar reviewed Sep 12, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

henbos and others added 2 commits September 12, 2024 16:21

Update index.html

1794afa

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Update index.html

02637b1

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

jan-ivar reviewed Sep 12, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

henbos and others added 3 commits September 12, 2024 16:21

Update index.html

555172f

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Update index.html

6f0ae22

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

Update index.html

ccd925e

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

jan-ivar reviewed Sep 12, 2024

View reviewed changes

index.html Outdated Show resolved Hide resolved

Update index.html

df349f7

Co-authored-by: Jan-Ivar Bruaroey <jan-ivar@users.noreply.github.com>

jan-ivar approved these changes Sep 12, 2024

View reviewed changes

jan-ivar reviewed Sep 12, 2024

View reviewed changes

jan-ivar approved these changes Sep 12, 2024

View reviewed changes

jan-ivar added the editors can integrate label Sep 12, 2024

henbos mentioned this pull request Sep 12, 2024

What happens if scaleTo: {360p, 720p} and frame is 360 #222

Closed

Remove part about not sending same size twice

0ec01b0

henbos merged commit 20a6eb2 into w3c:main Sep 12, 2024
1 of 2 checks passed

chromium-wpt-export-bot mentioned this pull request Sep 13, 2024

Implement RTCRtpEncodingParameters.scaleResolutionDownTo. web-platform-tests/wpt#48155

Merged

henbos mentioned this pull request Sep 16, 2024

nit: r/are/MUST be downscaled #223

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add scaleResolutionDownTo to RTCRtpEncodingParameters #221

Add scaleResolutionDownTo to RTCRtpEncodingParameters #221

henbos commented Aug 29, 2024 •

edited by pr-preview bot

Loading

henbos Aug 30, 2024 •

edited

Loading

alvestrand Aug 30, 2024

henbos Aug 30, 2024

jan-ivar Sep 5, 2024

henbos Sep 5, 2024

jan-ivar Sep 5, 2024

jan-ivar Sep 5, 2024

henbos Sep 6, 2024

henbos Sep 5, 2024

henbos Sep 6, 2024

henbos commented Sep 6, 2024

jan-ivar Sep 12, 2024

henbos Sep 12, 2024

jan-ivar left a comment

Add scaleResolutionDownTo to RTCRtpEncodingParameters #221

Add scaleResolutionDownTo to RTCRtpEncodingParameters #221

Conversation

henbos commented Aug 29, 2024 • edited by pr-preview bot Loading

henbos Aug 30, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

henbos commented Sep 6, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

jan-ivar left a comment

Choose a reason for hiding this comment

henbos commented Aug 29, 2024 •

edited by pr-preview bot

Loading

henbos Aug 30, 2024 •

edited

Loading