Create VA surface with correct VA RT format for MJPEG decode acceleration

Create VA surface with correct VA RT format for MJPEG decode acceleration

BUG=chromium:335778, chrome-os-partner:41112
TEST=decode 420 and 422 jpeg images using the same VaapiJpegDecodeAccelerator

Committed: https://crrev.com/4ecfb3c8da350c278ac8dfe7ad7be634ae52c939
Cr-Commit-Position: refs/heads/master@{#338481}

[kcwu, 2015-07-08 12:43:06]

kcwu@chromium.org changed reviewers:
+ henryhsu@chromium.org, posciak@chromium.org, wuchengli@chromium.org

[kcwu, 2015-07-08 12:43:07]

PTAL
Henry, this should fix the issue raised by your test.

[henryhsu, 2015-07-09 06:56:26]

And I also ran jpeg_decode_accelerator_unittest again with this patch. All test
cases passed.

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
File content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc (right):

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc:33: const
media::JpegParseResult& parse_result) {
Can we pass JpegFrameHeader directly since we don't use other parts in
JpegParseResult?

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc:239:
media::JpegDecodeAccelerator::UNSUPPORTED_JPEG);
You don't need "media::JpegDecodeAccelerator::" since VaapiJpegDecodeAccelerator
inherits from media::JpegDecodeAccelerator.
Same as others.

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
File content/common/gpu/media/vaapi_wrapper.h (right):

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
content/common/gpu/media/vaapi_wrapper.h:86: // the destructor.
describe |va_format| in comment.

[kcwu, 2015-07-13 05:45:16]

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
File content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc (right):

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc:33: const
media::JpegParseResult& parse_result) {
On 2015/07/09 06:56:26, henryhsu wrote:
> Can we pass JpegFrameHeader directly since we don't use other parts in
> JpegParseResult?

Done.

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc:239:
media::JpegDecodeAccelerator::UNSUPPORTED_JPEG);
On 2015/07/09 06:56:26, henryhsu wrote:
> You don't need "media::JpegDecodeAccelerator::" since
VaapiJpegDecodeAccelerator
> inherits from media::JpegDecodeAccelerator.
> Same as others.

Done.

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
File content/common/gpu/media/vaapi_wrapper.h (right):

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
content/common/gpu/media/vaapi_wrapper.h:86: // the destructor.
On 2015/07/09 06:56:26, henryhsu wrote:
> describe |va_format| in comment.

Done.

[wuchengli, 2015-07-13 07:26:22]

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
File content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc (right):

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc:32: unsigned int
VaSurfaceFormatForJpeg(
static

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc:40:
parse_result.frame_header.components[i].horizontal_sampling_factor, 4);
Is this a bug or invalid jpeg? If it's an invalid jpeg, it shouldn't crash in
debug mode and should be changed to DVLOG.

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc:67: }
DVLOG the values of h and v here.

[wuchengli, 2015-07-13 07:28:37]

LGTM

[kcwu, 2015-07-13 07:39:40]

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
File content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc (right):

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc:32: unsigned int
VaSurfaceFormatForJpeg(
On 2015/07/13 07:26:22, wuchengli wrote:
> static

Done.

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc:40:
parse_result.frame_header.components[i].horizontal_sampling_factor, 4);
On 2015/07/13 07:26:22, wuchengli wrote:
> Is this a bug or invalid jpeg? If it's an invalid jpeg, it shouldn't crash in
> debug mode and should be changed to DVLOG.

Invalid jpeg should already be handled by jpeg parser. So it is bug if they
reached here.

https://codereview.chromium.org/1209033018/diff/1/content/common/gpu/media/va...
content/common/gpu/media/vaapi_jpeg_decode_accelerator.cc:67: }
On 2015/07/13 07:26:22, wuchengli wrote:
> DVLOG the values of h and v here.

Done.

[kcwu, 2015-07-13 07:47:16]

The CQ bit was checked by kcwu@chromium.org

[kcwu, 2015-07-13 07:47:17]

The patchset sent to the CQ was uploaded after l-g-t-m from
wuchengli@chromium.org
Link to the patchset: https://codereview.chromium.org/1209033018/#ps60001
(title: " ")

[commit-bot: I haz the power, 2015-07-13 07:47:25]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/1209033018/60001

[commit-bot: I haz the power, 2015-07-13 08:22:15]

Committed patchset #4 (id:60001)

[commit-bot: I haz the power, 2015-07-13 08:24:10]

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

[dgozman, 2015-07-13 09:51:54]

A revert of this CL (patchset #4 id:60001) has been created in
https://codereview.chromium.org/1239443003/ by dgozman@chromium.org.

The reason for reverting is: Broke compile. See
https://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20x86-ge....