Add I420Buffer::Copy method taking plane pointers as input.

Add I420Buffer::Copy method taking plane pointers as input.

BUG=None.

Committed: https://crrev.com/2f58ec82fc26700ff47f8e99effa7da94a3e46e3
Cr-Commit-Position: refs/heads/master@{#15259}

[nisse-webrtc, 2016-11-25 15:07:46]

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

[nisse-webrtc, 2016-11-25 15:07:47]

Please take a look. I think Copy is a better name than Create for this one.

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/i420_vide...
File webrtc/common_video/i420_video_frame_unittest.cc (left):

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/i420_vide...
webrtc/common_video/i420_video_frame_unittest.cc:163:
rtc::Callback0<void>([](){})))),
The main point of this change is to get rid of this awkward construction. I know
of one downstream project doing the same, and that blocks landing of
https://codereview.webrtc.org/2528143002/

[magjed_webrtc, 2016-11-28 08:39:56]

lgtm

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/include/v...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/include/v...
webrtc/common_video/include/video_frame_buffer.h:64: public:
The order must be public, protected, private.

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/include/v...
webrtc/common_video/include/video_frame_buffer.h:100: static
rtc::scoped_refptr<I420Buffer> Copy(const VideoFrameBuffer& buffer);
Can you move both these static methods up to the other static methods?

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/video_fra...
File webrtc/common_video/video_frame_buffer.cc (right):

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/video_fra...
webrtc/common_video/video_frame_buffer.cc:139:
RTC_CHECK(libyuv::I420Copy(data_y, stride_y,
RTC_CHECK_EQ(0, ...)

[nisse-webrtc, 2016-11-28 09:44:54]

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/include/v...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/include/v...
webrtc/common_video/include/video_frame_buffer.h:64: public:
On 2016/11/28 08:39:56, magjed_webrtc wrote:
> The order must be public, protected, private.

Done.

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/include/v...
webrtc/common_video/include/video_frame_buffer.h:100: static
rtc::scoped_refptr<I420Buffer> Copy(const VideoFrameBuffer& buffer);
On 2016/11/28 08:39:56, magjed_webrtc wrote:
> Can you move both these static methods up to the other static methods?

Done, except for the deprecated version of the Copy method, which is grouped
with the other deprecated methods. 

I guess I should reorder the implementations in the .cc file too, for
consistency?

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/video_fra...
File webrtc/common_video/video_frame_buffer.cc (right):

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/video_fra...
webrtc/common_video/video_frame_buffer.cc:139:
RTC_CHECK(libyuv::I420Copy(data_y, stride_y,
On 2016/11/28 08:39:56, magjed_webrtc wrote:
> RTC_CHECK_EQ(0, ...)

Done.

https://codereview.webrtc.org/2528153002/diff/20001/webrtc/common_video/video...
File webrtc/common_video/video_frame_buffer.cc (right):

https://codereview.webrtc.org/2528153002/diff/20001/webrtc/common_video/video...
webrtc/common_video/video_frame_buffer.cc:110: RTC_CHECK(src->DataY());
I also changed these from DCHECK to CHECK. I think a check failure is preferable
to a crash in I420Rotate.

https://codereview.webrtc.org/2528153002/diff/20001/webrtc/common_video/video...
webrtc/common_video/video_frame_buffer.cc:128: RTC_CHECK_EQ(0,
libyuv::I420Rotate(
And this is changed from res = ...; RTC_DCHECK_EQ(res, 0);

[magjed_webrtc, 2016-11-28 10:18:20]

lgtm

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/include/v...
File webrtc/common_video/include/video_frame_buffer.h (right):

https://codereview.webrtc.org/2528153002/diff/1/webrtc/common_video/include/v...
webrtc/common_video/include/video_frame_buffer.h:100: static
rtc::scoped_refptr<I420Buffer> Copy(const VideoFrameBuffer& buffer);
On 2016/11/28 09:44:54, nisse-webrtc wrote:
> On 2016/11/28 08:39:56, magjed_webrtc wrote:
> > Can you move both these static methods up to the other static methods?
> 
> Done, except for the deprecated version of the Copy method, which is grouped
> with the other deprecated methods. 
> 
> I guess I should reorder the implementations in the .cc file too, for
> consistency?

Yeah, it's probably best to reorder the .cc file too.

[nisse-webrtc, 2016-11-28 10:51:57]

The CQ bit was checked by nisse@webrtc.org

[commit-bot: I haz the power, 2016-11-28 10:52:03]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.webrtc.org/...

[commit-bot: I haz the power, 2016-11-28 11:47:55]

CQ is committing da patch.

Bot data: {"patchset_id": 20001, "attempt_start_ts": 1480330317668090,
"parent_rev": "8ba09ad5ce00bc6522001c8425619785a3a1f435", "commit_rev":
"3b15f6a0d46a2985c158271e619c6b72c1269330"}

[commit-bot: I haz the power, 2016-11-28 11:47:57]

Committed patchset #2 (id:20001)

[commit-bot: I haz the power, 2016-11-28 11:48:07]

Description was changed from

==========
Add I420Buffer::Copy method taking plane pointers as input.

BUG=None.
==========

to

==========
Add I420Buffer::Copy method taking plane pointers as input.

BUG=None.

Committed: https://crrev.com/2f58ec82fc26700ff47f8e99effa7da94a3e46e3
Cr-Commit-Position: refs/heads/master@{#15259}
==========

[commit-bot: I haz the power, 2016-11-28 11:48:08]

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