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

[Stefano Sabatini]

On date Thursday 2013-03-28 10:08:14 +0800, Wei Gao encoded:
[...]
> > > > The main problem with this design is that different threads and
> > > > components can messup with the global environment.
> > > >
> > > > For example you may want to init a filter, this creates a global
> > > > environment, then you create another filter/component which requires
> > > > to build a different kernel etc., which can't be done since you're
> > > > supposed to init the global environment just once.
> > > >
> >
> > > all the opencl code an use the same, such as reuse context, command
> > queue,
> > > device, platform....... just init once is enough.
> >
> > Yes, but if opencl was already inited it is not possible to add a new
> > kernel, withouth destroying the global environment. And in general you
> > may want to add a new kernel on the fly, after the library/opencl have
> > been already inited.
> 
> the global environment should not be destroied becaust all the kernels will
> share the environment..the create kernel functions is like this
> 
>  cl_int status;
>     if (strlen(kernel_entry_point) > sizeof(env->kernel_entry_point)) {
>         av_log(&openclutils, AV_LOG_ERROR, "Created kernel name %s is too
> long\n", kernel_entry_point);
>         return AVERROR(EINVAL);
>     }
>     if (!env->kernel) {
>         env->kernel = clCreateKernel(gpu_env.program, kernel_entry_point,
> &status);
>         if (status != CL_SUCCESS) {
>             av_log(&openclutils, AV_LOG_ERROR, "Could not create OpenCL
> kernel: %s\n", opencl_errstr(status));
>             return AVERROR_EXTERNAL;
>         }
>         env->context       = gpu_env.context;
>         env->command_queue = gpu_env.command_queue;
>         env->program       = gpu_env.program;
>         av_strlcpy(env->kernel_entry_point, kernel_entry_point,
> sizeof(env->kernel_entry_point));
>     }
>     return 0;
> so it just call the api to create a kernel and pass the global enviroment,
> you can create kernel every where, but the kernel code must  registerd
> before I think it is a operation like that you should register a av_filter
> then use it, is is right?

Uhm so this is:

- register all components (which use OpenCL)
- init opencl (compile code for registered components)
- create kernel, use it

> 
> > >
> > > >
> > > > Ideally we should have one OpenCL context per component, so we don't
> > > > need to know everything (kernel code and functions) when we init the
> > > > OpenCL system, and by using a global environment you are prevented
> > > > from doing that.
> > > >
> > > > all kernels can reuse the opencl environment, no need to create for
> > > each.also the init function accept the externel OpenCL environment
> > > "AVOpenCLExternalInfo" from application, if the environment is created by
> > > application, the library will not release the environment.
> >
> > The problem again is that there is no way to register and compile new
> > code on the fly for a new component, after the system have been
> > already inited, unless you release the old opencl environment.
> >
> > So you can have this sequence:
> >
> > register kernels
> > av_opencl_init()
> > register kernels
> > av_opencl_init() -> no op: the new kernels are never compiled
> >
> all the kernels have registered in function
> "av_opencl_register_kernel_function", and when init, it will compiled all
> the kernel, so the secode calling don't need to compile the functions
> again.
> 
> >
> > > > In a similar way, when you uninit the OpenCL system you don't know if
> > > > other components are actually using it, so the only safe way is to
> > > > uninit() it when you close the *application*, which is not ideal for a
> > > > library.
> > > >
> > > I set a counter to count whether all kernels are released
> > > "gpu_env.runtime_kernel_count--; ", if all the kernel is released ,then
> > > release the OpenCL environment(it should wait for all the kernels have
> > been
> > > released.).the unint function just release the environment created by the
> > > library itself,"gpu_env.is_user_created" can indicate that whether the
> > > OpenCL enviroment is created by application or the opencl library itself,
> > > it only set to 1 if the OpenCL enviroment is created by application in
> > > av_opencl_init.There are two paths, one is application create the opencl
> > > library itself,then it release it. the other is created by application,
> > > then application release it.
> >

> > gpu_env.runtime_kernel_count is increased when a new function is
> > registered.
> > Also note that you increase the counter every time, so you can have:
> >
> > gpu_env.runtime_kernel_count = 0;
> >
> > av_opencl_register_kernel_function("foobar", fn)
> > gpu_env.runtime_kernel_count -> 1
> >
> > av_opencl_register_kernel_function("foobar", fn)
> > // registered again, the new function overwrites the old one
> > gpu_env.runtime_kernel_count -> 2
> >
> > av_opencl_uninit()
> > gpu_env.runtime_kernel_count--;
> > gpu_env.runtime_kernel_count -> 1
> > -> not released
> >

> why call  av_opencl_register_kernel_function twice and uninit once? at the
> start we design that when call uninit ,it will release all kernels and
> environment. but it should placed in the applications uninit code. Do you
> think so? and second is like this, when you register a kernel function, you
> should call the uninit, the should exist in pairs

SO basically init() initialize the global context, but uninit() must
be called the number of registered functions, and doesn't consider
the case when register() is called two or more times on the same
kernel, which is confusing and brittle (I'd expect uninit() to be
the opposite of init(), to be called just once).

Also, why don't you set the function handler when you create the
kernel (in av_opencl_create_kernel())?
-- 
FFmpeg = Fascinating & Furious Multimedia Pacific Ecletic Guide