Unify FrameType and VideoFrameType.

Unify FrameType and VideoFrameType.

Prevents some heap allocation and frame-type conversion since interfaces
mismatch. Also it's less confusing to have one type for this.

BUG=webrtc:5042
R=magjed@webrtc.org, mflodman@webrtc.org, henrik.lundin@webrtc.org, solenberg@webrtc.org, stefan@webrtc.org

Committed: https://crrev.com/22993e1a0c114122fc1b9de0fc74d4096ec868bd
Cr-Commit-Position: refs/heads/master@{#10320}

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

PTAL, this is mostly regexed ofc. :)

[pbos-webrtc, 2015-09-30 15:53:29]

On 2015/09/30 15:53:09, pbos-webrtc wrote:
> PTAL, this is mostly regexed ofc. :)

There are some temporary aliases in webrtc/common_types.h until we clean up
Chromium

[the sun, 2015-09-30 17:21:09]

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

[the sun, 2015-09-30 17:21:11]

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

https://codereview.webrtc.org/1371043003/diff/1/webrtc/common_types.h#newcode159
webrtc/common_types.h:159: enum FrameType {
enum class again?

[hlundin-webrtc, 2015-10-01 08:54:03]

henrik.lundin@webrtc.org changed reviewers:
+ henrik.lundin@webrtc.org

[hlundin-webrtc, 2015-10-01 08:54:04]

webrtc/modules/audio_coding/: lgtm with one nit.

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

https://codereview.webrtc.org/1371043003/diff/1/webrtc/common_types.h#newcode173
webrtc/common_types.h:173: typedef FrameType VideoFrameType;
I prefer C++11 "using VideoFrameType = FrameType;" but that's totally optional.

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/audio_coding/m...
File
webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc
(right):

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/audio_coding/m...
webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc:411:
} expectation[] = {
I'd rather you didn't reformat this.

[stefan-webrtc, 2015-10-01 09:21:21]

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:39: return "empty";
skip?

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/video_coding/m...
File webrtc/modules/video_coding/main/source/generic_encoder.cc (right):

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/video_coding/m...
webrtc/modules/video_coding/main/source/generic_encoder.cc:142: const
std::vector<FrameType>& frameTypes) {
DCHECK on frame types being valid video types?

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/video_coding/m...
File webrtc/modules/video_coding/main/source/session_info.cc (right):

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/video_coding/m...
webrtc/modules/video_coding/main/source/session_info.cc:519: } else if
(frame_type_ == kSkipFrame && packet.frameType != kSkipFrame) {
I don't really like the name kSkipFrame here... Maybe it's better to call it
kEmptyFrame?

https://codereview.webrtc.org/1371043003/diff/1/webrtc/video/encoded_frame_ca...
File webrtc/video/encoded_frame_callback_adapter.cc (right):

https://codereview.webrtc.org/1371043003/diff/1/webrtc/video/encoded_frame_ca...
webrtc/video/encoded_frame_callback_adapter.cc:31: encodedImage._frameType);
DCHECK frame type being of video type. (or maybe we should do it in the callback
in generic_encoder.cc instead?)

[pbos-webrtc, 2015-10-06 15:33:50]

rebase + kEmptyFrame

[pbos-webrtc, 2015-10-06 15:41:59]

more feedback woo

[pbos-webrtc, 2015-10-06 15:42:46]

PTAL woo

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

https://codereview.webrtc.org/1371043003/diff/1/webrtc/common_types.h#newcode159
webrtc/common_types.h:159: enum FrameType {
On 2015/09/30 17:21:10, the sun wrote:
> enum class again?

Nope, since I don't want to complicate rolling into Chromium. :(

https://codereview.webrtc.org/1371043003/diff/1/webrtc/common_types.h#newcode173
webrtc/common_types.h:173: typedef FrameType VideoFrameType;
On 2015/10/01 08:54:04, hlundin-webrtc wrote:
> I prefer C++11 "using VideoFrameType = FrameType;" but that's totally
optional.

Done.

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/audio_coding/m...
File
webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc
(right):

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/audio_coding/m...
webrtc/modules/audio_coding/main/acm2/audio_coding_module_unittest_oldapi.cc:411:
} expectation[] = {
On 2015/10/01 08:54:04, hlundin-webrtc wrote:
> I'd rather you didn't reformat this.

Done. But clang-format is awesome. :(

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/rtp_rtcp/sourc...
File webrtc/modules/rtp_rtcp/source/rtp_sender.cc (right):

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/rtp_rtcp/sourc...
webrtc/modules/rtp_rtcp/source/rtp_sender.cc:39: return "empty";
On 2015/10/01 09:21:20, stefan-webrtc (holmer) wrote:
> skip?

kEmptyFrame

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/video_coding/m...
File webrtc/modules/video_coding/main/source/generic_encoder.cc (right):

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/video_coding/m...
webrtc/modules/video_coding/main/source/generic_encoder.cc:142: const
std::vector<FrameType>& frameTypes) {
On 2015/10/01 09:21:20, stefan-webrtc (holmer) wrote:
> DCHECK on frame types being valid video types?

Done.

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/video_coding/m...
File webrtc/modules/video_coding/main/source/session_info.cc (right):

https://codereview.webrtc.org/1371043003/diff/1/webrtc/modules/video_coding/m...
webrtc/modules/video_coding/main/source/session_info.cc:519: } else if
(frame_type_ == kSkipFrame && packet.frameType != kSkipFrame) {
On 2015/10/01 09:21:20, stefan-webrtc (holmer) wrote:
> I don't really like the name kSkipFrame here... Maybe it's better to call it
> kEmptyFrame?

Done.

https://codereview.webrtc.org/1371043003/diff/1/webrtc/video/encoded_frame_ca...
File webrtc/video/encoded_frame_callback_adapter.cc (right):

https://codereview.webrtc.org/1371043003/diff/1/webrtc/video/encoded_frame_ca...
webrtc/video/encoded_frame_callback_adapter.cc:31: encodedImage._frameType);
On 2015/10/01 09:21:20, stefan-webrtc (holmer) wrote:
> DCHECK frame type being of video type. (or maybe we should do it in the
callback
> in generic_encoder.cc instead?)

Done (in generic_encoder.cc).

[stefan-webrtc, 2015-10-13 14:57:30]

lgtm

[pbos-webrtc, 2015-10-13 16:29:03]

solenberg@ are you fine with me submitting this without enum classing it?

[The Sun (google.com), 2015-10-14 08:04:35]

On 2015/10/13 16:29:03, pbos-webrtc wrote:
> solenberg@ are you fine with me submitting this without enum classing it?

well, yeah. lgtm

[pbos-webrtc, 2015-10-14 09:44:38]

rebase

[pbos-webrtc, 2015-10-14 09:44:52]

The CQ bit was checked by pbos@webrtc.org

[pbos-webrtc, 2015-10-14 09:44:52]

The patchset sent to the CQ was uploaded after l-g-t-m from
henrik.lundin@webrtc.org, solenberg@google.com, stefan@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1371043003/#ps60001 (title:
"rebase")

[commit-bot: I haz the power, 2015-10-14 09:45:07]

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

[commit-bot: I haz the power, 2015-10-14 09:48:07]

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

[commit-bot: I haz the power, 2015-10-14 09:48:08]

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

[pbos-webrtc, 2015-10-14 09:51:18]

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

[pbos-webrtc, 2015-10-14 09:51:19]

+R mflodman@ for:

    talk/app/webrtc/java/jni/androidmediaencoder_jni.cc
    webrtc/common_types.h
    webrtc/frame_callback.h
    webrtc/video_encoder.h
    webrtc/video_frame.h

[the sun, 2015-10-14 13:24:04]

On 2015/10/14 09:51:19, pbos-webrtc wrote:
> +R mflodman@ for:
> 
>     talk/app/webrtc/java/jni/androidmediaencoder_jni.cc
>     webrtc/common_types.h
>     webrtc/frame_callback.h
>     webrtc/video_encoder.h
>     webrtc/video_frame.h

oh sorry, lgtm, now with correct persona

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

On 2015/10/14 13:24:04, the sun wrote:
> On 2015/10/14 09:51:19, pbos-webrtc wrote:
> > +R mflodman@ for:
> > 
> >     talk/app/webrtc/java/jni/androidmediaencoder_jni.cc
> >     webrtc/common_types.h
> >     webrtc/frame_callback.h
> >     webrtc/video_encoder.h
> >     webrtc/video_frame.h
> 
> oh sorry, lgtm, now with correct persona

still not sufficient, but thanks :)

[mflodman, 2015-10-16 11:48:49]

LGTM for:

talk/app/webrtc/java/jni/androidmediaencoder_jni.cc
webrtc/common_types.h
webrtc/frame_callback.h
webrtc/video_encoder.h
webrtc/video_frame.h

[pbos-webrtc, 2015-10-16 13:14:18]

rebase

[pbos-webrtc, 2015-10-16 15:14:04]

The CQ bit was checked by pbos@webrtc.org

[pbos-webrtc, 2015-10-16 15:14:05]

The patchset sent to the CQ was uploaded after l-g-t-m from
solenberg@webrtc.org, henrik.lundin@webrtc.org, solenberg@google.com,
stefan@webrtc.org, mflodman@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1371043003/#ps80001 (title:
"rebase")

[commit-bot: I haz the power, 2015-10-16 15:14:25]

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

[commit-bot: I haz the power, 2015-10-16 15:19:33]

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

[commit-bot: I haz the power, 2015-10-16 15:19:34]

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

[pbos-webrtc, 2015-10-19 08:44:54]

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

[pbos-webrtc, 2015-10-19 08:44:55]

+R magjed@ for:

talk/app/webrtc/java/jni/androidmediaencoder_jni.cc

[magjed_webrtc, 2015-10-19 08:47:15]

talk/app/webrtc/java/jni/androidmediaencoder_jni.cc lgtm

[pbos-webrtc, 2015-10-19 08:48:49]

The CQ bit was checked by pbos@webrtc.org

[commit-bot: I haz the power, 2015-10-19 08:49:15]

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

[commit-bot: I haz the power, 2015-10-19 09:39:13]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-10-19 09:39:23]

Patchset 5 (id:??) landed as
https://crrev.com/22993e1a0c114122fc1b9de0fc74d4096ec868bd
Cr-Commit-Position: refs/heads/master@{#10320}