[FFmpeg-devel] [PATCH 2/3] libavutil/libavfilter: add spec opencl device APIs 20130410
Stefano Sabatini
stefasab at gmail.com
Wed Apr 10 14:53:40 CEST 2013
On date Wednesday 2013-04-10 17:32:41 +0800, Wei Gao encoded:
>
> From 538527370e0f4b8fc321580792384db7c9c2ab73 Mon Sep 17 00:00:00 2001
> From: highgod0401 <highgod0401 at gmail.com>
> Date: Wed, 10 Apr 2013 17:18:24 +0800
> Subject: [PATCH 2/3] add spec opencl device APIs 20130410
>
> ---
> libavfilter/deshake_opencl.c | 5 +--
> libavutil/opencl.c | 83 +++++++++++++++++++++++++++-----------------
> libavutil/opencl.h | 39 +++++++++++++++++----
> libavutil/version.h | 2 +-
> 4 files changed, 87 insertions(+), 42 deletions(-)
Missing docs. You could create a doc/opencl.texi file.
> diff --git a/libavfilter/deshake_opencl.c b/libavfilter/deshake_opencl.c
> index 63d144a..0f6dcc4 100644
> --- a/libavfilter/deshake_opencl.c
> +++ b/libavfilter/deshake_opencl.c
> @@ -98,10 +98,7 @@ int ff_opencl_deshake_init(AVFilterContext *ctx)
> {
> int ret = 0;
> DeshakeContext *deshake = ctx->priv;
> - AVDictionary *options = NULL;
> - av_dict_set(&options, "build_options", "-I.", 0);
> - ret = av_opencl_init(options, NULL);
> - av_dict_free(&options);
> + ret = av_opencl_init(NULL);
> if (ret < 0)
> return ret;
> deshake->opencl_ctx.matrix_size = MATRIX_SIZE;
> diff --git a/libavutil/opencl.c b/libavutil/opencl.c
> index ca5f6d7..53eb0a5 100644
> --- a/libavutil/opencl.c
> +++ b/libavutil/opencl.c
> @@ -24,6 +24,7 @@
> #include "avstring.h"
> #include "log.h"
> #include "avassert.h"
> +#include "opt.h"
>
> #if HAVE_PTHREADS
>
> @@ -73,10 +74,23 @@ typedef struct {
> const AVClass *class;
> int log_offset;
> void *log_ctx;
> + int init_flag;
> + int platform_idx;
> + int device_idx;
> + char *build_options;
> } OpenclUtils;
>
> +#define OFFSET(x) offsetof(OpenclUtils, x)
> +
> +static const AVOption opencl_options[] = {
> + { "platform", "set platform index value", OFFSET(platform_idx), AV_OPT_TYPE_INT, {.i64=-1}, -1, INT_MAX},
> + { "device", "set device index value", OFFSET(device_idx), AV_OPT_TYPE_INT, {.i64=-1}, -1, INT_MAX},
Nit: maybe add _idx for consistency.
> + { "build_options", "build options of opencl", OFFSET(build_options), AV_OPT_TYPE_STRING, {.str="-I."}, CHAR_MIN, CHAR_MAX},
> +};
> +
> static const AVClass openclutils_class = {
> .class_name = "OPENCLUTILS",
> + .option = opencl_options,
> .item_name = av_default_item_name,
> .version = LIBAVUTIL_VERSION_INT,
> .log_level_offset_offset = offsetof(OpenclUtils, log_offset),
> @@ -317,6 +331,33 @@ void av_opencl_free_device_list(AVOpenCLDeviceList **device_list)
> av_freep(device_list);
> }
>
> +int av_opencl_set_option(const char *key, const char *val)
> +{
> + int ret = 0;
> + LOCK_OPENCL
> + if (!openclutils.init_flag) {
> + av_opt_set_defaults(&openclutils);
> + openclutils.init_flag = 1;
> + }
> + ret = av_opt_set(&openclutils, key, val, 0);
> + if (ret < 0) {
> + av_log(&openclutils, AV_LOG_ERROR,
> + "Error setting options: option ='%s', value = '%s'\n", key, val);
> + return ret;
> + }
you can probably skip this error message in case av_opt_set() already
does that.
> + UNLOCK_OPENCL
> + return ret;
> +}
> +
> +int av_opencl_get_option(const char *key, uint8_t **out_val)
> +{
> + int ret = 0;
> + LOCK_OPENCL
> + ret = av_opt_get(&openclutils, key, 0, out_val);
> + UNLOCK_OPENCL
> + return ret;
> +}
> +
> AVOpenCLExternalEnv *av_opencl_alloc_external_env(void)
> {
> AVOpenCLExternalEnv *ext = av_mallocz(sizeof(AVOpenCLExternalEnv));
> @@ -564,46 +605,22 @@ end:
> return ret;
> }
>
> -int av_opencl_init(AVDictionary *options, AVOpenCLExternalEnv *ext_opencl_env)
> +int av_opencl_init(AVOpenCLExternalEnv *ext_opencl_env)
As noted, you could keep the options field to allow options setting
when doing init.
> {
> int ret = 0;
> - AVDictionaryEntry *opt_build_entry;
> - AVDictionaryEntry *opt_platform_entry;
> - AVDictionaryEntry *opt_device_entry;
> - char *pos;
> LOCK_OPENCL
> if (!gpu_env.init_count) {
> - opt_platform_entry = av_dict_get(options, "platform_idx", NULL, 0);
> - opt_device_entry = av_dict_get(options, "device_idx", NULL, 0);
> - /* initialize devices, context, command_queue */
> - gpu_env.platform_idx = -1;
> - gpu_env.device_idx = -1;
> - if (opt_platform_entry) {
> - gpu_env.platform_idx = strtol(opt_platform_entry->value, &pos, 10);
> - if (pos == opt_platform_entry->value) {
> - av_log(&openclutils, AV_LOG_ERROR, "Platform index should be a number\n");
> - ret = AVERROR(EINVAL);
> - goto end;
> - }
> - }
> - if (opt_device_entry) {
> - gpu_env.device_idx = strtol(opt_device_entry->value, &pos, 10);
> - if (pos == opt_platform_entry->value) {
> - av_log(&openclutils, AV_LOG_ERROR, "Device index should be a number\n");
> - ret = AVERROR(EINVAL);
> - goto end;
> - }
> + if (!openclutils.init_flag) {
> + av_opt_set_defaults(&openclutils);
> + openclutils.init_flag = 1;
> }
> + gpu_env.device_idx = openclutils.device_idx;
> + gpu_env.platform_idx = openclutils.platform_idx;
A good idea would be to replace openclutils with the gpu_env context,
so you set the options directly on the global gpu context (which could
be renamed OpenCLContext) and you don't need to move data around.
Not blocking, but could be considered in another patch.
The main advantage would be that you could use the full opt API for
working on the context, the main issue is that you would need to
protect the access to the global context.
Maybe something like this:
void *av_opencl_get_context(void);
void av_opencl_release_context(void *opencl_context);
that you use like:
--------------8<------------->8----------------------
void *opencl_ctx = *av_opencl_get_context();
av_opt_set (opencl_ctx, "build_options", "-I -blah -blargh", 0);
av_opt_set_int (opencl_ctx, "device_idx", 42, 0);
...
av_opt_set_dict(opencl_ctx, ...);
av_opencl_release_context(opencl_ctx);
--------------8<------------->8----------------------
may be good enough.
> ret = init_opencl_env(&gpu_env, ext_opencl_env);
> if (ret < 0)
> goto end;
> }
> - /*initialize program, kernel_name, kernel_count*/
> - opt_build_entry = av_dict_get(options, "build_options", NULL, 0);
> - if (opt_build_entry)
> - ret = compile_kernel_file(&gpu_env, opt_build_entry->value);
> - else
> - ret = compile_kernel_file(&gpu_env, NULL);
> + ret = compile_kernel_file(&gpu_env, openclutils.build_options);
> if (ret < 0)
> goto end;
> if (gpu_env.kernel_code_count <= 0) {
> @@ -657,6 +674,10 @@ void av_opencl_uninit(void)
> }
> release_device_list(&gpu_env.device_list);
> end:
> + if ((gpu_env.init_count <= 0) && (gpu_env.kernel_count <= 0)) {
> + av_freep(&openclutils.build_options);
> + openclutils.init_flag = 0;
> + }
why is this needed?
> UNLOCK_OPENCL
> }
>
> diff --git a/libavutil/opencl.h b/libavutil/opencl.h
> index 5c19cf1..c068fa7 100644
> --- a/libavutil/opencl.h
> +++ b/libavutil/opencl.h
> @@ -94,6 +94,38 @@ AVOpenCLDeviceList *av_opencl_get_device_list(void);
> void av_opencl_free_device_list(AVOpenCLDeviceList **device_list);
>
> /**
> +* Currently, the only accepted option are "buildoptions", "platform", "device".
> +*
> +* build_options:used to set options to compile registered kernels code. See reference "OpenCL
> +* Specification Version: 1.2 chapter 5.6.4".
> +*
> +* platform: index of platform in device list
> +*
> +* device: index of device in device list
> +*
> +* @param key key of options
> +* @param val value of options
> +* @return >=0 on success, a negative error code in case of failure
> +*/
> +int av_opencl_set_option(const char *key, const char *val);
/**
* Set option in the global OpenCL context.
*
* This options affect the operation performed by the next
* av_opencl_init() operation.
*
* The currently accepted options are:
* - build_options: set options to compile registered kernels code
* - platform: set index of platform in device list
* - device: set index of device in device list
*
* See reference "OpenCL Specification Version: 1.2 chapter X.Y.Z".
*
* @param key option key
* @param val option value
* @return >=0 on success, a negative error code in case of failure
* @see av_opencl_get_option()
*/
int av_opencl_set_option(const char *key, const char *val);
> +
> +/**
> +* Currently, the only accepted option are "buildoptions", "platform", "device".
> +*
> +* build_options:used to set options to compile registered kernels code. See reference "OpenCL
> +* Specification Version: 1.2 chapter 5.6.4".
> +*
> +* platform: index of platform in device list
> +*
> +* device: index of device in device list
> +*
> +* @param key key of options
> +* @param out_val value of the option will be written here
> +* @return >=0 on success, a negative error code in case of failure
> +*/
> +int av_opencl_get_option(const char *key, uint8_t **out_val);
/**
* Get option value from the global OpenCL context.
*
* @param key option key
* @param out_val pointer to location where option value will be
* written, must be freed with av_freep()
* @return >=0 on success, a negative error code in case of failure
* @see av_opencl_set_option()
*/
int av_opencl_get_option(const char *key, uint8_t **out_val);
> +
> +/**
> * Allocate OpenCL external environment.
> *
> * It must be freed with av_opencl_free_external_env().
> @@ -125,16 +157,11 @@ int av_opencl_register_kernel_code(const char *kernel_code);
> * Initialize the run time OpenCL environment and compile the kernel
> * code registered with av_opencl_register_kernel_code().
> *
> - * Currently, the only accepted option is "build_options", used to set
> - * options to compile registered kernels code. See reference "OpenCL
> - * Specification Version: 1.2 chapter 5.6.4".
> - *
> - * @param options dictionary of key/value options
> * @param ext_opencl_env external OpenCL environment, created by an
> * application program, ignored if set to NULL
> * @return >=0 on success, a negative error code in case of failure
> */
> - int av_opencl_init(AVDictionary *options, AVOpenCLExternalEnv *ext_opencl_env);
> + int av_opencl_init(AVOpenCLExternalEnv *ext_opencl_env);
>
> /**
> * Create kernel object in the specified kernel environment.
> diff --git a/libavutil/version.h b/libavutil/version.h
> index 7d1ab9c..6531397 100644
> --- a/libavutil/version.h
> +++ b/libavutil/version.h
> @@ -75,7 +75,7 @@
> */
>
> #define LIBAVUTIL_VERSION_MAJOR 52
> -#define LIBAVUTIL_VERSION_MINOR 25
> +#define LIBAVUTIL_VERSION_MINOR 26
> #define LIBAVUTIL_VERSION_MICRO 100
>
> #define LIBAVUTIL_VERSION_INT AV_VERSION_INT(LIBAVUTIL_VERSION_MAJOR, \
> --
> 1.7.11.msysgit.1
--
FFmpeg = Funny and Fierce Majestic Puristic Earthshaking Gymnast
More information about the ffmpeg-devel
mailing list