[Mesa-dev] [PATCH mesa] omx: always define ENABLE_ST_OMX_{BELLAGIO, TIZONIA}

[Dylan Baker]

Quoting Eric Engestrom (2018-03-12 11:05:51)
> On Monday, 2018-03-12 10:19:49 -0700, Dylan Baker wrote:
> > Quoting Eric Engestrom (2018-03-12 07:33:27)
> > > We're trying to be -Wundef clean so that we can turn it on (and
> > > eventually make it an error).
> > > 
> > > Note that the OMX code already used `#if ENABLE_ST_OMX_BELLAGIO` instead
> > > of #ifdef; I could've changed these, but the point of -Wundef is to
> > > catch typos, so we might as well make the change the right way.
> > > 
> > > Fixes: 83d4a5d5aea5a8a05be2 "st/omx/tizonia: Add H.264 decoder"
> > > Fixes: b2f2236dc565dd1460f0 "st/omx/tizonia: Add H.264 encoder"
> > > Fixes: c62cf1f165919bc74296 "st/omx/tizonia/h264d: Add EGLImage support"
> > > Cc: Gurkirpal Singh <gurkirpal204 at gmail.com>
> > > Signed-off-by: Eric Engestrom <eric.engestrom at imgtec.com>
> > > ---
> > > The meson hunk doesn't look pretty at all, but I'm planning on replacing
> > > all the `pre_args` with a configuration_data(), which will allow to
> > > simplify a lot of this #defines code.
> > > ---
> > >  configure.ac |  4 ++++
> > >  meson.build  | 11 +++++++++--
> > >  2 files changed, 13 insertions(+), 2 deletions(-)
> > > 
> > > diff --git a/configure.ac b/configure.ac
> > > index 1553ce99da44bca4e826..6de4ceb2fb715505120e 100644
> > > --- a/configure.ac
> > > +++ b/configure.ac
> > > @@ -2281,6 +2281,8 @@ if test "x$enable_omx_bellagio" = xyes; then
> > >      PKG_CHECK_MODULES([OMX_BELLAGIO], [libomxil-bellagio >= $LIBOMXIL_BELLAGIO_REQUIRED])
> > >      gallium_st="$gallium_st omx_bellagio"
> > >      AC_DEFINE([ENABLE_ST_OMX_BELLAGIO], 1, [Use Bellagio for OMX IL])
> > > +else
> > > +    AC_DEFINE([ENABLE_ST_OMX_BELLAGIO], 0)
> > >  fi
> > >  AM_CONDITIONAL(HAVE_ST_OMX_BELLAGIO, test "x$enable_omx_bellagio" = xyes)
> > >  
> > > @@ -2294,6 +2296,8 @@ if test "x$enable_omx_tizonia" = xyes; then
> > >                         libtizplatform >= $LIBOMXIL_TIZONIA_REQUIRED])
> > >      gallium_st="$gallium_st omx_tizonia"
> > >      AC_DEFINE([ENABLE_ST_OMX_TIZONIA], 1, [Use Tizoina for OMX IL])
> > > +else
> > > +    AC_DEFINE([ENABLE_ST_OMX_TIZONIA], 0)
> > >  fi
> > >  AM_CONDITIONAL(HAVE_ST_OMX_TIZONIA, test "x$enable_omx_tizonia" = xyes)
> > >  
> > > diff --git a/meson.build b/meson.build
> > > index b6e9692f192c528520e7..b9f7cd2aff5fc49e0d93 100644
> > > --- a/meson.build
> > > +++ b/meson.build
> > > @@ -504,7 +504,7 @@ if with_gallium_omx == 'bellagio' or with_gallium_omx == 'auto'
> > >      'libomxil-bellagio', required : with_gallium_omx == 'bellagio'
> > >    )
> > >    if dep_omx.found()
> > > -    pre_args += '-DENABLE_ST_OMX_BELLAGIO'
> > > +    pre_args += '-DENABLE_ST_OMX_BELLAGIO=1'
> > >      with_gallium_omx = 'bellagio'
> > >    endif
> > >  endif
> > > @@ -525,7 +525,7 @@ if with_gallium_omx == 'tizonia' or with_gallium_omx == 'auto'
> > >        dependency('tizilheaders', required : with_gallium_omx == 'tizonia'),
> > >      ]
> > >      if dep_omx.found() and dep_omx_other[0].found() and dep_omx_other[1].found()
> > > -      pre_args += '-DENABLE_ST_OMX_TIZONIA'
> > > +      pre_args += '-DENABLE_ST_OMX_TIZONIA=1'
> > >        with_gallium_omx = 'tizonia'
> > >      else
> > >        with_gallium_omx = 'disabled'
> > > @@ -533,6 +533,13 @@ if with_gallium_omx == 'tizonia' or with_gallium_omx == 'auto'
> > >    endif
> > >  endif
> > >  
> > > +if with_gallium_omx != 'bellagio'
> > > +  pre_args += '-DENABLE_ST_OMX_BELLAGIO=0'
> > > +endif
> > > +if with_gallium_omx != 'tizonia'
> > > +  pre_args += '-DENABLE_ST_OMX_TIZONIA=0'
> > > +endif
> > > +
> > 
> > This is fine as-is, but if you wanted to clean it up a little, you could do
> > something like:
> > 
> > pre_args += [
> >   '-DENABLE_ST_OMX_BELLAGIO=' + with_gallium_omx == 'bellagio ? '1' : '0',
> >   '-DENABLE_ST_OMX_TIZONIA=' + with_gallium_omx == 'tizonia ? '1' : '0',
> > ]
> 
> That's what I was looking for but too tired to figure out :]
> Thanks, I'll send a v2 with that tomorrow!

Yup!

I haven't tested this, and I have noticed some problems with using the ternary
construct in meson (there's some problems with the parser), so this may not
actually work.

Dylan
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 228 bytes
Desc: signature
URL: <https://lists.freedesktop.org/archives/mesa-dev/attachments/20180312/524915c2/attachment.sig>