Skip to content
This repository has been archived by the owner on Apr 3, 2020. It is now read-only.

Introduce vaLockBuffer & vaUnlockBuffer APIs in libva. #183

Merged
merged 1 commit into from
Sep 5, 2014

Conversation

shaochangbin
Copy link
Contributor

Reason that add this change:
1080p video playback on Tizen is quite unsmooth on VTC-1010, with only 15 render
fps and ~70% CPU usage. Ozone-wl landed a workaround solution that enables a
zero buffer copy method that uses libva vaLockBuffer APIs which gains much better
performance than previous method.
intel/ozone-wayland#260

In order to make the change also benifit Crosswalk/Tizen, we should add the
vaLockBuffer API declarations inside Chromium Crosswalk to compile
the code successfully.

The long term goal of our work would be landing those API declarations inside
Chromium upstream, after the API implementations are landed in libva and
libva-driver upstream.

Therefore, this CL is a temporary solution. It would be reverted after the long
term goal is achived.

@crosswalk-trybot
Copy link

Testing patch series with shaochangbin/chromium-crosswalk@dec0dea as its head.

Bot Status
Content Shell Linux [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/106)
Content Shell Android-x86 [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/105)

@shaochangbin
Copy link
Contributor Author

Please have a look, thanks!
@darktears @kenchris @tiagovignatti

@crosswalk-trybot
Copy link

Testing patch series with shaochangbin/chromium-crosswalk@2317739 as its head.

Bot Status
Content Shell Linux [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/107)
Content Shell Android-x86 [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/106)

@rakuco
Copy link

rakuco commented Aug 8, 2014

I've just cloned libva from freedesktop.org and master doesn't contain these changes either. I'm rather uncomfortable with having to carry these changes forever on our side.

@shaochangbin
Copy link
Contributor Author

There are APIs named vaAcquireBufferHandle and vaReleaseBufferHandle in libva 'staging' branch in freedesktop.org, which are newer version of the vaLockBuffer & vaUnlockBuffer. vaAcquireBufferHandle APIs will be merged in master branch after some time if there is no report issues over them.
For Tizen IVI, the libva is picked from libva upstream 'staging' branch, but is a bit old and at the moment it hasn't been updated for some time. we can find the vaLockBuffer patches from IVI libva rpm.
As for Chromium libva upstream, I am not clear how often they update the repo, but i guess they will only update it when there is need to do this.

To sum up, IMHO it requires time to make vaAcquireBufferHandle APIs landed in Chromium libva upstream, we can use vaLockBuffer as temporary option atm. wdyt?

@rakuco
Copy link

rakuco commented Aug 8, 2014

I think Chromium updates its libva checkout as needed, and they probably wouldn't object if you helped with that -- for example, libva 1.2.1 was released in the end of June last year and made it to Chromium in November; on the other hand, 1.3.1 was released this May and Chromium updated its copy in June.

In any case, I've got a few additional questions based on your explanation:

  • Do you know when the libva team intends to release a new version that includes this new API?
  • What if you add vaAcquireBufferHandle and vaReleaseBufferHandle instead? Does that mean it won't work with IVI? If that's the case, isn't that bound to cause problems in a way or another once a newer libva is released with the new symbols?
  • Does this approach cause any problems if one has a normal libva installed, like undefined symbol errors when the stubs are resolved?

@shaochangbin
Copy link
Contributor Author

@rakuco thanks for your suggestion!
My concern is it may requires quite a long time to make the new APIs merged in Chromium upstream. We firstly need to push to land it in libva upstream, then help to make it ready in chromium upsream.

For Q1, actually I've know idea when they will include new APIs in master branch. Think libva owners can give us more information.
Q2, as my knowledge, IVI libva is out of maintenance at the moment and it doesn't support the new APIs. So the old APIs works fine for IVI. If we want to add the new APIs, then should find someone to maintain/update the libva rpms.
Q3, it has no problem for IVI platform, due to libva isn't maintained. If there are case that uses the vaLockBuffer or vaAcquireBufferHandle APIs but only has a normal libva installed, then there is problem.

@tiagovignatti
Copy link
Contributor

hi @gbeauchesne, hi @halleyzhao. Any idea about when can we get these new API in libva?

@gbeauchesne
Copy link

Hi,

The API is available in libva "staging" branch. The associated VA driver support patch is still available in the libva mailing list. I will push it to the "staging" branch by the end of this week.

Now, the next steps are to get the changes merged back to "master" and get a release of it. For that to happen, I would appreciate compelling examples of usages. Chromium could be one. Likewise for the pending Mesa changes. It should not be hard to get in, but I first want to be sure that the APIs and implementations are going to work and match user expectations. Chromium is one.

Are you satisified with those APIs? Does this match your expectations?

Thanks,
Gwenole.

@shaochangbin
Copy link
Contributor Author

Hi, @gbeauchesne thanks a lot for your reply.
From my perspective, i would say it's a very impressive change and willing to see this in master branch ASAP.
Btw, as for Mesa change, do you mean the VA_SURFACE_ATTRIB_MEM_TYPE_KERNEL_DRM_BO support? I'm using the VA_SURFACE_ATTRIB_MEM_TYPE_KERNEL_DRM memory type atm.

@gbeauchesne
Copy link

Hi,

2014-08-13 7:22 GMT+02:00 shaochangbin notifications@github.com:

Hi, @gbeauchesne https://github.com/gbeauchesne thanks a lot for your
reply.
From my perspective, i would say it's a very impressive change and willing
to see this in master branch ASAP.
Btw, as for Mesa change, do you mean the
VA_SURFACE_ATTRIB_MEM_TYPE_KERNEL_DRM_BO support? I'm using the
VA_SURFACE_ATTRIB_MEM_TYPE_KERNEL_DRM memory type atm.

On the Mesa side, that's support for EXT_image_dma_buf_import. Since we
don't support sampling from NV12 images yet, we still need to import each
surface plane into separate EGLImage and use a shader for color conversion
for instance. That's what my Mesa patches do. I am currently uploading my
Mesa repo to github for simpler reference from here. :)

Meanwhile, the older patches are still available here:
http://cgit.freedesktop.org/~gb/mesa/log/?h=20.EXT_image_dma_buf_import
(20.EXT_image_dma_buf_import branch)

I will also push a simple test program to show off how this is supposed to
work altogether.

@tiagovignatti
Copy link
Contributor

@gbeauchesne, what happens with those new API calls when EXT_image_dma_buf_import is lacking? Thanks.

@gbeauchesne
Copy link

Hi,

2014-08-13 9:41 GMT+02:00 Tiago Vignatti notifications@github.com:

@gbeauchesne https://github.com/gbeauchesne, what happens with those
new API calls when EXT_image_dma_buf_import is lacking? Thanks.

You only need the proposed Mesa patches if you want to import planar YUV
surfaces and that the Intel 3D driver does not support
GL_OES_EGL_image_external. Meanwhile, without those Mesa patches, you would
have to convert that VA surface (internally in NV12 format) to something
that is readily supported, e.g. RGBA. You can use a VA/VPP pipeline for it.

@shaochangbin
Copy link
Contributor Author

That's the option that we use inside Ozone-WL implementation, firstly convert the VA surface to BGRX format.

@gbeauchesne
Copy link

2014-08-13 15:21 GMT+02:00 shaochangbin notifications@github.com:

That's the option that we use inside Ozone-WL implementation, firstly
convert the VA surface to BGRX format.

OK, that's not really zero-copy then but that's fine for a general fallback
without a particular support in Mesa, and starting to exercise the APIs. :)

BTW, if you have means to get display-rect hints from the render process,
that would also be nice to lower memory bandwidth. i.e. imagine you decode
1080p30 videos but only intend to display on 720p30 screens, there are
additional savings to get here, by firstly converting to a lower size.

@crosswalk-trybot
Copy link

Testing patch series with shaochangbin/chromium-crosswalk@e06df34 as its head.

Bot Status
Content Shell Linux [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/110)
Content Shell Android-x86 [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/109)

@shaochangbin
Copy link
Contributor Author

Hi, all, any review comments?
@rakuco @kalyankondapally @tiagovignatti @darktears @kenchris

@kalyankondapally
Copy link
Contributor

@shaochangbin This should go into oz-wl side.

@halton
Copy link

halton commented Sep 5, 2014

@shaochangbin I think this is value to upstream, isn't it?

@shaochangbin
Copy link
Contributor Author

@halton, thanks for your suggestion. It's our long term goal to upstream it in Chromium, due to the libva side change is not ready atm.

@shaochangbin
Copy link
Contributor Author

@kalyan, my understanding is the patches/ folder under oz-wl takes effect when building oz-wl standalone. We can't remove this CL in Chromium-crosswalk, otherwise it compiles error in Crosswalk if includes intel/ozone-wayland#260.

So my conclusion is we need two copies of the CL, one in Chromium-Crosswalk for Crosswalk building;
the other one under oz-wl /patches, for oz-wl standalone building.
Please correct me if my understanding is wrong.

@kalyankondapally
Copy link
Contributor

@shaochangbin That's true. For now you need this in both places.

@crosswalk-trybot
Copy link

Testing patch series with shaochangbin/chromium-crosswalk@bec472e as its head.

Bot Status
Content Shell Linux [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/111)
Content Shell Android-x86 [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/110)

@rakuco
Copy link

rakuco commented Sep 5, 2014

There's a long story behind this commit, so please expand your one-line commit message.

Reason that add this change:
1080p video playback on Tizen is quite unsmooth on VTC-1010, with only 15 render
fps and ~70% CPU usage. Ozone-wl landed a workaround solution that enables a
zero buffer copy method that uses libva vaLockBuffer APIs which gains much better
performance than previous method.
intel/ozone-wayland#260

In order to make the change also benifit Crosswalk/Tizen, we should add the
vaLockBuffer API declarations inside Chromium Crosswalk to compile
the code successfully.

The long term goal of our work would be landing those API declarations inside
Chromium upstream, after the API implementations are landed in libva and
libva-driver upstream.

Therefore, this CL is a temporary solution. It would be reverted after the long
term goal is achived.
@crosswalk-trybot
Copy link

Testing patch series with shaochangbin/chromium-crosswalk@d85e4ed as its head.

Bot Status
Content Shell Linux [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Linux/builds/112)
Content Shell Android-x86 [FAILED 💔](https://build.crosswalk-project.org/try/builders/Content Shell Android-x86/builds/111)

@shaochangbin
Copy link
Contributor Author

Update commit message, please review, thanks!
@rakuco

rakuco pushed a commit that referenced this pull request Sep 5, 2014
Introduce vaLockBuffer & vaUnlockBuffer APIs in libva.
@rakuco rakuco merged commit 15a0cc4 into crosswalk-project:master Sep 5, 2014
@rakuco
Copy link

rakuco commented Sep 5, 2014

Landed at last :-)

@shaochangbin
Copy link
Contributor Author

thanks for your review:)

darktears pushed a commit that referenced this pull request Sep 17, 2014
Tabbing should navigate through all the visible controls.

I've changed the EditablePhotoButton to actually be a button and
not just an image, so that tabbing and pressing enter actually
works.

BUG=405056
TBR=noms@chromium.org

Review URL: https://codereview.chromium.org/489143002

Cr-Commit-Position: refs/heads/master@{#292455}
(cherry picked from commit aaacc30)

Review URL: https://codereview.chromium.org/533833003

Cr-Commit-Position: refs/branch-heads/2125@{#183}
Cr-Branched-From: b68026d-refs/heads/master@{#290040}
darktears pushed a commit that referenced this pull request Nov 17, 2014
…vised user creation flow is active"

BUG=422918
TBR=nkostylev

Review URL: https://codereview.chromium.org/654173005

Cr-Commit-Position: refs/heads/master@{#299881}
(cherry picked from commit dcf3361)

Review URL: https://codereview.chromium.org/655963004

Cr-Commit-Position: refs/branch-heads/2171@{#183}
Cr-Branched-From: 267aeeb-refs/heads/master@{#297060}
rakuco pushed a commit that referenced this pull request Feb 27, 2015
The CL corrects a bug where clock_gettime was placed in
a DCHECK call and thus not executed on release builds.

Contributed by sheckylin@chromium.org

BUG=chrome-os-partner:35111
TEST=samus ChromeOS build

Review URL: https://codereview.chromium.org/884313002

Cr-Commit-Position: refs/heads/master@{#313634}
(cherry picked from commit c246784)

Review URL: https://codereview.chromium.org/895673003

Cr-Commit-Position: refs/branch-heads/2272@{#183}
Cr-Branched-From: 827a380-refs/heads/master@{#310958}
rakuco pushed a commit that referenced this pull request Mar 12, 2015
Now supported: 1.02.3.4
Remains unsupported: 01.2.3.4

BUG=460642

Review URL: https://codereview.chromium.org/949573002

Cr-Commit-Position: refs/heads/master@{#317524}
(cherry picked from commit f151249)

Review URL: https://codereview.chromium.org/991883003

Cr-Commit-Position: refs/branch-heads/2311@{#183}
Cr-Branched-From: 09b7de5-refs/heads/master@{#317474}
rakuco pushed a commit that referenced this pull request Apr 28, 2015
Cr-Commit-Position: refs/branch-heads/2357@{#183}
Cr-Branched-From: 59d4494-refs/heads/master@{#323860}
mrunalk pushed a commit that referenced this pull request Sep 22, 2015
Cr-Commit-Position: refs/branch-heads/2490@{#183}
Cr-Branched-From: 7790a35-refs/heads/master@{#344925}
mrunalk pushed a commit that referenced this pull request Dec 18, 2015
In some cases, a CAPTCHA solution will itself redirect (e.g., from
http:// to https://). This CL fixes the double-counting problem.

BUG=560470

Review URL: https://codereview.chromium.org/1465313003

Cr-Commit-Position: refs/heads/master@{#362156}
(cherry picked from commit 6bbab2e)

Review URL: https://codereview.chromium.org/1492513002 .

Cr-Commit-Position: refs/branch-heads/2564@{#183}
Cr-Branched-From: 1283eca-refs/heads/master@{#359700}
rakuco pushed a commit that referenced this pull request Mar 3, 2016
BUG=581949

Review URL: https://codereview.chromium.org/1648783003

Cr-Commit-Position: refs/heads/master@{#372188}

Review URL: https://codereview.chromium.org/1645983003 .

Cr-Commit-Position: refs/branch-heads/2623@{#183}
Cr-Branched-From: 92d7753-refs/heads/master@{#369907}
huningxin pushed a commit to huningxin/chromium-croswalk that referenced this pull request May 17, 2016
Blink and cc have different notions of scroll offset - Blink's is relative to
the scroll origin which is non-zero in RTL documents.

The CompositorScrollOffsetAnimationCurve must work entirely in cc scroll offsets
since it is handed over to cc which doesn't know about the scroll origin.

This patch teaches ScrollAnimator and ProgrammaticScrollAnimator to convert in
both directions when creating and using the curve object.

BUG=593186

Review URL: https://codereview.chromium.org/1776503002

Cr-Commit-Position: refs/heads/master@{#379974}
(cherry picked from commit 0bb1660)

Review URL: https://codereview.chromium.org/1780993003 .

Cr-Commit-Position: refs/branch-heads/2661@{crosswalk-project#183}
Cr-Branched-From: ef6f6ae-refs/heads/master@{#378081}
huningxin pushed a commit to huningxin/chromium-croswalk that referenced this pull request Oct 9, 2016
The existing DOM configuration code for installAttribute only applies to object
templates, and is intended for use when setting up the DOM template. This patch
adds a separate method which can be used to add attributes into a specific V8
context, by providing an object instance rather than a template (this parallels
the existing installAccessor code).

This is intended to be used by origin trials, to configure attributes in a
context-dependent manner.

BUG=584367

Review-Url: https://codereview.chromium.org/2002613002
Cr-Commit-Position: refs/heads/master@{#395659}
(cherry picked from commit 9dc629c)

Review URL: https://codereview.chromium.org/2037493002 .

Cr-Commit-Position: refs/branch-heads/2743@{crosswalk-project#183}
Cr-Branched-From: 2b3ae3b-refs/heads/master@{#394939}
imreotto pushed a commit to tenta-browser/chromium-crosswalk that referenced this pull request Nov 2, 2017
…on BT with Mobile Data.

This makes it easier to keep the actual powered state of the Bluetooth
adapter synchronized with user prefs.

TBR=lesliewatkins@chromium.org

(cherry picked from commit 8fd52f8)

Bug: 672263, 762345
Change-Id: I7a3a46217e669b8b70fe3301245c3735bc5a6be1
Reviewed-on: https://chromium-review.googlesource.com/656380
Commit-Queue: Leslie Watkins <lesliewatkins@chromium.org>
Reviewed-by: James Cook <jamescook@chromium.org>
Reviewed-by: Kyle Horimoto <khorimoto@chromium.org>
Cr-Original-Commit-Position: refs/heads/master@{#500980}
Reviewed-on: https://chromium-review.googlesource.com/663942
Cr-Commit-Position: refs/branch-heads/3202@{crosswalk-project#183}
Cr-Branched-From: fa6a5d8-refs/heads/master@{#499098}
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants