[Mesa-dev] [PATCH v2 2/4] st/omx_tizonia: Add --enable-omx-tizonia flag and build files
Gurkirpal Singh
gurkirpal204 at gmail.com
Mon Aug 14 16:25:31 UTC 2017
On Mon, Aug 14, 2017 at 8:55 PM, Leo Liu <leo.liu at 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.liu at 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 <gurkirpal204 at gmail.com>
>> wrote:
>>
>>>
>>>
>>> On Sun, Aug 13, 2017 at 8:47 AM, Leo Liu <leo.liu at 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 <gurkirpal204 at gmail.com>
>>>>> Reviewed-and-Tested-by: Julien Isorce <julien.iso... at 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 at lists.freedesktop.org
>>> https://lists.freedesktop.org/mailman/listinfo/mesa-dev
>>>
>>>
>>
>>
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20170814/5963198e/attachment-0001.html>
More information about the mesa-dev
mailing list