Memory copy the VideoFrame to match the requirement for HW encoders.

Memory copy the VideoFrame to match the requirement for HW encoders.

BUG=578938
Committed: https://crrev.com/0c71f2a2df4b4cada834528289fae181e4ac7706
Cr-Commit-Position: refs/heads/master@{#393062}

[xjz, 2016-05-09 21:34:07]

xjz@chromium.org changed reviewers:
+ miu@chromium.org, xhwang@chromium.org

[xjz, 2016-05-09 21:34:08]

Initialized the VideoFrame in unit tests to pass the msan. PTAL.

[xhwang, 2016-05-10 19:04:04]

lgtm % nits

https://chromiumcodereview.appspot.com/1958913005/diff/20001/media/base/video...
File media/base/video_util_unittest.cc (right):

https://chromiumcodereview.appspot.com/1958913005/diff/20001/media/base/video...
media/base/video_util_unittest.cc:18: void InitializePlane(uint8_t* data, int
stride, const gfx::Size& visible_size) {
"Initialize" is too generic. How about something like FillPlaneWithPattern()

https://chromiumcodereview.appspot.com/1958913005/diff/20001/media/base/video...
media/base/video_util_unittest.cc:19: if (!data || visible_size.width() >
stride)
DCHECK here. Otherwise we could silently return.

https://chromiumcodereview.appspot.com/1958913005/diff/20001/media/base/video...
media/base/video_util_unittest.cc:30: // Create a VideoFrame and initialize the
visible rect using |InitializePlane|.
Could you add a comment why CreateColorFrame() won't work here?

https://chromiumcodereview.appspot.com/1958913005/diff/20001/media/base/video...
media/base/video_util_unittest.cc:31: scoped_refptr<media::VideoFrame>
CreateInitializedFrame(
ditto: "Initialized" seems too generic.

https://chromiumcodereview.appspot.com/1958913005/diff/20001/media/base/video...
media/base/video_util_unittest.cc:38: format, coded_size, visible_rect,
natural_size, timestamp));
nit: add an empty line here

[miu, 2016-05-10 20:51:32]

lgtm, but agree with xhwang's comments (need to be addressed before commit).

[xjz, 2016-05-10 21:08:44]

Thanks Xiaohan and Yuri! Addressed comments.

https://codereview.chromium.org/1958913005/diff/20001/media/base/video_util_u...
File media/base/video_util_unittest.cc (right):

https://codereview.chromium.org/1958913005/diff/20001/media/base/video_util_u...
media/base/video_util_unittest.cc:18: void InitializePlane(uint8_t* data, int
stride, const gfx::Size& visible_size) {
On 2016/05/10 19:04:04, xhwang wrote:
> "Initialize" is too generic. How about something like FillPlaneWithPattern()

Done.

https://codereview.chromium.org/1958913005/diff/20001/media/base/video_util_u...
media/base/video_util_unittest.cc:19: if (!data || visible_size.width() >
stride)
On 2016/05/10 19:04:04, xhwang wrote:
> DCHECK here. Otherwise we could silently return.

Done.

https://codereview.chromium.org/1958913005/diff/20001/media/base/video_util_u...
media/base/video_util_unittest.cc:30: // Create a VideoFrame and initialize the
visible rect using |InitializePlane|.
On 2016/05/10 19:04:04, xhwang wrote:
> Could you add a comment why CreateColorFrame() won't work here?

Done.

https://codereview.chromium.org/1958913005/diff/20001/media/base/video_util_u...
media/base/video_util_unittest.cc:31: scoped_refptr<media::VideoFrame>
CreateInitializedFrame(
On 2016/05/10 19:04:04, xhwang wrote:
> ditto: "Initialized" seems too generic.

Done.

https://codereview.chromium.org/1958913005/diff/20001/media/base/video_util_u...
media/base/video_util_unittest.cc:38: format, coded_size, visible_rect,
natural_size, timestamp));
On 2016/05/10 19:04:04, xhwang wrote:
> nit: add an empty line here

Done.

[xjz, 2016-05-10 21:09:02]

The CQ bit was checked by xjz@chromium.org

[xjz, 2016-05-10 21:09:03]

The patchset sent to the CQ was uploaded after l-g-t-m from xhwang@chromium.org,
miu@chromium.org
Link to the patchset: https://codereview.chromium.org/1958913005/#ps40001
(title: "Addressed comments.")

[commit-bot: I haz the power, 2016-05-10 21:09:22]

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

[commit-bot: I haz the power, 2016-05-11 01:57:49]

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

[commit-bot: I haz the power, 2016-05-11 01:57:50]

Try jobs failed on following builders:
  win_chromium_rel_ng on tryserver.chromium.win (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.win/builders/win_chromium_rel_...)

[xjz, 2016-05-11 16:49:26]

The CQ bit was checked by xjz@chromium.org

[commit-bot: I haz the power, 2016-05-11 16:49:46]

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

[commit-bot: I haz the power, 2016-05-11 21:17:35]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-05-11 21:26:16]

Description was changed from

==========
Memory copy the VideoFrame to match the requirement for HW encoders.

BUG=578938
==========

to

==========
Memory copy the VideoFrame to match the requirement for HW encoders.

BUG=578938

Committed: https://crrev.com/0c71f2a2df4b4cada834528289fae181e4ac7706
Cr-Commit-Position: refs/heads/master@{#393062}
==========

[commit-bot: I haz the power, 2016-05-11 21:26:17]

Patchset 3 (id:??) landed as
https://crrev.com/0c71f2a2df4b4cada834528289fae181e4ac7706
Cr-Commit-Position: refs/heads/master@{#393062}