Add use_v4lplugin flag to enable v4l2 library

Add use_v4lplugin flag to enable v4l2 library

Dynamic load v4l-utils related libraries in unittest.

BUG=405861
TEST=use video_encode_accelerator_unittest to make sure
v4l2_open and plugin_init is called. Related shared libraries are from
chromeos. When use_v4lplugin=0, unittest doesn't open v4l2 libraries.

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

[henryhsu, 2014-11-06 11:29:00]

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

[henryhsu, 2014-11-18 03:58:53]

henryhsu@chromium.org changed reviewers:
+ cpu@chromium.org

[henryhsu, 2014-11-18 03:58:54]

Ready for review, PTAL.

[Pawel Osciak, 2014-11-18 04:55:39]

On 2014/11/18 03:58:54, henryhsu wrote:
> Ready for review, PTAL.

Please also TEST=/verify on existing platforms that don't use plugins.

[henryhsu, 2014-11-18 06:01:50]

On 2014/11/18 04:55:39, Pawel Osciak wrote:
> On 2014/11/18 03:58:54, henryhsu wrote:
> > Ready for review, PTAL.
> 
> Please also TEST=/verify on existing platforms that don't use plugins.

now use_v4lplugin flag is passed from chromeos. So I use "gyp_chromium
-Duse_v4lplugin=1" to test.
I think we can wait chromeos-chrome ebuild landed to ToT and then check the flag
in different platforms.

[Pawel Osciak, 2014-11-18 08:36:30]

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
File content/common/gpu/media/exynos_v4l2_video_device.cc (right):

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:21: #  include<libv4l2.h>
No indent please. And space before '<'.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:32: #  define v4l2_close
close
No indent please.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:34: #  define v4l2_read
read
We don't use read or write.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:36: #  define v4l2_mmap
mmap
We don't want to do this if we don't do conversion, there is a lot of stuff
going on in there and the mmap call in the library doesn't seem to be checking
for the flag. Is there a fast path through v4l2_mmap if we passed
DISABLE_CONVERSION?
For now I'd remove this. Maybe we can have a patch for the library to ensure
DISABLE_CONVERSION is honored everywhere?

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:37: #  define v4l2_munmap
munmap
Ditto.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:38: int v4l2_fd_open(int
fd, int v4l2_flags) {
Better just ifdef the call out instead of this and the define for the flag.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:55: #ifdef USE_LIBV4L2
Please extract this to PostSandboxInitialization(). See example at
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu...

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:59: LOG(ERROR) << "Failed
to initialize LIBV4L2 libs";
Shouldn't this be a critical error? Please do this in Initialize() so that we
can return an error.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:65:
v4l2_close(device_poll_interrupt_fd_);
We should not use v4l2_* functions on interrupt fd, this is not a v4l2 fd.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:164: if
(v4l2_fd_open(device_fd_, V4L2_DISABLE_CONVERSION) == -1) {
Do we need to HANDLE_EINTR?

[Pawel Osciak, 2014-11-18 08:36:48]

posciak@chromium.org changed reviewers:
+ phajdan.jr@chromium.org

[Pawel Osciak, 2014-11-18 08:37:57]

By the way, I think it would be best if you separated the third_party/
submission from changes to other code please.

[henryhsu, 2014-11-19 03:49:47]

all done. PTAL

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
File content/common/gpu/media/exynos_v4l2_video_device.cc (right):

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:21: #  include<libv4l2.h>
On 2014/11/18 08:36:30, Pawel Osciak wrote:
> No indent please. And space before '<'.

Done.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:32: #  define v4l2_close
close
On 2014/11/18 08:36:30, Pawel Osciak wrote:
> No indent please.

Done.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:34: #  define v4l2_read
read
On 2014/11/18 08:36:29, Pawel Osciak wrote:
> We don't use read or write.

Done.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:36: #  define v4l2_mmap
mmap
On 2014/11/18 08:36:30, Pawel Osciak wrote:
> We don't want to do this if we don't do conversion, there is a lot of stuff
> going on in there and the mmap call in the library doesn't seem to be checking
> for the flag. Is there a fast path through v4l2_mmap if we passed
> DISABLE_CONVERSION?
> For now I'd remove this. Maybe we can have a patch for the library to ensure
> DISABLE_CONVERSION is honored everywhere?

Agree. use mmap first and then add a patch at the next step.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:37: #  define v4l2_munmap
munmap
On 2014/11/18 08:36:30, Pawel Osciak wrote:
> Ditto.

Done.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:38: int v4l2_fd_open(int
fd, int v4l2_flags) {
On 2014/11/18 08:36:30, Pawel Osciak wrote:
> Better just ifdef the call out instead of this and the define for the flag.

Done.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:55: #ifdef USE_LIBV4L2
On 2014/11/18 08:36:30, Pawel Osciak wrote:
> Please extract this to PostSandboxInitialization(). See example at
>
https://code.google.com/p/chromium/codesearch#chromium/src/content/common/gpu...

Done.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:59: LOG(ERROR) << "Failed
to initialize LIBV4L2 libs";
On 2014/11/18 08:36:30, Pawel Osciak wrote:
> Shouldn't this be a critical error? Please do this in Initialize() so that we
> can return an error.

Done.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:65:
v4l2_close(device_poll_interrupt_fd_);
On 2014/11/18 08:36:30, Pawel Osciak wrote:
> We should not use v4l2_* functions on interrupt fd, this is not a v4l2 fd.

Done.

https://codereview.chromium.org/700383004/diff/60001/content/common/gpu/media...
content/common/gpu/media/exynos_v4l2_video_device.cc:164: if
(v4l2_fd_open(device_fd_, V4L2_DISABLE_CONVERSION) == -1) {
On 2014/11/18 08:36:30, Pawel Osciak wrote:
> Do we need to HANDLE_EINTR?

Done.

[Pawel Osciak, 2014-11-19 10:57:05]

Please update the CL message.

[Pawel Osciak, 2014-11-19 10:59:54]

I don't think CQ-DEPEND works for Chrome? You'll have to wait for the other CL
to get in before CQ-ing this one?

[Pawel Osciak, 2014-11-19 11:05:21]

posciak@chromium.org changed reviewers:
+ jorgelo@chromium.org

[Pawel Osciak, 2014-11-19 11:05:22]

+Jorge FYI (new library in GPU process)

And one last nit.

https://chromiumcodereview.appspot.com/700383004/diff/100001/content/common/g...
File content/common/gpu/media/exynos_v4l2_video_device.cc (right):

https://chromiumcodereview.appspot.com/700383004/diff/100001/content/common/g...
content/common/gpu/media/exynos_v4l2_video_device.cc:21: #include <libv4l2.h>
Please instead "third_party/v4l-utils/lib/include/libv4l2.h"

[henryhsu, 2014-11-20 05:11:58]

PTAL

https://codereview.chromium.org/700383004/diff/100001/content/common/gpu/medi...
File content/common/gpu/media/exynos_v4l2_video_device.cc (right):

https://codereview.chromium.org/700383004/diff/100001/content/common/gpu/medi...
content/common/gpu/media/exynos_v4l2_video_device.cc:21: #include <libv4l2.h>
On 2014/11/19 11:05:22, Pawel Osciak wrote:
> Please instead "third_party/v4l-utils/lib/include/libv4l2.h"

Done.

[Pawel Osciak, 2014-11-23 14:32:35]

lgtm

[henryhsu, 2015-01-09 02:26:04]

The CQ bit was checked by henryhsu@chromium.org

[commit-bot: I haz the power, 2015-01-09 02:27:44]

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

[commit-bot: I haz the power, 2015-01-09 03:28:18]

Committed patchset #10 (id:180001)

[commit-bot: I haz the power, 2015-01-09 03:30:00]

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