Add PIXEL_FORMAT_I422.

Add PIXEL_FORMAT_I422.

The Mediatek JPEG decoder uses the multi-planar buffer format
V4L2_PIX_FMT_YUV422M.

(https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv422m.html)

Apart from how the buffer is allocated the format is basically
idential to I422 (a.k.a YU16).

BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Committed: https://crrev.com/3837dbaf61fa08ae501d4bb7374c6930b8d9ff68
Cr-Commit-Position: refs/heads/master@{#439324}

[jcliang, 2016-12-14 07:51:41]

Description was changed from

==========
Add PIXEL_FORMAT_I422.

BUG=655890
BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
==========

to

==========
Add PIXEL_FORMAT_I422.

BUG=655890
BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[jcliang, 2016-12-14 08:01:38]

Description was changed from

==========
Add PIXEL_FORMAT_I422.

BUG=655890
BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Add PIXEL_FORMAT_I422.

BUG=655890
BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[jcliang, 2016-12-14 08:01:39]

jcliang@chromium.org changed reviewers:
+ brianderson@chromium.org, erickung@chromium.org, liberato@chromium.org,
miu@chromium.org

[wuchengli, 2016-12-14 08:13:19]

The CQ bit was checked by wuchengli@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-14 08:13:30]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[wuchengli, 2016-12-14 08:16:53]

- Explain what this format is and why we need this format in the change
description.
- BUG=655890 can be removed because that's for MPLANE.

https://codereview.chromium.org/2571163002/diff/1/media/base/video_types.h
File media/base/video_types.h (right):

https://codereview.chromium.org/2571163002/diff/1/media/base/video_types.h#ne...
media/base/video_types.h:64: PIXEL_FORMAT_I422 =
I prefer PIXEL_FORMAT_YU16 to be consistent with PIXEL_FORMAT_YV16. Let's see
what media OWNER think about the naming.

https://codereview.chromium.org/2571163002/diff/1/media/base/video_types.h#ne...
media/base/video_types.h:65: 27,  // 16bpp YUV planar 1x1 Y, 2x2 UV samples,
a.k.a. YU16.
This should be 2x1 UV samples.

[jcliang, 2016-12-14 08:25:41]

https://codereview.chromium.org/2571163002/diff/1/media/base/video_types.h
File media/base/video_types.h (right):

https://codereview.chromium.org/2571163002/diff/1/media/base/video_types.h#ne...
media/base/video_types.h:64: PIXEL_FORMAT_I422 =
On 2016/12/14 08:16:53, wuchengli wrote:
> I prefer PIXEL_FORMAT_YU16 to be consistent with PIXEL_FORMAT_YV16. Let's see
> what media OWNER think about the naming.

I followed the naming of libyuv which lists YU16 as an alias to I422. Not sure
what the best naming is here...

https://codereview.chromium.org/2571163002/diff/1/media/base/video_types.h#ne...
media/base/video_types.h:65: 27,  // 16bpp YUV planar 1x1 Y, 2x2 UV samples,
a.k.a. YU16.
On 2016/12/14 08:16:53, wuchengli wrote:
> This should be 2x1 UV samples.

Done.

[jcliang, 2016-12-14 08:34:57]

Description was changed from

==========
Add PIXEL_FORMAT_I422.

BUG=655890
BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Add PIXEL_FORMAT_I422.

The Mediatek JPEG decoder uses the multi-planar buffer format
V4L2_PIX_FMT_YUV422M.

(https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv422m.html)

Apart from how the buffer is allocated the format is basically
idential to I422 (a.k.a YU16).

BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
==========

[jcliang, 2016-12-14 08:46:48]

Description was changed from

==========
Add PIXEL_FORMAT_I422.

The Mediatek JPEG decoder uses the multi-planar buffer format
V4L2_PIX_FMT_YUV422M.

(https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv422m.html)

Apart from how the buffer is allocated the format is basically
idential to I422 (a.k.a YU16).

BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
==========

to

==========
Add PIXEL_FORMAT_I422.

The Mediatek JPEG decoder uses the multi-planar buffer format
V4L2_PIX_FMT_YUV422M.

(https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv422m.html)

Apart from how the buffer is allocated the format is basically
idential to I422 (a.k.a YU16).

BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[wuchengli, 2016-12-14 08:50:51]

lgtm

[jcliang, 2016-12-14 08:52:11]

The CQ bit was checked by jcliang@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-12-14 08:52:25]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-14 10:33:48]

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

[commit-bot: I haz the power, 2016-12-14 10:33:49]

Dry run: This issue passed the CQ dry run.

[liberato (no reviews please), 2016-12-14 17:51:44]

i prefer I422 also, since 'YU' and 'YV' are visually very similar.

lgtm % a comment.

thanks
-fl

https://codereview.chromium.org/2571163002/diff/40001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2571163002/diff/40001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:90: DCHECK(video_frame->format() ==
PIXEL_FORMAT_I420 ||
seems like I422 will get here via UpdateLastImage, since NumPlanes > 1.  lots of
formats do, too, and will hit this DCHECK.  i'm not sure what's up.

If this fn actually works for I422, then please update the DCHECK to allow it.

either way, would you mind adding a comment that we should compare the DCHECK vs
when UpdateLastPlanes calls it?  please reference crbug.com/674185, which i just
opened to track this.

[brianderson, 2016-12-14 19:32:59]

cc lgtm

[erickung1, 2016-12-14 19:42:26]

media/remoting lgtm

https://codereview.chromium.org/2571163002/diff/40001/media/remoting/rpc/prot...
File media/remoting/rpc/proto_enum_utils.cc (right):

https://codereview.chromium.org/2571163002/diff/40001/media/remoting/rpc/prot...
media/remoting/rpc/proto_enum_utils.cc:313:
CASE_RETURN_OTHER(PIXEL_FORMAT_I422);
any reason to add it here rather than add it at the end of switch case (line
#339) in order to make it consistent with enum value order ? since there is no
grouping logic in the switch case

same question in line #350

[jcliang, 2016-12-15 04:28:43]

https://codereview.chromium.org/2571163002/diff/40001/media/remoting/rpc/prot...
File media/remoting/rpc/proto_enum_utils.cc (right):

https://codereview.chromium.org/2571163002/diff/40001/media/remoting/rpc/prot...
media/remoting/rpc/proto_enum_utils.cc:313:
CASE_RETURN_OTHER(PIXEL_FORMAT_I422);
On 2016/12/14 19:42:26, erickung1 wrote:
> any reason to add it here rather than add it at the end of switch case (line
> #339) in order to make it consistent with enum value order ? since there is no
> grouping logic in the switch case
> 
> same question in line #350

Nope. I put it here only because the name was similar to I420. I'll move it to
the end to align with the enum value order.

https://codereview.chromium.org/2571163002/diff/40001/media/renderers/skcanva...
File media/renderers/skcanvas_video_renderer.cc (right):

https://codereview.chromium.org/2571163002/diff/40001/media/renderers/skcanva...
media/renderers/skcanvas_video_renderer.cc:90: DCHECK(video_frame->format() ==
PIXEL_FORMAT_I420 ||
On 2016/12/14 17:51:44, liberato wrote:
> seems like I422 will get here via UpdateLastImage, since NumPlanes > 1.  lots
of
> formats do, too, and will hit this DCHECK.  i'm not sure what's up.
> 
> If this fn actually works for I422, then please update the DCHECK to allow it.
> 
> either way, would you mind adding a comment that we should compare the DCHECK
vs
> when UpdateLastPlanes calls it?  please reference crbug.com/674185, which i
just
> opened to track this.

Done.

I'm not familiar with this module, but from line 146 I assume this function
currently only intends to work with I420 or NV12.

[jcliang, 2016-12-15 04:29:09]

The CQ bit was checked by jcliang@chromium.org to run a CQ dry run

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

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-12-15 06:14:27]

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

[commit-bot: I haz the power, 2016-12-15 06:14:28]

Dry run: This issue passed the CQ dry run.

[miu, 2016-12-15 22:55:42]

lgtm (took a quick look--I trust the other reviewers lgtm's)

[jcliang, 2016-12-16 04:20:27]

The CQ bit was checked by jcliang@chromium.org

[jcliang, 2016-12-16 04:20:27]

The patchset sent to the CQ was uploaded after l-g-t-m from
liberato@chromium.org, wuchengli@chromium.org, erickung@chromium.org,
brianderson@chromium.org
Link to the patchset: https://codereview.chromium.org/2571163002/#ps60001
(title: "move switch cases to align with the enum value order")

[commit-bot: I haz the power, 2016-12-16 04:21:03]

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

[commit-bot: I haz the power, 2016-12-16 04:55:13]

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

[commit-bot: I haz the power, 2016-12-16 04:55:14]

Try jobs failed on following builders:
  mac_chromium_compile_dbg_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_comp...)

[wuchengli, 2016-12-16 06:03:21]

The CQ bit was checked by wuchengli@chromium.org

[commit-bot: I haz the power, 2016-12-16 06:03:52]

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

[commit-bot: I haz the power, 2016-12-16 08:50:30]

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

[commit-bot: I haz the power, 2016-12-16 08:50:31]

Try jobs failed on following builders:
  mac_chromium_rel_ng on master.tryserver.chromium.mac (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[jcliang, 2016-12-17 03:28:51]

The CQ bit was checked by jcliang@chromium.org

[commit-bot: I haz the power, 2016-12-17 03:29:13]

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

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

CQ is committing da patch.

Bot data: {"patchset_id": 60001, "attempt_start_ts": 1481945331939290,
"parent_rev": "dbf4176257c2f2798a54a0450fe9cc6032e44cbe", "commit_rev":
"93c0fd0b2f4d919f76d6d08082b546cb3b888eb6"}

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

Description was changed from

==========
Add PIXEL_FORMAT_I422.

The Mediatek JPEG decoder uses the multi-planar buffer format
V4L2_PIX_FMT_YUV422M.

(https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv422m.html)

Apart from how the buffer is allocated the format is basically
idential to I422 (a.k.a YU16).

BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Add PIXEL_FORMAT_I422.

The Mediatek JPEG decoder uses the multi-planar buffer format
V4L2_PIX_FMT_YUV422M.

(https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv422m.html)

Apart from how the buffer is allocated the format is basically
idential to I422 (a.k.a YU16).

BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2571163002
==========

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

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2016-12-17 04:48:20]

Description was changed from

==========
Add PIXEL_FORMAT_I422.

The Mediatek JPEG decoder uses the multi-planar buffer format
V4L2_PIX_FMT_YUV422M.

(https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv422m.html)

Apart from how the buffer is allocated the format is basically
idential to I422 (a.k.a YU16).

BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2571163002
==========

to

==========
Add PIXEL_FORMAT_I422.

The Mediatek JPEG decoder uses the multi-planar buffer format
V4L2_PIX_FMT_YUV422M.

(https://linuxtv.org/downloads/v4l-dvb-apis/uapi/v4l/pixfmt-yuv422m.html)

Apart from how the buffer is allocated the format is basically
idential to I422 (a.k.a YU16).

BUG=chrome-os-partner:43703
TEST=Build chrome and test with jpeg decoder that uses I422 format.
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Committed: https://crrev.com/3837dbaf61fa08ae501d4bb7374c6930b8d9ff68
Cr-Commit-Position: refs/heads/master@{#439324}
==========

[commit-bot: I haz the power, 2016-12-17 04:48:21]

Patchset 4 (id:??) landed as
https://crrev.com/3837dbaf61fa08ae501d4bb7374c6930b8d9ff68
Cr-Commit-Position: refs/heads/master@{#439324}