[FFmpeg-devel] [PATCH] libavutil/libavfilter:add opencl warpper and opencl deshake filter

[Stefano Sabatini]

On date Tuesday 2013-03-19 16:54:21 +0800, Wei Gao encoded:
> Hi,
> 
> Thank you for your reviewing Stefano Savatini, I still have some questions,
> and some explanations as follows.
> 
> Thanks
> Best regards
> 
[...]
> > > +static int binary_generated(cl_context context, const char *
> > cl_file_name, FILE ** fhandle)
> >
> > The usual convenction for a function name is to use a verb indicating
> > what the function does. This could be "generate_binary".
> >
> This function is use to detect whether the compiled binary file is exist.
> The binary file is that when run the kernel at the first time, opencl
> wrapper will record the opencl kernel compile information and create a
> binary file.And if run the kernel again, it will read the binary file and
> don't need to do the compile kernel operation. it means that you just
> compile the kernel once and it can save compile time.

What about:
check_generated_binary()?

[...]
> > > +    /* dump out each binary into its own separate file. */
> > > +    for (i = 0; i < numdevices; i++) {
> > > +        char filename[256] = {0};
> > > +        char cl_name[128] = {0};
> > > +        FILE *output = NULL;
> > > +        if (binarysizes[i] != 0) {
> > > +            char devicename[1024];
> > > +            status = clGetDeviceInfo(devices[i],
> > > +                                     CL_DEVICE_NAME,
> > > +                                     sizeof(devicename),
> > > +                                     devicename,
> > > +                                     NULL);
> > > +            if (status != CL_SUCCESS) {
> > > +
> >  av_log(&openclutils,AV_LOG_ERROR,"generat_bin_from_kernel_source
> > error,clGetDeviceInfo:%s\n",opencl_errstr(status));
> > > +                return AVERROR_EXTERNAL;
> > > +            }
> > > +            memcpy(cl_name, cl_file_name, strlen(cl_file_name));
> > > +            cl_name[strlen(cl_file_name) + 1] = '\0';
> > > +            snprintf(filename,sizeof(filename), "./%s-%s.bin", cl_name,
> > devicename);
> > > +            output = fopen(filename, "wb");
> > > +            if(!output)
> > > +                return AVERROR_EXTERNAL;
> > > +            fwrite(binaries[i], sizeof(char), binarysizes[i], output);
> > > +            fclose(output);
> > > +        }
> > > +    }
> >
> > duplicated code with binary_generated()?
> >
> the  binary_generated is read the binary file, the above code is
> write(create) the binary file.

You could try to create a routine factorizing the shared code,
something like:

access_binaries(..., int write)

[...]
> > > +static int cached_of_kerner_prg(const GPUEnv *gpu_env, const char *
> > cl_file_name)
> >
> > obfuscated function name
> >
> this function is detect whether the kernel programs is existed, is
> "detect_kernel_program" OK?

sure it's better

[...]

I replied to the other questions in another mail.
-- 
FFmpeg = Fancy Furious Muttering Perfectionist Explosive Gladiator