Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

A series patches for Android #37

Merged
merged 10 commits into from
May 2, 2017
Merged

A series patches for Android #37

merged 10 commits into from
May 2, 2017

Conversation

cwhuang
Copy link
Contributor

@cwhuang cwhuang commented Apr 7, 2017

These patches fix building issues of Android on the master branch.

Tested OK on Android-x86 7.1 cm branch.

@xuguangxin
Copy link
Contributor

@LindaYu17 please help review this

@xhaihao xhaihao requested a review from LindaYu17 April 7, 2017 08:30
Copy link
Contributor

@LindaYu17 LindaYu17 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could you please put libva*.so and to /vendor/lib folder, all vendor provided driver/hal should come to this folder.

@cwhuang
Copy link
Contributor Author

cwhuang commented Apr 10, 2017

Personally I don't think libva*.so should go to /vendor/lib folder. That folder is usually for vendor's proprietary libraries. But libva and vaapi are open source libs. I understand they are mainly developed by Intel. But I believe you have developers or contributors from others. Right? In particular, libva is not limited to be used by one vendor. So we should not considered them as vendor provided libraries. We should just treat them as system libs, just like the normal linux desktop.

But if you insist, I can add additional patches to do that.

@xhaihao
Copy link
Contributor

xhaihao commented Apr 10, 2017

Agree with @cwhuang, besides intel-vaapi-driver, libva supports other vaapi backend drivers, such as r600, nouveau and radeonsi.

@kalyankondapally
Copy link

HALS/Drivers going into vendor/ can be updated independent of system changes i.e. driver bug fixng etc. We already have patches floating here: https://github.com/android-ia/libva/commits/master.

@LindaYu17
Copy link
Contributor

relative path should be set to /vendor/lib but not /vendor/lib/hw to avoid linking error.

+LOCAL_PROPRIETARY_MODULE := true
+LOCAL_MODULE_RELATIVE_PATH := .

@@ -194,7 +194,7 @@ extern "C" {
VAStatus vaPutSurface (
VADisplay dpy,
VASurfaceID surface,
sp<ISurface> draw, /* Android Surface/Window */
sp<ANativeWindow> draw, /* Android Native Window */
short srcx,
short srcy,
unsigned short srcw,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

are you using vaPutSurface on Android?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

no, we don't use it in Android.

endif
LOCAL_CFLAGS := \
$(if $(filter user,$(TARGET_BUILD_VARIANT)),,-DENABLE_VA_MESSAGING) \
-DLOG_TAG=\"libva\"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

is the build variant actually needing a full trace from libva?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @LindaYu17 , any comments for this?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not? It's just for debug build.

Besides, what trace should I see? Except these I didn't see other log from libva:

04-23 22:04:40.498 1453 3028 I libva : VA-API version 0.40.0
04-23 22:04:40.498 1453 3028 I libva : Open DRM devices /dev/dri/card0 for (null)
04-23 22:04:40.498 1453 3028 I libva : va_getDriverName() returns 0
04-23 22:04:40.498 1453 3028 I libva : Trying to open /system/lib/dri/i965_drv_video.so
04-23 22:04:40.498 1453 3028 I libva : Found init function __vaDriverInit_0_33
04-23 22:04:40.499 1453 3028 I libva : va_openDriver() returns 0

How could I enable more verbose log?

@xuguangxin
Copy link
Contributor

xuguangxin commented Apr 19, 2017

@kalyankondapally seems it's hard to align on directory thing. Do you think it's ok if we maintain a local patch on Android IA to change directory to vendor/?

@xuguangxin
Copy link
Contributor

Hi @cwhuang
@kalyankondapally convinced @xhaihao and me, Could you help to change directory as we did in https://github.com/android-ia/libva/commits/master?

Also please address Daniel's second comments

is the build variant actually needing a full trace from libva?

thanks

LindaYu17 and others added 10 commits April 26, 2017 13:25
Jira: https://01.org/jira/browser/AIA-65
Test: vainfo

Signed-off-by: Linda Yu <linda.yu@intel.com>
Android source tree has to be read-only. The generated files should
be put to the $(OUT)/gen/ dir. Besides, LOCAL_GENERATED_SOURCES must
be set before include $(BUILD_SHARED_LIBRARY). Otherwise it has no
effect.

Signed-off-by: Chih-Wei Huang <cwhuang@linux.org.tw>
The documents of Android build system explicitly said LOCAL_COPY_HEADERS
and LOCAL_COPY_HEADERS_TO are deprecated.

Replace them by the LOCAL_EXPORT_C_INCLUDE_DIRS variable. The modules
that use libva will get the include path automatically.

Signed-off-by: Chih-Wei Huang <cwhuang@linux.org.tw>
It has already been defined by the Android build system.

Signed-off-by: Chih-Wei Huang <cwhuang@linux.org.tw>
To support older versions, just check if ALOGx are defined.

Signed-off-by: Chih-Wei Huang <cwhuang@linux.org.tw>
Signed-off-by: Chih-Wei Huang <cwhuang@linux.org.tw>
Use the path similar to linux desktop.

Signed-off-by: Chih-Wei Huang <cwhuang@linux.org.tw>
These files are not executables.

Signed-off-by: Chih-Wei Huang <cwhuang@linux.org.tw>
Signed-off-by: Chih-Wei Huang <cwhuang@linux.org.tw>
Signed-off-by: Chih-Wei Huang <cwhuang@linux.org.tw>
@cwhuang
Copy link
Contributor Author

cwhuang commented Apr 26, 2017

Sorry for reply late. I just rebase my patches and change libs path to /vendor as request.
(though I still didn't see any real benefit from it)

@xuguangxin
Copy link
Contributor

@cwhuang, thanks, According to @kalyankondapally , /system will control by google, vendor/ will control by the vendor. If will in a different partition. It's easy to update when google have new android version

@xuguangxin
Copy link
Contributor

@LindaYu17 , please help to review.thanks

@LindaYu17
Copy link
Contributor

+1

@xhaihao xhaihao merged commit 22bbdaa into intel:master May 2, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants