subject:"Re\: \[FFmpeg\-devel\] \[PATCH\] lavfi\: add opencl tonemap filter."

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-06-04 Thread Song, Ruiling



> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Mark Thompson
> Sent: Monday, June 4, 2018 7:20 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.
> 
> On 29/05/18 06:54, Ruiling Song wrote:
> > 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@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
> >
> > v2:
> > add peak detection.
> >
> > Signed-off-by: Ruiling Song 
> > ---
> >  configure  |   1 +
> >  libavfilter/Makefile   |   2 +
> >  libavfilter/allfilters.c   |   1 +
> >  libavfilter/colorspace_basic.c |  89 +
> >  libavfilter/colorspace_basic.h |  40 ++
> >  libavfilter/opencl/colorspace_basic.cl | 187 ++
> >  libavfilter/opencl/tonemap.cl  | 278 ++
> >  libavfilter/opencl_source.h|   2 +
> >  libavfilter/vf_tonemap_opencl.c| 655
> +
> >  9 files changed, 1255 insertions(+)
> >  create mode 100644 libavfilter/colorspace_basic.c
> >  create mode 100644 libavfilter/colorspace_basic.h
> >  create mode 100644 libavfilter/opencl/colorspace_basic.cl
> >  create mode 100644 libavfilter/opencl/tonemap.cl
> >  create mode 100644 libavfilter/vf_tonemap_opencl.c
> 
> This segfaults when run on CPU implementations (both AMD and Intel on
> Windows) - can you check that?  Maybe an out-of-bounds memory reference
I am setting up the windows environment to check it, but could you show me how 
do you test it? So I can reproduce it.

> which doesn't get noticed on a GPU.  (Apologies for the terrible report - I 
> can
> only see it on opaque proprietary drivers, where it dies on some internal 
> thread
> with no information at all.  The filter unfortunately can't run on pocl 
> because of
> lack of R/RG support there.)
Already documented in TODO list.

> 
> Still not sure why it fails on Mali (it doesn't feel like it uses a lot of 
> memory so
> I'm not sure what's going wrong), but it does work well on AMD on Windows.
> 
> What set of implementations have you tested on?
I have only tested Beignet + vaapi use-case.

> 
> > diff --git a/configure b/configure
> > index e52f8f8..ee3586b 100755
> > --- a/configure
> > +++ b/configure
> > @@ -3401,6 +3401,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"
> >  unsharp_opencl_filter_deps="opencl"
> >  uspp_filter_deps="gpl avcodec"
> >  vaguedenoiser_filter_deps="gpl"
> > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > index c68ef05..0915656 100644
> > --- a/libavfilter/Makefile
> > +++ b/libavfilter/Makefile
> > @@ -352,6 +352,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_basic.o opencl.o \
> > +opencl/tonemap.o 
> > opencl/colorspace_basic.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 b44093d..6873bab 100644
> > --- a/libavfilter/allfilters.c
> > +++ b/libavfilter/allfilters.c
> > @@ -343,6 +343,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

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-06-03 Thread Mark Thompson

On 29/05/18 06:54, Ruiling Song wrote:
> 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@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
> 
> v2:
> add peak detection.
> 
> Signed-off-by: Ruiling Song 
> ---
>  configure  |   1 +
>  libavfilter/Makefile   |   2 +
>  libavfilter/allfilters.c   |   1 +
>  libavfilter/colorspace_basic.c |  89 +
>  libavfilter/colorspace_basic.h |  40 ++
>  libavfilter/opencl/colorspace_basic.cl | 187 ++
>  libavfilter/opencl/tonemap.cl  | 278 ++
>  libavfilter/opencl_source.h|   2 +
>  libavfilter/vf_tonemap_opencl.c| 655 
> +
>  9 files changed, 1255 insertions(+)
>  create mode 100644 libavfilter/colorspace_basic.c
>  create mode 100644 libavfilter/colorspace_basic.h
>  create mode 100644 libavfilter/opencl/colorspace_basic.cl
>  create mode 100644 libavfilter/opencl/tonemap.cl
>  create mode 100644 libavfilter/vf_tonemap_opencl.c

This segfaults when run on CPU implementations (both AMD and Intel on Windows) 
- can you check that?  Maybe an out-of-bounds memory reference which doesn't 
get noticed on a GPU.  (Apologies for the terrible report - I can only see it 
on opaque proprietary drivers, where it dies on some internal thread with no 
information at all.  The filter unfortunately can't run on pocl because of lack 
of R/RG support there.)

Still not sure why it fails on Mali (it doesn't feel like it uses a lot of 
memory so I'm not sure what's going wrong), but it does work well on AMD on 
Windows.

What set of implementations have you tested on?

> diff --git a/configure b/configure
> index e52f8f8..ee3586b 100755
> --- a/configure
> +++ b/configure
> @@ -3401,6 +3401,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"
>  unsharp_opencl_filter_deps="opencl"
>  uspp_filter_deps="gpl avcodec"
>  vaguedenoiser_filter_deps="gpl"
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index c68ef05..0915656 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -352,6 +352,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_basic.o opencl.o \
> +opencl/tonemap.o 
> opencl/colorspace_basic.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 b44093d..6873bab 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -343,6 +343,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_basic.c b/libavfilter/colorspace_basic.c
> new file mode 100644
> index 000..93f9f08
> --- /dev/null
> +++ b/libavfilter/colorspace_basic.c

The name of this file feels strange to me.  It's common parts used by 
colorspace-related filters, so maybe just colorspace.c?

> @@ -0,0 +1,89 @@
> +/*
> + * 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_basic.h"
> +

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-31 Thread Song, Ruiling



> -Original Message-
> From: Song, Ruiling
> Sent: Tuesday, May 29, 2018 4:47 PM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: RE: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.
> 
> 
> 
> > -Original Message-
> > From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On
> Behalf
> > Of myp...@gmail.com
> > Sent: Tuesday, May 29, 2018 3:40 PM
> > To: FFmpeg development discussions and patches  > de...@ffmpeg.org>
> > Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.
> >
> > 2018-05-29 13:54 GMT+08:00 Ruiling Song :
> > > 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@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
> > >
> > > v2:
> > > add peak detection.
> > >
> > > Signed-off-by: Ruiling Song 
> > > +static int tonemap_opencl_config_output(AVFilterLink *outlink)
> > > +{
> > > +AVFilterContext *avctx = outlink->src;
> > > +TonemapOpenCLContext *s = avctx->priv;
> > > +int ret;
> > > +if (s->format == AV_PIX_FMT_NONE)
> > > +av_log(avctx, AV_LOG_WARNING, "format not set, use default
> > format NV12\n");
> >  I think we can give a default format with AV_PIX_FMT_NV12 in
> > tonemap_opencl_options[] for this case
> > and I think now we only support NV12/P010 output in current implement.
> Sounds good.
Hi Jun,

I realized that if I default it to NV12 in the table, I would have no chance to 
catch whether user
already passed in a format or not. In fact, I still prefer user explicitly 
passed in the format that they want (10bit or 8bit).
If user did not choose a format, I would generate a warning message that 
default was used.
Does this make sense?

Thanks!
Ruiling

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-29 Thread Niklas Haas

I see no obvious issues with the algorithm. (Though I haven't tested it)

So "LGTM"

On Tue, 29 May 2018 13:54:27 +0800, Ruiling Song  wrote:
> 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@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
> 
> v2:
> add peak detection.
> 
> Signed-off-by: Ruiling Song 
> ---
>  configure  |   1 +
>  libavfilter/Makefile   |   2 +
>  libavfilter/allfilters.c   |   1 +
>  libavfilter/colorspace_basic.c |  89 +
>  libavfilter/colorspace_basic.h |  40 ++
>  libavfilter/opencl/colorspace_basic.cl | 187 ++
>  libavfilter/opencl/tonemap.cl  | 278 ++
>  libavfilter/opencl_source.h|   2 +
>  libavfilter/vf_tonemap_opencl.c| 655 
> +
>  9 files changed, 1255 insertions(+)
>  create mode 100644 libavfilter/colorspace_basic.c
>  create mode 100644 libavfilter/colorspace_basic.h
>  create mode 100644 libavfilter/opencl/colorspace_basic.cl
>  create mode 100644 libavfilter/opencl/tonemap.cl
>  create mode 100644 libavfilter/vf_tonemap_opencl.c
> 
> diff --git a/configure b/configure
> index e52f8f8..ee3586b 100755
> --- a/configure
> +++ b/configure
> @@ -3401,6 +3401,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"
>  unsharp_opencl_filter_deps="opencl"
>  uspp_filter_deps="gpl avcodec"
>  vaguedenoiser_filter_deps="gpl"
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index c68ef05..0915656 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -352,6 +352,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_basic.o opencl.o \
> +opencl/tonemap.o 
> opencl/colorspace_basic.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 b44093d..6873bab 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -343,6 +343,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_basic.c b/libavfilter/colorspace_basic.c
> new file mode 100644
> index 000..93f9f08
> --- /dev/null
> +++ b/libavfilter/colorspace_basic.c
> @@ -0,0 +1,89 @@
> +/*
> + * 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_basic.h"
> +
> +
> +void invert_matrix3x3(const double in[3][3], double out[3][3])
> +{
> +double m00 = in[0][0], m01 = in[0][1], m02 = in[0][2],
> +   m10 = in[1][0], m11 = in[1][1], m12 = in[1][2],
> +   m20 = in[2][0], m21 = in[2][1], m22 = in[2][2];
> +int i, j;
> +double det;
> +
> +out[0][0] =  (m11 * m22 - m21 * m12);
> +out[0][1] = -(m01 * m22 - m21 * m02);
> +out[0][2] =  (m01 * m12 - m11 * m02);
> +out[1][0] = -(m10 * m22 - m20 * m12);
> +out[1][1] =  (m00 * m22 - m20 * m02);
> +out[1][2] = -(m00 * m12 - m10 * m02);
> +out[2][0] =  (m10 * m21 - m20 * m11);
> +out[2][1] = -(m00 * m21 - m20 *

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-29 Thread Song, Ruiling



> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of myp...@gmail.com
> Sent: Tuesday, May 29, 2018 3:40 PM
> To: FFmpeg development discussions and patches  de...@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.
> 
> 2018-05-29 13:54 GMT+08:00 Ruiling Song :
> > 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@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
> >
> > v2:
> > add peak detection.
> >
> > Signed-off-by: Ruiling Song 
> > +static int tonemap_opencl_config_output(AVFilterLink *outlink)
> > +{
> > +AVFilterContext *avctx = outlink->src;
> > +TonemapOpenCLContext *s = avctx->priv;
> > +int ret;
> > +if (s->format == AV_PIX_FMT_NONE)
> > +av_log(avctx, AV_LOG_WARNING, "format not set, use default
> format NV12\n");
>  I think we can give a default format with AV_PIX_FMT_NV12 in
> tonemap_opencl_options[] for this case
> and I think now we only support NV12/P010 output in current implement.
Sounds good.

> > +{ "format","output pixel format", OFFSET(format),
> AV_OPT_TYPE_PIXEL_FMT, {.i64 = AV_PIX_FMT_NONE},
> AV_PIX_FMT_NONE, AV_PIX_FMT_GBRAP12LE, FLAGS, "fmt" },
> Missing the sub-option nv12 and p010 ?
Seems like using AV_OPT_TYPE_PIXEL_FMT, the framework parsed the user format 
argument correctly.
So I think no need to add sub-options?

Thanks!
Ruiling
> > +{ "peak",  "signal peak override", OFFSET(peak),
> AV_OPT_TYPE_DOUBLE, {.dbl = 0}, 0, DBL_MAX, FLAGS },
> > +{ "param", "tonemap parameter",   OFFSET(param),
> AV_OPT_TYPE_DOUBLE, {.dbl = NAN}, DBL_MIN, DBL_MAX, FLAGS },
> > +{ "desat", "desaturation parameter",   OFFSET(desat_param),
> AV_OPT_TYPE_DOUBLE, {.dbl = 0.5}, 0, DBL_MAX, FLAGS },
> > +{ "threshold", "scene detection threshold",   OFFSET(scene_threshold),
> AV_OPT_TYPE_DOUBLE, {.dbl = 0.2}, 0, DBL_MAX, FLAGS },
> > +{ NULL }
> > +};
> > +
> > +AVFILTER_DEFINE_CLASS(tonemap_opencl);
> > +
> > +static const AVFilterPad tonemap_opencl_inputs[] = {
> > +{
> > +.name = "default",
> > +.type = AVMEDIA_TYPE_VIDEO,
> > +.filter_frame = _opencl_filter_frame,
> > +.config_props = _opencl_filter_config_input,
> > +},
> > +{ NULL }
> > +};
> > +
> > +static const AVFilterPad tonemap_opencl_outputs[] = {
> > +{
> > +.name = "default",
> > +.type = AVMEDIA_TYPE_VIDEO,
> > +.config_props = _opencl_config_output,
> > +},
> > +{ NULL }
> > +};
> > +
> > +AVFilter ff_vf_tonemap_opencl = {
> > +.name   = "tonemap_opencl",
> > +.description= NULL_IF_CONFIG_SMALL("perform HDR to SDR
> conversion with tonemapping"),
> > +.priv_size  = sizeof(TonemapOpenCLContext),
> > +.priv_class = _opencl_class,
> > +.init   = _opencl_filter_init,
> > +.uninit = _opencl_uninit,
> > +.query_formats  = _opencl_filter_query_formats,
> > +.inputs = tonemap_opencl_inputs,
> > +.outputs= tonemap_opencl_outputs,
> > +.flags_internal = FF_FILTER_FLAG_HWFRAME_AWARE,
> > +};
> > --
> > 2.7.4
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> ___
> ffmpeg-devel mailing list
> ffmpeg-devel@ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-29 Thread myp...@gmail.com

2018-05-29 13:54 GMT+08:00 Ruiling Song :
> 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@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
>
> v2:
> add peak detection.
>
> Signed-off-by: Ruiling Song 
> ---
>  configure  |   1 +
>  libavfilter/Makefile   |   2 +
>  libavfilter/allfilters.c   |   1 +
>  libavfilter/colorspace_basic.c |  89 +
>  libavfilter/colorspace_basic.h |  40 ++
>  libavfilter/opencl/colorspace_basic.cl | 187 ++
>  libavfilter/opencl/tonemap.cl  | 278 ++
>  libavfilter/opencl_source.h|   2 +
>  libavfilter/vf_tonemap_opencl.c| 655 
> +
>  9 files changed, 1255 insertions(+)
>  create mode 100644 libavfilter/colorspace_basic.c
>  create mode 100644 libavfilter/colorspace_basic.h
>  create mode 100644 libavfilter/opencl/colorspace_basic.cl
>  create mode 100644 libavfilter/opencl/tonemap.cl
>  create mode 100644 libavfilter/vf_tonemap_opencl.c
>
> diff --git a/configure b/configure
> index e52f8f8..ee3586b 100755
> --- a/configure
> +++ b/configure
> @@ -3401,6 +3401,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"
>  unsharp_opencl_filter_deps="opencl"
>  uspp_filter_deps="gpl avcodec"
>  vaguedenoiser_filter_deps="gpl"
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index c68ef05..0915656 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -352,6 +352,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_basic.o opencl.o \
> +opencl/tonemap.o 
> opencl/colorspace_basic.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 b44093d..6873bab 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -343,6 +343,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_basic.c b/libavfilter/colorspace_basic.c
> new file mode 100644
> index 000..93f9f08
> --- /dev/null
> +++ b/libavfilter/colorspace_basic.c
> @@ -0,0 +1,89 @@
> +/*
> + * 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_basic.h"
> +
> +
> +void invert_matrix3x3(const double in[3][3], double out[3][3])
> +{
> +double m00 = in[0][0], m01 = in[0][1], m02 = in[0][2],
> +   m10 = in[1][0], m11 = in[1][1], m12 = in[1][2],
> +   m20 = in[2][0], m21 = in[2][1], m22 = in[2][2];
> +int i, j;
> +double det;
> +
> +out[0][0] =  (m11 * m22 - m21 * m12);
> +out[0][1] = -(m01 * m22 - m21 * m02);
> +out[0][2] =  (m01 * m12 - m11 * m02);
> +out[1][0] = -(m10 * m22 - m20 * m12);
> +out[1][1] =  (m00 * m22 - m20 * m02);
> +out[1][2] = -(m00 * m12 - m10 * m02);
> +out[2][0] =  (m10 * m21 - m20 * m11);
> +out[2][1] = -(m00 * m21 - m20 * m01);
> +out[2][2] =  (m00 * m11 - m10 * m01);
> +
> +det = m00 * out[0][0] + m10 * out[0][1] +

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-24 Thread Niklas Haas

On Thu, 24 May 2018 08:58:22 +, "Song, Ruiling"  
wrote:
> Where comes the "1000 cd/m² is the reference display peak"? seems no clear 
> statement in BT2100?

The concept of there being a "standard" 1000 cd/m² display is introduced
in multiple places. Refer to Table 5 of ITU-R BT.2100, specifically the
section called "HLG Reference EOTF, in particular this definition:

> γ = 1.2 at the nominal display peak luminance of 1 000 cd/m². [5d, 5e, 5f]

And also the Note 5e below it, which explains how to adjust the gamma
if you are displaying on a display with a peak luminance that is
different from 1000 cd/m².

This is pretty much as close to saying that a reference HLG display
should have a peak of 1000 cd/m² as you can get without explicitly
saying it, since that value is essentially the assumption they
hard-coded into their formula.

In addition to this, the ITU-R further reinforces this concept heavily
throughout their ITU-R Report BT.2390, which includes e.g. such
sentences:

> In order to determine the appropriate system gamma for a 1 000 cd/m²
> reference display, NHK conducted a series of experiments with an
> indoor test scene.

So the concept of a “HLG reference display” is not something I made up.
(Incidentally, 1000 cd/m² is also the value you hard-code in your OOTF)

> If that is true, my code is wrong to detect peak of untagged source.
> if (!peak)
> peak = in->color_trc == AVCOL_TRC_SMPTE2084 ? 100.0f : 12.0f;
> so here I should change it from 12.0f to 10.0f?

Yes, given that you're defaulting the OOTF to 1000 this is definitely a
good idea, otherwise white won't map to white. (Observe that you have
the scaling factor in your ootf_hlg hard-coded as 1000.0f / REFERENCE_WHITE
= 10.0)

> Thanks for point this out. I mis-understand the code in libplacebo, because 
> inverse_ootf() was also called if need_ootf is true, which makes I fail to 
> understand it correctly.

pl_shader_inverse_ootf() is parametrized by the actual OOTF to use.
Observe that the inverse_ootf() definition in libplacebo is a no-op if
the `enum pl_color_light` is given as PL_COLOR_LIGHT_DISPLAY.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-24 Thread Song, Ruiling



> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Niklas Haas
> Sent: Wednesday, May 23, 2018 7:27 PM
> To: Song, Ruiling <ruiling.s...@intel.com>
> Cc: Mark Thompson <s...@jkqxz.net>; FFmpeg development discussions and
> patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.
> 
> > Excellent explanation. I think I get your idea. Will refine the code per 
> > your
> suggestion.
> > But still some question, will people/tools tend to fill in the mastering
> information for HLG video?
> > I currently see no document that recommend to fill the mastering display for
> HLG.
> > I only have one HLG sample download from 4kmedia.org. seems it has no
> mastering metadata.
> > Do you have any more HLG videos that show it will often be filled in?
> > My concern here is will all video players correctly parse the mastering 
> > display
> metadata to decode HLG, or just skip it because most HLG video has no
> metadata?
> 
> I think there's probably going to be three ways to approach this
> situation. Part of the problem is surely the fact that HLG is sort of
> designed to be "implicitly" tone-mapped. That is, the way the HLG
> standard is written, you'd just always encode things so that 12.0 is the
> brightest peak brightness, and a user with a 500 cd/m² peak TV would
> just apply the HLG OOTF tuned for 500 cd/m² on the original signal as
> received from the (e.g. blu-ray) source. Sure, the mastering engineer
> may have used a 1500 cd/m² screen to master it, but since the HLG
> OOTF-OOTF round-trip essentially constitutes a simple form of
> tone-mapping, the overall result on-screen will look more or less
> reasonable. (Certainly more reasonable than e.g. PQ)
> 
> So surely there's the camp of people that believe HLG doesn't need
> mastering metadata and will therefore not include it, because the end
> result without metadata looks more or less good enough. However, I
> disagree with this result. First of all, it prevents color-accurate
> round-trips. The HLG OOTF is inherently color-distorting, so in a
> color-managed workflow with calibrated devices, this methodology will
> not be sufficient to ensure perceptually accurate reproduction. The
> second reason is that as I said, the HLG OOTF-OOTF interaction
> essentially constitutes a simple form of tone-mapping; but we can do
> significantly better. I believe our tone mapping algorithm produces a
> far better result (visually) than applying the HLG OOTF as-is,
> especially when going to an SDR display. (If you're using mpv, you can
> test this by playing a HLG source once with --vf=format:peak=10 and once
> with --vf=format:peak=1. In the latter case, the only tone mapping being
> done is the implicit HLG tone mapping). Not only are HLG sources I've
> found inconsistently encoded, but also I find that the inherent HLG
> tone-mapping tends to over-saturate the signal (similar to the result we
> get if the desaturation strength is 0.0) while also boosting the gamma.
I agree with you. Thanks for detailed explanation.

> 
> So if we subscribe to the idea that we need metadata to do
> color-accurate tone mapping and reproduction, then the question becomes:
> what do we do for un-tagged sources? The obvious approach is to assume a
> (display-referred) signal peak of 10.0 (thus corresponding to a display
> peak of 1000 cd/m², i.e. the HLG reference device). But I think if I was
> to make a HLG release of my own, I would definitely try and include the
> most accurate tagging possible. For example, if we have a clip available
> in both PQ and HLG, I would use the PQ version's mastering metadata for
> HLG as well.
Where comes the "1000 cd/m² is the reference display peak"? seems no clear 
statement in BT2100?
If that is true, my code is wrong to detect peak of untagged source.
if (!peak)
peak = in->color_trc == AVCOL_TRC_SMPTE2084 ? 100.0f : 12.0f;
so here I should change it from 12.0f to 10.0f?
 
> Finally, to put the nail in the coffin of the idea that HLG doesn't need
> metadata, we should realize that the mastering metadata isn't just there
> to help you tone map the brightness levels, it also includes the
> display's gamut capabilities - and for a good reason. When doing
> desaturation in order to fit the BT.2020 signal into a (typically far
> less than BT.2020) display response, knowing the gamut limitations of
> the signal can similarly help users do a far better job than having to
> assume the worst case scenario - for much the same reason that knowing
> the signal's actual peak brightness can help users do a far better job
> tone-mapping than having to as

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-24 Thread Song, Ruiling



> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Mark Thompson
> Sent: Tuesday, May 22, 2018 8:41 PM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.
> 
> On 22/05/18 09:48, Song, Ruiling wrote:
> >> -Original Message-
> >> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf
> Of
> >> Mark Thompson
> >> Sent: Tuesday, May 22, 2018 8:19 AM
> >> To: ffmpeg-devel@ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.
> >>
> >> On 21/05/18 07:50, Ruiling Song wrote:
> >>> 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@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.s...@intel.com>
> >>> ---

> >>> +
> >>> +err = av_frame_copy_props(output, input);
> >>> +if (err < 0)
> >>> +goto fail;
> 
> av_frame_copy_props() copies the side-data which will include the
> mastering/light-level information, but that's no longer valid after 
> tonemapping?
I think so, but I am not sure how to update this information correctly.
Using result peak and result color-space primaries to replace original 
metadata? Sounds ok?

Ruiling

___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-23 Thread Niklas Haas

On Mon, 21 May 2018 14:50:17 +0800, Ruiling Song  wrote:
> +float3 map_one_pixel_rgb(float3 rgb, float peak, float average) {
> +float sig = max(max(rgb.x, max(rgb.y, rgb.z)), 1e-6f);
> +// de-saturate
> +if (desat_param > 0.0f) {
> +float luma = get_luma_dst(rgb);
> +float base = 0.18f * dst_peak;
> +float coeff = max(sig - base, 1e-6f) / max(sig, 1e-6f);
> +coeff = native_powr(coeff, 10.0f / desat_param);
> +rgb = mix(rgb, (float3)luma, (float3)coeff);
> +sig = mix(sig, luma, coeff);
> +}
> +
> +float sig_old = sig;
> +float slope = min(1.0f, sdr_avg / average);
> +sig *= slope;
> +peak *= slope;
> +
> +sig = TONE_FUNC(sig, peak);
> +rgb *= (sig/sig_old);
> +return rgb;

Actually a better way to do this is to swap the order of the `slope`
adjustment  and the desaturation step. This prevents a problematic case
where very bright (badly mastered) sources ended up getting too
aggressively desaturated.

Some care needs to be taken when swapping the order in order to scale
the multiplication in the correct way. This should work:

  float3 map_one_pixel_rgb(float3 rgb, float peak, float average) {
  float sig = max(max(rgb.x, max(rgb.y, rgb.z)), 1e-6f);
  float sig_old = sig;
  float slope = min(1.0f, sdr_avg / average);
  sig *= slope;
  peak *= slope;

  // de-saturate
  if (desat_param > 0.0f) {
  float luma = get_luma_dst(rgb);
  float base = 0.18f * dst_peak;
  float coeff = max(sig - base, 1e-6f) / max(sig, 1e-6f);
  coeff = native_powr(coeff, 10.0f / desat_param);
  rgb = mix(rgb, (float3)luma, (float3)coeff);
  sig = mix(sig, luma * slope, coeff);
  }

  sig = TONE_FUNC(sig, peak);
  rgb *= (sig/sig_old);
  return rgb;

I found out about this while testing some pathological HLG sources
earlier today.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-23 Thread Niklas Haas

> Excellent explanation. I think I get your idea. Will refine the code per your 
> suggestion.
> But still some question, will people/tools tend to fill in the mastering 
> information for HLG video?
> I currently see no document that recommend to fill the mastering display for 
> HLG.
> I only have one HLG sample download from 4kmedia.org. seems it has no 
> mastering metadata.
> Do you have any more HLG videos that show it will often be filled in?
> My concern here is will all video players correctly parse the mastering 
> display metadata to decode HLG, or just skip it because most HLG video has no 
> metadata?

I think there's probably going to be three ways to approach this
situation. Part of the problem is surely the fact that HLG is sort of
designed to be "implicitly" tone-mapped. That is, the way the HLG
standard is written, you'd just always encode things so that 12.0 is the
brightest peak brightness, and a user with a 500 cd/m² peak TV would
just apply the HLG OOTF tuned for 500 cd/m² on the original signal as
received from the (e.g. blu-ray) source. Sure, the mastering engineer
may have used a 1500 cd/m² screen to master it, but since the HLG
OOTF-OOTF round-trip essentially constitutes a simple form of
tone-mapping, the overall result on-screen will look more or less
reasonable. (Certainly more reasonable than e.g. PQ)

So surely there's the camp of people that believe HLG doesn't need
mastering metadata and will therefore not include it, because the end
result without metadata looks more or less good enough. However, I
disagree with this result. First of all, it prevents color-accurate
round-trips. The HLG OOTF is inherently color-distorting, so in a
color-managed workflow with calibrated devices, this methodology will
not be sufficient to ensure perceptually accurate reproduction. The
second reason is that as I said, the HLG OOTF-OOTF interaction
essentially constitutes a simple form of tone-mapping; but we can do
significantly better. I believe our tone mapping algorithm produces a
far better result (visually) than applying the HLG OOTF as-is,
especially when going to an SDR display. (If you're using mpv, you can
test this by playing a HLG source once with --vf=format:peak=10 and once
with --vf=format:peak=1. In the latter case, the only tone mapping being
done is the implicit HLG tone mapping). Not only are HLG sources I've
found inconsistently encoded, but also I find that the inherent HLG
tone-mapping tends to over-saturate the signal (similar to the result we
get if the desaturation strength is 0.0) while also boosting the gamma.

So if we subscribe to the idea that we need metadata to do
color-accurate tone mapping and reproduction, then the question becomes:
what do we do for un-tagged sources? The obvious approach is to assume a
(display-referred) signal peak of 10.0 (thus corresponding to a display
peak of 1000 cd/m², i.e. the HLG reference device). But I think if I was
to make a HLG release of my own, I would definitely try and include the
most accurate tagging possible. For example, if we have a clip available
in both PQ and HLG, I would use the PQ version's mastering metadata for
HLG as well.

Finally, to put the nail in the coffin of the idea that HLG doesn't need
metadata, we should realize that the mastering metadata isn't just there
to help you tone map the brightness levels, it also includes the
display's gamut capabilities - and for a good reason. When doing
desaturation in order to fit the BT.2020 signal into a (typically far
less than BT.2020) display response, knowing the gamut limitations of
the signal can similarly help users do a far better job than having to
assume the worst case scenario - for much the same reason that knowing
the signal's actual peak brightness can help users do a far better job
tone-mapping than having to assume a worst-case peak of 10,000 cd/m².
Indeed, in the best case scenario (your own display's gamut and
brightness capabilities match or exceed the mastering display's), both
of these can just be no-ops.

So if mastering metadata is beneficial at all, then we should also agree
that mastering metadata is beneficial to BT.2020 + HLG sources, simply
for the gamut data alone. The fact that HLG is ill-defined without
knowing the mastering display's brightness is just icing on the cake at
this point.

> As what I do now is tone mapping from HDR to SDR, do you think it is 
> meaningful to add the metadata for SDR video?

The mastering metadata is still useful for the gamut information as
explained. Since you're (most likely) encoding a BT.2020 signal that
doesn't use the full gamut range of BT.2020, even for SDR curves it can
be a good idea to preserve it.

> And looks like using a peak of 100 in inverse_ootf() when tone-mapping to sdr 
> is just ok?

Sure. That won't blow up, but using HLG to store an SDR signal is sort
of wasteful/pointless. Might as well use an actual SDR curve and skip
the inverse_ootf step altogether.

> 
> Thanks again for your

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-22 Thread Song, Ruiling



> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Niklas Haas
> Sent: Tuesday, May 22, 2018 8:54 PM
> To: Song, Ruiling <ruiling.s...@intel.com>
> Cc: Mark Thompson <s...@jkqxz.net>; FFmpeg development discussions and
> patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.
> 
> On Tue, 22 May 2018 08:56:37 +, "Song, Ruiling" <ruiling.s...@intel.com>
> wrote:
> > Yes, your idea sounds reasonable. But it may need much effort to 
> > re-structure
> the code to make it (that would launch two kernels, and we may need a wait
> between them) and evaluate the performance.
> 
> Actually, a brute force solution to solve the missing peak problem would
> be to filter the first frame twice and discard the first result. (After
> that, you only need to filter each frame once, so the overall
> performance characteristic is unchanged for videos)
> 
> That requires minimal code change, and it still allows it to work for
> single-frame video sources. It also prevents an initial flash of the
> wrong brightness level for transcoded videos.
For the single frame video, do you mean still image?
I am not sure whether current OpenCL acceleration well designed for that?
My feeling is that people mainly use OpenCL for video acceleration,
esp. interop with hardware-accelerated codecs. Welcome to correct me on this.

For the very first frame, I think it is not easy to notice a flash.
Because a default peak value was used for the first frame which is 100 for PQ,
we would just get the first frame a little dimmer.

> 
> Also, performnace wise, I'm not sure how this works in OpenCL land, but
> in OpenGL/Vulkan, you'd just need to emit a pipeline barrier. That
> allows the kernels to synchronize without having to stall the pipeline
> by doing a CPU wait. (And, in general, you'd need a pipeline barrier
> even if you *are* running glFinish() afterwards - the pipeline barrier
> isn't just for timing, it's also for flushing the appropriate caches. In
> general, write visibility on storage buffers requires a pipeline
> barrier. Are you sure this is not the case for OpenCL as well?)
I think it again, the two OpenCL kernel launch needs no wait. It is just two 
kernel launched from host.
The performance I said is we need to read the image twice, which is obviously 
not as efficient as read once.
> 
> > Although we are developing offline filter, I think that performance is 
> > still very
> important as well as quality.
> > Given that the current implementation does well for video transcoding. I
> would leave it in my TODO list. Sounds ok?
> 
> ACK. It's not my decision, I'm just offering advice.
> 
> > Are you talking about display-referred HLG? I didn't update frame side 
> > channel
> data.
> > I am not sure when do I need to update it. I thought all HLG should be 
> > scene-
> referred, seems not?
> > Could you tell me more about display-referred HLG?
> 
> There's no such thing as "display-referred HLG". HLG by definition is
> encoded as scene-referred, but the OOTF to convert from scene-referred
> to display-referred is part of the EOTF (also by definition).
> 
> So the HLG EOTF inputs scene-referred and outputs display-referred. When
> you apply the EOTF (including the OOTF) as part of your processing
> chain, you're turning it into a linear light display referred signal.
> The tone mapping then happens on this signal (in display light), and
> then to turn it back to HLG after you're done tone-mapping you apply the
> inverse OOTF + OETF, thus turning it back into scene referred light.
> 
> The HLG OOTF (and therefore the EOTF) is parametrized by the display
> peak. Even though the HLG signal is stored in the range 0.0 - 12.0
> (scene referred), the output range depends on how you tuned the EOTF. If
> you tuned it for the 1000 cd/m^2 reference display, then an input of
> 12.0 will get turned into an output value of 1000 cd/m^2.
> 
> If we then tone-map this to a brightness of 500 cd/m^2, and pass it back
> through the same OOTF, it would get turned into 6.0 rather than the
> 12.0. While this may ultimately reproduce the correct result on-screen
> (assuming the end user of the video file also uses a peak of 1000 cd/m^2
> to decode the file), it's a suboptimal use of the encoding range and
> also not how HLG is designed to operate. (For example, it would affect
> the "SDR backwards compatibility" property of HLG, which is the whole
> reason for the peak-dependent encoding)
> 
> That's why the correct thing to do would be to re-encode the file using
> an inverse OOTF tuned for 500 cd/m², thus taking our tone mapped va

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-22 Thread Niklas Haas

On Tue, 22 May 2018 08:56:37 +, "Song, Ruiling"  
wrote:
> Yes, your idea sounds reasonable. But it may need much effort to re-structure 
> the code to make it (that would launch two kernels, and we may need a wait 
> between them) and evaluate the performance.

Actually, a brute force solution to solve the missing peak problem would
be to filter the first frame twice and discard the first result. (After
that, you only need to filter each frame once, so the overall
performance characteristic is unchanged for videos)

That requires minimal code change, and it still allows it to work for
single-frame video sources. It also prevents an initial flash of the
wrong brightness level for transcoded videos.

Also, performnace wise, I'm not sure how this works in OpenCL land, but
in OpenGL/Vulkan, you'd just need to emit a pipeline barrier. That
allows the kernels to synchronize without having to stall the pipeline
by doing a CPU wait. (And, in general, you'd need a pipeline barrier
even if you *are* running glFinish() afterwards - the pipeline barrier
isn't just for timing, it's also for flushing the appropriate caches. In
general, write visibility on storage buffers requires a pipeline
barrier. Are you sure this is not the case for OpenCL as well?)

> Although we are developing offline filter, I think that performance is still 
> very important as well as quality.
> Given that the current implementation does well for video transcoding. I 
> would leave it in my TODO list. Sounds ok?

ACK. It's not my decision, I'm just offering advice.

> Are you talking about display-referred HLG? I didn't update frame side 
> channel data.
> I am not sure when do I need to update it. I thought all HLG should be 
> scene-referred, seems not?
> Could you tell me more about display-referred HLG?

There's no such thing as "display-referred HLG". HLG by definition is
encoded as scene-referred, but the OOTF to convert from scene-referred
to display-referred is part of the EOTF (also by definition).

So the HLG EOTF inputs scene-referred and outputs display-referred. When
you apply the EOTF (including the OOTF) as part of your processing
chain, you're turning it into a linear light display referred signal.
The tone mapping then happens on this signal (in display light), and
then to turn it back to HLG after you're done tone-mapping you apply the
inverse OOTF + OETF, thus turning it back into scene referred light.

The HLG OOTF (and therefore the EOTF) is parametrized by the display
peak. Even though the HLG signal is stored in the range 0.0 - 12.0
(scene referred), the output range depends on how you tuned the EOTF. If
you tuned it for the 1000 cd/m^2 reference display, then an input of
12.0 will get turned into an output value of 1000 cd/m^2.

If we then tone-map this to a brightness of 500 cd/m^2, and pass it back
through the same OOTF, it would get turned into 6.0 rather than the
12.0. While this may ultimately reproduce the correct result on-screen
(assuming the end user of the video file also uses a peak of 1000 cd/m^2
to decode the file), it's a suboptimal use of the encoding range and
also not how HLG is designed to operate. (For example, it would affect
the "SDR backwards compatibility" property of HLG, which is the whole
reason for the peak-dependent encoding)

That's why the correct thing to do would be to re-encode the file using
an inverse OOTF tuned for 500 cd/m², thus taking our tone mapped value
in question back to the (scene-referred) value of 12.0, and update the
tagged peak to also read 500 cd/m². Now a spec-conforming implementation
of a video player (e.g. mpv or VLC) that plays this file would use the
same tuned EOTF to decode it back to the value of 500 cd/m², thus
ensuring it round trips correctly.

> I don't find anything about it. What metadata in HEVC indicate 
> display-referred?
> Any display-referred HLG video sample?

As mentioned, the HLG EOTF by definition requires transforming to
display-referred space. The mastering display metadata *is* what
describes how this (definitively display-referred) space behaves. So
when decoding HLG, you use the tagged mastering metadata's peak as the
parametrization for the EOTF. (This is what e.g. mpv and VLC do)

For a better explanation of this (admittedly confusing) topic, see Annex
1 of ITU-R Recommendation BT.2100.

Here is a relevant excerpt: http://0x0.st/se7O.png
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-22 Thread Mark Thompson

On 22/05/18 09:48, Song, Ruiling wrote:
>> -Original Message-
>> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
>> Mark Thompson
>> Sent: Tuesday, May 22, 2018 8:19 AM
>> To: ffmpeg-devel@ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.
>>
>> On 21/05/18 07:50, Ruiling Song wrote:
>>> 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@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.s...@intel.com>
>>> ---
>>
>> ...
>>
>>
>> On Mali:
>>
>> $ ./ffmpeg_g -v 55 -y -i ~/test/The\ World\ in\ HDR.mkv -init_hw_device 
>> opencl
>> -filter_hw_device opencl0 -an -vf
>> 'format=p010,hwupload,tonemap_opencl=t=bt2020:tonemap=linear:format=p0
>> 10,hwdownload,format=p010' -c:v libx264 out.mp4
>> ...
>> [tonemap_opencl @ 0x8201d7c0] Filter input: opencl, 3840x2160 (0).
>> [Parsed_tonemap_opencl_2 @ 0x8201d760] Failed to enqueue kernel: -5.
> The error seems map to OpenCL error CL_OUT_OF_RESOURCES. I don't have any 
> idea yet.
> May be some limitation in the driver not queried?
> 
>>
>> That's an RK3288 with a Mali T760, clinfo: <https://0x0.st/se5r.txt>, full 
>> log:
>> <https://0x0.st/se5s.log>.
>>
>> (The Rockchip hardware decoder can do H.265 Main 10, but the output format
>> isn't P010 so it's easier to use VP9 here.)
> Not p010? Then which format? Planar?

It's two-plane like P010, but with the samples packed together to minimise the 
memory use - I think it uses all the bits to give you four (4 x 10 = 40 bits) 
luma samples (or two times two component chroma samples) in each five bytes (5 
x 8 = 40 bits).  This form probably isn't usable directly by anything generic 
like OpenCL without more magic, though Rockchip's KMS and related processing 
code can handle it.

> And I don't quite understand here. What the relationship of format with VP9?

Oh, sorry - that's coming from the mostly-unrelated point that VP9 has much 
better software decode support in libavcodec.  Irrelevant, really - H.265 will 
also work.


>>> ...
>>> +
>>> +// detect peak/average signal of a frame, the algorithm was ported from:
>>> +// libplacebo (https://github.com/haasn/libplacebo)
>>> +struct detection_result
>>> +detect_peak_avg(global uint *util_buf, __local uint *sum_wg,
>>> +float signal, float peak) {
>>> +global uint *avg_buf = util_buf;
>>> +global uint *peak_buf = avg_buf + DETECTION_FRAMES + 1;
>>> +global uint *counter_wg_p = peak_buf + DETECTION_FRAMES + 1;
>>> +global uint *max_total_p = counter_wg_p + 1;
>>> +global uint *avg_total_p = max_total_p + 1;
>>> +global uint *frame_idx_p = avg_total_p + 1;
>>> +global uint *scene_frame_num_p = frame_idx_p + 1;
>>> +
>>> +uint frame_idx = *frame_idx_p;
>>> +uint scene_frame_num = *scene_frame_num_p;
>>> +
>>> +size_t lidx = get_local_id(0);
>>> +size_t lidy = get_local_id(1);
>>> +size_t lsizex = get_local_size(0);
>>> +size_t lsizey = get_local_size(1);
>>> +uint num_wg = get_num_groups(0) * get_num_groups(1);
>>> +size_t group_idx = get_group_id(0);
>>> +size_t group_idy = get_group_id(1);
>>> +struct detection_result r = {peak, sdr_avg};
>>> +*sum_wg = 0;
>>
>> This is technically a data race - maybe set it in only the first workitem?
> When writing same value to it, this may be fine, we should still get correct 
> result.
> But I agree it is better to only ask the first work-item to do the 
> initialization.

C/C++ make it undefined behaviour, so even if when it's benign like this 
(writing the same value) I would prefer to avoid it.

>>> +barrier(CLK_LOCAL_MEM_FENCE);
>>> +
>>> +// update workgroup sum
>>> +atomic_add(sum_wg, (uint)(signal * REFERENCE_WHITE));
>>
>> I think the numbers you're adding together here sum to at most something like
>> 16 * 16 * 100 * 1023?  Can you make sure this can't overflow and add a
>> comment on tha

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-22 Thread Song, Ruiling



> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Niklas Haas
> Sent: Tuesday, May 22, 2018 10:28 AM
> To: ffmpeg-devel@ffmpeg.org
> Cc: Mark Thompson <s...@jkqxz.net>
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.
> 
> On Tue, 22 May 2018 01:18:30 +0100, Mark Thompson <s...@jkqxz.net> wrote:
> > On 21/05/18 07:50, Ruiling Song wrote:
> > > 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@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.s...@intel.com>
> > > ---
> >
> > I assume you're testing with Beignet for this sort of mapping to work?  I 
> > tried it
> with Beignet on Coffee Lake with 10-bit videos and it looks sensible, though 
> it is
> rather hard to tell whether it is in some sense "correct".
> 
> It's also rather hard to define whether it is in some sense "correct".
> The methodology employed here is generally based on ITU-R
> recommendations, however the ITU-R advises multiple possible ways of
> doing tone-mapping. They also highlight their own curve function, which
> we don't use (for performance/simplicity reasons - iirc I gave it a try
> and the result was not visually dissimilar enough from the hable
> function, but my memory could be wrong). There's nothing resembling an
> official "standard" way to tone-map defined by the ITU-R.
> 
> This algorithm is also generally based on the results obtained from the
> "official" ACES implementation of HDR->SDR tone mapping (obtainable
> here: https://github.com/ampas/aces-dev), with the key difference that
> we do chroma-invariant tone mapping whereas hollywood tends to use
> channel-independent tone mapping. I think the latter distorts the colors
> too much for taste and generally results in a worse looking image. The
> only important bit to make chroma-invariant tone mapping work well,
> however, is the need for a good desaturation algorithm. This one is
> based on original research and experimentation. The desaturation
> strength with a parameter of 1.0 is comparable to the one achieved by
> the ACES algorithm, although I pick a lower strength by default (0.5),
> because I found it too damaging for some types of sources (particularly
> bright skies) as a result of the chroma-invariant nature.
> 
> In addition to the desaturation step, the other key innovation which I
> cannot find mentioned in ITU-R literature is the importance of adjusting
> the overall average brightness before tone mapping. I suspect the reason
> this isn't considered by the ITU-R is because the ITU-R assumes that HDR
> sources actually follow their standards, which in practice none seem to
> do. In theory, HDR material isn't supposed to have a significantly
> higher average brightness than SDR material. Apart from the addition of
> isolated super-highlights, nothing should have changed about the image
> appearance. In practice, HDR directors like to point their expensive
> cameras at very bright objects (e.g. the sun) and blind viewers' eyes by
> failing to adjust the brightness during the mastering step. Our
> algorithm compensates for this by essentially "correcting" the bad
> mastering in real-time. [1] Of course, the result here is not as good as
> doing it ahead of time by a human, but unfortunately we don't have a say
> in this matter.
> 
> As long as the implementation is correct, I'd be confident in assuming
> that this produces pleasurable results for all the sources I've thrown
> at it, often even exceeding in quality the "official" SDR-mapped blu-ray
> versions of the same sources on the same scenes. (Partially due to the
> preserved higher color gamut)
> 
> In order to ascertain whether or not the implementation is correct, you
> could compare it to results obtained by latest `mpv` (might need git
> master) or `libplacebo`, both of which implement the same algorithm.
> 
> 
> [1] The algorithm I use in mpv and libplacebo does this with one frame
> of latency, because I don't want to round-trip through an intermediate
> buffer in my processing chain, and there's no other way to communicate
> back the measured fr

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-22 Thread Song, Ruiling



> -Original Message-
> From: ffmpeg-devel [mailto:ffmpeg-devel-boun...@ffmpeg.org] On Behalf Of
> Mark Thompson
> Sent: Tuesday, May 22, 2018 8:19 AM
> To: ffmpeg-devel@ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.
> 
> On 21/05/18 07:50, Ruiling Song wrote:
> > 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@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.s...@intel.com>
> > ---
> 
> I assume you're testing with Beignet for this sort of mapping to work?  I 
> tried it
> with Beignet on Coffee Lake with 10-bit videos and it looks sensible, though 
> it is
> rather hard to tell whether it is in some sense "correct".
> 
> Given a non-P010 input video it fails with build errors when compling the 
> kernels:
> 
> [Parsed_tonemap_opencl_1 @ 0x55b700e51540] Failed to build program: -11.
> [Parsed_tonemap_opencl_1 @ 0x55b700e51540] Build log:
> /home/mrt/video/ffmpeg/opencl/libavfilter/opencl/colorspace_basic.cl:125:19:
> error: use of undeclared identifier 'null'; did you mean 'all'?
> stringInput.cl:7:21: note: expanded from macro 'rgb_matrix'
> 
> That case should probably be caught earlier and rejected with a clear message.
Will fix it.

> 
> 
> On Mali:
> 
> $ ./ffmpeg_g -v 55 -y -i ~/test/The\ World\ in\ HDR.mkv -init_hw_device opencl
> -filter_hw_device opencl0 -an -vf
> 'format=p010,hwupload,tonemap_opencl=t=bt2020:tonemap=linear:format=p0
> 10,hwdownload,format=p010' -c:v libx264 out.mp4
> ...
> [tonemap_opencl @ 0x8201d7c0] Filter input: opencl, 3840x2160 (0).
> [Parsed_tonemap_opencl_2 @ 0x8201d760] Failed to enqueue kernel: -5.
The error seems map to OpenCL error CL_OUT_OF_RESOURCES. I don't have any idea 
yet.
May be some limitation in the driver not queried?

> 
> That's an RK3288 with a Mali T760, clinfo: <https://0x0.st/se5r.txt>, full 
> log:
> <https://0x0.st/se5s.log>.
> 
> (The Rockchip hardware decoder can do H.265 Main 10, but the output format
> isn't P010 so it's easier to use VP9 here.)
Not p010? Then which format? Planar?
And I don't quite understand here. What the relationship of format with VP9?

> 
> 
> Some more thoughts below, I haven't read through all of it carefully.
Thanks for your comments. Answers inline.

> 
> Thanks,
> 
> - Mark
> 
> 
> >  configure  |   1 +
> >  libavfilter/Makefile   |   2 +
> >  libavfilter/allfilters.c   |   1 +
> >  libavfilter/colorspace_basic.c |  89 ++
> >  libavfilter/colorspace_basic.h |  40 +++
> >  libavfilter/opencl/colorspace_basic.cl | 179 +++
> >  libavfilter/opencl/tonemap.cl  | 258 +++
> >  libavfilter/opencl_source.h|   2 +
> >  libavfilter/vf_tonemap_opencl.c| 560
> +
> >  9 files changed, 1132 insertions(+)
> >  create mode 100644 libavfilter/colorspace_basic.c
> >  create mode 100644 libavfilter/colorspace_basic.h
> >  create mode 100644 libavfilter/opencl/colorspace_basic.cl
> >  create mode 100644 libavfilter/opencl/tonemap.cl
> >  create mode 100644 libavfilter/vf_tonemap_opencl.c
> >
> > ...
> > diff --git a/libavfilter/opencl/tonemap.cl b/libavfilter/opencl/tonemap.cl
> > new file mode 100644
> > index 000..03cf3e2
> > --- /dev/null
> > +++ b/libavfilter/opencl/tonemap.cl
> > @@ -0,0 +1,258 @@
> > +/*
> > + * 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 no

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-21 Thread Niklas Haas

On Tue, 22 May 2018 01:18:30 +0100, Mark Thompson  wrote:
> On 21/05/18 07:50, Ruiling Song wrote:
> > 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@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 
> > ---
> 
> I assume you're testing with Beignet for this sort of mapping to work?  I 
> tried it with Beignet on Coffee Lake with 10-bit videos and it looks 
> sensible, though it is rather hard to tell whether it is in some sense 
> "correct".

It's also rather hard to define whether it is in some sense "correct".
The methodology employed here is generally based on ITU-R
recommendations, however the ITU-R advises multiple possible ways of
doing tone-mapping. They also highlight their own curve function, which
we don't use (for performance/simplicity reasons - iirc I gave it a try
and the result was not visually dissimilar enough from the hable
function, but my memory could be wrong). There's nothing resembling an
official "standard" way to tone-map defined by the ITU-R.

This algorithm is also generally based on the results obtained from the
"official" ACES implementation of HDR->SDR tone mapping (obtainable
here: https://github.com/ampas/aces-dev), with the key difference that
we do chroma-invariant tone mapping whereas hollywood tends to use
channel-independent tone mapping. I think the latter distorts the colors
too much for taste and generally results in a worse looking image. The
only important bit to make chroma-invariant tone mapping work well,
however, is the need for a good desaturation algorithm. This one is
based on original research and experimentation. The desaturation
strength with a parameter of 1.0 is comparable to the one achieved by
the ACES algorithm, although I pick a lower strength by default (0.5),
because I found it too damaging for some types of sources (particularly
bright skies) as a result of the chroma-invariant nature.

In addition to the desaturation step, the other key innovation which I
cannot find mentioned in ITU-R literature is the importance of adjusting
the overall average brightness before tone mapping. I suspect the reason
this isn't considered by the ITU-R is because the ITU-R assumes that HDR
sources actually follow their standards, which in practice none seem to
do. In theory, HDR material isn't supposed to have a significantly
higher average brightness than SDR material. Apart from the addition of
isolated super-highlights, nothing should have changed about the image
appearance. In practice, HDR directors like to point their expensive
cameras at very bright objects (e.g. the sun) and blind viewers' eyes by
failing to adjust the brightness during the mastering step. Our
algorithm compensates for this by essentially "correcting" the bad
mastering in real-time. [1] Of course, the result here is not as good as
doing it ahead of time by a human, but unfortunately we don't have a say
in this matter.

As long as the implementation is correct, I'd be confident in assuming
that this produces pleasurable results for all the sources I've thrown
at it, often even exceeding in quality the "official" SDR-mapped blu-ray
versions of the same sources on the same scenes. (Partially due to the
preserved higher color gamut)

In order to ascertain whether or not the implementation is correct, you
could compare it to results obtained by latest `mpv` (might need git
master) or `libplacebo`, both of which implement the same algorithm.

[1] The algorithm I use in mpv and libplacebo does this with one frame
of latency, because I don't want to round-trip through an intermediate
buffer in my processing chain, and there's no other way to communicate
back the measured frame statistics to the rest of the kernels in
OpenGL/Vulkan land. I do this because of my realtime requirements as
well as the structure of my processing chain.

Since you are implementing an offline filter and neither of these
restrictions apply to you, I would recommend changing the implementation
to separate the peak measurement step from the tone mapping step, so
that the former completes first and then the second runs from scratch
and can use the results computed by the former in the same frame.

If you don't do this, you run the risk of failing to tone map single
frame data (e.g. screenshots), because no data about the previous frame
is available at the time.

> > +// detect peak/average signal of a frame, the algorithm was ported from:
> > +// libplacebo (https://github.com/haasn/libplacebo)
> > +struct detection_result
> > +detect_peak_avg(global

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-21 Thread Song, Ruiling



> -Original Message-
> From: myp...@gmail.com [mailto:myp...@gmail.com]
> Sent: Monday, May 21, 2018 3:23 PM
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Cc: s...@jkqxz.net; ffm...@haasn.xyz; Song, Ruiling <ruiling.s...@intel.com>
> Subject: Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.
> 
> 2018-05-21 14:50 GMT+08:00 Ruiling Song <ruiling.s...@intel.com>:
> > 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@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.s...@intel.com>
> > ---
> >  configure  |   1 +
> >  libavfilter/Makefile   |   2 +
> >  libavfilter/allfilters.c   |   1 +
> >  libavfilter/colorspace_basic.c |  89 ++
> >  libavfilter/colorspace_basic.h |  40 +++
> >  libavfilter/opencl/colorspace_basic.cl | 179 +++
> >  libavfilter/opencl/tonemap.cl  | 258 +++
> >  libavfilter/opencl_source.h|   2 +
> >  libavfilter/vf_tonemap_opencl.c| 560
> +
> >  9 files changed, 1132 insertions(+)
> >  create mode 100644 libavfilter/colorspace_basic.c
> >  create mode 100644 libavfilter/colorspace_basic.h
> >  create mode 100644 libavfilter/opencl/colorspace_basic.cl
> >  create mode 100644 libavfilter/opencl/tonemap.cl
> >  create mode 100644 libavfilter/vf_tonemap_opencl.c
> >
> > diff --git a/libavfilter/opencl/colorspace_basic.cl
> b/libavfilter/opencl/colorspace_basic.cl
> > new file mode 100644
> > index 000..ffd98c2
> > --- /dev/null
> > +++ b/libavfilter/opencl/colorspace_basic.cl
> > @@ -0,0 +1,179 @@
> > +/*
> > + * 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
> > + */
> > +
> > +#define ST2084_MAX_LUMINANCE 1.0f
> > +#define REFERENCE_WHITE 100.0f
> > +constant const float ST2084_M1 = 0.1593017578125f;
> > +constant const float ST2084_M2 = 78.84375f;
> > +constant const float ST2084_C1 = 0.8359375f;
> > +constant const float ST2084_C2 = 18.8515625f;
> > +constant const float ST2084_C3 = 18.6875f;
> > +
> > +// TODO Move these colorspace matrix to .cpp files
> what's .cpp files? is it porting from some cpp file?
Sorry, this is a typo. It should be '.c'. It's better to move this matrix 
generation into .c file, so that it is easy to support more color spaces.
But currently, it is not so urgent as the tonemap only cares about bt709/bt2020 
now.

Thanks!
Ruiling

> > +__constant float yuv2rgb_bt2020[] = {
> > +1.0f, 0.0f, 1.4746f,
> > +1.0f, -0.16455f, -0.57135f,
> > +1.0f, 1.8814f, 0.0f
> > +};
> > +
> >
> > ___
> > ffmpeg-devel mailing list
> > ffmpeg-devel@ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> 
> 
> 
> --
> ===
> Pixelworks
> Room 301-303 No. 88,Lane 887 Zuchongzhi Road, Zhangjiang Hi-tech Park,
> Shanghai 201203, China
> Best Regards,
> Jun zhao/赵军
> +++
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-21 Thread Mark Thompson

On 21/05/18 07:50, Ruiling Song wrote:
> 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@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 
> ---

I assume you're testing with Beignet for this sort of mapping to work?  I tried 
it with Beignet on Coffee Lake with 10-bit videos and it looks sensible, though 
it is rather hard to tell whether it is in some sense "correct".

Given a non-P010 input video it fails with build errors when compling the 
kernels:

[Parsed_tonemap_opencl_1 @ 0x55b700e51540] Failed to build program: -11.
[Parsed_tonemap_opencl_1 @ 0x55b700e51540] Build log:
/home/mrt/video/ffmpeg/opencl/libavfilter/opencl/colorspace_basic.cl:125:19: 
error: use of undeclared identifier 'null'; did you mean 'all'?
stringInput.cl:7:21: note: expanded from macro 'rgb_matrix'

That case should probably be caught earlier and rejected with a clear message.


On Mali:

$ ./ffmpeg_g -v 55 -y -i ~/test/The\ World\ in\ HDR.mkv -init_hw_device opencl 
-filter_hw_device opencl0 -an -vf 
'format=p010,hwupload,tonemap_opencl=t=bt2020:tonemap=linear:format=p010,hwdownload,format=p010'
 -c:v libx264 out.mp4
...
[tonemap_opencl @ 0x8201d7c0] Filter input: opencl, 3840x2160 (0).
[Parsed_tonemap_opencl_2 @ 0x8201d760] Failed to enqueue kernel: -5.

That's an RK3288 with a Mali T760, clinfo: , full log: 
.

(The Rockchip hardware decoder can do H.265 Main 10, but the output format 
isn't P010 so it's easier to use VP9 here.)


Some more thoughts below, I haven't read through all of it carefully.

Thanks,

- Mark


>  configure  |   1 +
>  libavfilter/Makefile   |   2 +
>  libavfilter/allfilters.c   |   1 +
>  libavfilter/colorspace_basic.c |  89 ++
>  libavfilter/colorspace_basic.h |  40 +++
>  libavfilter/opencl/colorspace_basic.cl | 179 +++
>  libavfilter/opencl/tonemap.cl  | 258 +++
>  libavfilter/opencl_source.h|   2 +
>  libavfilter/vf_tonemap_opencl.c| 560 
> +
>  9 files changed, 1132 insertions(+)
>  create mode 100644 libavfilter/colorspace_basic.c
>  create mode 100644 libavfilter/colorspace_basic.h
>  create mode 100644 libavfilter/opencl/colorspace_basic.cl
>  create mode 100644 libavfilter/opencl/tonemap.cl
>  create mode 100644 libavfilter/vf_tonemap_opencl.c
> 
> ...
> diff --git a/libavfilter/opencl/tonemap.cl b/libavfilter/opencl/tonemap.cl
> new file mode 100644
> index 000..03cf3e2
> --- /dev/null
> +++ b/libavfilter/opencl/tonemap.cl
> @@ -0,0 +1,258 @@
> +/*
> + * 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
> + */
> +
> +#define REFERENCE_WHITE 100.0f
> +extern float3 lrgb2yuv(float3);
> +extern float3 yuv2lrgb(float3);
> +extern float3 lrgb2lrgb(float3);
> +extern float get_luma_src(float3);
> +extern float get_luma_dst(float3);
> +extern float3 ootf(float3);
> +extern float3 inverse_ootf(float3);
> +struct detection_result {
> +float peak;
> +float average;
> +};
> +
> +float hable_f(float in) {
> +float a = 0.15f, b = 0.50f, c = 0.10f, d = 0.20f, e = 0.02f, f = 0.30f;
> +return (in * (in * a + b * c) + d * e) / (in * (in * a + b) + d * f) - e 
> / f;
> +}
> +
> +float direct(float s, float peak) {
> +return s;
> +}
> +
> +float linear(float s, float peak) {
> +return s * tone_param / peak;
> +}
> +
> +float gamma(float s, float peak) {
> +float p = s > 0.05f ? s /peak : 0.05f / peak;
> +float v = pow(p, 1.0f / tone_param);
> +return s > 0.05f ? v : (s * v /0.05f);
> +}
> +
> +float clip(float s, float peak) {
> +return clamp(s * tone_param, 0.0f, 1.0f);
> +}
> +
> +float reinhard(float s, float peak) {
> +return s / (s + tone_param) * (peak + tone_param) / peak;
> +}
> +
>

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

2018-05-21 Thread myp...@gmail.com

2018-05-21 14:50 GMT+08:00 Ruiling Song :
> 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@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 
> ---
>  configure  |   1 +
>  libavfilter/Makefile   |   2 +
>  libavfilter/allfilters.c   |   1 +
>  libavfilter/colorspace_basic.c |  89 ++
>  libavfilter/colorspace_basic.h |  40 +++
>  libavfilter/opencl/colorspace_basic.cl | 179 +++
>  libavfilter/opencl/tonemap.cl  | 258 +++
>  libavfilter/opencl_source.h|   2 +
>  libavfilter/vf_tonemap_opencl.c| 560 
> +
>  9 files changed, 1132 insertions(+)
>  create mode 100644 libavfilter/colorspace_basic.c
>  create mode 100644 libavfilter/colorspace_basic.h
>  create mode 100644 libavfilter/opencl/colorspace_basic.cl
>  create mode 100644 libavfilter/opencl/tonemap.cl
>  create mode 100644 libavfilter/vf_tonemap_opencl.c
>
> diff --git a/configure b/configure
> index e52f8f8..ee3586b 100755
> --- a/configure
> +++ b/configure
> @@ -3401,6 +3401,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"
>  unsharp_opencl_filter_deps="opencl"
>  uspp_filter_deps="gpl avcodec"
>  vaguedenoiser_filter_deps="gpl"
> diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> index c68ef05..0915656 100644
> --- a/libavfilter/Makefile
> +++ b/libavfilter/Makefile
> @@ -352,6 +352,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_basic.o opencl.o \
> +opencl/tonemap.o 
> opencl/colorspace_basic.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 b44093d..6873bab 100644
> --- a/libavfilter/allfilters.c
> +++ b/libavfilter/allfilters.c
> @@ -343,6 +343,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_basic.c b/libavfilter/colorspace_basic.c
> new file mode 100644
> index 000..93f9f08
> --- /dev/null
> +++ b/libavfilter/colorspace_basic.c
> @@ -0,0 +1,89 @@
> +/*
> + * 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_basic.h"
> +
> +
> +void invert_matrix3x3(const double in[3][3], double out[3][3])
> +{
> +double m00 = in[0][0], m01 = in[0][1], m02 = in[0][2],
> +   m10 = in[1][0], m11 = in[1][1], m12 = in[1][2],
> +   m20 = in[2][0], m21 = in[2][1], m22 = in[2][2];
> +int i, j;
> +double det;
> +
> +out[0][0] =  (m11 * m22 - m21 * m12);
> +out[0][1] = -(m01 * m22 - m21 * m02);
> +out[0][2] =  (m01 * m12 - m11 * m02);
> +out[1][0] = -(m10 * m22 - m20 * m12);
> +out[1][1] =  (m00 * m22 - m20 * m02);
> +out[1][2] = -(m00 * m12 - m10 * m02);
> +out[2][0] =  (m10 * m21 - m20 * m11);
> +out[2][1] = -(m00 * m21 - m20 * m01);
> +out[2][2] =  (m00 * m11 - m10 * m01);
> +
> +det = m00 * out[0][0]

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

Re: [FFmpeg-devel] [PATCH] lavfi: add opencl tonemap filter.

20 matches

Site Navigation

Mail list logo

Footer information