Cleanup of webrtc::VideoFrame.

Cleanup of webrtc::VideoFrame.

Delete EqualsFrame method, used only by tests. Delete one of the
CreateFrame methods. Drop return value for CreateEmptyFrame, CreateFrame
and CopyFrame.

BUG=webrtc:5426
Committed: https://crrev.com/208019637bfed975f8f13b16d40b90e200763cd6
Cr-Commit-Position: refs/heads/master@{#11783}

R=mflodman@webrtc.org, pbos@webrtc.org, perkj@webrtc.org, pthatcher@webrtc.org

Committed: https://crrev.com/739fcb989dc292bc62c8296a8bbdaa9bf53d7602
Cr-Commit-Position: refs/heads/master@{#11811}

[nisse-webrtc, 2016-02-09 15:14:21]

nisse@webrtc.org changed reviewers:
+ pbos@webrtc.org, perkj@webrtc.org, pthatcher@webrtc.org

[nisse-webrtc, 2016-02-09 15:14:21]

This is a warmup, before getting to cricket::VideoFrame cruft.

[pbos-webrtc, 2016-02-09 15:18:14]

https://codereview.webrtc.org/1679323002/diff/1/webrtc/common_video/incoming_...
File webrtc/common_video/incoming_video_stream.cc (right):

https://codereview.webrtc.org/1679323002/diff/1/webrtc/common_video/incoming_...
webrtc/common_video/incoming_video_stream.cc:93: int32_t
IncomingVideoStream::SetStartImage(const VideoFrame& video_frame) {
Can we make this void?

https://codereview.webrtc.org/1679323002/diff/1/webrtc/media/webrtc/fakewebrt...
File webrtc/media/webrtc/fakewebrtcvideocapturemodule.h (right):

https://codereview.webrtc.org/1679323002/diff/1/webrtc/media/webrtc/fakewebrt...
webrtc/media/webrtc/fakewebrtcvideocapturemodule.h:93: bool SendFrame(int w, int
h) {
Remove bool

https://codereview.webrtc.org/1679323002/diff/1/webrtc/modules/video_processi...
File webrtc/modules/video_processing/test/denoiser_test.cc (right):

https://codereview.webrtc.org/1679323002/diff/1/webrtc/modules/video_processi...
webrtc/modules/video_processing/test/denoiser_test.cc:19: // Duplicated in
i420_video_frame_unittest.cc
Put this somewhere common, even if it's only test code. webrtc/test might have
something that you can use in both test targets.

[nisse-webrtc, 2016-02-09 15:31:34]

https://codereview.webrtc.org/1679323002/diff/1/webrtc/common_video/incoming_...
File webrtc/common_video/incoming_video_stream.cc (right):

https://codereview.webrtc.org/1679323002/diff/1/webrtc/common_video/incoming_...
webrtc/common_video/incoming_video_stream.cc:93: int32_t
IncomingVideoStream::SetStartImage(const VideoFrame& video_frame) {
On 2016/02/09 15:18:14, pbos-webrtc wrote:
> Can we make this void?

Maybe. I have no idea what this code does or from where it is called. And
int32_t is a very peculiar type for a success/fail indication.

But I can try to make it void, and follow it up the call chain converting
additional functions to void.

https://codereview.webrtc.org/1679323002/diff/1/webrtc/modules/video_processi...
File webrtc/modules/video_processing/test/denoiser_test.cc (right):

https://codereview.webrtc.org/1679323002/diff/1/webrtc/modules/video_processi...
webrtc/modules/video_processing/test/denoiser_test.cc:19: // Duplicated in
i420_video_frame_unittest.cc
On 2016/02/09 15:18:14, pbos-webrtc wrote:
> Put this somewhere common, even if it's only test code. webrtc/test might have
> something that you can use in both test targets.

I see no obvious place. Any suggestion for file (and namespace?) to put it in?

A new file webrtc/test/frame_utils.h? Is it good enough with only a header file
including the implementation?

[pbos-webrtc, 2016-02-09 15:36:10]

https://codereview.webrtc.org/1679323002/diff/1/webrtc/common_video/incoming_...
File webrtc/common_video/incoming_video_stream.cc (right):

https://codereview.webrtc.org/1679323002/diff/1/webrtc/common_video/incoming_...
webrtc/common_video/incoming_video_stream.cc:93: int32_t
IncomingVideoStream::SetStartImage(const VideoFrame& video_frame) {
On 2016/02/09 15:31:33, nisse-webrtc wrote:
> On 2016/02/09 15:18:14, pbos-webrtc wrote:
> > Can we make this void?
> 
> Maybe. I have no idea what this code does or from where it is called. And
> int32_t is a very peculiar type for a success/fail indication.
> 
> But I can try to make it void, and follow it up the call chain converting
> additional functions to void.

All of our error codes were int32 historically. We've stopped doing that but
have a bunch of legacy code.

https://codereview.webrtc.org/1679323002/diff/1/webrtc/modules/video_processi...
File webrtc/modules/video_processing/test/denoiser_test.cc (right):

https://codereview.webrtc.org/1679323002/diff/1/webrtc/modules/video_processi...
webrtc/modules/video_processing/test/denoiser_test.cc:19: // Duplicated in
i420_video_frame_unittest.cc
On 2016/02/09 15:31:33, nisse-webrtc wrote:
> On 2016/02/09 15:18:14, pbos-webrtc wrote:
> > Put this somewhere common, even if it's only test code. webrtc/test might
have
> > something that you can use in both test targets.
> 
> I see no obvious place. Any suggestion for file (and namespace?) to put it in?
> 
> A new file webrtc/test/frame_utils.h? Is it good enough with only a header
file
> including the implementation?

I'll come over and sync with you.

[pthatcher1, 2016-02-09 18:12:53]

lgtm, assuming you address pbos's comments.

[nisse-webrtc, 2016-02-10 09:32:25]

It seems I'm a little unstructured. Patchset 2 not only moves the testcode, it
addresses most but not all of pbos comments. New patchset coming soon.

[nisse-webrtc, 2016-02-10 09:37:27]

I think patchset 3 addresses all comments.

https://codereview.webrtc.org/1679323002/diff/1/webrtc/media/webrtc/fakewebrt...
File webrtc/media/webrtc/fakewebrtcvideocapturemodule.h (right):

https://codereview.webrtc.org/1679323002/diff/1/webrtc/media/webrtc/fakewebrt...
webrtc/media/webrtc/fakewebrtcvideocapturemodule.h:93: bool SendFrame(int w, int
h) {
On 2016/02/09 15:18:14, pbos-webrtc wrote:
> Remove bool

Done.

[nisse-webrtc, 2016-02-10 09:38:09]

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

[commit-bot: I haz the power, 2016-02-10 09:38:15]

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

[commit-bot: I haz the power, 2016-02-10 10:05:28]

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

[commit-bot: I haz the power, 2016-02-10 10:05:28]

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

[pbos-webrtc, 2016-02-10 13:49:24]

lgtm

https://codereview.webrtc.org/1679323002/diff/40001/webrtc/modules/video_proc...
File webrtc/modules/video_processing/test/denoiser_test.cc (right):

https://codereview.webrtc.org/1679323002/diff/40001/webrtc/modules/video_proc...
webrtc/modules/video_processing/test/denoiser_test.cc:152: ASSERT_EQ(
ASSERT_TRUE

https://codereview.webrtc.org/1679323002/diff/40001/webrtc/test/frame_utils.h
File webrtc/test/frame_utils.h (right):

https://codereview.webrtc.org/1679323002/diff/40001/webrtc/test/frame_utils.h...
webrtc/test/frame_utils.h:10: #ifndef WEBRTC_COMMON_VIDEO_TEST_FRAME_UTILS_H_
remove COMMON_VIDEO_ to pass the presubmit

[nisse-webrtc, 2016-02-10 13:54:06]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-02-10 13:54:07]

The patchset sent to the CQ was uploaded after l-g-t-m from
pthatcher@webrtc.org, pbos@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1679323002/#ps80001 (title:
"Changed ASSERT_EQ to ASSERT_TRUE.")

[commit-bot: I haz the power, 2016-02-10 13:54:11]

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

[nisse-webrtc, 2016-02-10 13:55:18]

https://codereview.webrtc.org/1679323002/diff/40001/webrtc/modules/video_proc...
File webrtc/modules/video_processing/test/denoiser_test.cc (right):

https://codereview.webrtc.org/1679323002/diff/40001/webrtc/modules/video_proc...
webrtc/modules/video_processing/test/denoiser_test.cc:152: ASSERT_EQ(
On 2016/02/10 13:49:24, pbos-webrtc wrote:
> ASSERT_TRUE

Done.

https://codereview.webrtc.org/1679323002/diff/40001/webrtc/test/frame_utils.h
File webrtc/test/frame_utils.h (right):

https://codereview.webrtc.org/1679323002/diff/40001/webrtc/test/frame_utils.h...
webrtc/test/frame_utils.h:10: #ifndef WEBRTC_COMMON_VIDEO_TEST_FRAME_UTILS_H_
On 2016/02/10 13:49:24, pbos-webrtc wrote:
> remove COMMON_VIDEO_ to pass the presubmit

Done.

[commit-bot: I haz the power, 2016-02-10 14:08:16]

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

[commit-bot: I haz the power, 2016-02-10 14:08:17]

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

[nisse-webrtc, 2016-02-10 14:30:26]

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

[nisse-webrtc, 2016-02-10 14:30:27]

Hi, please take a look. I lack owner's approval for these changes.

[perkj_webrtc, 2016-02-11 08:52:26]

On 2016/02/10 14:30:27, nisse-webrtc wrote:
> Hi, please take a look. I lack owner's approval for these changes.

lgtm but I think you need mflodman anyway.

[mflodman, 2016-02-24 10:02:33]

On 2016/02/11 08:52:26, perkj_webrtc wrote:
> On 2016/02/10 14:30:27, nisse-webrtc wrote:
> > Hi, please take a look. I lack owner's approval for these changes.
> 
> lgtm but I think you need mflodman anyway.

Which parts specifically did you want me to take a look at?

[pbos-webrtc, 2016-02-24 10:10:13]

I think webrtc/video_frame.h is the important one.

[mflodman, 2016-02-26 07:52:16]

video_frame LGTM

[nisse-webrtc, 2016-02-26 10:00:35]

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

[nisse-webrtc, 2016-02-26 10:00:36]

@marpan: Can you please have a look at the (small) changes under
webrtc/modules/video_processing?

This cl is a small step in refactoring of the frame classes, where eventually
we'd like to get rid of at least one of cricket::VideoFrame and
webrtc::VideoFrame.

[mflodman, 2016-02-26 10:03:51]

root OWNER LGTM for video_processing, since it's a trivial change.

Marco,
Feel free to comment after the fact.

[nisse-webrtc, 2016-02-26 10:10:15]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-02-26 10:10:23]

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

[commit-bot: I haz the power, 2016-02-26 10:12:03]

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

[commit-bot: I haz the power, 2016-02-26 10:12:04]

Try jobs failed on following builders:
  ios_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/11593)
  mac_compile_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_compile_dbg/builds/...)
  mac_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_dbg/builds/1086)
  mac_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_rel/builds/7320)
  mac_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/12992)

[nisse-webrtc, 2016-02-26 10:40:18]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-02-26 10:40:18]

The patchset sent to the CQ was uploaded after l-g-t-m from pbos@webrtc.org,
perkj@webrtc.org, mflodman@webrtc.org, pthatcher@webrtc.org
Link to the patchset: https://codereview.webrtc.org/1679323002/#ps100001 (title:
"Rebase.")

[commit-bot: I haz the power, 2016-02-26 10:40:35]

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

[commit-bot: I haz the power, 2016-02-26 12:41:05]

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

[commit-bot: I haz the power, 2016-02-26 12:41:07]

Try jobs failed on following builders:
  android_arm64_rel on tryserver.webrtc (JOB_TIMED_OUT, no build URL)
  win_baremetal on tryserver.webrtc (JOB_TIMED_OUT, no build URL)

[nisse-webrtc, 2016-02-26 14:38:56]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-02-26 14:39:03]

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

[commit-bot: I haz the power, 2016-02-26 14:40:45]

Committed patchset #6 (id:100001)

[commit-bot: I haz the power, 2016-02-26 14:40:54]

Description was changed from

==========
Cleanup of webrtc::VideoFrame.

Delete EqualsFrame method, used only by tests. Delete one of the
CreateFrame methods. Drop return value for CreateEmptyFrame, CreateFrame
and CopyFrame.

BUG=webrtc:5426
==========

to

==========
Cleanup of webrtc::VideoFrame.

Delete EqualsFrame method, used only by tests. Delete one of the
CreateFrame methods. Drop return value for CreateEmptyFrame, CreateFrame
and CopyFrame.

BUG=webrtc:5426

Committed: https://crrev.com/208019637bfed975f8f13b16d40b90e200763cd6
Cr-Commit-Position: refs/heads/master@{#11783}
==========

[commit-bot: I haz the power, 2016-02-26 14:40:55]

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

[kjellander_webrtc, 2016-02-26 15:38:32]

A revert of this CL (patchset #6 id:100001) has been created in
https://codereview.webrtc.org/1743613002/ by kjellander@webrtc.org.

The reason for reverting is: Breaks downstream compilation. Please make
non-breaking API changes for the reland or coordinate fixing downstream code
quickly with the sheriff..

[nisse-webrtc, 2016-02-29 08:36:26]

Description was changed from

==========
Cleanup of webrtc::VideoFrame.

Delete EqualsFrame method, used only by tests. Delete one of the
CreateFrame methods. Drop return value for CreateEmptyFrame, CreateFrame
and CopyFrame.

BUG=webrtc:5426

Committed: https://crrev.com/208019637bfed975f8f13b16d40b90e200763cd6
Cr-Commit-Position: refs/heads/master@{#11783}
==========

to

==========
Cleanup of webrtc::VideoFrame.

Delete EqualsFrame method, used only by tests. Delete one of the
CreateFrame methods. Drop return value for CreateEmptyFrame, CreateFrame
and CopyFrame.

BUG=webrtc:5426

Committed: https://crrev.com/208019637bfed975f8f13b16d40b90e200763cd6
Cr-Commit-Position: refs/heads/master@{#11783}
==========

[nisse-webrtc, 2016-02-29 12:12:01]

Description was changed from

==========
Cleanup of webrtc::VideoFrame.

Delete EqualsFrame method, used only by tests. Delete one of the
CreateFrame methods. Drop return value for CreateEmptyFrame, CreateFrame
and CopyFrame.

BUG=webrtc:5426

Committed: https://crrev.com/208019637bfed975f8f13b16d40b90e200763cd6
Cr-Commit-Position: refs/heads/master@{#11783}
==========

to

==========
Cleanup of webrtc::VideoFrame.

Delete EqualsFrame method, used only by tests. Delete one of the
CreateFrame methods. Drop return value for CreateEmptyFrame, CreateFrame
and CopyFrame.

BUG=webrtc:5426

Committed: https://crrev.com/208019637bfed975f8f13b16d40b90e200763cd6
Cr-Commit-Position: refs/heads/master@{#11783}

R=mflodman@webrtc.org, pbos@webrtc.org, perkj@webrtc.org, pthatcher@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/739fcb989dc292bc62c8296a8...
==========

[commit-bot: I haz the power, 2016-02-29 12:12:02]

Description was changed from

==========
Cleanup of webrtc::VideoFrame.

Delete EqualsFrame method, used only by tests. Delete one of the
CreateFrame methods. Drop return value for CreateEmptyFrame, CreateFrame
and CopyFrame.

BUG=webrtc:5426

Committed: https://crrev.com/208019637bfed975f8f13b16d40b90e200763cd6
Cr-Commit-Position: refs/heads/master@{#11783}

R=mflodman@webrtc.org, pbos@webrtc.org, perkj@webrtc.org, pthatcher@webrtc.org

Committed:
https://chromium.googlesource.com/external/webrtc/+/739fcb989dc292bc62c8296a8...
==========

to

==========
Cleanup of webrtc::VideoFrame.

Delete EqualsFrame method, used only by tests. Delete one of the
CreateFrame methods. Drop return value for CreateEmptyFrame, CreateFrame
and CopyFrame.

BUG=webrtc:5426

Committed: https://crrev.com/208019637bfed975f8f13b16d40b90e200763cd6
Cr-Commit-Position: refs/heads/master@{#11783}

R=mflodman@webrtc.org, pbos@webrtc.org, perkj@webrtc.org, pthatcher@webrtc.org

Committed: https://crrev.com/739fcb989dc292bc62c8296a8bbdaa9bf53d7602
Cr-Commit-Position: refs/heads/master@{#11811}
==========

[nisse-webrtc, 2016-02-29 12:12:03]

Committed patchset #7 (id:120001) manually as
739fcb989dc292bc62c8296a8bbdaa9bf53d7602 (presubmit successful).

[commit-bot: I haz the power, 2016-02-29 12:12:04]

Patchset 7 (id:??) landed as
https://crrev.com/739fcb989dc292bc62c8296a8bbdaa9bf53d7602
Cr-Commit-Position: refs/heads/master@{#11811}