[FFmpeg-devel] [PATCH 2/3] libavutil/libavfilter: add spec opencl device APIs 20130410
Stefano Sabatini
stefasab at gmail.com
Wed Apr 10 16:51:36 CEST 2013
On date Wednesday 2013-04-10 22:22:04 +0800, Wei Gao encoded:
> 2013/4/10 Stefano Sabatini <stefasab at gmail.com>
>
> > 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.
>
> Can I add the opencl.texi in a later patch? thanks
@chapter OpenCL Options
When FFmpeg is configured with @code{--enable-opencl}, it is possible
to set the options to set in the global OpenCL context. The list of
supported options follows:
@table @option
@item build_options
...
@item platform_idx
...
@item device_idx
...
@end table
...
Then you need to add an include in doc/all_components.texi,
ffmpeg-utils.texi and libavutil.texi. Ideally we should merge these
files in a single utils.texi, so all libavutil doc is contained in a
single file.
>
> >
> >
> > --
> > > 1.7.11.msysgit.1
> > --
> > FFmpeg = Funny and Fierce Majestic Puristic Earthshaking Gymnast
> > _______________________________________________
> > ffmpeg-devel mailing list
> > ffmpeg-devel at ffmpeg.org
> > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
> >
> From 0f4bd860a835c414f1c4bda14539e60a726c0668 Mon Sep 17 00:00:00 2001
> From: highgod0401 <highgod0401 at gmail.com>
> Date: Wed, 10 Apr 2013 22:18:54 +0800
> Subject: [PATCH 2/3] add spec opencl device APIs 20130410 new
>
> ---
> libavfilter/deshake_opencl.c | 5 +--
> libavutil/opencl.c | 76 +++++++++++++++++++++++++-------------------
> libavutil/opencl.h | 38 ++++++++++++++++++----
> libavutil/version.h | 2 +-
> 4 files changed, 78 insertions(+), 43 deletions(-)
>
> 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 2e5797e..ddcbf50 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_idx", "set platform index value", OFFSET(platform_idx), AV_OPT_TYPE_INT, {.i64=-1}, -1, INT_MAX},
> + { "device_idx", "set device index value", OFFSET(device_idx), AV_OPT_TYPE_INT, {.i64=-1}, -1, INT_MAX},
> + { "build_options", "build options of opencl", OFFSET(build_options), AV_OPT_TYPE_STRING, {.str="-I."}, CHAR_MIN, CHAR_MAX},
nit: weird align
> +};
> +
> 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),
> @@ -311,6 +325,29 @@ 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);
> + UNLOCK_OPENCL
> + return ret;
> +}
> +
> +
Nit+++: just one empty line is enough
> +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));
> @@ -561,47 +598,22 @@ end:
> return ret;
> }
>
> -int av_opencl_init(AVDictionary *options, AVOpenCLExternalEnv *ext_opencl_env)
> +int av_opencl_init(AVOpenCLExternalEnv *ext_opencl_env)
> {
> int ret = 0;
> - AVDictionaryEntry *opt_build_entry;
> - AVDictionaryEntry *opt_platform_entry;
> - AVDictionaryEntry *opt_device_entry;
> - char *pos;
> - AVOpenCLDeviceList *device_list;
> 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;
> 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) {
We should probably free openclutils with av_opt_free() at some point,
but I can't see a safe way to do it. Ideally if you set it once in the
global context, build_options should not be reset when doing
av_opencl_init() again. A possibility would be to have a dedicated
function av_opencl_free() to free global data, but looks somehow
awkward for the user.
You may add a note somewhere in the code:
FIXME: free openclutils context
unless someone has a better idea.
> diff --git a/libavutil/opencl.h b/libavutil/opencl.h
> index a34b1b7..1dce028 100644
> --- a/libavutil/opencl.h
> +++ b/libavutil/opencl.h
> @@ -97,6 +97,37 @@ int av_opencl_get_device_list(AVOpenCLDeviceList **device_list);
> void av_opencl_free_device_list(AVOpenCLDeviceList **device_list);
>
> /**
> + * 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 5.6.4".
> + *
> + * @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);
> +
> +/**
> + * 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().
> @@ -128,16 +159,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);
LGTM otherwise, thanks.
--
FFmpeg = Frenzy and Fostering Minimal Pitiless Eager Gymnast
More information about the ffmpeg-devel
mailing list