[FFmpeg-devel] [PATCH v4 1/2] lavfi: add opencl tonemap filter.

[Mark Thompson]

On 19/06/18 02:57, 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 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>
> ---
> As I didn't receive any other comment on v3, this version only fix the comment from Michael.
> And also include some little change to leverage CL_SET_KERNEL_ARG() macro.
> 
> Thanks!
> Ruiling
> 
>  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(+)
>  create mode 100644 libavfilter/colorspace.c
>  create mode 100644 libavfilter/colorspace.h
>  create mode 100644 libavfilter/opencl/colorspace_common.cl
>  create mode 100644 libavfilter/opencl/tonemap.cl
>  create mode 100644 libavfilter/vf_tonemap_opencl.c

I did a bit more testing, LGTM; therefore applied.

Some further thoughts:
* Did you get anywhere with testing on other platforms?  (I'm happy with the current state with it working on at least two different platforms including the actually-useful GPU-with-interop ones, but it would be nice to know what was going wrong in the other cases.)
* The single-frame-delay effect ends up looking a bit weird when I go looking for it - I now keep seeing the flash of a single frame at a different brightness on some transitions, though I'm not sure it's obvious enough that I would notice often if I didn't already know it was there.  How much performance would it actually cost to use the correct frame rather than the previous one?
* +1 to the comment from Moritz about documentation if you wouldn't mind writing some as a separate patch.

Thanks,

- Mark