[FFmpeg-devel] [PATCH] lavfi: add nlmeans_opencl filter

[Mark Thompson]

On 12/04/2019 08:38, Song, Ruiling wrote:
>>>> +#define RELEASE_KERNEL(k)                                    \
>>>> +do {                                                         \
>>>> +    if (k) {                                                 \
>>>> +        cle = clReleaseKernel(k);                            \
>>>> +        if (cle != CL_SUCCESS)                               \
>>>> +            av_log(avctx, AV_LOG_ERROR, "Failed to release " \
>>>> +                   "kernel: %d.\n", cle);                    \
>>>> +    }                                                        \
>>>> +} while(0)
>>>
>>> This appears multiple times here and also in other filters.  Maybe it should be a
>>> macro in opencl.h like CL_SET_KERNEL_ARG?
> Hi Mark,
> 
> I am rethinking about this problem, can we just simply call clReleaseKernel() and not checking the input and the error_code.
> OpenCL spec has require implementation to check the input argument. So I think we can just ignore the if-null check.

I'm not sure that's true?  The spec allows a CL_INVALID_KERNEL error, but doesn't offer any clear indication of when it should be returned (NULL is distinguished in other cases, but not here).  Random pointers certainly do crash implementations, so they aren't interpreting it as a requirement to validate the pointer generally (against some list in the context, say).

The standard ICD loader does have a null check returning CL_INVALID_KERNEL, but there is no requirement that it is used rather than linking to a particular ICD directly.

> As we are destroying the objects, is it still useful to care the error code returned?

Probably not, I agree.

- Mark