Android bp #1856
Android bp #1856
Conversation
JeevakaPrabu
force-pushed
the
android_bp
branch
from
d9d115e
to
c632490
Compare
JeevakaPrabu
force-pushed
the
android_bp
branch
from
c632490
to
774eb01
Compare
| @@ -0,0 +1,2265 @@ | |||
| package { | |||
There was a problem hiding this comment.
nit: Do you need a copyright notice?
| "-Wno-parentheses", | ||
| "-Wno-delete-incomplete", | ||
| "-Wno-overloaded-virtual", | ||
| "-std=c++14", |
There was a problem hiding this comment.
I noticed that this is in media_driver/cmake/linux/media_compile_flags_linux.cmake but not in media_softlet/cmake/linux/media_compile_flags_linux.cmake. Does this matter?
Android.bp
Outdated
| "-Wno-delete-incomplete", | ||
| "-Wno-overloaded-virtual", | ||
| "-std=c++14", | ||
| "-m64", |
There was a problem hiding this comment.
I would expect -m64 to be added by Soong. See this.
Android.bp
Outdated
| "-m64", | ||
| "-DNO_RTTI", | ||
| "-DNO_EXCEPTION_HANDLING", | ||
| "-Wno-ignored-optimization-argument", |
There was a problem hiding this comment.
Consider removing -Wno-ignored-optimization-argument and -finline-limit=100 because:
- According to media_driver/cmake/linux/media_compile_flags_linux.cmake and media_softlet/cmake/linux/media_compile_flags_linux.cmake,
-finline-limit=100is removed when using clang, and I'm pretty sure clang is the primary compiler for AOSP. - If
-finline-limit=100is removed, it looks like things build without-Wno-ignored-optimization-argument.
| "-DIGFX_XEHP_SDV_SUPPORTED", | ||
| "-DIGFX_XE_HPG_CMFCPATCH_SUPPORTED", | ||
| "-DIGFX_XE_HPG_SUPPORTED", | ||
| "-DMEDIA_VERSION=\"24.3.4\"", |
There was a problem hiding this comment.
How can we ensure MEDIA_VERSION and MEDIA_VERSION_DETAILS don't become stale?
For example, currently, CMakeLists.txt contains 24.4.0.
Additionally, MEDIA_VERSION_DETAILS is obtained dynamically using git rev-parse --short HEAD.
There was a problem hiding this comment.
Android.bp can be generated now by running python3 Tools/MediaDriverTools/Android/GenerateAndroidBp.py. Pre-requisite is libva and gmmlib are compiled and installed in the build machine.
When GenerateAndroidBp.py script is used, Version and version_details gets generated instead of hardcoding.
cmrtlib/Android.bp
Outdated
| "-DISTDLIB_UMD", | ||
| "-DVPHAL", | ||
| "-D__CT__", | ||
| "-fno-strict-aliasing", |
There was a problem hiding this comment.
-fno-strict-aliasing is globally added, consider removing it from here.
cmrtlib/Android.bp
Outdated
| "-DVPHAL", | ||
| "-D__CT__", | ||
| "-fno-strict-aliasing", | ||
| "-D_FORTIFY_SOURCE=2", |
There was a problem hiding this comment.
You get -D_FORTIFY_SOURCE=2 for free as well.
| "-D__CT__", | ||
| "-fno-strict-aliasing", | ||
| "-D_FORTIFY_SOURCE=2", | ||
| ], |
There was a problem hiding this comment.
I happened to notice that the cmrt CMakeLists.txt adds the -fstack-protector-all option. Consider adding it here as well.
Note: it looks like Soong adds -fstack-protector-strong automatically, but if I understand correctly, it offers less protection than -fstack-protector-all.
cmrtlib/Android.bp
Outdated
| ], | ||
|
|
||
| cflags: [ | ||
| "-Wno-error", |
There was a problem hiding this comment.
Reconsider the -W flags here. I tried the following that seems more strict and it still builds:
"-Werror",
"-Wno-non-virtual-dtor",
"-Wno-implicit-fallthrough",
"-Wno-reorder",
| @@ -40,7 +40,7 @@ | |||
| #include <sys/types.h> | |||
| #include <sys/sem.h> | |||
| #include <sys/mman.h> | |||
| #include "mos_compat.h" // libc variative definitions: backtrace | |||
| //#include "mos_compat.h" // libc variative definitions: backtrace | |||
There was a problem hiding this comment.
Did you mean to just remove this line?
JeevakaPrabu
force-pushed
the
android_bp
branch
from
774eb01
to
741511a
Compare
* Fixed some compilation issues * Update GenerateAndroidMk.py fix compile issue and update tpl for cmrt and media_driver. * Add 'media_add_curr_to_include_path()' to media_srcs.cmake Tracked-On: OAM-117146 Signed-off-by: zhangyichix <[email protected]> Signed-off-by: zhepeng.xu <[email protected]>
JeevakaPrabu
force-pushed
the
android_bp
branch
from
741511a
to
b7aa05c
Compare
Test: make iHD_drv_video for x86_64 Change-Id: I9582630dc52b0780c1a63ab8ead63a1823186445 Signed-off-by: JeevakaPrabu <[email protected]>
JeevakaPrabu
force-pushed
the
android_bp
branch
from
b7aa05c
to
869a466
Compare
There was a problem hiding this comment.
I tried python3 Tools/MediaDriverTools/Android/GenerateAndroidBp.py
it generate
./gmmlib/Android.bp
./media-driver/cmrtlib/Android.bp
./media-driver/Android.bp
from feature wise, it do generate the Android.bp . it works.
but from my personal perspective:
- I am expecting a better GenerateAndroidBp.py , because from the sequence description here (https://github.com/intel/media-driver/blob/master/Tools/MediaDriverTools/Android/GenerateAndroidMk.py#L25-L30), this script run cmake firstly, then search file and dependencies from the Makefile. so, cmake system and Makefile works, in theory, this script should works without the changes in cmake file , such as "adding media_add_curr_to_include_path". we should not keep it scalable for future platform.
- if there are already such script as above, we could remove Android.bp in the top folder. anyone who has such requirement , could generate it by himself. maybe we could add a BKM in the wiki.
if 1 is not doable now. lets keep current solution as a out-of-tree patch, or keep it by down stream.
No description provided.