Use generate_stubs.py for VAAPI symbols.

Use generate_stubs.py for VAAPI symbols and remove VAAPI < 0.34 TODO.
    
VAAPI on ChromeOS is updated to 0.34, so remove the vaCreateSurfaces() TODO.
And use generate_stubs.py for simplify.
    
BUG=None
TEST=content_shell and chrome to view H264 oneline videos.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=256781

[haltonhuo, 2014-02-25 09:17:45]

Here is presubmit errors, since common/gpu/media/va_stubs.h is generated code, I
think it is safe to bypass it.

** Presubmit ERRORS **
You added one or more #includes that violate checkdeps rules.
  content/common/gpu/media/vaapi_wrapper.cc
      Illegal include: "common/gpu/media/va_stubs.h"
    Because of no rule applying.

[Ami GONE FROM CHROMIUM, 2014-02-25 09:24:33]

On 2014/02/25 09:17:45, haltonhuo wrote:
> Here is presubmit errors, since common/gpu/media/va_stubs.h is generated code,
I
> think it is safe to bypass it.
> 
> ** Presubmit ERRORS **
> You added one or more #includes that violate checkdeps rules.
>   content/common/gpu/media/vaapi_wrapper.cc
>       Illegal include: "common/gpu/media/va_stubs.h"
>     Because of no rule applying.

I don't think you should ignore this.  I think this indicates you've got your
gyp code slightly off, since it's emitting to common/gpu/media instead of
content/common/gpu/media/.

CL description should answer the question of "why are you doing this?".

[haltonhuo, 2014-02-25 09:33:05]

On 2014/02/25 09:24:33, Ami Fischman wrote:
> I don't think you should ignore this.  I think this indicates you've got your
> gyp code slightly off, since it's emitting to common/gpu/media instead of
> content/common/gpu/media/.
> 
> CL description should answer the question of "why are you doing this?".

It is doing same thing as ffmpeg and pulse does.
 *
https://code.google.com/p/chromium/codesearch#chromium/src/media/base/media_p...

 *
https://code.google.com/p/chromium/codesearch#chromium/src/media/audio/pulse/...

va_stubs.h is generated under out/Debug/gen/va/common/gpu/media/va_stubs.h. So
in content/content_common.gypi, <(SHARED_INTERMEDIATE_DIR)/va is added in
include_dirs.

[Pawel Osciak, 2014-02-25 10:34:01]

https://codereview.chromium.org/179543002/diff/1/content/common/gpu/media/vaa...
File content/common/gpu/media/vaapi_wrapper.cc (left):

https://codereview.chromium.org/179543002/diff/1/content/common/gpu/media/vaa...
content/common/gpu/media/vaapi_wrapper.cc:63: // TODO(chihchung): Remove the old
path once ChromeOS updates to 1.2.1.
We can do this already. We are at 1.2.1/0.34.

[haltonhuo, 2014-02-25 10:56:14]

On 2014/02/25 10:34:01, Pawel Osciak wrote:
>
https://codereview.chromium.org/179543002/diff/1/content/common/gpu/media/vaa...
> File content/common/gpu/media/vaapi_wrapper.cc (left):
> 
>
https://codereview.chromium.org/179543002/diff/1/content/common/gpu/media/vaa...
> content/common/gpu/media/vaapi_wrapper.cc:63: // TODO(chihchung): Remove the
old
> path once ChromeOS updates to 1.2.1.
> We can do this already. We are at 1.2.1/0.34.

Pawel, thanks for the info, I'll update the patch to use 0.34 above
vaCreateSurface api(8 parameters) only.

[haltonhuo, 2014-02-25 11:33:41]

Pawel and Ami, patch set2 remove the <0.34 TODO, please review.

[Ami GONE FROM CHROMIUM, 2014-02-25 19:04:26]

Can you paste the generated files to gist.github.com or similar and point to
them from here?

https://codereview.chromium.org/179543002/diff/20001/content/content_common.gypi
File content/content_common.gypi (right):

https://codereview.chromium.org/179543002/diff/20001/content/content_common.g...
content/content_common.gypi:578: 'project_path': 'common/gpu/media',
If you change this to content/common/gpu/media can the offending #include be
changed to have content/ and avoid the checkdeps warning?

[haltonhuo, 2014-02-26 02:33:25]

On 2014/02/25 19:04:26, Ami Fischman wrote:
> Can you paste the generated files to http://gist.github.com or similar and
point to
> them from here?
> 
>
https://codereview.chromium.org/179543002/diff/20001/content/content_common.gypi
> File content/content_common.gypi (right):
> 
>
https://codereview.chromium.org/179543002/diff/20001/content/content_common.g...
> content/content_common.gypi:578: 'project_path': 'common/gpu/media',
> If you change this to content/common/gpu/media can the offending #include be
> changed to have content/ and avoid the checkdeps warning?

Ami, patch set3 fixes the presubmit warning, many thanks for the hint. Please
review.

FYI, the generated files are pasted at https://gist.github.com/halton/9222419

[Ami GONE FROM CHROMIUM, 2014-02-26 19:30:48]

LGTM but I'd like to have posciak@ bless this as well.

https://codereview.chromium.org/179543002/diff/40001/content/common/gpu/media...
File content/common/gpu/media/vaapi_wrapper.cc (right):

https://codereview.chromium.org/179543002/diff/40001/content/common/gpu/media...
content/common/gpu/media/vaapi_wrapper.cc:11: // Auto-generated for dlopen libva
libraries
Drop?

[Pawel Osciak, 2014-02-27 01:04:08]

On 2014/02/26 19:30:48, Ami Fischman wrote:
> LGTM but I'd like to have posciak@ bless this as well.
> 

Please don't commit before I can test this. I will confirm once everything looks
fine. Thank you.

[haltonhuo, 2014-02-27 01:07:26]

On 2014/02/27 01:04:08, Pawel Osciak wrote:
> On 2014/02/26 19:30:48, Ami Fischman wrote:
> > LGTM but I'd like to have posciak@ bless this as well.
> > 
> 
> Please don't commit before I can test this. I will confirm once everything
looks
> fine. Thank you.

Thanks for let me know. I guess you're trying it on ChromeOS. If anything I can
help, please let me know. Before that, I need to know how you test on ChromeOS?

[dshwang, 2014-02-28 10:48:12]

https://codereview.chromium.org/179543002/diff/40001/content/common/gpu/media...
File content/common/gpu/media/vaapi_wrapper.cc (left):

https://codereview.chromium.org/179543002/diff/40001/content/common/gpu/media...
content/common/gpu/media/vaapi_wrapper.cc:310: if (VAAPIVersionLessThan(0, 34))
{
Hi, could you postpone dropping <0.34 support?
After this CL, we can not test accelerated video using CrOS executable on Ubuntu
machine. I guess that compiling and flashing to chromebook and testing routine
is a bit burdensome.

I hope this be postponed until Ubuntu supports VAAPI 0.34 officially.

[haltonhuo, 2014-03-03 02:53:26]

On 2014/02/28 10:48:12, dshwang wrote:
> Hi, could you postpone dropping <0.34 support?
> After this CL, we can not test accelerated video using CrOS executable on
Ubuntu
> machine. I guess that compiling and flashing to chromebook and testing routine
> is a bit burdensome.
> 
> I hope this be postponed until Ubuntu supports VAAPI 0.34 officially.

dshwang, VAAPI won't be enabled on linux unless apply some patches(I maintained
two). Ami and myslef were discussed it year ago, chromium upstream only want to
enable it on ChromeOS, I do not think it is changed now. So, Ubuntu or other
distros's VAAPI change won't be considered.

And for Ubuntu, users/developers could install latest VAAPI(0.34) via
https://01.org/linuxgraphics/community/vaapi. That is how I verified this patch
is working.

[dshwang, 2014-03-03 16:07:50]

On 2014/03/03 02:53:26, haltonhuo wrote:
> dshwang, VAAPI won't be enabled on linux unless apply some patches(I
maintained
> two). Ami and myslef were discussed it year ago, chromium upstream only want
to
> enable it on ChromeOS, I do not think it is changed now. So, Ubuntu or other
> distros's VAAPI change won't be considered.

I guess there may be some difference between 1 year ago and now.
http://crbug.com/137247 just mentions
"I marked it WontFix intentionally, because nobody is working on implementing
this now, and nobody's planning on doing so in the future.  When that changes,
this bug can be reopened/assigned."
So I try to enable it in Linux by default: http://crrev.com/176883018

> And for Ubuntu, users/developers could install latest VAAPI(0.34) via
> https://01.org/linuxgraphics/community/vaapi. That is how I verified this
patch
> is working.

Thank you for good information.

[Ami GONE FROM CHROMIUM, 2014-03-03 17:26:34]

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

https://codereview.chromium.org/179543002/diff/60001/content/common/gpu/media...
content/common/gpu/media/vaapi_wrapper.cc:5: #include
"content/common/gpu/media/vaapi_wrapper.h"
This CL doesn't remove the third_party/libva/va #include's from vaapi_wrapper.h.
 Can they be removed now?

[haltonhuo, 2014-03-04 01:41:57]

On 2014/03/03 17:26:34, Ami Fischman wrote:
>
https://codereview.chromium.org/179543002/diff/60001/content/common/gpu/media...
> File content/common/gpu/media/vaapi_wrapper.cc (right):
> 
>
https://codereview.chromium.org/179543002/diff/60001/content/common/gpu/media...
> content/common/gpu/media/vaapi_wrapper.cc:5: #include
> "content/common/gpu/media/vaapi_wrapper.h"
> This CL doesn't remove the third_party/libva/va #include's from
vaapi_wrapper.h.
>  Can they be removed now?

Ami, patch set remove va include in vaapi_wrapper.h and verified.

[Ami GONE FROM CHROMIUM, 2014-03-04 02:51:34]

WHat about content/common/gpu/media/va_surface.h?


On Mon, Mar 3, 2014 at 5:41 PM, <halton.huo@intel.com> wrote:

> On 2014/03/03 17:26:34, Ami Fischman wrote:
>
> https://codereview.chromium.org/179543002/diff/60001/
> content/common/gpu/media/vaapi_wrapper.cc
>
>> File content/common/gpu/media/vaapi_wrapper.cc (right):
>>
>
>
> https://codereview.chromium.org/179543002/diff/60001/
> content/common/gpu/media/vaapi_wrapper.cc#newcode5
>
>> content/common/gpu/media/vaapi_wrapper.cc:5: #include
>> "content/common/gpu/media/vaapi_wrapper.h"
>> This CL doesn't remove the third_party/libva/va #include's from
>>
> vaapi_wrapper.h.
>
>>   Can they be removed now?
>>
>
> Ami, patch set remove va include in vaapi_wrapper.h and verified.
>
> https://codereview.chromium.org/179543002/
>

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

[haltonhuo, 2014-03-04 03:06:34]

On 2014/03/04 02:51:34, Ami Fischman wrote:
> WHat about content/common/gpu/media/va_surface.h?

No va.h can not removed from va_surface.h because it reference VA structures
such as VASurfaceID.

I'm sorry that va_x11.h can not removed from vaapi_wrapper.h as well, same
reason as va_surface.h. va.h can be removed.

In that case do you want me remove va.h from vaapi_wrapper.h and leave va_x11.h
there?

[Ami GONE FROM CHROMIUM, 2014-03-04 03:22:42]

If libva can't be removed as a build time dependency then what exactly does
this cl buy us?
On Mar 3, 2014 7:06 PM, <halton.huo@intel.com> wrote:

> On 2014/03/04 02:51:34, Ami Fischman wrote:
>
>> WHat about content/common/gpu/media/va_surface.h?
>>
>
> No va.h can not removed from va_surface.h because it reference VA
> structures
> such as VASurfaceID.
>
> I'm sorry that va_x11.h can not removed from vaapi_wrapper.h as well, same
> reason as va_surface.h. va.h can be removed.
>
> In that case do you want me remove va.h from vaapi_wrapper.h and leave
> va_x11.h
> there?
>
>
>
> https://codereview.chromium.org/179543002/
>

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

[Pawel Osciak, 2014-03-04 03:26:13]

Please let me know when the change is ready and I will test it before approving.

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

https://codereview.chromium.org/179543002/diff/100001/content/common/gpu/medi...
content/common/gpu/media/vaapi_wrapper.cc:19: // libva-x11 depend on libva, so
dlopen libva-x11 is enough
s/depend/depends/

https://codereview.chromium.org/179543002/diff/100001/content/common/gpu/medi...
content/common/gpu/media/vaapi_wrapper.cc:26: << " VA error: " <<
vaErrorStr(va_error); \
Please align the backslash.

[haltonhuo, 2014-03-04 04:59:49]

On 2014/03/04 03:26:13, Pawel Osciak wrote:
> Please let me know when the change is ready and I will test it before
approving.
> 
>
https://codereview.chromium.org/179543002/diff/100001/content/common/gpu/medi...
> File content/common/gpu/media/vaapi_wrapper.cc (right):
> 
>
https://codereview.chromium.org/179543002/diff/100001/content/common/gpu/medi...
> content/common/gpu/media/vaapi_wrapper.cc:19: // libva-x11 depend on libva, so
> dlopen libva-x11 is enough
> s/depend/depends/
> 
>
https://codereview.chromium.org/179543002/diff/100001/content/common/gpu/medi...
> content/common/gpu/media/vaapi_wrapper.cc:26: << " VA error: " <<
> vaErrorStr(va_error); \
> Please align the backslash.

Pawel, thanks for reviewing. Patch set7 addressed your comments.

[Pawel Osciak, 2014-03-12 07:35:29]

lgtm after testing and as long as Ami still approves
+Jorge as fyi

[haltonhuo, 2014-03-12 07:48:00]

The CQ bit was checked by halton.huo@intel.com

[commit-bot: I haz the power, 2014-03-12 08:05:51]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/halton.huo@intel.com/179543002/120001

[commit-bot: I haz the power, 2014-03-12 09:12:59]

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

[commit-bot: I haz the power, 2014-03-12 09:12:59]

Try jobs failed on following builders: linux_chromium_chromeos_clang_dbg

[haltonhuo, 2014-03-13 01:34:39]

The CQ bit was checked by halton.huo@intel.com

[commit-bot: I haz the power, 2014-03-13 01:35:59]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/halton.huo@intel.com/179543002/120001

[commit-bot: I haz the power, 2014-03-13 04:08:25]

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

[commit-bot: I haz the power, 2014-03-13 04:08:26]

Try jobs failed on following builders:
  tryserver.chromium on linux_chromium_chromeos_clang_dbg

[haltonhuo, 2014-03-13 04:58:28]

On 2014/03/13 04:08:26, I haz the power (commit-bot) wrote:
> Try jobs failed on following builders:
>   tryserver.chromium on linux_chromium_chromeos_clang_dbg

From
http://build.chromium.org/p/tryserver.chromium/builders/linux_chromium_chrome...,
I do not think it is related. Anything need I do?

[haltonhuo, 2014-03-13 05:04:47]

The CQ bit was checked by halton.huo@intel.com

[commit-bot: I haz the power, 2014-03-13 05:12:49]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/halton.huo@intel.com/179543002/120001

[commit-bot: I haz the power, 2014-03-13 10:05:43]

Change committed as 256781