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

[Wei Gao]

Hi, Thanks for your reply. some questions and explanations

Thanks
Best regards


2013/3/29 Stefano Sabatini <stefasab at gmail.com>

> On date Thursday 2013-03-28 20:57:44 +0800, Wei Gao encoded:
> >
>
> > From 38aa9bb65c63d44cd1586383f37197bee93c61ab Mon Sep 17 00:00:00 2001
> > From: highgod0401 <highgod0401 at gmail.com>
> > Date: Thu, 28 Mar 2013 20:42:21 +0800
> > Subject: [PATCH 1/2] opencl wrapper based on comments on 20130328
> >
> > ---
>
>
> > +        /*initialize devices, context, command_queue*/
> > +        AVDictionaryEntry *opt_entry = av_dict_get(options,
> "build_option", NULL, 0);
>
> "build_options"?
>
> "build_flags" also seems fine, whatever is more consistent with the
> specification text.
>
I reference the document of OpenCL Specification Version: 1.2, it named
build options.

>
> > +        ret = init_opencl_env(&gpu_env, ext_opencl_info);
> > +        if (ret < 0)
> > +            goto end;
> > +        /*initialize program, kernel_name, kernel_count*/
>
> > +        ret = compile_kernel_file(&gpu_env, opt_entry->value);
>
> crash if opt_entry is NULL?
>
will not crush, the OpenCL API "clBuildProgram" accept the NULL pointer,
the kernel can be compiled too.

>
>  *
>  * A kernel with name kernel_name must have been registered and
>  * created with av_opencl_create_kernel(). This is requested for
>  * executing the kernel code through av_opencl_run_kernel().
>  *
>  * @param kernel_name      name used to find the kernel in OpenCL runtime
> environment
>  * @param function         user defined function, should not be NULL, used
> to set the input parameter in the kernel environment
>  * @return >=0 on success, a negative error code on failure
>  */
> int av_opencl_register_kernel_function(const char *kernel_name,
> av_opencl_kernel_function function);
>
> Also: why don't you register the function handle when you create the
> kernel?
> This would simplify usage and reduce API complexity.
>

I think you  confuse the parameter of av_opencl_create_kernel, it is my
mistake. the kernel_name in this function should be renamed to
register_name, it is a index to find kernel code in
av_opencl_register_kernel_code and find function registered by
av_opencl_register_kernel_function when run av_opencl_run_kernel, the
function is not equal to kernel, it may call many kernels when runing. it
is a user defined function to run kernels, you can reference the deshake
code in tranform_opencl.c


> > +
> > +
> > +/**
> > + * Create kernel object on the specified OpenCL run in env.
> > + *
> > + * @param env                   the kernel environment which has been
> created by av_opencl_init
>
> why do you need to pass this (since there is a unique global context,
> why do you need to pass it)?
>

sorry about the comment of env it should be "it is used for user to get the
kernel env created by av_opencl_init, user will use it to run kernel".

>
> > + * @param kernel_entry_point      kernel name
>
> wrong
>


> Also you may specify that this is the name of a function which must be
> specified in the kernel code (BTW what happens in case it is not?).
>

this is the real kernel function name which is in the kernel code. the
function wtich have the  prefix "kernel" or "__kernel" is the kernel
function. this name is the function's name, you can reference
deshake_kernel.h OpenCL API clCreateKernel will use this name to find
whether the kernel is in the program which has been compiled. if yes, it
will return a handle cl_kernel, and user can use it to call the function in
GPU. So it is different from register_name
if the name is not found in program, the API will return error, and handle
should be NULL.

>
> > + * @return >=0 on success, a negative error code on failure
> > + */
> > +int av_opencl_create_kernel(AVOpenCLKernelEnv *env, const char
> *kernel_entry_point);
>
> See my comment about av_opencl_register_kernel_function().
>
> > +
> > +/**
> > + *  Release kernel object.
> > + *
> > + *@param env  the kernel environment which has been created by
> av_opencl_init.
> > + */
> > +void av_opencl_release_kernel(AVOpenCLKernelEnv *env);
> > +
>
> ...
>
> Sorry this is taking more time than expected, but as I explained
> public API is critical and code and API is pretty complex.
> --
> FFmpeg = Free Fierce Mean Portable Elastic God
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>