On Mon, Aug 14, 2017 at 8:55 PM, Leo Liu <leo....@amd.com> wrote: > > > On 08/14/2017 11:19 AM, Gurkirpal Singh wrote: > > > > On Mon, Aug 14, 2017 at 8:05 PM, Leo Liu <leo....@amd.com> wrote: > >> >> >> On 08/14/2017 05:46 AM, Julien Isorce wrote: >> >> Hi Leo, >> >> >It would be better if you can extract the common code between bellagio >> and tizonia to avoid the duplication. >> >> This is something Gurkirpal and me discussed, like having >> state_trackers/omx/common, state_trackers/omx/bellagio, >> state_trackers/omx/tizonia. To anticipate that Gurkirpal sent an email >> https://www.mail-archive.com/mesa-dev@lists.freedeskto >> p.org/msg153562.html during the Community Bonding period in May (a few >> weeks from the date (early May) students are officially accepted to the >> date (end May) where student actually starts working) >> >> The problem with the directory layout above is that it would look like it >> will be built together in the same shared library. This is not good because >> it is suppose to export OMX_ComponentInit for each target. Of course this >> would still be possible to have 2 shared libs but I believe in >> state_trackers dir, all sub dir are for one target. Also we were afraid >> that there would be other limitations so we decided to go for a separate >> directory. >> And since there were no objections in Gurkirpal's mail above we went this >> way. >> >> Now if I look into state_trackers/omx_tizonia, in fact the common code >> with state_trackers/omx_bellagio does not have anything to do with omx. For >> example "slice_header". Maybe it can be moved to gallium/auxiliary/vl/ like >> it is done already for vl_rbsp_init. Same for omx_get_screen. >> >> So I suggest this 'factorization' to be not a blocker for merging >> state_trackers/omx_tizonia into usptream. But later on we can move >> gradually bits from state_trackers/omx_bellagio to gallium/auxiliary. And >> then make state_trackers/omx_tizonia use it as well. >> This way you will only bother about maintaining >> state_trackers/omx_bellagio the time being. This also allows to not slowly >> get its work lost. >> >> Of course we would have done differently if we knew advance. But as today >> Gurkirpal won't have enough time to do this factorization/move as the >> project ends in 1 week. Having all of this merged in upstream is not >> mandatory to succeed the project but Gurkirpal will need some rest after >> these 3 months of hard work. And who knows what happens after, whether he >> will still be around after sometimes or not. And this is entirely up to him. >> >> So again I suggest this factorization/move not to be a blocker for the >> reasons above. What do you think? >> >> >> When the whole project will be completed, as least same status as >> bellagio? Is there anyone keep working on this project after this step 1? >> > Hi, I plan on continuing working on this after the GSoC project ends. > > > Is the GSoC project done with your current patches? > > Apart from these patches, at least two more will be needed for the project. One for adding H.264 enc and other for adding EGLImage support to H.264 dec. Since I keep revising the commits quite frequently I'll give the link to the branch https://github.com/gpalsingh/mesa/commits/gsoc
Cheers > > Regards, > Leo > > > >> The left stuff compared to bellagio are codecs of mpeg2 and hevc, and >> encoding that is more important, since the most usage for omx currently is >> to use transcoding from one codec to another. >> >> Other is the transcoding performance, at least meet the performance of >> bellagio's. >> >> If the above could be addressed at early time, we could remove Bellagio >> completely, then there is no need to do any refactor now. >> > The original plan was this but making gst-omx OMX IL 1.2 compatible has > been a bit time consuming. > >> >> >> BTW what's the plan for new feature like EGL Images? >> > We've been able to make it work but with wrong colours in output. I've > written more about it and the performance comparison in my blog post here > https://singhcodes.wordpress.com/2017/08/04/gsoc-2017-third-phase-starts/ > But recently the EGLImage hook stopped working and I've opened the issue > https://github.com/gpalsingh/mesa/issues/15 to keep track of it. > This happened after we fixed another issue which made changes to gst-omx > which might be the cause https://github.com/gpalsingh/mesa/issues/2# > issuecomment-321839195 > I'm still looking into this. > > Cheers > >> >> Regards, >> Leo >> >> >> >> Thx >> Julien >> >> On 13 August 2017 at 16:52, Gurkirpal Singh <gurkirpal...@gmail.com> >> wrote: >> >>> >>> >>> On Sun, Aug 13, 2017 at 8:47 AM, Leo Liu <leo....@amd.com> wrote: >>> >>>> Where is the patch 1? >>> >>> Sorry for the patches got messed up somehow while sending, I could only >>> see two patches on mail-archive but three on patchwork. >>> Tried two times and same result. >>> About the first one I got a mail saying that it was too large has been >>> put aside for mod approval. >>> The changes I made were to just rename the st/omx directory to >>> st/omx_bellagio (the reason it became large) >>> and renaming bits in the configure.ac and Makefiles. >>> >>>> >>>> >>>> >>>> On 08/12/2017 12:07 PM, Gurkirpal Singh wrote: >>>> >>>>> Coexist with --enable-omx so they can be built independently >>>>> Detect tizonia package config file >>>>> Generate libomxtiz_mesa.so and install it to libtizcore.pc::pluginsdir >>>>> Only compile empty source (target.c) for now. >>>>> >>>>> v2: Show error message when --enable-omx is used (Christian) >>>>> Use single PKG_CHECK_MODULES for omx_tizonia checks (Emil) >>>>> Use spaces instead of tabs >>>>> Add checks around omx-tizonia >>>>> >>>>> GSoC Project link: https://summerofcode.withgoogl >>>>> e.com/projects/#4737166321123328 >>>>> >>>>> Signed-off-by: Gurkirpal Singh <gurkirpal...@gmail.com> >>>>> Reviewed-and-Tested-by: Julien Isorce <julien.iso...@gmail.com> >>>>> --- >>>>> configure.ac | 40 ++++++++++++++- >>>>> src/gallium/Makefile.am | 4 ++ >>>>> src/gallium/targets/omx-tizonia/Makefile.am | 77 >>>>> +++++++++++++++++++++++++++++ >>>>> src/gallium/targets/omx-tizonia/omx.sym | 11 +++++ >>>>> src/gallium/targets/omx-tizonia/target.c | 2 + >>>>> 5 files changed, 132 insertions(+), 2 deletions(-) >>>>> create mode 100644 src/gallium/targets/omx-tizonia/Makefile.am >>>>> create mode 100644 src/gallium/targets/omx-tizonia/omx.sym >>>>> create mode 100644 src/gallium/targets/omx-tizonia/target.c >>>>> >>>>> diff --git a/configure.ac b/configure.ac >>>>> index 38af96a..5669695 100644 >>>>> --- a/configure.ac >>>>> +++ b/configure.ac >>>>> @@ -85,6 +85,7 @@ dnl Versions for external dependencies >>>>> DRI2PROTO_REQUIRED=2.8 >>>>> GLPROTO_REQUIRED=1.4.14 >>>>> LIBOMXIL_BELLAGIO_REQUIRED=0.0 >>>>> +LIBOMXIL_TIZONIA_REQUIRED=0.9.0 >>>>> LIBVA_REQUIRED=0.38.0 >>>>> VDPAU_REQUIRED=1.1 >>>>> WAYLAND_REQUIRED=1.11 >>>>> @@ -1216,14 +1217,19 @@ AC_ARG_ENABLE([vdpau], >>>>> [enable_vdpau=auto]) >>>>> AC_ARG_ENABLE([omx], >>>>> [AS_HELP_STRING([--enable-omx], >>>>> - [DEPRECATED: Use --enable-omx-bellagio instead >>>>> @<:@default=auto@:>@])], >>>>> - [AC_MSG_ERROR([--enable-omx is deprecated. Use >>>>> --enable-omx-bellagio instead.])], >>>>> >>>> >>>> Is this in patch 1? >>> >>> Yes, it is so. >>> >>>> >>>> >>>> + [DEPRECATED: Use --enable-omx-bellagio or >>>>> --enable-omx-tizonia instead @<:@default=auto@:>@])], >>>>> + [AC_MSG_ERROR([--enable-omx is deprecated. Use >>>>> --enable-omx-bellagio or --enable-omx-tizonia instead.])], >>>>> []) >>>>> AC_ARG_ENABLE([omx-bellagio], >>>>> [AS_HELP_STRING([--enable-omx-bellagio], >>>>> [enable OpenMAX Bellagio library @<:@default=disabled@ >>>>> :>@])], >>>>> [enable_omx_bellagio="$enableval"], >>>>> [enable_omx_bellagio=no]) >>>>> +AC_ARG_ENABLE([omx-tizonia], >>>>> + [AS_HELP_STRING([--enable-omx-tizonia], >>>>> + [enable OpenMAX Tizonia library @<:@default=disabled@:>@])], >>>>> + [enable_omx_tizonia="$enableval"], >>>>> + [enable_omx_tizonia=no]) >>>>> AC_ARG_ENABLE([va], >>>>> [AS_HELP_STRING([--enable-va], >>>>> [enable va library @<:@default=auto@:>@])], >>>>> @@ -1275,6 +1281,7 @@ if test "x$enable_opengl" = xno -a \ >>>>> "x$enable_xvmc" = xno -a \ >>>>> "x$enable_vdpau" = xno -a \ >>>>> "x$enable_omx_bellagio" = xno -a \ >>>>> + "x$enable_omx_tizonia" = xno -a \ >>>>> "x$enable_va" = xno -a \ >>>>> "x$enable_opencl" = xno; then >>>>> AC_MSG_ERROR([at least one API should be enabled]) >>>>> @@ -2121,6 +2128,10 @@ if test -n "$with_gallium_drivers" -a >>>>> "x$with_gallium_drivers" != xswrast; then >>>>> PKG_CHECK_EXISTS([libomxil-bellagio >= >>>>> $LIBOMXIL_BELLAGIO_REQUIRED], [enable_omx_bellagio=yes], >>>>> [enable_omx_bellagio=no]) >>>>> fi >>>>> + if test "x$enable_omx_tizonia" = xauto -a "x$have_omx_platform" >>>>> = xyes; then >>>>> + PKG_CHECK_EXISTS([libtizonia >= $LIBOMXIL_TIZONIA_REQUIRED], >>>>> [enable_omx_tizonia=yes], [enable_omx_tizonia=no]) >>>>> + fi >>>>> + >>>>> if test "x$enable_va" = xauto -a "x$have_va_platform" = xyes; >>>>> then >>>>> PKG_CHECK_EXISTS([libva >= $LIBVA_REQUIRED], >>>>> [enable_va=yes], [enable_va=no]) >>>>> fi >>>>> @@ -2130,6 +2141,7 @@ if test "x$enable_dri" = xyes -o \ >>>>> "x$enable_xvmc" = xyes -o \ >>>>> "x$enable_vdpau" = xyes -o \ >>>>> "x$enable_omx_bellagio" = xyes -o \ >>>>> + "x$enable_omx_tizonia" = xyes -o \ >>>>> "x$enable_va" = xyes; then >>>>> need_gallium_vl=yes >>>>> fi >>>>> @@ -2138,6 +2150,7 @@ AM_CONDITIONAL(NEED_GALLIUM_VL, test >>>>> "x$need_gallium_vl" = xyes) >>>>> if test "x$enable_xvmc" = xyes -o \ >>>>> "x$enable_vdpau" = xyes -o \ >>>>> "x$enable_omx_bellagio" = xyes -o \ >>>>> + "x$enable_omx_tizonia" = xyes -o \ >>>>> "x$enable_va" = xyes; then >>>>> PKG_CHECK_MODULES([VL], [x11-xcb xcb xcb-dri2 >= >>>>> $XCBDRI2_REQUIRED]) >>>>> need_gallium_vl_winsys=yes >>>>> @@ -2172,6 +2185,18 @@ if test "x$enable_omx_bellagio" = xyes; then >>>>> fi >>>>> AM_CONDITIONAL(HAVE_ST_OMX_BELLAGIO, test "x$enable_omx_bellagio" = >>>>> xyes) >>>>> +if test "x$enable_omx_tizonia" = xyes; then >>>>> + if test "x$have_omx_platform" != xyes; then >>>>> + AC_MSG_ERROR([OMX requires at least one of the x11 or drm >>>>> platforms]) >>>>> + fi >>>>> + PKG_CHECK_MODULES([OMX_TIZONIA], >>>>> + [libtizonia >= $LIBOMXIL_TIZONIA_REQUIRED >>>>> + tizilheaders >= $LIBOMXIL_TIZONIA_REQUIRED >>>>> + libtizplatform >= $LIBOMXIL_TIZONIA_REQUIRED]) >>>>> + gallium_st="$gallium_st omx_tizonia" >>>>> +fi >>>>> +AM_CONDITIONAL(HAVE_ST_OMX_TIZONIA, test "x$enable_omx_tizonia" = >>>>> xyes) >>>>> + >>>>> if test "x$enable_va" = xyes; then >>>>> if test "x$have_va_platform" != xyes; then >>>>> AC_MSG_ERROR([VA requires at least one of the x11 drm or >>>>> wayland platforms]) >>>>> @@ -2342,6 +2367,15 @@ AC_ARG_WITH([omx-bellagio-libdir], >>>>> $PKG_CONFIG >>>>> --define-variable=libdir=\$libdir --variable=pluginsdir >>>>> libomxil-bellagio`]) >>>>> AC_SUBST([OMX_BELLAGIO_LIB_INSTALL_DIR]) >>>>> +dnl Directory for OMX_TIZONIA libs >>>>> + >>>>> +AC_ARG_WITH([omx-tizonia-libdir], >>>>> + [AS_HELP_STRING([--with-omx-tizonia-libdir=DIR], >>>>> + [directory for the OMX_TIZONIA libraries])], >>>>> + [OMX_TIZONIA_LIB_INSTALL_DIR="$withval"], >>>>> + [OMX_TIZONIA_LIB_INSTALL_DIR=`$PKG_CONFIG >>>>> --define-variable=libdir=\$libdir --variable=pluginsdir libtizcore`]) >>>>> +AC_SUBST([OMX_TIZONIA_LIB_INSTALL_DIR]) >>>>> + >>>>> dnl Directory for VA libs >>>>> AC_ARG_WITH([va-libdir], >>>>> @@ -2840,6 +2874,7 @@ AC_CONFIG_FILES([Makefile >>>>> src/gallium/state_trackers/glx/xlib/Makefile >>>>> src/gallium/state_trackers/nine/Makefile >>>>> src/gallium/state_trackers/omx_bellagio/Makefile >>>>> + src/gallium/state_trackers/omx_tizonia/Makefile >>>>> src/gallium/state_trackers/osmesa/Makefile >>>>> src/gallium/state_trackers/va/Makefile >>>>> src/gallium/state_trackers/vdpau/Makefile >>>>> @@ -2850,6 +2885,7 @@ AC_CONFIG_FILES([Makefile >>>>> src/gallium/targets/dri/Makefile >>>>> src/gallium/targets/libgl-xlib/Makefile >>>>> src/gallium/targets/omx-bellagio/Makefile >>>>> + src/gallium/targets/omx-tizonia/Makefile >>>>> src/gallium/targets/opencl/Makefile >>>>> src/gallium/targets/opencl/mesa.icd >>>>> src/gallium/targets/osmesa/Makefile >>>>> diff --git a/src/gallium/Makefile.am b/src/gallium/Makefile.am >>>>> index 2b930ac..e17c679 100644 >>>>> --- a/src/gallium/Makefile.am >>>>> +++ b/src/gallium/Makefile.am >>>>> @@ -153,6 +153,10 @@ if HAVE_ST_OMX_BELLAGIO >>>>> SUBDIRS += state_trackers/omx_bellagio targets/omx-bellagio >>>>> endif >>>>> +if HAVE_ST_OMX_TIZONIA >>>>> +SUBDIRS += state_trackers/omx_tizonia targets/omx-tizonia >>>>> +endif >>>>> + >>>>> >>>> >>>> So now we have 2 OMX ST under gallium state trackers >>>> >>>> Is possible that we could have both tizonia and bellagio as >>>> sub-directory under st/omx?, so that they could share the same Makefile.am, >>>> Makefile.source, and entrypoint.[ch] etc. >>>> even maybe same idea for gallium/target? >>>> >>>> Regards, >>>> Leo >>>> >>>> The original plan was to completely replace the existing OMX IL ST >>> however porting wasn't that straightforward since the new OMX IL version >>> breaks backward compatibility. >>> I think Julien will be able to expain better here. >>> >>> So the plan is still to replace the ST but for now we've postponed that >>> task until all the components have been ported. As it is now we'll later >>> just remove omx_bellagio and rename omx_tizonia to omx. >>> I think that merging them both will make the transition a little bit >>> harder. >>> >>> So maybe if you still insist can we mark it as good-to-have for later? >>> Actually Julien gave the same idea earlier when we started on the project >>> and decided to postpone it for later due to project time restraints. >>> >>> Cheers >>> >>> >>>> >>>> if HAVE_GALLIUM_OSMESA >>>>> SUBDIRS += state_trackers/osmesa targets/osmesa >>>>> endif >>>>> diff --git a/src/gallium/targets/omx-tizonia/Makefile.am >>>>> b/src/gallium/targets/omx-tizonia/Makefile.am >>>>> new file mode 100644 >>>>> index 0000000..6baacaa >>>>> --- /dev/null >>>>> +++ b/src/gallium/targets/omx-tizonia/Makefile.am >>>>> @@ -0,0 +1,77 @@ >>>>> +include $(top_srcdir)/src/gallium/Automake.inc >>>>> + >>>>> +AM_CFLAGS = \ >>>>> + $(GALLIUM_TARGET_CFLAGS) >>>>> + >>>>> +omxdir = $(OMX_TIZONIA_LIB_INSTALL_DIR) >>>>> +omx_LTLIBRARIES = libomxtiz_mesa.la >>>>> + >>>>> +nodist_EXTRA_libomxtiz_mesa_la_SOURCES = dummy.cpp >>>>> +libomxtiz_mesa_la_SOURCES = >>>>> + >>>>> +libomxtiz_mesa_la_LDFLAGS = \ >>>>> + -shared \ >>>>> + -module \ >>>>> + -no-undefined \ >>>>> + -avoid-version \ >>>>> + $(GC_SECTIONS) \ >>>>> + $(LD_NO_UNDEFINED) >>>>> + >>>>> +if HAVE_LD_VERSION_SCRIPT >>>>> +libomxtiz_mesa_la_LDFLAGS += \ >>>>> + -Wl,--version-script=$(top_srcdir)/src/gallium/targets/omx- >>>>> tizonia/omx.sym >>>>> +endif # HAVE_LD_VERSION_SCRIPT >>>>> + >>>>> +libomxtiz_mesa_la_LIBADD = \ >>>>> + $(top_builddir)/src/gallium/state_trackers/omx_tizonia/libo >>>>> mxtiztracker.la \ >>>>> + $(top_builddir)/src/gallium/auxiliary/libgalliumvlwinsys.la \ >>>>> + $(top_builddir)/src/gallium/auxiliary/libgalliumvl.la \ >>>>> + $(top_builddir)/src/gallium/auxiliary/libgallium.la \ >>>>> + $(top_builddir)/src/util/libmesautil.la \ >>>>> + $(OMX_TIZONIA_LIBS) \ >>>>> + $(OMX_TIZILHEADERS_LIBS) \ >>>>> + $(OMX_TIZPLATFORM_LIBS) \ >>>>> + $(LIBDRM_LIBS) \ >>>>> + $(GALLIUM_COMMON_LIB_DEPS) >>>>> + >>>>> +if HAVE_PLATFORM_X11 >>>>> +libomxtiz_mesa_la_LIBADD += \ >>>>> + $(VL_LIBS) \ >>>>> + $(XCB_DRI3_LIBS) >>>>> +endif >>>>> + >>>>> +EXTRA_libomxtiz_mesa_la_DEPENDENCIES = omx.sym >>>>> +EXTRA_DIST = omx.sym >>>>> + >>>>> +if HAVE_GALLIUM_STATIC_TARGETS >>>>> + >>>>> +TARGET_DRIVERS = >>>>> +TARGET_CPPFLAGS = >>>>> +TARGET_LIB_DEPS = >>>>> + >>>>> + >>>>> +include $(top_srcdir)/src/gallium/drivers/nouveau/Automake.inc >>>>> + >>>>> +include $(top_srcdir)/src/gallium/drivers/r600/Automake.inc >>>>> +include $(top_srcdir)/src/gallium/drivers/radeonsi/Automake.inc >>>>> + >>>>> +libomxtiz_mesa_la_SOURCES += target.c >>>>> +libomxtiz_mesa_la_CPPFLAGS = $(TARGET_CPPFLAGS) >>>>> +libomxtiz_mesa_la_LIBADD += \ >>>>> + $(top_builddir)/src/gallium/auxiliary/pipe-loader/libpipe_l >>>>> oader_static.la \ >>>>> + $(GALLIUM_PIPE_LOADER_WINSYS_LIBS) \ >>>>> + $(TARGET_LIB_DEPS) \ >>>>> + $(TARGET_COMPILER_LIB_DEPS) \ >>>>> + $(TARGET_RADEON_WINSYS) $(TARGET_RADEON_COMMON) >>>>> + >>>>> +else # HAVE_GALLIUM_STATIC_TARGETS >>>>> + >>>>> +libomxtiz_mesa_la_LIBADD += \ >>>>> + $(top_builddir)/src/gallium/auxiliary/pipe-loader/libpipe_l >>>>> oader_dynamic.la >>>>> + >>>>> +endif # HAVE_GALLIUM_STATIC_TARGETS >>>>> + >>>>> +if HAVE_GALLIUM_LLVM >>>>> +libomxtiz_mesa_la_LIBADD += $(LLVM_LIBS) >>>>> +libomxtiz_mesa_la_LDFLAGS += $(LLVM_LDFLAGS) >>>>> +endif >>>>> diff --git a/src/gallium/targets/omx-tizonia/omx.sym >>>>> b/src/gallium/targets/omx-tizonia/omx.sym >>>>> new file mode 100644 >>>>> index 0000000..2aafb29 >>>>> --- /dev/null >>>>> +++ b/src/gallium/targets/omx-tizonia/omx.sym >>>>> @@ -0,0 +1,11 @@ >>>>> +{ >>>>> + global: >>>>> + OMX_ComponentInit; >>>>> + >>>>> + # Workaround for an LLVM warning with >>>>> -simplifycfg-sink-common >>>>> + # due to LLVM being initialized multiple times. >>>>> + radeon_drm_winsys_create; >>>>> + amdgpu_winsys_create; >>>>> + local: >>>>> + *; >>>>> +}; >>>>> diff --git a/src/gallium/targets/omx-tizonia/target.c >>>>> b/src/gallium/targets/omx-tizonia/target.c >>>>> new file mode 100644 >>>>> index 0000000..308e23b >>>>> --- /dev/null >>>>> +++ b/src/gallium/targets/omx-tizonia/target.c >>>>> @@ -0,0 +1,2 @@ >>>>> +#include "target-helpers/drm_helper.h" >>>>> +#include "target-helpers/sw_helper.h" >>>>> >>>> >>>> >>> >>> _______________________________________________ >>> mesa-dev mailing list >>> mesa-dev@lists.freedesktop.org >>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev >>> >>> >> >> > >
_______________________________________________ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
