1326442 - VideoConduit code should simply reconfigure the VideoSendStream when possible on a configuration change

Status: RESOLVED FIXED

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

Description

[Paul Kerr [:pkerr]]

The VideoConduit module has been updated to work with the new Call API found in the branch 49 version of the webrtc.org media stack. In its current form, the Encoder is stopped, re-created, and started for even small changes in codec configuration. When the changes can be handled by the VideoSendStream::ReconfigureVideoEncoder method the update should be made using this mechanism without destroying the current VideoSendStream. Otherwise, a new VideoSendStream will need to be created.

Comment 1

[Paul Kerr [:pkerr]]

Attachment: All fast SendCodec reconfigure when appropriate

Comment 2

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

I believe resolution changes can all be handled without destroying.  Some parts of this have already landed as part of fixing ConfigureSendMediaCodec() to install the new codec params.

Comment 3

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

Attachment: All fast SendCodec reconfigure when appropriate

rebased, and rather modified.  Also I'm assuming we don't need to destroy on a resolution change - Chrome does, but they hadn't fixed the vp8/vp9 things that break on resolution changes - we have

Comment 4

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8828361 [details] [diff] [review]
All fast SendCodec reconfigure when appropriate

Review of attachment 8828361 [details] [diff] [review]:
-----------------------------------------------------------------

One issue with duplicated call.

::: media/webrtc/signaling/src/media-conduit/VideoConduit.cpp
@@ +628,5 @@
> +    if (!RequiresNewSendStream(*codecConfig)) {
> +      if (!mSendStream->ReconfigureVideoEncoder(mEncoderConfig.GenerateConfig())) {
> +        CSFLogError(logTag, "%s: ReconfigureVideoEncoder failed", __FUNCTION__);
> +        // This will cause a new encoder to be created by StartTransmitting()
> +        condError = StopTransmitting();

This is redundant with the StopTransmitting call at line 643.

Comment 5

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

Attachment: All fast SendCodec reconfigure when appropriate

yup, removed that (though stopping twice doesn't hurt)

Comment 6

[Nico Grunbaum [:ng, @chew:mozilla.org]]

Comment on attachment 8828539 [details] [diff] [review]
All fast SendCodec reconfigure when appropriate

Review of attachment 8828539 [details] [diff] [review]:
-----------------------------------------------------------------

r+, pending try

Comment 7

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/mozilla-inbound/rev/cdc8e1a140e2
Use reconfigure whenever possible on video renegotiation r=ng

Comment 8

[Carsten Book [:Tomcat]]

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