1241153 - Find some way to make setParameters and GMP init less fragile

Status: RESOLVED FIXED

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

Description

[Byron Campen [:bwc]]

Right now, setParameters will not work if GMP is not initted yet, since addTrack will not have completed. We need to work out some way of handling this better.

Comment 1

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

What does this block (or make a pain)?

Comment 2

[Byron Campen [:bwc]]

I can think of several ways to do this for setParameters, but I think getParameters is going to be harder. getParameters is synchronous, and since we cannot init GMP synchronously, we are left with a few options:

1. Ensure GMP is initted before a PeerConnection is ever created (basically init GMP on startup).

   This ensures that content JS has an accurate picture of what is supported, allows us to remove a bunch of complexity, but is maybe wasteful of resources.

2. Create tracks with the full list of codecs we _might_ support, and if we don't end up supporting H264 or something, oh well we tried.

   This is pretty efficient, allows content JS to tweak codec params without a hacky wait on GMP init, and simplifies the code a little. However, it is kinda lame that we advertise stuff to content JS that we don't know we support yet.

3. Create tracks with only what we know we support, and update things once GMP init is done.

   This is again pretty efficient, and allows for some simplification of existing code. However, content JS still needs to do a hacky wait on GMP init to figure out the full list of codecs.

Comment 3

[Byron Campen [:bwc]]

I suppose a fourth option is to change the W3C spec to make addTrack async.

Comment 4

[Byron Campen [:bwc]]

What do you guys think about these options?

Comment 5

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

How often does the information from GMP change? Could we perhaps cache information from last GMP was up? This might make a version of #2 bearable, where we return cached information from last boot until the GMP has loaded on this boot. That should make the information stable most of the time, right? We could even do #1 on the very first launch of a clean profile.

Comment 6

[Byron Campen [:bwc]]

(In reply to Jan-Ivar Bruaroey [:jib] from comment #5)
> How often does the information from GMP change? Could we perhaps cache
> information from last GMP was up? This might make a version of #2 bearable,
> where we return cached information from last boot until the GMP has loaded
> on this boot. That should make the information stable most of the time,
> right? We could even do #1 on the very first launch of a clean profile.

   This information changes pretty infrequently I would think. Caching might work most of the time, but if the performance characteristics of option 1 aren't too onerous that would be my preference.

Comment 7

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

Wfm. Caching may be an additional optimization we could pursue if option 1 gets blowback.

Comment 8

[Byron Campen [:bwc]]

I could also live with option 2, and don't think we'd see much push-back.

Comment 9

[Byron Campen [:bwc]]

Attachment: MozReview Request: Bug 1241153: Stop blocking addTrack on GMP init, and update codecs on existing tracks when GMP init finishes. r?jesup

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

Comment 10

[Byron Campen [:bwc]]

Comment on attachment 8718970 [details]
MozReview Request: Bug 1241153: Stop blocking addTrack on GMP init, and update codecs on existing tracks when GMP init finishes. r?jesup

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

Comment 11

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

Comment on attachment 8718970 [details]
MozReview Request: Bug 1241153: Stop blocking addTrack on GMP init, and update codecs on existing tracks when GMP init finishes. r?jesup

https://reviewboard.mozilla.org/r/34817/#review32151

Comment 12

[Pulsebot]

https://hg.mozilla.org/integration/mozilla-inbound/rev/c1ce4aaa3587

Comment 13

[Carsten Book [:Tomcat]]

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