Generalize FlexfecReceiveStream::Config. (CL1)

Generalize FlexfecReceiveStream::Config.

- Adding information about RTCP and RTP header extensions.
- Renaming flexfec_payload_type -> payload_type and
  flexfec_ssrc -> remote_ssrc.

BUG=webrtc:5654
R=stefan@webrtc.org, philipel@webrtc.org

Committed: https://crrev.com/1cfbd6003b91dca69f485f17f03ad44212f4218a
Cr-Commit-Position: refs/heads/master@{#15477}

[brandtr, 2016-12-02 08:28:50]

Description was changed from

==========
Generalize FlexfecReceiveStream::Config.

- Adding information about RTCP and RTP header extensions.
- Renaming flexfec_payload_type -> payload_type and
  flexfec_ssrc -> remote_ssrc.

BUG=webrtc:5654
R=stefan@webrtc.org, philipel@webrtc.org
==========

to

==========
Generalize FlexfecReceiveStream::Config.

- Adding information about RTCP and RTP header extensions.
- Renaming flexfec_payload_type -> payload_type and
  flexfec_ssrc -> remote_ssrc.

BUG=webrtc:5654
R=stefan@webrtc.org, philipel@webrtc.org
==========

[brandtr, 2016-12-02 09:10:09]

Please take a look at this set of four (five) related changes, whose goal is to
feed the BWE with RTP headers from the FlexFEC packets. RTCP reporting will be
added in future CLs. Don't be scared by the large number of lines being changed,
since many of the line changes are due to renaming and/or trivial additions.
This is the main reason that I have split this one logical change into four
separate CLs.

CL0: Change some FlexReceiveStream::Config method signatures.
CL1: Add more information to the FlexfecReceiveStream config. This will be
needed by the BWE changes in this set of CLs, and the future addition of RTCP
reporting.
CL2: Get the necessary BWE information to the FlexfecReceiver.
CL3: Get the necessary BWE object to the FlexfecReceiver.
CL4: Connect everything and test it.

[philipel, 2016-12-02 09:33:40]

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
File webrtc/api/call/flexfec_receive_stream.h (right):

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
webrtc/api/call/flexfec_receive_stream.h:18: #include "webrtc/config.h"
Is config.h still used?

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
webrtc/api/call/flexfec_receive_stream.h:38: Config()
For small structs like this I think it's cleaner to set the default values of
the members as you declare them, for example:
int payload_type = -1;

Then you can remove the ctor.

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
webrtc/api/call/flexfec_receive_stream.h:46: extensions() {}
newline after ctor

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
webrtc/api/call/flexfec_receive_stream.h:47: ~Config() = default;
Remove default dtor declaration.

[brandtr, 2016-12-02 09:47:57]

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
File webrtc/api/call/flexfec_receive_stream.h (right):

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
webrtc/api/call/flexfec_receive_stream.h:18: #include "webrtc/config.h"
On 2016/12/02 09:33:40, philipel wrote:
> Is config.h still used?

Yep, for RtpExtension and RtcpMode.

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
webrtc/api/call/flexfec_receive_stream.h:38: Config()
On 2016/12/02 09:33:40, philipel wrote:
> For small structs like this I think it's cleaner to set the default values of
> the members as you declare them, for example:
> int payload_type = -1;
> 
> Then you can remove the ctor.

Done.

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
webrtc/api/call/flexfec_receive_stream.h:46: extensions() {}
On 2016/12/02 09:33:40, philipel wrote:
> newline after ctor

Done.

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
webrtc/api/call/flexfec_receive_stream.h:47: ~Config() = default;
On 2016/12/02 09:33:40, philipel wrote:
> Remove default dtor declaration.

I think this is a non-POD struct (due to the std::vector's), so isn't it nice to
show that the destructor is not trivial by including it like this?

[philipel, 2016-12-02 10:14:51]

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
File webrtc/api/call/flexfec_receive_stream.h (right):

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
webrtc/api/call/flexfec_receive_stream.h:47: ~Config() = default;
On 2016/12/02 09:47:57, brandtr wrote:
> On 2016/12/02 09:33:40, philipel wrote:
> > Remove default dtor declaration.
> 
> I think this is a non-POD struct (due to the std::vector's), so isn't it nice
to
> show that the destructor is not trivial by including it like this?

I can't find that rule in the style guide, so I don't think anyone else would
know a declared dtor means this is a non-POD struct.

Also, I think this should be a class since this is a non-POD object, the style
guide is a bit vague so I'm not sure.
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

[brandtr, 2016-12-02 10:36:51]

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
File webrtc/api/call/flexfec_receive_stream.h (right):

https://codereview.webrtc.org/2542413002/diff/1/webrtc/api/call/flexfec_recei...
webrtc/api/call/flexfec_receive_stream.h:47: ~Config() = default;
On 2016/12/02 10:14:51, philipel wrote:
> On 2016/12/02 09:47:57, brandtr wrote:
> > On 2016/12/02 09:33:40, philipel wrote:
> > > Remove default dtor declaration.
> > 
> > I think this is a non-POD struct (due to the std::vector's), so isn't it
nice
> to
> > show that the destructor is not trivial by including it like this?
> 
> I can't find that rule in the style guide, so I don't think anyone else would
> know a declared dtor means this is a non-POD struct.
> 
> Also, I think this should be a class since this is a non-POD object, the style
> guide is a bit vague so I'm not sure.
>
https://engdoc.corp.google.com/eng/doc/devguide/cpp/styleguide.shtml?cl=head#...

I didn't get this from the style guide, but rather from danilchap who sometimes
told me to add destructors like this :)

This config is modeled after the corresponding VideoReceiveStream config:
https://cs.chromium.org/chromium/src/third_party/webrtc/video_receive_stream....,
which is a non-POD struct. So I think it's fine to do the same here. The purpose
of this config is in line with the gist of the style guide ("Use a struct only
for passive objects that carry data"), although the user of the config could do
advanced things with the vectors ("The accessing/setting of fields is done by
directly accessing the fields rather than through method invocations.")

https://codereview.webrtc.org/2542413002/diff/20001/webrtc/api/call/flexfec_r...
File webrtc/api/call/flexfec_receive_stream.h (right):

https://codereview.webrtc.org/2542413002/diff/20001/webrtc/api/call/flexfec_r...
webrtc/api/call/flexfec_receive_stream.h:38: ~Config() = default;
Removed.

[philipel, 2016-12-02 10:42:19]

lgtm

[stefan-webrtc, 2016-12-07 16:55:23]

lgtm

[brandtr, 2016-12-08 11:22:16]

Rebase.

[brandtr, 2016-12-08 11:22:54]

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

[commit-bot: I haz the power, 2016-12-08 11:23:01]

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, 2016-12-08 11:54:14]

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

[commit-bot: I haz the power, 2016-12-08 11:54:15]

Dry run: This issue passed the CQ dry run.

[brandtr, 2016-12-08 12:15:47]

The CQ bit was checked by brandtr@webrtc.org

[brandtr, 2016-12-08 12:15:49]

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

[commit-bot: I haz the power, 2016-12-08 12:15:58]

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, 2016-12-08 12:17:53]

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481199347768700,
"parent_rev": "17c61d5f75f746f56e5c3ac2c42c1b256ffb4b1c", "commit_rev":
"6e374dfe60f150a351cb6abb4dba64a88d2eac7e"}

[commit-bot: I haz the power, 2016-12-08 12:17:59]

Description was changed from

==========
Generalize FlexfecReceiveStream::Config.

- Adding information about RTCP and RTP header extensions.
- Renaming flexfec_payload_type -> payload_type and
  flexfec_ssrc -> remote_ssrc.

BUG=webrtc:5654
R=stefan@webrtc.org, philipel@webrtc.org
==========

to

==========
Generalize FlexfecReceiveStream::Config.

- Adding information about RTCP and RTP header extensions.
- Renaming flexfec_payload_type -> payload_type and
  flexfec_ssrc -> remote_ssrc.

BUG=webrtc:5654
R=stefan@webrtc.org, philipel@webrtc.org
==========

[commit-bot: I haz the power, 2016-12-08 12:18:00]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-08 12:18:13]

Description was changed from

==========
Generalize FlexfecReceiveStream::Config.

- Adding information about RTCP and RTP header extensions.
- Renaming flexfec_payload_type -> payload_type and
  flexfec_ssrc -> remote_ssrc.

BUG=webrtc:5654
R=stefan@webrtc.org, philipel@webrtc.org
==========

to

==========
Generalize FlexfecReceiveStream::Config.

- Adding information about RTCP and RTP header extensions.
- Renaming flexfec_payload_type -> payload_type and
  flexfec_ssrc -> remote_ssrc.

BUG=webrtc:5654
R=stefan@webrtc.org, philipel@webrtc.org

Committed: https://crrev.com/1cfbd6003b91dca69f485f17f03ad44212f4218a
Cr-Commit-Position: refs/heads/master@{#15477}
==========

[commit-bot: I haz the power, 2016-12-08 12:18:14]

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