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

Michael Niedermayer michaelni at gmx.at
Sat Mar 30 12:57:14 CET 2013


On Sat, Mar 30, 2013 at 06:47:42PM +0800, Wei Gao wrote:
> 
[...]
> +static const char *opencl_errstr(cl_int status)
> +{
> +    int i;
> +    for (i = 0; i < sizeof(opencl_err_msg); i++) {
> +        if (opencl_err_msg[i].err_code == status)
> +            return opencl_err_msg[i].err_str;
> +    }
> +    return "unknown error";
> +}
> +

> +AVOpenCLExternalInfo *av_opencl_malloc_external_environment(void)
> +{

> +    AVOpenCLExternalInfo *ext = (AVOpenCLExternalInfo *)av_mallocz(sizeof(AVOpenCLExternalInfo));

unneeded cast


> +    if (!ext) {
> +        av_log(&openclutils, AV_LOG_ERROR,
> +         "Could not malloc external opencl environment data space\n");
> +    }
> +    return ext;
> +}
> +

> +void av_opencl_free_external_environment(AVOpenCLExternalInfo *ext)
> +{
> +    av_free(ext);
> +}

A release function similar to av_freep() would safer as it also zeros
the pointer


> +
> +AVUserSpecificDevInfo *av_opencl_malloc_user_spec_dev_info(void)
> +{
> +    AVUserSpecificDevInfo *dev_info = (AVUserSpecificDevInfo *)av_mallocz(sizeof(AVUserSpecificDevInfo));

unneeded cast


> +    if (!dev_info) {
> +        av_log(&openclutils, AV_LOG_ERROR, "Could not malloc device info data space\n");
> +    }
> +    return dev_info;
> +}
> +

> +void av_opencl_free_user_spec_dev_info(AVUserSpecificDevInfo *dev_info)
> +{
> +    av_free(dev_info);
> +}

A release function similar to av_freep() would safer as it also zeros
the pointer


> +
> +int av_opencl_register_kernel_code(const char *kernel_code)
> +{
> +    int ret = 0;
> +    int i;
> +    LOCK_OPENCL;
> +    if (gpu_env.kernel_code_count >= MAX_REG_NUM) {
> +        av_log(&openclutils, AV_LOG_ERROR,
> +         "Could not register kernel code, maximum number of registered kernel code %d already reached\n",
> +         MAX_REG_NUM);
> +        ret = AVERROR(EINVAL);
> +        goto end;
> +    }
> +    for (i = 0;i < gpu_env.kernel_code_count;i++) {
> +        if (gpu_env.kernel_code[i] == kernel_code) {
> +            av_log(&openclutils, AV_LOG_WARNING, "Same kernel code has been registered\n");
> +            goto end;
> +        }
> +    }
> +    gpu_env.kernel_code[gpu_env.kernel_code_count] = kernel_code;
> +    gpu_env.kernel_code_count++;
> +end:
> +    UNLOCK_OPENCL;
> +    return ret;
> +}

iam not sure droping the kernel name is a good idea, detecting
duplicated registrations now only works if the duplicates are
represented by the same pointer


[...]
> +int av_opencl_init(AVDictionary *options, AVOpenCLExternalInfo *ext_opencl_info, AVUserSpecificDevInfo *user_specific_dev_info)
> +{
> +    int ret = 0;
> +    LOCK_OPENCL
> +    if (!gpu_env.opencl_is_inited) {
> +        /*initialize devices, context, command_queue*/
> +        AVDictionaryEntry *opt_entry = av_dict_get(options, "build_option", NULL, 0);

> +        if (user_specific_dev_info) {
> +            gpu_env.user_spec_dev_flag = 1;
> +            gpu_env.usr_spec_dev_info.dev_idx = user_specific_dev_info->dev_idx;
> +            gpu_env.usr_spec_dev_info.platform_idx= user_specific_dev_info->platform_idx;
> +        }

Why is AVUserSpecificDevInfo used here and not the AVDictionary ?
(maybe theres a reason, but it seems unneeded to use 2 different
 systems)


[...]
> +/**
> + * Initialize the run time OpenCL environment.
> + *
> + * This must be done after all the kernels have been registered with
> + * the av_opencl_register_kernel_code().

Thats not so easy

consider that libavutil is used by 2 libraries
both libraries register their kernels and then init them
its hard to gurantee that this happens in order
consider one lib to be avfilter and one avcodec
The user application doesnt (need to) know about open cl it just knows
about the avcodec and avfilter APIs and and may very well register
and init one before registering anything from the other.

also consider that libavcodec and libavfilter themselfs can be used
by other libs, it quickly gets hard to ensure their register and
init happens in order in the final application


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

it is not once nor twice but times without number that the same ideas make
their appearance in the world. -- Aristotle
-------------- 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/20130330/940f9afc/attachment.asc>


More information about the ffmpeg-devel mailing list