Adding support for RTCRtpEncodingParameters.active flag.

Adding support for RTCRtpEncodingParameters.active flag.

This will allow a sender to stop/start sending media on the
application's demand.

Among other things, this can allow an application to set a track on a
sender while the encoding(s) are inactive, allowing the encoder to be
initialized for that track, then later set the encodings to "active"
to instantly start sending the track.

Committed: https://crrev.com/dbe2b8744f77b630af36f4ffc436b55f90e121f0
Cr-Commit-Position: refs/heads/master@{#12094}

[Taylor Brandstetter, 2016-03-22 00:43:37]

deadbeef@webrtc.org changed reviewers:
+ pbos@webrtc.org, pthatcher@webrtc.org

[Taylor Brandstetter, 2016-03-22 00:43:38]

PTAL.

[pbos-webrtc, 2016-03-22 18:13:43]

https://codereview.webrtc.org/1822923002/diff/20001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/1822923002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:1990: rtc::CritScope cs(&lock_);
Don't take the lock here. (see header comment)

Also put a TODO on that encodings[>0].active should be handled in the future.

https://codereview.webrtc.org/1822923002/diff/20001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.h (right):

https://codereview.webrtc.org/1822923002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.h:260: rtc::CritScope cs(&lock_);
This became non-trivial, move to .cc file.

https://codereview.webrtc.org/1822923002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.h:274: void UpdateSendState();
put EXCLUSIVE_LOCKS_REQUIRED(lock_) before ; to have it only callable from under
a lock.

Also move this down to around line 340 or so where other methods are (internal
classes/structs are declared before methods)

[Taylor Brandstetter, 2016-03-22 18:32:43]

https://codereview.webrtc.org/1822923002/diff/20001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/1822923002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:1990: rtc::CritScope cs(&lock_);
On 2016/03/22 18:13:43, pbos-webrtc wrote:
> Don't take the lock here. (see header comment)
> 
> Also put a TODO on that encodings[>0].active should be handled in the future.

Done.

https://codereview.webrtc.org/1822923002/diff/20001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.h (right):

https://codereview.webrtc.org/1822923002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.h:260: rtc::CritScope cs(&lock_);
On 2016/03/22 18:13:43, pbos-webrtc wrote:
> This became non-trivial, move to .cc file.

Done.

https://codereview.webrtc.org/1822923002/diff/20001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.h:274: void UpdateSendState();
On 2016/03/22 18:13:43, pbos-webrtc wrote:
> put EXCLUSIVE_LOCKS_REQUIRED(lock_) before ; to have it only callable from
under
> a lock.
> 
> Also move this down to around line 340 or so where other methods are (internal
> classes/structs are declared before methods)

Somehow I didn't notice the other methods and that macro, oops.

[pbos-webrtc, 2016-03-22 18:35:45]

lgtm, thanks

make sure this runs under tsan so we don't have any lock-order inversions due to
some new stuff being taken under two locks

https://codereview.webrtc.org/1822923002/diff/40001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/1822923002/diff/40001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:1891: if (sending_ &&
rtp_parameters_.encodings[0].active) {
Can you also DCHECK that !encodings.empty()?

[Taylor Brandstetter, 2016-03-22 19:05:38]

I also removed a DCHECK that shouldn't have been there in the first place (it
was moved to UpdateSendState but I forgot to delete it).

https://codereview.webrtc.org/1822923002/diff/40001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/1822923002/diff/40001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:1891: if (sending_ &&
rtp_parameters_.encodings[0].active) {
On 2016/03/22 18:35:45, pbos-webrtc wrote:
> Can you also DCHECK that !encodings.empty()?

Done.

[pbos-webrtc, 2016-03-22 19:11:03]

ltgm

[pbos-webrtc, 2016-03-22 19:11:26]

On 2016/03/22 19:11:03, pbos-webrtc wrote:
> ltgm

looks too good (for) me..? :\

lgtm

[pthatcher1, 2016-03-22 19:46:22]

lgtm

[Taylor Brandstetter, 2016-03-22 22:40:22]

The CQ bit was checked by deadbeef@webrtc.org

[commit-bot: I haz the power, 2016-03-22 22:40:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1822923002/60001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1822923002/60001

[commit-bot: I haz the power, 2016-03-22 22:42:04]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-03-22 22:42:12]

Description was changed from

==========
Adding support for RTCRtpEncodingParameters.active flag.

This will allow a sender to stop/start sending media on the
application's demand.

Among other things, this can allow an application to set a track on a
sender while the encoding(s) are inactive, allowing the encoder to be
initialized for that track, then later set the encodings to "active"
to instantly start sending the track.
==========

to

==========
Adding support for RTCRtpEncodingParameters.active flag.

This will allow a sender to stop/start sending media on the
application's demand.

Among other things, this can allow an application to set a track on a
sender while the encoding(s) are inactive, allowing the encoder to be
initialized for that track, then later set the encodings to "active"
to instantly start sending the track.

Committed: https://crrev.com/dbe2b8744f77b630af36f4ffc436b55f90e121f0
Cr-Commit-Position: refs/heads/master@{#12094}
==========

[commit-bot: I haz the power, 2016-03-22 22:42:13]

Patchset 4 (id:??) landed as
https://crrev.com/dbe2b8744f77b630af36f4ffc436b55f90e121f0
Cr-Commit-Position: refs/heads/master@{#12094}