[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