Revert of Delete webrtc::VideoFrame methods buffer and stride.

Revert of Delete webrtc::VideoFrame methods buffer and stride. (patchset #2 id:290001 of https://codereview.webrtc.org/1963413004/ )

Reason for revert:
Speculative revert to see if failures on the DrMemory bot are related to this cl.  See e.g. here:
https://build.chromium.org/p/client.webrtc/builders/Win%20DrMemory%20Full/builds/4243

UNINITIALIZED READ: reading 0x04980040-0x04980060 32 byte(s) within 0x04980040-0x04980060
# 0 CopyRow_AVX
# 1 CopyPlane
# 2 I420Copy
# 3 webrtc::ExtractBuffer
# 4 cricket::WebRtcVideoCapturer::SignalFrameCapturedOnStartThread
# 5 cricket::WebRtcVideoCapturer::OnIncomingCapturedFrame
# 6 FakeWebRtcVideoCaptureModule::SendFrame
# 7 WebRtcVideoCapturerTest_TestCaptureVcm_Test::TestBody
# 8 testing::internal::HandleSehExceptionsInMethodIfSupported<>

Original issue's description:
> Reland of Delete webrtc::VideoFrame methods buffer and stride. (patchset #1 id:1 of https://codereview.webrtc.org/1935443002/ )
>
> Reason for revert:
> I plan to reland this change in a week or two, after downstream users are updated.
>
> Original issue's description:
> > Revert of Delete webrtc::VideoFrame methods buffer and stride. (patchset #14 id:250001 of https://codereview.webrtc.org/1900673002/ )
> >
> > Reason for revert:
> > Breaks chrome FYI bots.
> >
> > Original issue's description:
> > > 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
> >
> > TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,stefan@webrtc.org
> > # Skipping CQ checks because original CL landed less than 1 days ago.
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > NOTRY=true
> > BUG=webrtc:5682
> >
> > Committed: https://crrev.com/5b3c443d301f2c2f18dac5b02652c08b91ea3828
> > Cr-Commit-Position: refs/heads/master@{#12558}
>
> TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,stefan@webrtc.org
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=webrtc:5682
>
> Committed: https://crrev.com/d0dc66e0ea30c8614001e425a4ae0aa7dd56c2a7
> Cr-Commit-Position: refs/heads/master@{#12721}

TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,stefan@webrtc.org,nisse@webrtc.org
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5682
Committed: https://crrev.com/d49c30cd2fe442f2b5b4ecec8d5cbaa430464725
Cr-Commit-Position: refs/heads/master@{#12745}

[tommi, 2016-05-14 10:17:49]

Created Revert of Delete webrtc::VideoFrame methods buffer and stride.

[commit-bot: I haz the power, 2016-05-14 10:17:55]

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

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

Description was changed from

==========
Revert of Delete webrtc::VideoFrame methods buffer and stride. (patchset #2
id:290001 of https://codereview.webrtc.org/1963413004/ )

Reason for revert:
Speculative revert to see if failures on the DrMemory bot are related to this
cl.  See e.g. here:
https://build.chromium.org/p/client.webrtc/builders/Win%20DrMemory%20Full/bui...

UNINITIALIZED READ: reading 0x04980040-0x04980060 32 byte(s) within
0x04980040-0x04980060
# 0 CopyRow_AVX
# 1 CopyPlane
# 2 I420Copy
# 3 webrtc::ExtractBuffer
# 4 cricket::WebRtcVideoCapturer::SignalFrameCapturedOnStartThread
# 5 cricket::WebRtcVideoCapturer::OnIncomingCapturedFrame
# 6 FakeWebRtcVideoCaptureModule::SendFrame
# 7 WebRtcVideoCapturerTest_TestCaptureVcm_Test::TestBody
# 8 testing::internal::HandleSehExceptionsInMethodIfSupported<>

Original issue's description:
> Reland of Delete webrtc::VideoFrame methods buffer and stride. (patchset #1
id:1 of https://codereview.webrtc.org/1935443002/ )
> 
> Reason for revert:
> I plan to reland this change in a week or two, after downstream users are
updated.
> 
> Original issue's description:
> > Revert of Delete webrtc::VideoFrame methods buffer and stride. (patchset #14
id:250001 of https://codereview.webrtc.org/1900673002/ )
> >
> > Reason for revert:
> > Breaks chrome FYI bots.
> >
> > Original issue's description:
> > > 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
> >
> >
TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,s...
> > # Skipping CQ checks because original CL landed less than 1 days ago.
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > NOTRY=true
> > BUG=webrtc:5682
> >
> > Committed: https://crrev.com/5b3c443d301f2c2f18dac5b02652c08b91ea3828
> > Cr-Commit-Position: refs/heads/master@{#12558}
> 
>
TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,s...
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=webrtc:5682
> 
> Committed: https://crrev.com/d0dc66e0ea30c8614001e425a4ae0aa7dd56c2a7
> Cr-Commit-Position: refs/heads/master@{#12721}

TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,s...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5682
==========

to

==========
Revert of Delete webrtc::VideoFrame methods buffer and stride. (patchset #2
id:290001 of https://codereview.webrtc.org/1963413004/ )

Reason for revert:
Speculative revert to see if failures on the DrMemory bot are related to this
cl.  See e.g. here:
https://build.chromium.org/p/client.webrtc/builders/Win%20DrMemory%20Full/bui...

UNINITIALIZED READ: reading 0x04980040-0x04980060 32 byte(s) within
0x04980040-0x04980060
# 0 CopyRow_AVX
# 1 CopyPlane
# 2 I420Copy
# 3 webrtc::ExtractBuffer
# 4 cricket::WebRtcVideoCapturer::SignalFrameCapturedOnStartThread
# 5 cricket::WebRtcVideoCapturer::OnIncomingCapturedFrame
# 6 FakeWebRtcVideoCaptureModule::SendFrame
# 7 WebRtcVideoCapturerTest_TestCaptureVcm_Test::TestBody
# 8 testing::internal::HandleSehExceptionsInMethodIfSupported<>

Original issue's description:
> Reland of Delete webrtc::VideoFrame methods buffer and stride. (patchset #1
id:1 of https://codereview.webrtc.org/1935443002/ )
> 
> Reason for revert:
> I plan to reland this change in a week or two, after downstream users are
updated.
> 
> Original issue's description:
> > Revert of Delete webrtc::VideoFrame methods buffer and stride. (patchset #14
id:250001 of https://codereview.webrtc.org/1900673002/ )
> >
> > Reason for revert:
> > Breaks chrome FYI bots.
> >
> > Original issue's description:
> > > 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
> >
> >
TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,s...
> > # Skipping CQ checks because original CL landed less than 1 days ago.
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > NOTRY=true
> > BUG=webrtc:5682
> >
> > Committed: https://crrev.com/5b3c443d301f2c2f18dac5b02652c08b91ea3828
> > Cr-Commit-Position: refs/heads/master@{#12558}
> 
>
TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,s...
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=webrtc:5682
> 
> Committed: https://crrev.com/d0dc66e0ea30c8614001e425a4ae0aa7dd56c2a7
> Cr-Commit-Position: refs/heads/master@{#12721}

TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,s...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5682
==========

[commit-bot: I haz the power, 2016-05-14 10:18:09]

Committed patchset #1 (id:1)

[commit-bot: I haz the power, 2016-05-14 10:18:21]

Description was changed from

==========
Revert of Delete webrtc::VideoFrame methods buffer and stride. (patchset #2
id:290001 of https://codereview.webrtc.org/1963413004/ )

Reason for revert:
Speculative revert to see if failures on the DrMemory bot are related to this
cl.  See e.g. here:
https://build.chromium.org/p/client.webrtc/builders/Win%20DrMemory%20Full/bui...

UNINITIALIZED READ: reading 0x04980040-0x04980060 32 byte(s) within
0x04980040-0x04980060
# 0 CopyRow_AVX
# 1 CopyPlane
# 2 I420Copy
# 3 webrtc::ExtractBuffer
# 4 cricket::WebRtcVideoCapturer::SignalFrameCapturedOnStartThread
# 5 cricket::WebRtcVideoCapturer::OnIncomingCapturedFrame
# 6 FakeWebRtcVideoCaptureModule::SendFrame
# 7 WebRtcVideoCapturerTest_TestCaptureVcm_Test::TestBody
# 8 testing::internal::HandleSehExceptionsInMethodIfSupported<>

Original issue's description:
> Reland of Delete webrtc::VideoFrame methods buffer and stride. (patchset #1
id:1 of https://codereview.webrtc.org/1935443002/ )
> 
> Reason for revert:
> I plan to reland this change in a week or two, after downstream users are
updated.
> 
> Original issue's description:
> > Revert of Delete webrtc::VideoFrame methods buffer and stride. (patchset #14
id:250001 of https://codereview.webrtc.org/1900673002/ )
> >
> > Reason for revert:
> > Breaks chrome FYI bots.
> >
> > Original issue's description:
> > > 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
> >
> >
TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,s...
> > # Skipping CQ checks because original CL landed less than 1 days ago.
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > NOTRY=true
> > BUG=webrtc:5682
> >
> > Committed: https://crrev.com/5b3c443d301f2c2f18dac5b02652c08b91ea3828
> > Cr-Commit-Position: refs/heads/master@{#12558}
> 
>
TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,s...
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=webrtc:5682
> 
> Committed: https://crrev.com/d0dc66e0ea30c8614001e425a4ae0aa7dd56c2a7
> Cr-Commit-Position: refs/heads/master@{#12721}

TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,s...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5682
==========

to

==========
Revert of Delete webrtc::VideoFrame methods buffer and stride. (patchset #2
id:290001 of https://codereview.webrtc.org/1963413004/ )

Reason for revert:
Speculative revert to see if failures on the DrMemory bot are related to this
cl.  See e.g. here:
https://build.chromium.org/p/client.webrtc/builders/Win%20DrMemory%20Full/bui...

UNINITIALIZED READ: reading 0x04980040-0x04980060 32 byte(s) within
0x04980040-0x04980060
# 0 CopyRow_AVX
# 1 CopyPlane
# 2 I420Copy
# 3 webrtc::ExtractBuffer
# 4 cricket::WebRtcVideoCapturer::SignalFrameCapturedOnStartThread
# 5 cricket::WebRtcVideoCapturer::OnIncomingCapturedFrame
# 6 FakeWebRtcVideoCaptureModule::SendFrame
# 7 WebRtcVideoCapturerTest_TestCaptureVcm_Test::TestBody
# 8 testing::internal::HandleSehExceptionsInMethodIfSupported<>

Original issue's description:
> Reland of Delete webrtc::VideoFrame methods buffer and stride. (patchset #1
id:1 of https://codereview.webrtc.org/1935443002/ )
>
> Reason for revert:
> I plan to reland this change in a week or two, after downstream users are
updated.
>
> Original issue's description:
> > Revert of Delete webrtc::VideoFrame methods buffer and stride. (patchset #14
id:250001 of https://codereview.webrtc.org/1900673002/ )
> >
> > Reason for revert:
> > Breaks chrome FYI bots.
> >
> > Original issue's description:
> > > 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
> >
> >
TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,s...
> > # Skipping CQ checks because original CL landed less than 1 days ago.
> > NOPRESUBMIT=true
> > NOTREECHECKS=true
> > NOTRY=true
> > BUG=webrtc:5682
> >
> > Committed: https://crrev.com/5b3c443d301f2c2f18dac5b02652c08b91ea3828
> > Cr-Commit-Position: refs/heads/master@{#12558}
>
>
TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,s...
> # Not skipping CQ checks because original CL landed more than 1 days ago.
> BUG=webrtc:5682
>
> Committed: https://crrev.com/d0dc66e0ea30c8614001e425a4ae0aa7dd56c2a7
> Cr-Commit-Position: refs/heads/master@{#12721}

TBR=perkj@webrtc.org,magjed@webrtc.org,pbos@webrtc.org,pthatcher@webrtc.org,s...
# Skipping CQ checks because original CL landed less than 1 days ago.
NOPRESUBMIT=true
NOTREECHECKS=true
NOTRY=true
BUG=webrtc:5682

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

[commit-bot: I haz the power, 2016-05-14 10:18:22]

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

[tommi, 2016-05-14 12:49:04]

On 2016/05/14 10:18:22, commit-bot: I haz the power wrote:
> Patchset 1 (id:??) landed as
> https://crrev.com/d49c30cd2fe442f2b5b4ecec8d5cbaa430464725
> Cr-Commit-Position: refs/heads/master@{#12745}

Looks like this was causing that failure.  The following build is green.

[nisse-webrtc, 2016-05-16 11:19:39]

On 2016/05/14 12:49:04, tommi-webrtc wrote:
> On 2016/05/14 10:18:22, commit-bot: I haz the power wrote:
> > Patchset 1 (id:??) landed as
> > https://crrev.com/d49c30cd2fe442f2b5b4ecec8d5cbaa430464725
> > Cr-Commit-Position: refs/heads/master@{#12745}
> 
> Looks like this was causing that failure.  The following build is green.

This is a bit confusing. The test creates an uninitialized frame, in
FakeWebRtcVideoCaptureModule,

  void SendFrame(int w, int h) {
    if (!running_) return;
    webrtc::VideoFrame sample;
    // Setting stride based on width.
    sample.CreateEmptyFrame(w, h, w, (w + 1) / 2, (w + 1) / 2);
    if (callback_) {
      callback_->OnIncomingCapturedFrame(id_, sample);
    }
  }

Then this is passed to ExtractBuffer, which makes a copy if the uninitialized
data.
The copying used to be a loop copying each line of each plane using memcpy,
while
my cl changes it to call I420Copy. 

For valgrind, copying uninitialized memory around is not considered a problem,
it warns only when 
a branch or address depends on uninitialized data. Is DrMemory different? Maybe
there's 
some supression somewhere that no longer applies?

[nisse-webrtc, 2016-05-16 12:43:23]

I've digged a bit further (thanks pbos), see

https://bugs.chromium.org/p/libyuv/issues/detail?id=377
https://webrtc-codereview.appspot.com/25919004/
https://webrtc-codereview.appspot.com/41029004/

My understanding is that copying uninitialized data is not considered a problem
and that DrMemory shouldn't warn, but it warns nevertheless because it doesn't 
handle AVX instructions correctly.

I think I'll try to make a separate cl which just initializes the data (similar
to 
magjed's cl above), and then try to reland this with no further changes.

[tommi, 2016-05-16 12:56:35]

On 2016/05/16 12:43:23, nisse-webrtc wrote:
> I've digged a bit further (thanks pbos), see
> 
> https://bugs.chromium.org/p/libyuv/issues/detail?id=377
> https://webrtc-codereview.appspot.com/25919004/
> https://webrtc-codereview.appspot.com/41029004/
> 
> My understanding is that copying uninitialized data is not considered a
problem
> and that DrMemory shouldn't warn, but it warns nevertheless because it doesn't

> handle AVX instructions correctly.
> 
> I think I'll try to make a separate cl which just initializes the data
(similar
> to 
> magjed's cl above), and then try to reland this with no further changes.

sgtm.

[nisse-webrtc, 2016-05-16 13:41:52]

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

The reason for reverting is: Should work after cl
https://codereview.webrtc.org/1985693002/ is landed, which initializes the
frames used by FakeWebRtcVideoCaptureModule. So intend to reland after that,
with no changes..