1780310 - scaleResolutionDownBy only produces a few coarse number of dimensions

Status: VERIFIED FIXED

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

Description

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

This doesn't appear to be a regression.

STRs:

1. Open https://jsfiddle.net/jib1/02j8q6h9/show and uncheck ☑ h264 (or not)
2. Click gUM or gDM to test with camera or screen share respectively
3. Drag the "Downscale" slider back and forth slowly (pause occasionally to give the text time to update)

Expected result (matching Chrome):

• The size of the left-most ("Receiver") video shrinks and grows smoothly

Actual result:

• The size of the left-most ("Receiver") video stays the same for a while until the slider is moved far enough when it shrinks or grows significantly in steps and jumps. Only certain sizes seem possible, which seems quite limiting:
  • For mac screen-share of 3072x1920 I only see these sizes (height): 960, 720, 480, 240, 180, ... etc.
  • For mac camera at 1280x720 I only see these sizes (height): 720, 540, 360, 270, ... etc.
  • For windows camera or screen-share at 1920x1080 I only see these sizes (height): 1080, 810, 540, 405, 270, 201... etc.

It's possible a newer update of libwebrtc may resolve this, but I wasn't able to test with https://treeherder.mozilla.org/jobs?repo=elm as I only see opt builds for linux there for some reason, and I don't have a linux box to test with (the debug builds won't cut it for this test)

Comment 1

[Jan-Ivar Bruaroey [:jib] (needinfo? me)]

I was able to get a macOS opt build, but other bugs prevented the test from succeeding (no receiver-side video output). I've already marked this bug as blocking on the libwebrtc fast forwarding project, and will look in on this when it has progressed to where this test can run unhindered.

Comment 2

[Andreas Pehrson [:pehrsons]]

Here is a recording: https://pernos.co/debug/f6fBr3JUB3I59Ef-OS2hSA/index.html
STR was:

• Uncheck h264
• Click gUM!
• Wait for the receiver to get full resolution, 1080p
• Slide the scaling slider slowly all the way to the left
• Close the window

Comment 3

[Dan Baker[:dbaker]]

Attachment: Bug 1780310 - POC testing handling scaleResolutionDownBy value outside libwebrtc.;r?pehrsons

Comment 4

[Dan Baker[:dbaker]]

From the looks of things VideoAdapter::FindScale is only going to give us a few resolutions. This seems to be the same in Chromium's code base https://source.chromium.org/chromium/chromium/src/+/main:third_party/webrtc/media/base/video_adapter.cc;l=99?q=videoadapter. It could be possible for us to look into doing work to implement scaling outside of libwebrtc as the POC patch can demonstrate. I'm unsure of full scope to properly implement improved handling of scaleResolutionDownBy and could use advice.

Comment 5

[Andreas Pehrson [:pehrsons]]

We should check whether libwebrtc can do the scaling for us first.
The VideoAdapter we are using for scaling currently may be libwebrtc code but we are using it in our code. Chromium may use something else for determining how to scale the frames.

Note that when VideoConduit does either: 1) call ReconfigureVideoEncoder or 2) pass a frame to libwebrtc of a different resolution than the last passed frame, then libwebrtc will ask the VideoStreamFactory (supplied by us) to CreateEncoderStreams.

So in order to check whether libwebrtc can handle scaling things, you need to:

• Let VideoConduit pass the frame straight through, but call ReconfigureVideoEncoder as appropriate (on scaleDownBy changes for instance, if we don't already)
• Let VideoStreamFactory calculate the scaling by spec and pass those streams to libwebrtc

To check whether this works, we first of all need to check it with simulcast. A single stream should work already, but the limitations in older upstream were with simulcast.
Running all tests takes a while, so start with the simulcast mochitests and gtests:

• ./mach mochitest dom/media/webrtc/tests/mochitest/test_peerConnection_simulcast --headless
• ./mach gtest *VideoConduit*

Comment 6

[Dan Baker[:dbaker]]

Attachment: Bug 1780310 - Let libwebrtc not VideoConduit be responsible for scaling video frames for encoding;r=pehrsons!

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/f802366520fd
Let libwebrtc not VideoConduit be responsible for scaling video frames for encoding;r=pehrsons

Comment 8

[Cristian Tuns]

Backed out for causing GTest failures in VideoConduitTest

• Backout link
• Push with failures
• Failure Log
• Failure line: TEST-UNEXPECTED-FAIL | VideoConduitTest.TestVideoEncodeMaxWidthAndHeight | Expected equality of these values:
  TEST-UNEXPECTED-FAIL | VideoConduitTest.TestVideoEncodeMaxWidthAndHeight | test completed (time: 3ms)

Comment 9

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/03d3f61f80e6
Let libwebrtc not VideoConduit be responsible for scaling video frames for encoding;r=pehrsons

Comment 10

[Iulian Moraru]

https://hg.mozilla.org/mozilla-central/rev/03d3f61f80e6

Comment 11

[Bogdan Maris, Desktop QA]

Reproduced the initial issue using an old Nightly from 2022-07-19, verified that using latest Firefox 108.0b7 the resize of the Receiver box is made smoothly on various resolution size (by changing the Downscale bar) for both gUM and gDM across platforms (Windows 10, macOS 11.6 and Ubuntu 18.04).