[Mesa-dev] [PATCH mesa] omx: always define ENABLE_ST_OMX_{BELLAGIO, TIZONIA}
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
Signed-off-by: Eric Engestrom
---
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
+
if with_gallium_omx != 'disabled'
omx_drivers_path = get_option('omx-libs-path')
# Figure out where to put the omx driver.
--
Cheers,
Eric
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] omx: always define ENABLE_ST_OMX_{BELLAGIO, TIZONIA}
On 12 March 2018 at 14:33, Eric Engestrom wrote: > 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 > Signed-off-by: Eric Engestrom > --- > 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. The meson hunk doesn't look that bad IMHO. Reviewed-by: Emil Velikov Thanks Emil P.S. The re-spinned upstream gtest patches, seems to have fallen through the cracks again :-\ ___ mesa-dev mailing list mesa-dev@lists.freedesktop.org https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] omx: always define ENABLE_ST_OMX_{BELLAGIO, TIZONIA}
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
> Signed-off-by: Eric Engestrom
> ---
> 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',
]
and take the pre_args out of the block above altogether.
either way,
Reviewed-by: Dylan Baker
signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] omx: always define ENABLE_ST_OMX_{BELLAGIO, TIZONIA}
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
> > Signed-off-by: Eric Engestrom
> > ---
> > 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!
>
> and take the pre_args out of the block above altogether.
>
> either way,
>
> Reviewed-by: Dylan Baker
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
Re: [Mesa-dev] [PATCH mesa] omx: always define ENABLE_ST_OMX_{BELLAGIO, TIZONIA}
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
> > > Signed-off-by: Eric Engestrom
> > > ---
> > > 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
signature.asc
Description: signature
___
mesa-dev mailing list
mesa-dev@lists.freedesktop.org
https://lists.freedesktop.org/mailman/listinfo/mesa-dev
