Remove contention between RTCP packets and encoding.

Remove contention between RTCP packets and encoding.

Receiving RTCP often caused the worker thread to stall for >20 ms
(>100ms observed) due to contention on VideoSender's send_crit_ (used to
protect encoding).

This change removes an unnecessary acquire of send_crit_ and caches
encoder settings in ViEEncoder instead of acquiring them through
::SendCodec() in VCM (which is blocking).

BUG=webrtc:5106
R=stefan@webrtc.org

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

[pbos-webrtc, 2015-11-09 15:11:38]

Description was changed from

==========
Remove contention between RTCP packets and encoding.

Receiving RTCP often caused the worker thread to stall for >20 ms
(>100ms observed) due to contention on VideoSender's send_crit_ used to
protect encoding.

This change removes an unnecessary acquire of send_crit_ and caches
encoder settings in ViEEncoder instead of acquiring them through
::SendCodec() in VCM (which is blocking).

BUG=webrtc:5106
R=stefan@webrtc.org
==========

to

==========
Remove contention between RTCP packets and encoding.

Receiving RTCP often caused the worker thread to stall for >20 ms
(>100ms observed) due to contention on VideoSender's send_crit_ (used to
protect encoding).

This change removes an unnecessary acquire of send_crit_ and caches
encoder settings in ViEEncoder instead of acquiring them through
::SendCodec() in VCM (which is blocking).

BUG=webrtc:5106
R=stefan@webrtc.org
==========

[pbos-webrtc, 2015-11-09 15:12:19]

PTAL, I'll run all tests locally before poking CQ (dry).

[pbos-webrtc, 2015-11-09 16:19:52]

fix bug + add EXPECT_ instead of printf error handling

[pbos-webrtc, 2015-11-09 16:41:25]

simplify fec/nack protection setting

[pbos-webrtc, 2015-11-09 16:55:34]

make sure encoder_config_ is set before adding bitrate observers, to update
based on current simulcast settings

[pbos-webrtc, 2015-11-09 16:57:18]

OK, PTAL. Hope this didn't go too far out of scope, I wanted to remove the
blocking SendCodec() calls completely.

[pbos-webrtc, 2015-11-09 16:57:25]

The CQ bit was checked by pbos@webrtc.org to run a CQ dry run

[commit-bot: I haz the power, 2015-11-09 16:57:38]

Dry run: CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1433703002/50001
View timeline at
 https://chromium-cq-status.appspot.com/patch-timeline/1433703002/50001

[commit-bot: I haz the power, 2015-11-09 18:40:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2015-11-09 18:40:21]

Dry run: This issue passed the CQ dry run.

[pbos-webrtc, 2015-11-10 15:08:14]

comment for SetProtectionMethod

[stefan-webrtc, 2015-11-10 15:15:26]

lgtm

https://codereview.webrtc.org/1433703002/diff/50001/webrtc/video/video_send_s...
File webrtc/video/video_send_stream.cc (right):

https://codereview.webrtc.org/1433703002/diff/50001/webrtc/video/video_send_s...
webrtc/video/video_send_stream.cc:199:
vie_encoder_->SetProtectionMethod(enable_protection_nack,
Why did you move this? If it needs to be here it should be commented on.

https://codereview.webrtc.org/1433703002/diff/50001/webrtc/video_engine/vie_e...
File webrtc/video_engine/vie_encoder.cc (left):

https://codereview.webrtc.org/1433703002/diff/50001/webrtc/video_engine/vie_e...
webrtc/video_engine/vie_encoder.cc:485: // The send codec must be registered to
set correct MTU.
So now the user of this class needs to make sure it calls SetSendCodec after
calling SetProtectionMethod?

https://codereview.webrtc.org/1433703002/diff/70001/webrtc/video_engine/vie_e...
File webrtc/video_engine/vie_encoder.h (right):

https://codereview.webrtc.org/1433703002/diff/70001/webrtc/video_engine/vie_e...
webrtc/video_engine/vie_encoder.h:102: // updated according to protection.
Add a TODO to move to construction, or in some way getting rid of this
dependency.

[pbos-webrtc, 2015-11-10 15:18:27]

added TODO in comment

[pbos-webrtc, 2015-11-10 15:18:52]

https://codereview.webrtc.org/1433703002/diff/50001/webrtc/video/video_send_s...
File webrtc/video/video_send_stream.cc (right):

https://codereview.webrtc.org/1433703002/diff/50001/webrtc/video/video_send_s...
webrtc/video/video_send_stream.cc:199:
vie_encoder_->SetProtectionMethod(enable_protection_nack,
On 2015/11/10 15:15:26, stefan-webrtc (holmer) wrote:
> Why did you move this? If it needs to be here it should be commented on.

Didn't move it, the call below was a no-op but there is a call here currently.

https://codereview.webrtc.org/1433703002/diff/50001/webrtc/video_engine/vie_e...
File webrtc/video_engine/vie_encoder.cc (left):

https://codereview.webrtc.org/1433703002/diff/50001/webrtc/video_engine/vie_e...
webrtc/video_engine/vie_encoder.cc:485: // The send codec must be registered to
set correct MTU.
On 2015/11/10 15:15:26, stefan-webrtc (holmer) wrote:
> So now the user of this class needs to make sure it calls SetSendCodec after
> calling SetProtectionMethod? 

Yep, commented.

https://codereview.webrtc.org/1433703002/diff/70001/webrtc/video_engine/vie_e...
File webrtc/video_engine/vie_encoder.h (right):

https://codereview.webrtc.org/1433703002/diff/70001/webrtc/video_engine/vie_e...
webrtc/video_engine/vie_encoder.h:102: // updated according to protection.
On 2015/11/10 15:15:26, stefan-webrtc (holmer) wrote:
> Add a TODO to move to construction, or in some way getting rid of this
> dependency.

Done.

[pbos-webrtc, 2015-11-10 15:27:27]

Committed patchset #6 (id:90001) manually as
d153a37801d9b1258ffff9f878064fa0ddfd3b0a (presubmit successful).

[commit-bot: I haz the power, 2015-11-10 15:27:27]

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