[FFmpeg-devel] [PATCH 1/2] libavutil/libavfilter: opencl wrapper based on comments on 20130401

[Michael Niedermayer]

On Mon, Apr 01, 2013 at 05:52:13PM +0800, Wei Gao wrote:
[...]
> +int av_opencl_create_kernel(AVOpenCLKernelEnv *env, const char *kernel_name)
> +{
> +    cl_int status;
> +    int i, ret = 0;
> +    LOCK_OPENCL;
> +    if (strlen(kernel_name) + 1 > AV_OPENCL_MAX_KERNEL_NAME_SIZE) {
> +        av_log(&openclutils, AV_LOG_ERROR, "Created kernel name %s is too long\n", kernel_name);
> +        ret = AVERROR(EINVAL);
> +        goto end;
> +    }
> +    if (!env->kernel) {
> +        if (gpu_env.kernel_count >= MAX_KERNEL_NUM) {
> +            av_log(&openclutils, AV_LOG_ERROR,
> +            "Could not create kernel with name '%s', maximum number of kernels %d already reached\n",
> +                kernel_name, MAX_KERNEL_NUM);
> +            ret = AVERROR(EINVAL);
> +            goto end;
> +        }
> +        for (i = 0; i < gpu_env.program_count; i++) {
> +            env->kernel = clCreateKernel(gpu_env.programs[i], kernel_name, &status);
> +            if (status == CL_SUCCESS)
> +                break;
> +        }
> +        if (status != CL_SUCCESS) {

if program_count is 0 then status isnt initialized


[...]
> +int av_opencl_init(AVDictionary *options, AVOpenCLExternalEnv *ext_opencl_env)
> +{
> +    int ret = 0;
> +    AVDictionaryEntry *opt_build_entry;
> +    AVDictionaryEntry *opt_platform_entry;
> +    AVDictionaryEntry *opt_device_entry;
> +    LOCK_OPENCL
> +    if (!gpu_env.init_count) {
> +        opt_platform_entry = av_dict_get(options, "platform_idx", NULL, 0);
> +        opt_device_entry   = av_dict_get(options, "device_idx", NULL, 0);
> +        /*initialize devices, context, command_queue*/
> +        gpu_env.usr_spec_dev_info.platform_idx = -1;
> +        gpu_env.usr_spec_dev_info.dev_idx = -1;
> +        if (opt_platform_entry) {
> +            gpu_env.usr_spec_dev_info.platform_idx = strtol(opt_platform_entry->value, NULL, 10);
> +        }
> +        if (opt_device_entry) {

> +            gpu_env.usr_spec_dev_info.dev_idx = strtol(opt_device_entry->value, NULL, 10);

strtol should be checked for failure


> +        }
> +        ret = init_opencl_env(&gpu_env, ext_opencl_env);
> +        if (ret < 0)
> +            goto end;
> +    }
> +    /*initialize program, kernel_name, kernel_count*/
> +    opt_build_entry = av_dict_get(options, "build_options", NULL, 0);
> +    if (opt_build_entry)
> +        ret = compile_kernel_file(&gpu_env, opt_build_entry->value);
> +    else
> +        ret = compile_kernel_file(&gpu_env, NULL);
> +    if (ret < 0)
> +        goto end;

> +    av_assert1(gpu_env.kernel_code_count > 0);

A user understandable error message is better here for the case
where no code was registered


[...]
> +/**
> + * Initialize the run time OpenCL environment and compile the kernel code registered with
> + * av_opencl_register_kernel_code().
> + *

> + * Currently, the only accepted option is "build_options", used to set
> + * options to compile registered kernels code. See reference "OpenCL
> + * Specification Version: 1.2 chapter 5.6.4".
> + *
> + * @param options                  dictionary of key/value options

missing platform_idx and device_idx

also please add yourself to MAINTAINERS and get some git repository
somewhere (for example github) so you can maintain the code.

Thanks!

[...]

--
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Why not whip the teacher when the pupil misbehaves? -- Diogenes of Sinope
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20130401/e235d625/attachment.asc>