Fix visible size for V4L2 VDA

Fix visible size for V4L2 VDA

Note this CL depends on s5p-mfc kernel driver fix https://chromium.googlesource.com/chromiumos/third_party/kernel/+/090b79f, which is available since 6840.0.0
Otherwise exynos chromeos will crash.

BUG=chromium:449920
TEST=apprtc loopback with resolution 640x354 on peach pit; run run video_VideoSanity

Committed: https://crrev.com/ead8f96b24fe7a382ab113c0f7a60c528025c0c6
Cr-Commit-Position: refs/heads/master@{#319059}

[kcwu, 2015-03-03 06:30:26]

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

[kcwu, 2015-03-03 06:31:59]

Patch set 1 is https://codereview.chromium.org/874083005
Patch set 2 is revision according to discussion in
https://code.google.com/p/chromium/issues/detail?id=449920
Please take a llok

[Owen Lin, 2015-03-03 07:35:22]

lgtm % nits

https://codereview.chromium.org/972063002/diff/20001/content/common/gpu/media...
File content/common/gpu/media/v4l2_video_decode_accelerator.cc (right):

https://codereview.chromium.org/972063002/diff/20001/content/common/gpu/media...
content/common/gpu/media/v4l2_video_decode_accelerator.cc:1661: <<
coded_size_.ToString();
Also log the visible size ?

https://codereview.chromium.org/972063002/diff/20001/content/common/gpu/media...
content/common/gpu/media/v4l2_video_decode_accelerator.cc:2015: bool ret =
GetFormatInfo(&format, &visible_size, &again);
[Optional] I would prefer we pass nullptr for visible_size if we don't need this
information.

[Pawel Osciak, 2015-03-03 08:28:28]

https://codereview.chromium.org/972063002/diff/20001/content/common/gpu/media...
File content/common/gpu/media/v4l2_video_decode_accelerator.cc (right):

https://codereview.chromium.org/972063002/diff/20001/content/common/gpu/media...
content/common/gpu/media/v4l2_video_decode_accelerator.cc:1669: bool
V4L2VideoDecodeAccelerator::GetVisibleSize(const gfx::Size& coded_size,
Could you make this return visible_size, and move all the logic for falling back
to using coded_size here as well?
Then you wouldn't need the bool return and could contain all that logic to this
method, which I think is where it belongs.

[kcwu, 2015-03-03 09:45:32]

https://codereview.chromium.org/972063002/diff/20001/content/common/gpu/media...
File content/common/gpu/media/v4l2_video_decode_accelerator.cc (right):

https://codereview.chromium.org/972063002/diff/20001/content/common/gpu/media...
content/common/gpu/media/v4l2_video_decode_accelerator.cc:1661: <<
coded_size_.ToString();
On 2015/03/03 07:35:22, Owen Lin wrote:
> Also log the visible size ?

Done.

https://codereview.chromium.org/972063002/diff/20001/content/common/gpu/media...
content/common/gpu/media/v4l2_video_decode_accelerator.cc:1669: bool
V4L2VideoDecodeAccelerator::GetVisibleSize(const gfx::Size& coded_size,
On 2015/03/03 08:28:28, Pawel Osciak wrote:
> Could you make this return visible_size, and move all the logic for falling
back
> to using coded_size here as well?
> Then you wouldn't need the bool return and could contain all that logic to
this
> method, which I think is where it belongs.

Done.

https://codereview.chromium.org/972063002/diff/20001/content/common/gpu/media...
content/common/gpu/media/v4l2_video_decode_accelerator.cc:2015: bool ret =
GetFormatInfo(&format, &visible_size, &again);
On 2015/03/03 07:35:22, Owen Lin wrote:
> [Optional] I would prefer we pass nullptr for visible_size if we don't need
this
> information.

Done.

[wuchengli, 2015-03-04 10:24:39]

Pawel. PTAL.

[Pawel Osciak, 2015-03-04 10:50:22]

lgtm

https://codereview.chromium.org/972063002/diff/60001/content/common/gpu/media...
File content/common/gpu/media/v4l2_video_decode_accelerator.h (right):

https://codereview.chromium.org/972063002/diff/60001/content/common/gpu/media...
content/common/gpu/media/v4l2_video_decode_accelerator.h:256: // cropping is not
supported or the crop size is not inside |coded_size|.
Mentioning cropping is not really relevant here I feel.

Please just say "Return visible size, or, if querying it is not supported or
produces invalid size, return |coded_size| instead."

[kcwu, 2015-03-04 13:05:04]

https://codereview.chromium.org/972063002/diff/60001/content/common/gpu/media...
File content/common/gpu/media/v4l2_video_decode_accelerator.h (right):

https://codereview.chromium.org/972063002/diff/60001/content/common/gpu/media...
content/common/gpu/media/v4l2_video_decode_accelerator.h:256: // cropping is not
supported or the crop size is not inside |coded_size|.
On 2015/03/04 10:50:22, Pawel Osciak wrote:
> Mentioning cropping is not really relevant here I feel.
> 
> Please just say "Return visible size, or, if querying it is not supported or
> produces invalid size, return |coded_size| instead."

Done.

[kcwu, 2015-03-04 13:05:13]

The CQ bit was checked by kcwu@chromium.org

[kcwu, 2015-03-04 13:05:14]

The patchset sent to the CQ was uploaded after l-g-t-m from
owenlin@chromium.org, posciak@chromium.org
Link to the patchset: https://codereview.chromium.org/972063002/#ps80001 (title:
" ")

[commit-bot: I haz the power, 2015-03-04 13:05:55]

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

[commit-bot: I haz the power, 2015-03-04 13:27:05]

Committed patchset #5 (id:80001)

[commit-bot: I haz the power, 2015-03-04 13:27:32]

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