MJPEG acceleration for video capture using VAAPI, the GPU host part

MJPEG acceleration for video capture, the GPU host part

BUG=335778
TEST=none

Committed: https://crrev.com/4073b070231e84d823cd368d478ec4014b9d2455
Cr-Commit-Position: refs/heads/master@{#335484}

[kcwu, 2015-06-08 13:32:55]

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

[kcwu, 2015-06-08 13:33:43]

This CL is split from https://codereview.chromium.org/1124423008/

[kcwu, 2015-06-08 14:07:08]

In summary, there are total 4 CLs for MJPEG acceleration. They are
- https://codereview.chromium.org/1124423008/ GPU and IPC part
- https://codereview.chromium.org/1165943008/ GPU host part <-- this one
- https://codereview.chromium.org/1132683004/ browser client to use
- https://codereview.chromium.org/1153163003/ flag to enable

[piman, 2015-06-08 21:22:18]

lgtm

https://codereview.chromium.org/1165943008/diff/20001/content/common/gpu/clie...
File content/common/gpu/client/gpu_jpeg_decode_accelerator_host.h (right):

https://codereview.chromium.org/1165943008/diff/20001/content/common/gpu/clie...
content/common/gpu/client/gpu_jpeg_decode_accelerator_host.h:38: bool
Initialize(media::JpegDecodeAccelerator::Client* client) override;
nit: can you add a comment that the Client is called on the IO thread, but is
never called into after the GpuJpegDecodeAcceleratorHost is destroyed?

[kcwu, 2015-06-09 08:34:43]

https://codereview.chromium.org/1165943008/diff/20001/content/common/gpu/clie...
File content/common/gpu/client/gpu_jpeg_decode_accelerator_host.h (right):

https://codereview.chromium.org/1165943008/diff/20001/content/common/gpu/clie...
content/common/gpu/client/gpu_jpeg_decode_accelerator_host.h:38: bool
Initialize(media::JpegDecodeAccelerator::Client* client) override;
On 2015/06/08 21:22:17, piman (Very slow to review) wrote:
> nit: can you add a comment that the Client is called on the IO thread, but is
> never called into after the GpuJpegDecodeAcceleratorHost is destroyed?

Done.

[kcwu, 2015-06-18 11:31:13]

The CQ bit was checked by kcwu@chromium.org

[kcwu, 2015-06-18 11:31:14]

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

[commit-bot: I haz the power, 2015-06-18 11:33:11]

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

[commit-bot: I haz the power, 2015-06-18 11:48:10]

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

[commit-bot: I haz the power, 2015-06-18 11:48:11]

Try jobs failed on following builders:
  linux_chromium_gn_chromeos_rel on tryserver.chromium.linux (JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[kcwu, 2015-06-18 11:51:36]

The CQ bit was checked by kcwu@chromium.org

[kcwu, 2015-06-18 11:51:37]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1165943008/#ps80001
(title: "fix compile")

[commit-bot: I haz the power, 2015-06-18 11:52:54]

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

[commit-bot: I haz the power, 2015-06-18 12:34:44]

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

[commit-bot: I haz the power, 2015-06-18 12:34:46]

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

[kcwu, 2015-06-18 14:11:32]

kcwu@chromium.org changed reviewers:
+ jschuh@chromium.org

[kcwu, 2015-06-18 14:11:32]

@jschuh, could you please advise how to deal SharedMemory cross-platform
properly in the following review comment?

Thanks

https://codereview.chromium.org/1165943008/diff/80001/content/common/gpu/clie...
File content/common/gpu/client/gpu_jpeg_decode_accelerator_host.cc (right):

https://codereview.chromium.org/1165943008/diff/80001/content/common/gpu/clie...
content/common/gpu/client/gpu_jpeg_decode_accelerator_host.cc:159:
base::ScopedFD(input_handle.fd);
(copy piman's comment from the CL before split
from
https://codereview.chromium.org/1124423008/diff/120001/content/common/gpu/cli...
)

This won't work on non-posix.
You can use SharedMemory::CloseHandle which is more portable, but unfortunately
neither one will do the right thing on Windows, because the returned handle is
for another process.
+jschuh do you have a best practice idea here? Can ShareToGpuProcess (i.e.
BrokerDuplicateHandle) fail, outside of either invalid handles or the target
process being closed?

[jschuh, 2015-06-19 14:43:19]

You should be very unlikely to get the handle copied on a failure, but it's not
totally impossible. If you can wait for the work on crbug.com/493414 to wrap up,
it should solve that problem.

[kcwu, 2015-06-20 01:11:53]

On 2015/06/19 14:43:19, jschuh (very slow) wrote:
> You should be very unlikely to get the handle copied on a failure, but it's
not
> totally impossible. If you can wait for the work on crbug.com/493414 to wrap
up,
> it should solve that problem.

Is crbug.com/493414 a big task? 
How about add a TODO mentions crbug.com/493414 and ignore the potential handle
leak temporarily?
Since there is no plan to use this code on Windows yet.

[piman, 2015-06-20 01:18:15]

On Fri, Jun 19, 2015 at 6:11 PM, <kcwu@chromium.org> wrote:

> On 2015/06/19 14:43:19, jschuh (very slow) wrote:
>
>> You should be very unlikely to get the handle copied on a failure, but
>> it's
>>
> not
>
>> totally impossible. If you can wait for the work on crbug.com/493414 to
>> wrap
>>
> up,
>
>> it should solve that problem.
>>
>
> Is crbug.com/493414 a big task?
> How about add a TODO mentions crbug.com/493414 and ignore the potential
> handle
> leak temporarily?
>

This is fine with me.


> Since there is no plan to use this code on Windows yet.
>
>
> https://codereview.chromium.org/1165943008/
>

To unsubscribe from this group and stop receiving emails from it, send an email
to chromium-reviews+unsubscribe@chromium.org.

[kcwu, 2015-06-20 01:39:58]

On 2015/06/20 01:18:15, piman (Very slow to review) wrote:
> On Fri, Jun 19, 2015 at 6:11 PM,
<https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=kcwu@chromium.org> wrote:
> 
> > On 2015/06/19 14:43:19, jschuh (very slow) wrote:
> >
> >> You should be very unlikely to get the handle copied on a failure, but
> >> it's
> >>
> > not
> >
> >> totally impossible. If you can wait for the work on crbug.com/493414 to
> >> wrap
> >>
> > up,
> >
> >> it should solve that problem.
> >>
> >
> > Is crbug.com/493414 a big task?
> > How about add a TODO mentions crbug.com/493414 and ignore the potential
> > handle
> > leak temporarily?
> >
> 
> This is fine with me.
TODO added.

[jschuh, 2015-06-20 02:49:55]

On 2015/06/20 01:39:58, kcwu wrote:
> On 2015/06/20 01:18:15, piman (Very slow to review) wrote:
> > On Fri, Jun 19, 2015 at 6:11 PM,
> <https://mail.google.com/mail/?view=cm&fs=1&tf=1&to=kcwu@chromium.org> wrote:
> > 
> > > On 2015/06/19 14:43:19, jschuh (very slow) wrote:
> > >
> > >> You should be very unlikely to get the handle copied on a failure, but
> > >> it's
> > >>
> > > not
> > >
> > >> totally impossible. If you can wait for the work on crbug.com/493414 to
> > >> wrap
> > >>
> > > up,
> > >
> > >> it should solve that problem.
> > >>
> > >
> > > Is crbug.com/493414 a big task?
> > > How about add a TODO mentions crbug.com/493414 and ignore the potential
> > > handle
> > > leak temporarily?
> > >
> > 
> > This is fine with me.
> TODO added.

Yeah, I'm fine with a TODO

[kcwu, 2015-06-22 10:14:29]

The CQ bit was checked by kcwu@chromium.org

[kcwu, 2015-06-22 10:14:30]

The patchset sent to the CQ was uploaded after l-g-t-m from piman@chromium.org
Link to the patchset: https://codereview.chromium.org/1165943008/#ps140001
(title: "rebase and fix compile on mac")

[commit-bot: I haz the power, 2015-06-22 10:14:39]

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

[commit-bot: I haz the power, 2015-06-22 10:17:53]

Committed patchset #8 (id:140001)

[commit-bot: I haz the power, 2015-06-22 10:19:50]

Patchset 8 (id:??) landed as
https://crrev.com/4073b070231e84d823cd368d478ec4014b9d2455
Cr-Commit-Position: refs/heads/master@{#335484}