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

[Stefano Sabatini]

On date Wednesday 2013-03-27 15:11:46 +0800, Wei Gao encoded:
> Hi,
> 
> Stefano Sabatini, thanks for your reply. some questions and explanations as
> follows
> 
> Thanks
> Best regards
> 
> 2013/3/27 Stefano Sabatini <stefasab at gmail.com>
> 
> > On date Tuesday 2013-03-26 18:55:05 +0800, Wei Gao encoded:
> > >
> >
> > > From f91df6a8315a1b7bdc7b69517831fc745fcbd4fd Mon Sep 17 00:00:00 2001
> > > From: highgod0401 <highgod0401 at gmail.com>
> > > Date: Tue, 26 Mar 2013 18:43:00 +0800
> > > Subject: [PATCH 1/2] opencl wrapper based on comments on 20130326
[...]
> > This is my understanding of how this OpenCL API works.
> >
> > You register some code with a name (which are currently stored
> > in the global environment), with av_opencl_register_kernel().
> >
> > Then the previously registered framments of code are compiled by
> > OpenCL, when doing: av_opencl_init() -> compile_kernel_file()
> >
> > With av_opencl_init() you also specify some parameters (build_options)
> > which are used when compiling the code of the specified functions.
> >
> > av_opencl_init() also creates the OpenCL program, which is stored in
> > the global environment. The program is unique for all the kernels
> > registered so far.
> >
> > At this point you need to create an entry point for each kernel, to
> > run a specific *function* defined within it. This is done by creating
> > a kernel, with av_opencl_create_kernel()
> >
> > av_opencl_create_kernel() is used to create a kernel (a sort of
> > handler to communicate with the *compiled* kernel). The kernel is
> > created specifying the name of the function to run *in the kernel
> > code*. The kernel is set in the passed AVOpenCLEnv environment.
> >
> > In order to run a function specified in a kernel, you also need to
> > provide some parameters/data to it.
> >
> > This is done through av_opencl_register_kernel_function(), which is
> > used to register a function which is associated to one of the
> > previously registered kernel in the global environment.
> >
> > so we have: kernel(global env) -> function(global env)
> >
> > To run the code of a kernel, av_opencl_run_kernel() must be called,
> > with the name of the registered kernel on which the function is to be
> > called.
> >
> > This function lookups the functions registered in the global
> > environment, and executes the registered function with provided user
> > data/parameters, which in particular must contain the opencl
> > environment. The environment should contain the kernel handler created
> > with av_opencl_create_kerne(), and is used to set the arguments for
> > the function defined in the kernel code, and eventually run the code
> > for it (see the deshake patch for an example of such usage).
> >
> > ...
> >
> > So basically this is the workflow:
> >
> > kernel code registration (done in the global environment)            ->
> > av_opencl_register_kernel()
> > kernel code compilation/init (always done in the global environment) ->
> > av_opencl_init()
> >
> > kernel function registration (can be eventually done *before* init)  ->
> > av_opencl_register_kernel_function()
> > kernel object creation, which is required to run the code            ->
> > av_opencl_create_kernel
> > kernel code execution with user data parameters                      ->
> > av_opencl_run_kernel()
> >
> > Cleanup:
> > kernel object (stored in an environment)                             ->
> > av_opencl_release_kernel()
> > global environment                                                   ->
> > av_opencl_uninit()
> >
> > ...
> 
> > Can you confirm that this is an accurate description of the
> > design/workflow?
> >
> yes,you are right
> 
> >
> > 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.

> 
> >
> > 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

> > 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

Also in case you never register a function you have:
gpu_env.runtime_kernel_count = 0;
av_opencl_uninit()
gpu_env.runtime_kernel_count--;
gpu_env.runtime_kernel_count -> -INT_MAX
-> not released

[...]

I'll try to suggest an alternative design when commenting on the new
patch.
-- 
FFmpeg = Fast and Faithful Mean Practical Exploitable Guru