Delete webrtc::VideoFrame methods buffer and stride.

Delete webrtc::VideoFrame methods buffer and stride.

To make the HasOneRef/IsMutable hack work, also had to change the
video_frame_buffer method to return a const ref to a scoped_ref_ptr,
to not imply an AddRef.

BUG=webrtc:5682
Committed: https://crrev.com/a0591b54730c097b2a1236b3343d258e397ed018
Cr-Commit-Position: refs/heads/master@{#12557}

[nisse-webrtc, 2016-04-18 15:39:27]

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

[nisse-webrtc, 2016-04-19 07:17:40]

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

[commit-bot: I haz the power, 2016-04-19 07:17:44]

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

[commit-bot: I haz the power, 2016-04-19 07:19:34]

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

[commit-bot: I haz the power, 2016-04-19 07:19:36]

Dry run: Try jobs failed on following builders:
  ios64_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/6776)
  ios_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/12960)
  linux_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_gn_rel/builds/10191)
  mac_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_dbg/builds/2430)
  mac_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_rel/builds/8655)

[nisse-webrtc, 2016-04-19 10:17:36]

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

[commit-bot: I haz the power, 2016-04-19 10:17:42]

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

[nisse-webrtc, 2016-04-19 10:53:39]

Now seems to compile on all platforms.

Some of the affected code should perhaps use VideoFrameBuffer or I420Buffer
instead of VideoFrame. Suggestions for TODO items welcome.

There's also an upcoming cl deleting corresponding methods in
cricket::VideoFrame.

[commit-bot: I haz the power, 2016-04-19 11:00:42]

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

[commit-bot: I haz the power, 2016-04-19 11:00:43]

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

[pbos-webrtc, 2016-04-19 11:28:15]

lgtm

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/common_video/libyu...
File webrtc/common_video/libyuv/webrtc_libyuv.cc (right):

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/common_video/libyu...
webrtc/common_video/libyuv/webrtc_libyuv.cc:146: // unused. Delete and let that
code call libyuv directly?
Yes, please inline all libyuv calls unless there's non-trivial methods here. I
don't think the webrtc wrapper is particularly helpful.

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/common_video/video...
File webrtc/common_video/video_frame.cc (right):

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/common_video/video...
webrtc/common_video/video_frame.cc:142: // TODO(nisse): Delete. Only one use, in
Can you do this now?

[pbos-webrtc, 2016-04-19 11:28:47]

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/video_frame.h
File webrtc/video_frame.h (right):

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/video_frame.h#newc...
webrtc/video_frame.h:122: // Creating a new reference breaks the (admittedly
brittle)
Don't think the "admittedly brittle" part is necessary here.

[magjed_webrtc, 2016-04-19 11:40:35]

lgtm

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/common_video/libyu...
File webrtc/common_video/libyuv/webrtc_libyuv.cc (right):

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/common_video/libyu...
webrtc/common_video/libyuv/webrtc_libyuv.cc:129:
frame.video_frame_buffer()->StrideY(), file) < 0)
nit: I'm not sure if the style guide requires it
https://google.github.io/styleguide/cppguide.html#Conditionals, but I would like
curly braces here.

[perkj_webrtc, 2016-04-19 12:28:13]

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/common_video/libyu...
File webrtc/common_video/libyuv/webrtc_libyuv.cc (right):

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/common_video/libyu...
webrtc/common_video/libyuv/webrtc_libyuv.cc:117: // TODO(nisse): Belongs with
the test code?
where is it used? Can it be moved there now?

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/common_video/libyu...
webrtc/common_video/libyuv/webrtc_libyuv.cc:145: // TODO(nisse): Used only by
I420Decoder, which in turn is mostly
now ?

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/common_video/video...
File webrtc/common_video/video_frame.cc (right):

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/common_video/video...
webrtc/common_video/video_frame.cc:176: const
rtc::scoped_refptr<VideoFrameBuffer>& VideoFrame::video_frame_buffer()
have you run git cl format?

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:1545: static void
CreateBlackFrame(webrtc::VideoFrame* video_frame,
change this to return a webrtc::VideoFrame instead of having video_frame as
input.

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h (right):

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h:239: static void
CreateImage(VideoFrame* frame, int plane_colors[kNumOfPlanes]) {
const scoped_refptr<VideoFrameBuffer>& buffer instead of |frame|?

[pbos-webrtc, 2016-04-19 13:18:34]

The msan errors look legit though, please look through those.

[nisse-webrtc, 2016-04-19 13:55:39]

I haven't yet looked into the msan failures.

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/common_video/libyu...
File webrtc/common_video/libyuv/webrtc_libyuv.cc (right):

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/common_video/libyu...
webrtc/common_video/libyuv/webrtc_libyuv.cc:117: // TODO(nisse): Belongs with
the test code?
On 2016/04/19 12:28:13, perkj_webrtc wrote:
> where is it used? Can it be moved there now? 

It's used in various unittests, and in the command line tools(?)
vp8_sequence_coder.cc and replay.cc. So not trivial to move.

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/common_video/libyu...
webrtc/common_video/libyuv/webrtc_libyuv.cc:145: // TODO(nisse): Used only by
I420Decoder, which in turn is mostly
On 2016/04/19 12:28:13, perkj_webrtc wrote:
> now ?

As I wrote in a different comment, including libyuv headers requires a
non-trivial change to the build system. Or to libyuv.

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/common_video/video...
File webrtc/common_video/video_frame.cc (right):

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/common_video/video...
webrtc/common_video/video_frame.cc:176: const
rtc::scoped_refptr<VideoFrameBuffer>& VideoFrame::video_frame_buffer()
On 2016/04/19 12:28:13, perkj_webrtc wrote:
> have you run git cl format?

Not really. I've run git cl format --diff, and applied its suggestions
selectively.

In this particular case, it *is* formatted according to git cl format.

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/media/engine/webrt...
File webrtc/media/engine/webrtcvideoengine2.cc (right):

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/media/engine/webrt...
webrtc/media/engine/webrtcvideoengine2.cc:1545: static void
CreateBlackFrame(webrtc::VideoFrame* video_frame,
On 2016/04/19 12:28:13, perkj_webrtc wrote:
> change this to return a webrtc::VideoFrame instead of having video_frame as
> input. 

Done.

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h (right):

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h:239: static void
CreateImage(VideoFrame* frame, int plane_colors[kNumOfPlanes]) {
On 2016/04/19 12:28:13, perkj_webrtc wrote:
> const scoped_refptr<VideoFrameBuffer>& buffer instead of |frame|? 

Maybe, I'll look into it.

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/common_video/libyu...
File webrtc/common_video/libyuv/webrtc_libyuv.cc (right):

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/common_video/libyu...
webrtc/common_video/libyuv/webrtc_libyuv.cc:129:
frame.video_frame_buffer()->StrideY(), file) < 0)
On 2016/04/19 11:40:35, magjed_webrtc wrote:
> nit: I'm not sure if the style guide requires it
> https://google.github.io/styleguide/cppguide.html#Conditionals, but I would
like
> curly braces here.

Done.

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/common_video/libyu...
webrtc/common_video/libyuv/webrtc_libyuv.cc:146: // unused. Delete and let that
code call libyuv directly?
On 2016/04/19 11:28:15, pbos-webrtc wrote:
> Yes, please inline all libyuv calls unless there's non-trivial methods here. I
> don't think the webrtc wrapper is particularly helpful.

It's difficult to inline libyuv calls, bacause including libyuv header generally
doesn't work. Problem is, I'd like to include
"third_party/libyuv/include/libyuv/convert.h", but that file wants to include
"libyuv/basic_types.h", which fails because that header can't be found in the
include path.

There's some special include_dir setting in common_video.gyp to let
webrtc_libyuv.cc include "libyuv.h". 

So I'm not doing anything about this now (and in addition, there were a few
other uses of ExtractBuffer).

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/common_video/video...
File webrtc/common_video/video_frame.cc (right):

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/common_video/video...
webrtc/common_video/video_frame.cc:142: // TODO(nisse): Delete. Only one use, in
On 2016/04/19 11:28:15, pbos-webrtc wrote:
> Can you do this now?

I can't, there are more uses (but in test code, apparently not indexed by code
search).

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/video_frame.h
File webrtc/video_frame.h (right):

https://codereview.webrtc.org/1900673002/diff/70032/webrtc/video_frame.h#newc...
webrtc/video_frame.h:122: // Creating a new reference breaks the (admittedly
brittle)
On 2016/04/19 11:28:47, pbos-webrtc wrote:
> Don't think the "admittedly brittle" part is necessary here.

Ok, dropped it.

[nisse-webrtc, 2016-04-20 12:23:51]

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

[commit-bot: I haz the power, 2016-04-20 12:23:57]

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

[nisse-webrtc, 2016-04-20 12:24:35]

Hittade msan problemet. Ett en-teckens-fel.

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/modules/video_codi...
File webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h (right):

https://codereview.webrtc.org/1900673002/diff/80001/webrtc/modules/video_codi...
webrtc/modules/video_coding/codecs/vp8/simulcast_unittest.h:239: static void
CreateImage(VideoFrame* frame, int plane_colors[kNumOfPlanes]) {
On 2016/04/19 12:28:13, perkj_webrtc wrote:
> const scoped_refptr<VideoFrameBuffer>& buffer instead of |frame|? 

Done.

[nisse-webrtc, 2016-04-20 12:35:37]

On 2016/04/20 12:24:35, nisse-webrtc wrote:
> Hittade msan problemet. Ett en-teckens-fel.

Translation: "Found the msan problem. A one-character error".

[commit-bot: I haz the power, 2016-04-20 12:57:13]

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

[commit-bot: I haz the power, 2016-04-20 12:57:15]

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

[nisse-webrtc, 2016-04-25 09:50:47]

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

[commit-bot: I haz the power, 2016-04-25 09:50:54]

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

[commit-bot: I haz the power, 2016-04-25 09:52:03]

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

[commit-bot: I haz the power, 2016-04-25 09:52:05]

Dry run: Try jobs failed on following builders:
  android_compile_x64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_compile_x64_dbg...)
  ios32_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios32_sim_dbg/builds/6970)
  ios64_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/6954)
  ios_arm64_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_dbg/builds/9288)
  ios_arm64_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_arm64_rel/builds/9211)
  ios_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_dbg/builds/14496)
  ios_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/13145)
  linux_libfuzzer_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/linux_libfuzzer_rel/bui...)
  mac_asan on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_asan/builds/14121)
  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/2620)
  mac_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_rel/builds/8844)
  mac_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_rel/builds/14559)

[nisse-webrtc, 2016-04-25 10:00:05]

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

[commit-bot: I haz the power, 2016-04-25 10:00:13]

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

[perkj_webrtc, 2016-04-25 11:41:25]

lgtm

[commit-bot: I haz the power, 2016-04-25 11:44:43]

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

[commit-bot: I haz the power, 2016-04-25 11:44:45]

Dry run: This issue passed the CQ dry run.

[nisse-webrtc, 2016-04-25 11:58:08]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-04-25 11:58:08]

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

[commit-bot: I haz the power, 2016-04-25 11:58:13]

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

[commit-bot: I haz the power, 2016-04-25 12:12:45]

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

[commit-bot: I haz the power, 2016-04-25 12:12:46]

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

[nisse-webrtc, 2016-04-25 12:17:23]

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

[nisse-webrtc, 2016-04-25 12:17:24]

Stefan, I need your approval for the changes to the video_processing module.

[stefan-webrtc, 2016-04-26 11:28:15]

lgtm

[nisse-webrtc, 2016-04-27 10:39:50]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-04-27 10:39:51]

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

[commit-bot: I haz the power, 2016-04-27 10:39:55]

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

[commit-bot: I haz the power, 2016-04-27 10:41:09]

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

[commit-bot: I haz the power, 2016-04-27 10:41:11]

Try jobs failed on following builders:
  android_gn_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/android_gn_rel/builds/1...)
  ios64_sim_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios64_sim_dbg/builds/7015)
  ios_rel on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/ios_rel/builds/13206)
  mac_gn_dbg on tryserver.webrtc (JOB_FAILED,
http://build.chromium.org/p/tryserver.webrtc/builders/mac_gn_dbg/builds/2690)

[nisse-webrtc, 2016-04-27 12:37:27]

The CQ bit was checked by nisse@webrtc.org

[nisse-webrtc, 2016-04-27 12:37:27]

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

[commit-bot: I haz the power, 2016-04-27 12:37:32]

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

[commit-bot: I haz the power, 2016-04-27 14:38:05]

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

[commit-bot: I haz the power, 2016-04-27 14:38:07]

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

[nisse-webrtc, 2016-04-29 08:10:28]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-04-29 08:10:40]

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

[commit-bot: I haz the power, 2016-04-29 09:09:43]

Committed patchset #14 (id:250001)

[nisse-webrtc, 2016-04-29 09:38:57]

A revert of this CL (patchset #14 id:250001) has been created in
https://codereview.webrtc.org/1935443002/ by nisse@webrtc.org.

The reason for reverting is: Breaks chrome FYI bots..

[commit-bot: I haz the power, 2016-05-01 22:01:43]

Description was changed from

==========
Delete webrtc::VideoFrame methods buffer and stride.

To make the HasOneRef/IsMutable hack work, also had to change the
video_frame_buffer method to return a const ref to a scoped_ref_ptr,
to not imply an AddRef.

BUG=webrtc:5682
==========

to

==========
Delete webrtc::VideoFrame methods buffer and stride.

To make the HasOneRef/IsMutable hack work, also had to change the
video_frame_buffer method to return a const ref to a scoped_ref_ptr,
to not imply an AddRef.

BUG=webrtc:5682

Committed: https://crrev.com/a0591b54730c097b2a1236b3343d258e397ed018
Cr-Commit-Position: refs/heads/master@{#12557}
==========

[commit-bot: I haz the power, 2016-05-01 22:01:44]

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