Unify newapi::RtcpMode and RTCPMethod.

Unify newapi::RtcpMode and RTCPMethod.

BUG=webrtc:1695
R=solenberg@webrtc.org, stefan@webrtc.org
TBR=mflodman@webrtc.org

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

[pbos-webrtc, 2015-09-28 17:07:24]

PTAL, this removes the newapi:: namespace (which was a bad idea to begin with).
:)

[pbos-webrtc, 2015-09-28 17:07:31]

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

[commit-bot: I haz the power, 2015-09-28 17:07:46]

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

[commit-bot: I haz the power, 2015-09-28 17:32:20]

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

[commit-bot: I haz the power, 2015-09-28 17:32:21]

Dry run: Try jobs failed on following builders:
  win_x64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/win_x64_rel/builds/9663)

[the sun, 2015-09-30 08:17:05]

solenberg@webrtc.org changed reviewers:
+ solenberg@webrtc.org

[the sun, 2015-09-30 08:17:06]

https://codereview.webrtc.org/1373903003/diff/1/webrtc/common_types.h
File webrtc/common_types.h (right):

https://codereview.webrtc.org/1373903003/diff/1/webrtc/common_types.h#newcode899
webrtc/common_types.h:899: enum RtcpMode { kRtcpOff, kRtcpCompound,
kRtcpReducedSize };
While messing with this, can we please also make it an enum class?
enum class RtcpMode { OFF, COMPOUND, REDUCED_SIZE };

http://www.stroustrup.com/C++11FAQ.html#enum

Yes, Chromium style guide allows this C++11 feature:
http://chromium-cpp.appspot.com/

[pbos-webrtc, 2015-09-30 09:49:45]

enum class

[pbos-webrtc, 2015-09-30 09:50:04]

Done, woo!

https://codereview.webrtc.org/1373903003/diff/1/webrtc/common_types.h
File webrtc/common_types.h (right):

https://codereview.webrtc.org/1373903003/diff/1/webrtc/common_types.h#newcode899
webrtc/common_types.h:899: enum RtcpMode { kRtcpOff, kRtcpCompound,
kRtcpReducedSize };
On 2015/09/30 08:17:05, the sun wrote:
> While messing with this, can we please also make it an enum class?
> enum class RtcpMode { OFF, COMPOUND, REDUCED_SIZE };
> 
> http://www.stroustrup.com/C++11FAQ.html#enum
> 
> Yes, Chromium style guide allows this C++11 feature:
> http://chromium-cpp.appspot.com/

Done.

[stefan-webrtc, 2015-09-30 09:58:13]

https://codereview.webrtc.org/1373903003/diff/20001/webrtc/common_types.h
File webrtc/common_types.h (right):

https://codereview.webrtc.org/1373903003/diff/20001/webrtc/common_types.h#new...
webrtc/common_types.h:899: enum class RtcpMode { OFF, COMPOUND, REDUCED_SIZE };
I would say it's more common to use kOff, kCompound, etc. in WebRTC code. I
don't have a strong opinion myself, but I am opposed to having a mix...

Also, isn't it strange to expose the option OFF in the API, and then DCHECK on
that no one uses that setting? Seems like that should be an internal option in
that case?

[pbos-webrtc, 2015-09-30 10:15:15]

kConstant

[pbos-webrtc, 2015-09-30 10:16:13]

https://codereview.webrtc.org/1373903003/diff/20001/webrtc/common_types.h
File webrtc/common_types.h (right):

https://codereview.webrtc.org/1373903003/diff/20001/webrtc/common_types.h#new...
webrtc/common_types.h:899: enum class RtcpMode { OFF, COMPOUND, REDUCED_SIZE };
On 2015/09/30 09:58:13, stefan-webrtc (holmer) wrote:
> I would say it's more common to use kOff, kCompound, etc. in WebRTC code. I
> don't have a strong opinion myself, but I am opposed to having a mix...
> 
> Also, isn't it strange to expose the option OFF in the API, and then DCHECK on
> that no one uses that setting? Seems like that should be an internal option in
> that case?

Changed to kConstant, that's my impression as well.

Yeah, a bit, but I think it's preferable to having to convert between types for
these as soon as they go into/out of the RTP module. Now it's one DCHECK.

[pbos-webrtc, 2015-09-30 10:17:13]

ehm, compile the code

[the sun, 2015-09-30 10:23:05]

lgtm

https://codereview.webrtc.org/1373903003/diff/40001/webrtc/video/end_to_end_t...
File webrtc/video/end_to_end_tests.cc (right):

https://codereview.webrtc.org/1373903003/diff/40001/webrtc/video/end_to_end_t...
webrtc/video/end_to_end_tests.cc:1107: case RtcpMode::kOff:
use default: instead; then code will break here if another mode is added and
this is somehow forgotten.

[stefan-webrtc, 2015-10-01 07:41:44]

lgtm

[pbos-webrtc, 2015-10-02 08:24:50]

The CQ bit was checked by pbos@webrtc.org

[commit-bot: I haz the power, 2015-10-02 08:25:13]

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

[commit-bot: I haz the power, 2015-10-02 08:27:12]

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

[commit-bot: I haz the power, 2015-10-02 08:27:13]

Try jobs failed on following builders:
  presubmit on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/presubmit/builds/1057)

[pbos-webrtc, 2015-10-02 09:30:32]

pbos@webrtc.org changed reviewers:
+ mflodman@webrtc.org

[pbos-webrtc, 2015-10-02 09:30:32]

TBR=mflodman@ for the root files. :)

[pbos-webrtc, 2015-10-02 09:30:38]

The CQ bit was checked by pbos@webrtc.org

[commit-bot: I haz the power, 2015-10-02 09:30:57]

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

[commit-bot: I haz the power, 2015-10-02 09:37:02]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-10-02 09:37:25]

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