1359668 - Screen-sharing track.getSettings() should reflect actual video width and height once frames come in.

Status: RESOLVED FIXED

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

Description

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

(A follow-up to bug 1359662) From my best reading of the mediacapture-main spec [1] track.getSettings() should return the device's current resolution in width and height.

For screen-sharing this would I suppose translate to the actual dimensions of the shared screen/window (after being proportionally scaled to fit within any max constraints).

STR:
 1. Open https://jsfiddle.net/jib1/77nqay6w/
 2. Pick Screen to share: "Entire screen", and share screen.
 3. Click the button named "160".

Expected result: Applied 160 x 120 got 176 x 110

(where 176 x 110 are the numbers that match the live video.videoWidth and video.videoHeight)

Actual result (after bug 1359662 fixed): Applied 160 x 120 got 160 x 120

The tricky part is that for screensharing we don't know the source dimensions until the first frame arrives. I see a couple of options:

 A) Return 160 x 120 until the first frame arrives, then 176 x 110 and update live from then on.

    (yes, resizing a shared window changes the video dimensions dynamically!)

 B) Somehow delay gUM success until frames arrive, or do a dry-run somehow.

I thinking (A) might suffice, and that people wait for video.onloadedmetadata.



The reason I broke this out as a separate bug is it's a bit tricky. See original patch [2] for background.

In short, dst_width and dst_height are known on the incoming frame on a render thread here [3].
We probably need to register some form of callback [4] to get this data out to the media thread where we send a runnable to main thread with the values (see code in bug 1359662).

[1] https://w3c.github.io/mediacapture-main/getusermedia.html#dom-constrainablepattern-getsettings()
[2] https://bugzilla.mozilla.org/page.cgi?id=splinter.html&bug=1211656&attachment=8683444
[3] https://dxr.mozilla.org/mozilla-central/rev/f229b7e5d91eb70d23d3e31db7caff9d69a2ef04/media/webrtc/trunk/webrtc/video_engine/desktop_capture_impl.cc#703
[4] https://dxr.mozilla.org/mozilla-central/rev/f229b7e5d91eb70d23d3e31db7caff9d69a2ef04/media/webrtc/trunk/webrtc/video_engine/desktop_capture_impl.cc#533

Comment 1

[Munro Mengjue Chiang [:mchiang] Be Mozillian since 1/19/2018]

Attachment: Bug 1359668 - update mSettings.mWidth and mSettings.mHeight when there is a frame size change.

Review commit: https://reviewboard.mozilla.org/r/134656/diff/#index_header
See other reviews: https://reviewboard.mozilla.org/r/134656/

Comment 2

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

Comment on attachment 8862776 [details]
Bug 1359668 - update mSettings.mWidth and mSettings.mHeight when there is a frame size change.

https://reviewboard.mozilla.org/r/134656/#review138104

Great! That was a lot simpler than I thought. Thanks! :)

Comment 3

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/cff101ea16af
update mSettings.mWidth and mSettings.mHeight when there is a frame size change. r=jib

Comment 4

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/cff101ea16af