[FFmpeg-devel] [PATCH 1/2] libavutil/libavfilter: opencl wrapper based on comments on 20130330
Wei Gao
highgod0401 at gmail.com
Sun Mar 31 09:36:35 CEST 2013
Hi,
Stefano Sabatini, thanks for your reviewing, will fix according the comments
Thanks
Best regards
2013/3/30 Stefano Sabatini <stefasab at gmail.com>
> On date Saturday 2013-03-30 18:47:42 +0800, Wei Gao encoded:
> >
>
> > From 9f092f2e65c09a5462ed243801cf954145a547e4 Mon Sep 17 00:00:00 2001
> > From: highgod0401 <highgod0401 at gmail.com>
> > Date: Sat, 30 Mar 2013 18:28:18 +0800
> > Subject: [PATCH 1/2] opencl wrapper based on comments on 20130330
> >
> > ---
> > configure | 4 +
> > libavutil/Makefile | 3 +
> > libavutil/opencl.c | 730
> +++++++++++++++++++++++++++++++++++++++++++++++++++++
> > libavutil/opencl.h | 205 +++++++++++++++
> > 4 files changed, 942 insertions(+)
> > create mode 100644 libavutil/opencl.c
> > create mode 100644 libavutil/opencl.h
> >
> [...]
> > +int av_opencl_buffer_create(cl_mem *cl_buf, size_t cl_buf_size, int
> flags, void *host_ptr)
> > +{
> > + cl_int status;
> > + *cl_buf = clCreateBuffer(gpu_env.context, flags, cl_buf_size,
> host_ptr, &status);
> > + if (status != CL_SUCCESS) {
> > + av_log(&openclutils, AV_LOG_ERROR, "Could not create OpenCL
> buffer: %s\n", opencl_errstr(status));
> > + return AVERROR_EXTERNAL;
> > + }
> > + return 0;
> > +}
> > +
>
> > +void av_opencl_buffer_release(cl_mem cl_buf)
> > +{
> > + cl_int status = 0;
>
> > + if (!cl_buf)
>
> this is never true, since cl_buf is an address to the stack.
>
> > + return;
>
> > + status = clReleaseMemObject(cl_buf);
> > + if (status != CL_SUCCESS) {
> > + av_log(&openclutils, AV_LOG_ERROR, "Could not release OpenCL
> buffer: %s\n", opencl_errstr(status));
> > + }
> > +}
>
> Maybe you mean:
>
> void av_opencl_buffer_release(cl_mem *cl_buf)
> {
> ...
> status = clReleaseMemObject(*cl_buf);
> ...
> }
>
> which should be safer since doesn't leave the unchanged value of
> cl_buf in the environment.
>
> > +
> > +int av_opencl_buffer_write(cl_mem dst_cl_buf, uint8_t *src_buf, size_t
> buf_size)
> > +{
> > + cl_int status;
> > + void *mapped = clEnqueueMapBuffer(gpu_env.command_queue, dst_cl_buf,
> > + CL_TRUE,CL_MAP_WRITE, 0,
> sizeof(uint8_t) * buf_size,
> > + 0, NULL, NULL, &status);
> > +
> > + if (status != CL_SUCCESS) {
> > + av_log(&openclutils, AV_LOG_ERROR, "Could not map OpenCL
> buffer: %s\n", opencl_errstr(status));
> > + return AVERROR_EXTERNAL;
> > + }
> > + memcpy(mapped, src_buf, buf_size);
> > +
> > + status = clEnqueueUnmapMemObject(gpu_env.command_queue, dst_cl_buf,
> mapped, 0, NULL, NULL);
> > + if (status != CL_SUCCESS) {
> > + av_log(&openclutils, AV_LOG_ERROR, "Could not unmap OpenCL
> buffer: %s\n", opencl_errstr(status));
> > + return AVERROR_EXTERNAL;
> > + }
> > + return 0;
> > +}
> > +
> > +int av_opencl_buffer_read(uint8_t *dst_buf, cl_mem src_cl_buf, size_t
> buf_size)
> > +{
> > + cl_int status;
> > + void *mapped = clEnqueueMapBuffer(gpu_env.command_queue, src_cl_buf,
> > + CL_TRUE,CL_MAP_READ, 0, buf_size,
> > + 0, NULL, NULL, &status);
> > +
> > + if (status != CL_SUCCESS) {
> > + av_log(&openclutils, AV_LOG_ERROR, "Could not map OpenCL
> buffer: %s\n", opencl_errstr(status));
> > + return AVERROR_EXTERNAL;
> > + }
> > + memcpy(dst_buf, mapped, buf_size);
> > +
> > + status = clEnqueueUnmapMemObject(gpu_env.command_queue, src_cl_buf,
> mapped, 0, NULL, NULL);
> > + if (status != CL_SUCCESS) {
> > + av_log(&openclutils, AV_LOG_ERROR, "Could not unmap OpenCL
> buffer: %s\n", opencl_errstr(status));
> > + return AVERROR_EXTERNAL;
> > + }
> > + return 0;
> > +}
> > +
> > +int av_opencl_buffer_write_image(cl_mem dst_cl_buf, size_t
> cl_buffer_size,
> > + uint8_t **src_data, int
> *plane_size, int plane_num, int offset)
> > +{
> > + int i, buffer_size = 0;
> > + uint8_t *temp;
> > + cl_int status;
> > + void *mapped;
> > + if ((unsigned int)plane_num > 8) {
> > + return AVERROR(EINVAL);
> > + }
> > + for (i = 0;i < plane_num;i++) {
> > + buffer_size += plane_size[i];
> > + }
> > + if (buffer_size > cl_buffer_size) {
> > + av_log(&openclutils, AV_LOG_ERROR, "Cannot write image to
> OpenCL buffer: buffer too small\n");
> > + return AVERROR(EINVAL);
> > + }
> > + mapped = clEnqueueMapBuffer(gpu_env.command_queue, dst_cl_buf,
> > + CL_TRUE,CL_MAP_WRITE, 0,
> buffer_size + offset,
> > + 0, NULL, NULL, &status);
> > + if (status != CL_SUCCESS) {
> > + av_log(&openclutils, AV_LOG_ERROR, "Could not map OpenCL
> buffer: %s\n", opencl_errstr(status));
> > + return AVERROR_EXTERNAL;
> > + }
> > + temp = mapped;
> > + temp += offset;
> > + for (i = 0; i < plane_num; i++) {
> > + memcpy(temp, src_data[i], plane_size[i]);
> > + temp += plane_size[i];
> > + }
> > + status = clEnqueueUnmapMemObject(gpu_env.command_queue, dst_cl_buf,
> mapped, 0, NULL, NULL);
> > + if (status != CL_SUCCESS) {
> > + av_log(&openclutils, AV_LOG_ERROR, "Could not unmap OpenCL
> buffer: %s\n", opencl_errstr(status));
> > + return AVERROR_EXTERNAL;
> > + }
> > + return 0;
> > +}
> > +
> > +int av_opencl_buffer_read_image(uint8_t **dst_data, int *plane_size,
> int plane_num,
> > + cl_mem src_cl_buf, size_t
> cl_buffer_size)
> > +{
> > + int i,buffer_size = 0,ret = 0;
> > + uint8_t *temp;
> > + void *mapped;
> > + cl_int status;
> > + if ((unsigned int)plane_num > 8) {
> > + return AVERROR(EINVAL);
> > + }
> > + for (i = 0;i < plane_num;i++) {
> > + buffer_size += plane_size[i];
> > + }
> > + if (buffer_size > cl_buffer_size) {
> > + av_log(&openclutils, AV_LOG_ERROR, "Cannot write image to CPU
> buffer: OpenCL buffer too small\n");
> > + return AVERROR(EINVAL);
> > + }
> > + mapped = clEnqueueMapBuffer(gpu_env.command_queue, src_cl_buf,
> > + CL_TRUE,CL_MAP_READ, 0,
> buffer_size,
> > + 0, NULL, NULL, &status);
> > +
> > + if (status != CL_SUCCESS) {
> > + av_log(&openclutils, AV_LOG_ERROR, "Could not map OpenCL
> buffer: %s\n", opencl_errstr(status));
> > + return AVERROR_EXTERNAL;
> > + }
> > + temp = mapped;
> > + if (ret >= 0) {
> > + for (i = 0;i < plane_num;i++) {
> > + memcpy(dst_data[i], temp, plane_size[i]);
> > + temp += plane_size[i];
> > + }
> > + }
> > + status = clEnqueueUnmapMemObject(gpu_env.command_queue, src_cl_buf,
> mapped, 0, NULL, NULL);
> > + if (status != CL_SUCCESS) {
> > + av_log(&openclutils, AV_LOG_ERROR, "Could not unmap OpenCL
> buffer: %s\n", opencl_errstr(status));
> > + return AVERROR_EXTERNAL;
> > + }
> > + return 0;
> > +}
> > +
> > diff --git a/libavutil/opencl.h b/libavutil/opencl.h
> > new file mode 100644
> > index 0000000..5fa835f
> > --- /dev/null
> > +++ b/libavutil/opencl.h
> [...]
> > +/**
> > + * Create OpenCL buffer, the buffer is used to save the data which is
> used or created by OpenCL kernel.
> > + *
> > + * @param cl_buf the pointer of OpenCL buffer, call
> av_opencl_buffer_release to release the buffer.
> > + * @param cl_buf_size size in bytes of the OpenCL buffer to create
> > + * @param flags the flags which used to control buffer
> attribute
>
> flags used to control buffer attributes
>
> > + * @param host_ptr the host pointer of OpenCL buffer
> > + * @return >=0 on success, a negative error code in case of failure
> > + */
> > +int av_opencl_buffer_create(cl_mem *cl_buf, size_t cl_buf_size, int
> flags, void *host_ptr);
> > +
>
> > +/**
> > + * Read data from OpenCL buffer to memory buffer.
> > + *
> > + * @param src_buf pointer to destination buffer (CPU memory)
> > + * @param dst_cl_buf pointer to source OpenCL buffer
> > + * @param buf_size size in bytes of the source and destination
> buffers
> > + * @return >=0 on success, a negative error code in case of failure
> > + */
> > +int av_opencl_buffer_write(cl_mem dst_cl_buf, uint8_t *src_buf, size_t
> buf_size);
>
> Inconsisent documentation and mismatch between documentation and
> implementation, the current documentation is backward.
>
> It should be something like:
> /**
> * Write OpenCL buffer with data from src_buf.
> *
> * @param dst_cl_buf pointer to OpenCL destination buffer
> * @param src_buf pointer to source buffer
> * @param buf_size size in bytes of the source and destination
> buffers
> * @return >=0 on success, a negative error code in case of failure
> */
> int av_opencl_buffer_write(cl_mem dst_cl_buf, uint8_t *src_buf, size_t
> buf_size);
>
> > +
> > +/**
> > + * Read data from OpenCL buffer to memory buffer.
> > + *
> > + * @param dst_buf pointer to destination buffer (CPU memory)
> > + * @param src_cl_buf pointer to source OpenCL buffer
> > + * @param buf_size size in bytes of the source and destination
> buffers
> > + * @return >=0 on success, a negative error code in case of failure
> > + */
> > +int av_opencl_buffer_read(uint8_t *dst_buf, cl_mem src_cl_buf, size_t
> buf_size);
> > +
>
> > +/**
> > + * Write data from memory to OpenCL buffer. Src is frames
> buffer(data[0],data[1]...), dst is OpenCL buffer.
> > + *
> > + * @param dst_cl_buf the pointer of destination buffer
> (GPU memory).
> > + * @param cl_buffer_size size in bytes of OpenCL buffer
>
> > + * @param src_data picture or audio data for
> each plane
>
> why do you mention audio data if the function is about image data?
>
> Unless you want to generalize the function, in that case it could be
> named:
> av_opencl_buffer_write_frame()
>
> Also in that case you should drop the limitation to 8 planes, since
> audio data can contain an arbitrary number of channels (and thus
> planes, in case of planar audio).
>
> > + * @param plane_size size in bytes of each plane
> > + * @param plane_num the input plane number, the value of
> this parameter should be 1 to 8
> > + * @param offset the offset of OpenCL buffer start
> position
> > + * @return >=0 on success, a negative error code in case of failure
> > + */
> > +int av_opencl_buffer_write_image(cl_mem dst_cl_buf, size_t
> cl_buffer_size,
> > + uint8_t **src_data, int
> *plane_size, int plane_num, int offset);
>
> /**
> * Write image data from memory to OpenCL buffer.
> *
> * The source must be an array of pointers to image plane buffers.
> *
> * @param dst_cl_buf pointer to destination OpenCL buffer
> * @param dst_cl_buf_size size in bytes of OpenCL buffer
> * @param src_data array of pointers to source plane buffers
> * @param src_plane_sizes array of sizes in bytes of the source plane
> buffers
> * @param src_plane_num number of source image planes
> * @param dst_cl_buf_offset the offset of the OpenCL buffer start position
> * @return >=0 on success, a negative error code in case of failure
> */
> int av_opencl_buffer_write_image(cl_mem dst_cl_buf, size_t dst_cl_buf_size,
> uint8_t **src_data, int *src_plane_size,
> int src_plane_num, int dst_cl_offset);
>
> dst_cl_offset could be moved just after the other dst_ parameters.
>
> > +
> > +/**
> > + * Read frame data from OpenCL buffer to frame buffer, src buffer is
> OpenCL buffer, dst buffer is frame buffer(data[0],data[1]....).
> > + *
> > + * @param dst_data picture or audio data for each
> plane
> > + * @param plane_size size in bytes of each plane
> > + * @param plane_num the input plane number, the value
> of this parameter should be 1 to 8
> > + * @param src_cl_inbuf the pointer of OpenCL buffer.
> > + * @param cl_buffer_size size in bytes of OpenCL buffer
> > + * @return >=0 on success, a negative error code in case of failure
> > + */
> > +int av_opencl_buffer_read_image(uint8_t **dst_data, int *plane_size,
> int plane_num,
> > + cl_mem src_cl_buf, size_t
> cl_buffer_size);
>
> /**
> * Read image data from OpenCL buffer.
> *
> * src buffer is OpenCL buffer, dst buffer is frame
> buffer(data[0],data[1]....).
> *
> * @param dst_data array of pointers to destination plane buffers
> * @param dst_plane_sizes array of pointers to destination plane buffers
> * @param dst_plane_num number of destination image planes
> * @param src_cl_buf pointer to source OpenCL buffer
> * @param src_cl_buf_size size in bytes of OpenCL buffer
> * @return >=0 on success, a negative error code in case of failure
> */
> int av_opencl_buffer_read_image(uint8_t **dst_data, int *dst_plane_sizes,
> int plane_num,
> cl_mem src_cl_buf, size_t src_cl_buf_size);
>
> > +/**
> > + * Release OpenCL buffer.
> > + *
> > + * @param cl_buf the OpenCL buffer need to be released, this
> buffer is created in function av_opencl_buffer_create
> > + */
> > +void av_opencl_buffer_release(cl_mem cl_buf);
>
> /**
> * Release OpenCL buffer.
> *
> * @param cl_buf OpenCL buffer to release, which was previously filled
> with av_opencl_buffer_create()
> */
> void av_opencl_buffer_release(cl_mem cl_buf);
>
> [...]
> --
> FFmpeg = Frenzy & Friendly Minimal Perfectionist Exploitable Gargoyle
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
More information about the ffmpeg-devel
mailing list