[FFmpeg-devel] [FFmpeg-cvslog] lavfi: add opencl tonemap filter
Song, Ruiling
ruiling.song at intel.com
Tue Jun 26 07:22:42 EEST 2018
> -----Original Message-----
> From: ffmpeg-devel [mailto:ffmpeg-devel-bounces at ffmpeg.org] On Behalf Of
> Michael Niedermayer
> Sent: Friday, June 22, 2018 2:32 AM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [FFmpeg-cvslog] lavfi: add opencl tonemap filter
>
> On Thu, Jun 21, 2018 at 12:23:26AM +0000, Ruiling Song wrote:
> > ffmpeg | branch: master | Ruiling Song <ruiling.song at intel.com> | Tue Jun 19
> 09:57:31 2018 +0800| [8b8b0e2cd26cf1f522c630859fcbcc62b6493fb9] |
> committer: Mark Thompson
> >
> > lavfi: add opencl tonemap filter
> >
> > This filter does HDR(HDR10/HLG) to SDR conversion with tone-mapping.
> >
> > An example command to use this filter with vaapi codecs:
> > FFMPEG -init_hw_device vaapi=va:/dev/dri/renderD128 -init_hw_device \
> > opencl=ocl at va -hwaccel vaapi -hwaccel_device va -hwaccel_output_format \
> > vaapi -i INPUT -filter_hw_device ocl -filter_complex \
> > '[0:v]hwmap,tonemap_opencl=t=bt2020:tonemap=linear:format=p010[x1]; \
> > [x1]hwmap=derive_device=vaapi:reverse=1' -c:v hevc_vaapi -profile 2
> OUTPUT
> >
> > Signed-off-by: Ruiling Song <ruiling.song at intel.com>
> >
> > >
> http://git.videolan.org/gitweb.cgi/ffmpeg.git/?a=commit;h=8b8b0e2cd26cf1f5
> 22c630859fcbcc62b6493fb9
> > ---
> >
> > configure | 1 +
> > libavfilter/Makefile | 2 +
> > libavfilter/allfilters.c | 1 +
> > libavfilter/colorspace.c | 90 +++++
> > libavfilter/colorspace.h | 41 +++
> > libavfilter/opencl/colorspace_common.cl | 220 +++++++++++
> > libavfilter/opencl/tonemap.cl | 272 ++++++++++++++
> > libavfilter/opencl_source.h | 2 +
> > libavfilter/vf_tonemap_opencl.c | 624
> ++++++++++++++++++++++++++++++++
> > 9 files changed, 1253 insertions(+)
> >
> > diff --git a/configure b/configure
> > index 8ca258691d..6ad5ce8eaf 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3412,6 +3412,7 @@ tinterlace_filter_deps="gpl"
> > tinterlace_merge_test_deps="tinterlace_filter"
> > tinterlace_pad_test_deps="tinterlace_filter"
> > tonemap_filter_deps="const_nan"
> > +tonemap_opencl_filter_deps="opencl const_nan"
> > unsharp_opencl_filter_deps="opencl"
> > uspp_filter_deps="gpl avcodec"
> > vaguedenoiser_filter_deps="gpl"
> > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > index 552499558d..589682f353 100644
> > --- a/libavfilter/Makefile
> > +++ b/libavfilter/Makefile
> > @@ -358,6 +358,8 @@ OBJS-$(CONFIG_TINTERLACE_FILTER) +=
> vf_tinterlace.o
> > OBJS-$(CONFIG_TLUT2_FILTER) += vf_lut2.o framesync.o
> > OBJS-$(CONFIG_TMIX_FILTER) += vf_mix.o framesync.o
> > OBJS-$(CONFIG_TONEMAP_FILTER) += vf_tonemap.o
> > +OBJS-$(CONFIG_TONEMAP_OPENCL_FILTER) += vf_tonemap_opencl.o
> colorspace.o opencl.o \
> > + opencl/tonemap.o opencl/colorspace_common.o
> > OBJS-$(CONFIG_TRANSPOSE_FILTER) += vf_transpose.o
> > OBJS-$(CONFIG_TRIM_FILTER) += trim.o
> > OBJS-$(CONFIG_UNPREMULTIPLY_FILTER) += vf_premultiply.o
> framesync.o
> > diff --git a/libavfilter/allfilters.c b/libavfilter/allfilters.c
> > index 2b44626028..e07fe67ec5 100644
> > --- a/libavfilter/allfilters.c
> > +++ b/libavfilter/allfilters.c
> > @@ -346,6 +346,7 @@ extern AVFilter ff_vf_tinterlace;
> > extern AVFilter ff_vf_tlut2;
> > extern AVFilter ff_vf_tmix;
> > extern AVFilter ff_vf_tonemap;
> > +extern AVFilter ff_vf_tonemap_opencl;
> > extern AVFilter ff_vf_transpose;
> > extern AVFilter ff_vf_trim;
> > extern AVFilter ff_vf_unpremultiply;
> > diff --git a/libavfilter/colorspace.c b/libavfilter/colorspace.c
> > new file mode 100644
> > index 0000000000..7fd7bdf0d9
> > --- /dev/null
> > +++ b/libavfilter/colorspace.c
> > @@ -0,0 +1,90 @@
> > +/*
> > + * Copyright (c) 2016 Ronald S. Bultje <rsbultje at gmail.com>
> > + * This file is part of FFmpeg.
> > + *
> > + * FFmpeg is free software; you can redistribute it and/or
> > + * modify it under the terms of the GNU Lesser General Public
> > + * License as published by the Free Software Foundation; either
> > + * version 2.1 of the License, or (at your option) any later version.
> > + *
> > + * FFmpeg is distributed in the hope that it will be useful,
> > + * but WITHOUT ANY WARRANTY; without even the implied warranty of
> > + * MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU
> > + * Lesser General Public License for more details.
> > + *
> > + * You should have received a copy of the GNU Lesser General Public
> > + * License along with FFmpeg; if not, write to the Free Software
> > + * Foundation, Inc., 51 Franklin Street, Fifth Floor, Boston, MA 02110-1301
> USA
> > + */
> > +
> > +#include "colorspace.h"
> > +
> > +
> > +void invert_matrix3x3(const double in[3][3], double out[3][3])
>
> this (and others) need some (ff_) prefix to not pollute namespace
Sounds reasonable. Mark already sent a patch to fix.
>
> [...]
> > +/*
> > + * see e.g.
> http://www.brucelindbloom.com/index.html?Eqn_RGB_XYZ_Matrix.html
> > + */
> > +void fill_rgb2xyz_table(const struct PrimaryCoefficients *coeffs,
> > + const struct WhitepointCoefficients *wp,
> > + double rgb2xyz[3][3])
> > +{
> > + double i[3][3], sr, sg, sb, zw;
> > +
> > + rgb2xyz[0][0] = coeffs->xr / coeffs->yr;
> > + rgb2xyz[0][1] = coeffs->xg / coeffs->yg;
> > + rgb2xyz[0][2] = coeffs->xb / coeffs->yb;
> > + rgb2xyz[1][0] = rgb2xyz[1][1] = rgb2xyz[1][2] = 1.0;
> > + rgb2xyz[2][0] = (1.0 - coeffs->xr - coeffs->yr) / coeffs->yr;
> > + rgb2xyz[2][1] = (1.0 - coeffs->xg - coeffs->yg) / coeffs->yg;
> > + rgb2xyz[2][2] = (1.0 - coeffs->xb - coeffs->yb) / coeffs->yb;
>
> > + invert_matrix3x3(rgb2xyz, i);
>
> gcc produces a warning for this:
>
> libavfilter/colorspace.c: In function ‘fill_rgb2xyz_table’:
> libavfilter/colorspace.c:76:5: warning: passing argument 1 of ‘invert_matrix3x3’
> from incompatible pointer type [enabled by default]
I think it is complaining about "implicit casting from double [*][3] to 'const double [*][3]'?
My test with gcc 5.4 on Ubuntu does not complain about this. I am not sure if gcc 5.4 has changed default behavior.
It hits a limitation in C, which does not allow mixed qualification in assignments for indirect pointers or two dimensional array.
There are some discussions at [1], you can read through the top-rated answer for more information.
Basically I think we have three ways to handle it:
a.) ignore the warning.
b.) add explicit type cast at all call sites, like invert_matrix3x3((const double [*][3])rgb2xyz, i);
c.) remove the const modifiers in function definition.
Personally I don't like b.) as it will obviously make code messy. I am not sure which one do you prefer?
Thanks!
Ruiling
[1] https://stackoverflow.com/questions/4573349/strange-warning-in-a-c-function-const-multidimensional-array-argument
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> it is not once nor twice but times without number that the same ideas make
> their appearance in the world. -- Aristotle
More information about the ffmpeg-devel
mailing list