1210660 - Change vp8 threshold for static images in screensharing mode

Status: RESOLVED FIXED

(Core :: WebRTC, defect, P2)

Description

[Randell Jesup [:jesup] (needinfo me)]

https://code.google.com/p/webrtc/issues/detail?id=5015

This is for cherry-picking the patch into our tree to improve screensharing, especially for 'static' content like slides.

Comment 1

[Randell Jesup [:jesup] (needinfo me)]

Attachment: upstream patch

FYI, I checked to be sure and we do set kScreensharing in MediaPipelineFactory.cpp

Comment 2

[Dan Minor [:dminor]]

Attachment: Bug 1210660 - Change vp8 threshold for static images in screensharing mode;

This changes the static threshold in screensharing mode and ensures that the
screensharing mode is in fact passed to the codec.

This also causes the peer connection to update the media pipelines when a track is
replaced to cause the codec to be notified that the source has changed and to
change settings appropriately. It seems to be a common use case to have a camera
track be replaced by a screenshare track during a call.

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

Comment 3

[Dan Minor [:dminor]]

Try run here: https://treeherder.mozilla.org/#/jobs?repo=try&revision=dc5727270eb376d9803b76cf4fc113e318a72188

Comment 4

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8762090 [details]
Bug 1210660 - Change vp8 threshold for static images in screensharing mode;

https://reviewboard.mozilla.org/r/58974/#review55976

Comment 5

[Dan Minor [:dminor]]

Comment on attachment 8762090 [details]
Bug 1210660 - Change vp8 threshold for static images in screensharing mode;

Review request updated; see interdiff: https://reviewboard.mozilla.org/r/58974/diff/1-2/

Comment 6

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/0ff8e745ca48
Change vp8 threshold for static images in screensharing mode; r=jesup

Comment 7

[Carsten Book [:Tomcat]]

https://hg.mozilla.org/mozilla-central/rev/0ff8e745ca48

Comment 8

[Maire Reavy [:mreavy]]

Randell -- Since Dan is on PTO, can you ask for uplift on this to Fx 49?

Comment 9

[Randell Jesup [:jesup] (needinfo me)]

Comment on attachment 8762090 [details]
Bug 1210660 - Change vp8 threshold for static images in screensharing mode;

Approval Request Comment
[Feature/regressing bug #]: N/A

[User impact if declined]: greatly increased bandwidth use (or lower quality for other streams) when sharing static or mostly-static screens/windows/etc.

[Describe test coverage new/current, TreeHerder]: Manual testing.  Used upstream by Chrome; this is an port over of what they did.

[Risks and why]: extremely low risk; just bookkeeping to know if it's a screenshare, and a parameter adjustment to the encoder.

[String/UUID change made/needed]: none

Comment 10

[Gerry Chang [:gchang]]

Comment on attachment 8762090 [details]
Bug 1210660 - Change vp8 threshold for static images in screensharing mode;

This patch improves the bandwidth when sharing static images. Take it in aurora.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/releases/mozilla-aurora/rev/ad0fd95d4831