VAVEA: Turn VAAPI encoder flag into a kill switch.

VAVEA: Turn VAAPI encoder flag into a kill switch.
    
Flip the flag to enable VAVEA by default.

Also update the CastStreamingApiTest to not require VP8, as we may be using
H264 as well, but just assert on non-empty string for the codec. Do the
same for the audio codec while we are here.


TEST=encoding scenarios
BUG=378962, 398322
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=288920

[Pawel Osciak, 2014-07-07 06:23:16]

ptal

[wuchengli, 2014-07-07 07:34:34]

lgtm

[piman, 2014-07-07 17:57:47]

https://codereview.chromium.org/374553002/diff/1/content/common/gpu/media/vaa...
File content/common/gpu/media/vaapi_video_encode_accelerator.cc (right):

https://codereview.chromium.org/374553002/diff/1/content/common/gpu/media/vaa...
content/common/gpu/media/vaapi_video_encode_accelerator.cc:117:
(cpu.vendor_name() == "GenuineIntel") && (cpu.model() == 55));
You said this was used in the renderer? Does this correctly work with the
sandbox? base::CPU needs to access files.

I'm not excited about pegging the encoder to the CPU type.

Why not keeping the enable flag and pass it from the board definition on the
Chrome OS side?

[Pawel Osciak, 2014-07-08 05:32:42]

https://codereview.chromium.org/374553002/diff/1/content/common/gpu/media/vaa...
File content/common/gpu/media/vaapi_video_encode_accelerator.cc (right):

https://codereview.chromium.org/374553002/diff/1/content/common/gpu/media/vaa...
content/common/gpu/media/vaapi_video_encode_accelerator.cc:117:
(cpu.vendor_name() == "GenuineIntel") && (cpu.model() == 55));
On 2014/07/07 17:57:47, piman (slow to review) wrote:
> You said this was used in the renderer? Does this correctly work with the
> sandbox? base::CPU needs to access files.

Yes, this is in renderer. PreSandboxStartup()
(https://code.google.com/p/chromium/codesearch#chromium/src/chrome/app/chrome_...)
prepopulates /proc/cpuinfo.

> I'm not excited about pegging the encoder to the CPU type.

Me neither of course, but there is a bug in the driver that says it's available
even if it's not. I'm waiting for a fix from Intel, so this should hopefully be
temporary.

> Why not keeping the enable flag and pass it from the board definition on the
> Chrome OS side?

We have quite a few boards that support this and this way feels to me like a
better idea rather than manually curating a board list in command line builder.

[piman, 2014-07-08 17:30:48]

On Mon, Jul 7, 2014 at 10:32 PM, <posciak@chromium.org> wrote:

>
> https://codereview.chromium.org/374553002/diff/1/content/
> common/gpu/media/vaapi_video_encode_accelerator.cc
> File content/common/gpu/media/vaapi_video_encode_accelerator.cc (right):
>
> https://codereview.chromium.org/374553002/diff/1/content/
> common/gpu/media/vaapi_video_encode_accelerator.cc#newcode117
> content/common/gpu/media/vaapi_video_encode_accelerator.cc:117:
> (cpu.vendor_name() == "GenuineIntel") && (cpu.model() == 55));
> On 2014/07/07 17:57:47, piman (slow to review) wrote:
>
>> You said this was used in the renderer? Does this correctly work with
>>
> the
>
>> sandbox? base::CPU needs to access files.
>>
>
> Yes, this is in renderer. PreSandboxStartup()
> (https://code.google.com/p/chromium/codesearch#chromium/
> src/chrome/app/chrome_main_delegate.cc&l=644)
> prepopulates /proc/cpuinfo.
>
>
>  I'm not excited about pegging the encoder to the CPU type.
>>
>
> Me neither of course, but there is a bug in the driver that says it's
> available even if it's not. I'm waiting for a fix from Intel, so this
> should hopefully be temporary.
>
>
>  Why not keeping the enable flag and pass it from the board definition
>>
> on the
>
>> Chrome OS side?
>>
>
> We have quite a few boards that support this and this way feels to me
> like a better idea rather than manually curating a board list in command
> line builder.
>

On the contrary. We already do this for e.g. GL vs GLES. Each board
(possibly via inheritance) just has a use flag.
The cpuid is certainly not the best indicator of what's available on the
SoC, let alone the software configuration.

This belongs on the board.


>
> https://codereview.chromium.org/374553002/
>

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

[Pawel Osciak, 2014-07-09 00:45:04]

On 2014/07/08 17:30:48, piman (slow to review) wrote:
> On Mon, Jul 7, 2014 at 10:32 PM, <mailto:posciak@chromium.org> wrote:
> 
> >
> > https://codereview.chromium.org/374553002/diff/1/content/
> > common/gpu/media/vaapi_video_encode_accelerator.cc
> > File content/common/gpu/media/vaapi_video_encode_accelerator.cc (right):
> >
> > https://codereview.chromium.org/374553002/diff/1/content/
> > common/gpu/media/vaapi_video_encode_accelerator.cc#newcode117
> > content/common/gpu/media/vaapi_video_encode_accelerator.cc:117:
> > (cpu.vendor_name() == "GenuineIntel") && (cpu.model() == 55));
> > On 2014/07/07 17:57:47, piman (slow to review) wrote:
> >
> >> You said this was used in the renderer? Does this correctly work with
> >>
> > the
> >
> >> sandbox? base::CPU needs to access files.
> >>
> >
> > Yes, this is in renderer. PreSandboxStartup()
> > (https://code.google.com/p/chromium/codesearch#chromium/
> > src/chrome/app/chrome_main_delegate.cc&l=644)
> > prepopulates /proc/cpuinfo.
> >
> >
> >  I'm not excited about pegging the encoder to the CPU type.
> >>
> >
> > Me neither of course, but there is a bug in the driver that says it's
> > available even if it's not. I'm waiting for a fix from Intel, so this
> > should hopefully be temporary.
> >
> >
> >  Why not keeping the enable flag and pass it from the board definition
> >>
> > on the
> >
> >> Chrome OS side?
> >>
> >
> > We have quite a few boards that support this and this way feels to me
> > like a better idea rather than manually curating a board list in command
> > line builder.
> >
> 
> On the contrary. We already do this for e.g. GL vs GLES. Each board
> (possibly via inheritance) just has a use flag.
> The cpuid is certainly not the best indicator of what's available on the
> SoC, let alone the software configuration.
> 
> This belongs on the board.
> 

We'd have to maintain a large set of seemingly arbitrary board names tied to a
flag, while using CPUID gives us a way of autodetecting the exact model of the
CPU that we know has this feature available. This is 100% hardware dependent and
no software decision is involved, while GL vs GLES is also dependent on the SW
configuration on the board, so it feels like it makes more sense to use a USE
flag there.

Also, this is temporary until the driver is fixed. I really feel that a
temporary solution that gives us reliable autodetection is batter than
maintaining a temporary USE flag for a large set of boards though...

[piman, 2014-07-09 01:29:55]

On 2014/07/09 00:45:04, Pawel Osciak wrote:
> On 2014/07/08 17:30:48, piman (slow to review) wrote:
> > On Mon, Jul 7, 2014 at 10:32 PM, <mailto:posciak@chromium.org> wrote:
> > 
> > >
> > > https://codereview.chromium.org/374553002/diff/1/content/
> > > common/gpu/media/vaapi_video_encode_accelerator.cc
> > > File content/common/gpu/media/vaapi_video_encode_accelerator.cc (right):
> > >
> > > https://codereview.chromium.org/374553002/diff/1/content/
> > > common/gpu/media/vaapi_video_encode_accelerator.cc#newcode117
> > > content/common/gpu/media/vaapi_video_encode_accelerator.cc:117:
> > > (cpu.vendor_name() == "GenuineIntel") && (cpu.model() == 55));
> > > On 2014/07/07 17:57:47, piman (slow to review) wrote:
> > >
> > >> You said this was used in the renderer? Does this correctly work with
> > >>
> > > the
> > >
> > >> sandbox? base::CPU needs to access files.
> > >>
> > >
> > > Yes, this is in renderer. PreSandboxStartup()
> > > (https://code.google.com/p/chromium/codesearch#chromium/
> > > src/chrome/app/chrome_main_delegate.cc&l=644)
> > > prepopulates /proc/cpuinfo.
> > >
> > >
> > >  I'm not excited about pegging the encoder to the CPU type.
> > >>
> > >
> > > Me neither of course, but there is a bug in the driver that says it's
> > > available even if it's not. I'm waiting for a fix from Intel, so this
> > > should hopefully be temporary.
> > >
> > >
> > >  Why not keeping the enable flag and pass it from the board definition
> > >>
> > > on the
> > >
> > >> Chrome OS side?
> > >>
> > >
> > > We have quite a few boards that support this and this way feels to me
> > > like a better idea rather than manually curating a board list in command
> > > line builder.
> > >
> > 
> > On the contrary. We already do this for e.g. GL vs GLES. Each board
> > (possibly via inheritance) just has a use flag.
> > The cpuid is certainly not the best indicator of what's available on the
> > SoC, let alone the software configuration.
> > 
> > This belongs on the board.
> > 
> 
> We'd have to maintain a large set of seemingly arbitrary board names tied to a
> flag, while using CPUID gives us a way of autodetecting the exact model of the
> CPU that we know has this feature available. This is 100% hardware dependent
and
> no software decision is involved, while GL vs GLES is also dependent on the SW
> configuration on the board, so it feels like it makes more sense to use a USE
> flag there.
> 
> Also, this is temporary until the driver is fixed. I really feel that a
> temporary solution that gives us reliable autodetection is batter than
> maintaining a temporary USE flag for a large set of boards though...

If you only need this as a workaround for a single board, then it's easy to add
a use flag for that one to enable the workaround.

[Pawel Osciak, 2014-07-24 08:01:17]

Removed CPU check and rebased. PTAL.

[piman, 2014-07-24 18:16:01]

LGTM, thanks!

[Pawel Osciak, 2014-07-28 08:03:18]

The CQ bit was checked by posciak@chromium.org

[commit-bot: I haz the power, 2014-07-28 08:03:43]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/posciak@chromium.org/374553002/20001

[commit-bot: I haz the power, 2014-07-28 11:15:28]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  linux_chromium_clang_dbg on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-07-28 11:23:57]

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

[commit-bot: I haz the power, 2014-07-28 11:23:59]

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

[Pawel Osciak, 2014-07-29 00:04:16]

The CQ bit was checked by posciak@chromium.org

[commit-bot: I haz the power, 2014-07-29 00:06:23]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/posciak@chromium.org/374553002/20001

[commit-bot: I haz the power, 2014-07-29 01:17:36]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-07-29 02:30:37]

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

[commit-bot: I haz the power, 2014-07-29 02:30:38]

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

[Pawel Osciak, 2014-07-29 02:32:22]

The CQ bit was checked by posciak@chromium.org

[commit-bot: I haz the power, 2014-07-29 02:34:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/posciak@chromium.org/374553002/20001

[commit-bot: I haz the power, 2014-07-29 03:45:01]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-07-29 05:00:37]

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

[commit-bot: I haz the power, 2014-07-29 05:00:38]

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

[posciak1, 2014-07-30 00:29:29]

miu@: chrome/test/data/extensions/api_test/cast_streaming/* please. Thanks!

[miu, 2014-07-30 00:33:13]

chrome/test/data/extensions/api_test/cast_streaming/* lgtm

[Pawel Osciak, 2014-07-30 00:35:15]

The CQ bit was checked by posciak@chromium.org

[commit-bot: I haz the power, 2014-07-30 00:38:58]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/posciak@chromium.org/374553002/40001

[Pawel Osciak, 2014-07-30 01:28:06]

The CQ bit was checked by posciak@chromium.org

[commit-bot: I haz the power, 2014-07-30 01:30:06]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/posciak@chromium.org/374553002/60001

[commit-bot: I haz the power, 2014-07-30 07:31:35]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-07-30 08:44:59]

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

[commit-bot: I haz the power, 2014-07-30 08:45:01]

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

[posciak1, 2014-07-30 09:20:29]

The CQ bit was checked by posciak@google.com

[posciak1, 2014-07-30 09:20:32]

The CQ bit was unchecked by posciak@google.com

[Pawel Osciak, 2014-07-30 09:20:51]

The CQ bit was checked by posciak@chromium.org

[commit-bot: I haz the power, 2014-07-30 09:22:40]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/posciak@chromium.org/374553002/60001

[commit-bot: I haz the power, 2014-07-30 10:37:12]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-07-30 11:51:59]

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

[commit-bot: I haz the power, 2014-07-30 11:52:02]

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

[Pawel Osciak, 2014-07-31 00:38:53]

The CQ bit was checked by posciak@chromium.org

[commit-bot: I haz the power, 2014-07-31 00:39:39]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/posciak@chromium.org/374553002/60001

[commit-bot: I haz the power, 2014-07-31 03:08:18]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[commit-bot: I haz the power, 2014-07-31 04:20:16]

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

[commit-bot: I haz the power, 2014-07-31 04:20:17]

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

[Pawel Osciak, 2014-08-12 00:23:04]

The CQ bit was checked by posciak@chromium.org

[commit-bot: I haz the power, 2014-08-12 00:29:37]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/posciak@chromium.org/374553002/60001

[commit-bot: I haz the power, 2014-08-12 00:41:07]

FYI, CQ is re-trying this CL (attempt #1).
The failing builders are:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[commit-bot: I haz the power, 2014-08-12 00:51:33]

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

[commit-bot: I haz the power, 2014-08-12 00:51:35]

Try jobs failed on following builders:
  linux_chromium_chromeos_rel_swarming on tryserver.chromium.linux
(http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)
  mac_chromium_rel_swarming on tryserver.chromium.mac
(http://build.chromium.org/p/tryserver.chromium.mac/builders/mac_chromium_rel_...)

[Pawel Osciak, 2014-08-12 01:04:22]

The CQ bit was checked by posciak@chromium.org

[commit-bot: I haz the power, 2014-08-12 01:17:54]

CQ is trying da patch. Follow status at
 https://chromium-status.appspot.com/cq/posciak@chromium.org/374553002/80001

[commit-bot: I haz the power, 2014-08-12 05:47:50]

Change committed as 288920