Add |protected_by_flexfec| flag to VideoReceiveStream::Config.

Add |protected_by_flexfec| flag to VideoReceiveStream::Config.

Use of FlexFEC is known when streams are created in
WebRtcVideoChannel2, so this replaces the code in Call to infer
FlexFEC config of video streams from the configuration of the FlexFEC
stream(s). This also allows us to switch to a more logical creation
order, where media streams are created before the FlexFEC stream.

This is done in preparation for a larger refactoring of the RTP
demuxing done in Call.

BUG=None

Review-Url: https://codereview.webrtc.org/2712683002
Cr-Commit-Position: refs/heads/master@{#17143}
Committed: https://chromium.googlesource.com/external/webrtc/+/c69385de8b614b7edba4f640bca6aa0963e1505c

[nisse-webrtc, 2017-02-22 13:20:36]

nisse@webrtc.org changed reviewers:
+ brandtr@webrtc.org

[nisse-webrtc, 2017-02-22 13:20:37]

PTAL. Surprisingly, no need to update tests have surfaced yet.

[brandtr, 2017-02-23 11:58:32]

lgtm!

https://codereview.webrtc.org/2712683002/diff/1/webrtc/media/engine/webrtcvid...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/2712683002/diff/1/webrtc/media/engine/webrtcvid...
webrtc/media/engine/webrtcvideoengine2.cc:2302: // the argument to
CreateVideoReceiveStream a const ref?
Agree that this should be cleaned up at some point. Right now, its a mixture of
copies and moves in the code that has to do with the VideoReceiveStream::Config.

https://codereview.webrtc.org/2712683002/diff/1/webrtc/video_receive_stream.h
File webrtc/video_receive_stream.h (right):

https://codereview.webrtc.org/2712683002/diff/1/webrtc/video_receive_stream.h...
webrtc/video_receive_stream.h:150: // Set if the stream is protected using
flexfec.
Nit: FlexFEC.

[nisse-webrtc, 2017-02-23 13:01:22]

nisse@webrtc.org changed reviewers:
+ stefan@webrtc.org

[nisse-webrtc, 2017-02-23 13:01:22]

Stefan, PTAL.

https://codereview.webrtc.org/2712683002/diff/1/webrtc/media/engine/webrtcvid...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/2712683002/diff/1/webrtc/media/engine/webrtcvid...
webrtc/media/engine/webrtcvideoengine2.cc:2302: // the argument to
CreateVideoReceiveStream a const ref?
On 2017/02/23 11:58:32, brandtr wrote:
> Agree that this should be cleaned up at some point. Right now, its a mixture
of
> copies and moves in the code that has to do with the
VideoReceiveStream::Config.

I noticed a std::move(configuration) in Call::CreateVideoReceiveStream, 
but it's not clear to me if it makes any difference.

Is there any easy way to eliminate at least the second Copy call here?

[brandtr, 2017-02-23 15:28:35]

https://codereview.webrtc.org/2712683002/diff/1/webrtc/media/engine/webrtcvid...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/2712683002/diff/1/webrtc/media/engine/webrtcvid...
webrtc/media/engine/webrtcvideoengine2.cc:2302: // the argument to
CreateVideoReceiveStream a const ref?
On 2017/02/23 13:01:22, nisse-webrtc wrote:
> On 2017/02/23 11:58:32, brandtr wrote:
> > Agree that this should be cleaned up at some point. Right now, its a mixture
> of
> > copies and moves in the code that has to do with the
> VideoReceiveStream::Config.
> 
> I noticed a std::move(configuration) in Call::CreateVideoReceiveStream, 
> but it's not clear to me if it makes any difference.
> 
> Is there any easy way to eliminate at least the second Copy call here?

I guess that std::move saves a copy at that call site.

I'm not sure if removing the copy here would change the logic overall in any
way. It might be good to be careful, it's my experience that it is fairly easy
to introduce bugs -- that are not found by tests -- in the configuration at this
layer.

[stefan-webrtc, 2017-03-09 13:08:30]

lgtm

[nisse-webrtc, 2017-03-09 13:09:39]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2017-03-09 13:09:39]

The patchset sent to the CQ was uploaded after l-g-t-m from brandtr@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2712683002/#ps20001 (title:
"CapitalizaTION fix.")

[commit-bot: I haz the power, 2017-03-09 13:09:46]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-03-09 13:12:15]

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

[commit-bot: I haz the power, 2017-03-09 13:12:17]

Try jobs failed on following builders:
  linux_memcheck on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_memcheck/builds/6093)
  linux_ubsan_vptr on master.tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_ubsan_vptr/builds...)

[nisse-webrtc, 2017-03-09 13:40:01]

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

[commit-bot: I haz the power, 2017-03-09 13:40:09]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-03-09 14:04:46]

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

[commit-bot: I haz the power, 2017-03-09 14:04:47]

Dry run: This issue passed the CQ dry run.

[nisse-webrtc, 2017-03-09 14:10:58]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2017-03-09 14:10:58]

The patchset sent to the CQ was uploaded after l-g-t-m from brandtr@webrtc.org,
stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/2712683002/#ps40001 (title:
"Rebase.")

[commit-bot: I haz the power, 2017-03-09 14:11:07]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2017-03-09 14:13:21]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1489068658489500,
"parent_rev": "94f4d9effc3fa1ee6a6a48a7bbdef6684defd1dc", "commit_rev":
"c69385de8b614b7edba4f640bca6aa0963e1505c"}

[commit-bot: I haz the power, 2017-03-09 14:13:23]

Description was changed from

==========
Add |protected_by_flexfec| flag to VideoReceiveStream::Config.

Use of FlexFEC is known when streams are created in
WebRtcVideoChannel2, so this replaces the code in Call to infer
FlexFEC config of video streams from the configuration of the FlexFEC
stream(s). This also allows us to switch to a more logical creation
order, where media streams are created before the FlexFEC stream.

This is done in preparation for a larger refactoring of the RTP
demuxing done in Call.

BUG=None
==========

to

==========
Add |protected_by_flexfec| flag to VideoReceiveStream::Config.

Use of FlexFEC is known when streams are created in
WebRtcVideoChannel2, so this replaces the code in Call to infer
FlexFEC config of video streams from the configuration of the FlexFEC
stream(s). This also allows us to switch to a more logical creation
order, where media streams are created before the FlexFEC stream.

This is done in preparation for a larger refactoring of the RTP
demuxing done in Call.

BUG=None

Review-Url: https://codereview.webrtc.org/2712683002
Cr-Commit-Position: refs/heads/master@{#17143}
Committed:
https://chromium.googlesource.com/external/webrtc/+/c69385de8b614b7edba4f640b...
==========

[commit-bot: I haz the power, 2017-03-09 14:13:24]

Committed patchset #3 (id:40001) as
https://chromium.googlesource.com/external/webrtc/+/c69385de8b614b7edba4f640b...