[FFmpeg-devel] [PATCH 2/2] libavutil/libavfilter: deshake opencl filter based on comments on 20130401

[Stefano Sabatini]

On date Tuesday 2013-04-02 09:09:06 +0800, Wei Gao encoded:
> Hi,
> 
> Thanks for your reviewing, some questions and explanations are as follows
> 
> Thanks
> Best regards
> 
> 
> 2013/4/1 Stefano Sabatini <stefasab at gmail.com>
> 
> > On date Monday 2013-04-01 17:52:51 +0800, Wei Gao encoded:
> > >
> >
> > > From 77b3811b748129c8018216989f5e9781d37d7ea0 Mon Sep 17 00:00:00 2001
> > > From: highgod0401 <highgod0401 at gmail.com>
> > > Date: Mon, 1 Apr 2013 17:48:34 +0800
> > > Subject: [PATCH 2/2] deshake opencl filter based on comments on 20130401
> > >
> >
> > > +        av_log(ctx, AV_LOG_ERROR, "error %s %d\n", "clSetKernelArg",
> > status );                               \
> >
> > av_log(ctx, AV_LOG_ERROR, "cannot set kernel argument: %d\n", status );
> >                             \
> >
> > or even better, export the error string API so you can put the OpenCL
> > error (even if I noted it would be much better if the OpenCL lib
> > supported such API).
> >

> should I add the api this patch or later? I think it just make static const
> char *opencl_errstr(cl_int status) as a api

As you prefer.

You may have to simply do:
opencl_errstr() -> av_opencl_errstr()

and add a declaration to libavutil/opencl.h.

or maybe -> av_opencl_strerror() for consistency with the standard
strerror().

> 
> >
> > > +        return AVERROR_EXTERNAL;
> >                                       \
> > > +    }
> > > +
> >

> > > +
> > > +#if CONFIG_OPENCL
> > > +
> > > +typedef struct {
> > > +    size_t buf_size;
> > > +    cl_mem cl_buf;
> > > +} OpenCLFrameBuf;
> >
> > For simplicity sake you should probably avoid this struct, and just
> > put the field in the context below.
> >
> > cl_mem cl_inbuf;
> > size_t cl_inbuf_size;
> >

> Can I add this  OpenCLFrameBuf to avfilter context? because all the opencl
> filters will use  OpenCLFrameBuf cl_inbuf and
> OpenCLFrameBuf cl_outbuf, and can I add a opencl flag to avfilter context?

> because it is can make the opencl share buffer operation easily without add
> api to opencl lib. share buffer means that if the current filter check that
> the previous filter is opencl filter, then it will use the OpenCLFrameBuf
> of pervious filter as a input( for image data), if the current filter check
> that the next filter is opencl filter, then it can avoid copy the data to
> CPU memory, it can save the time of copy data from CPU and GPU.

I don't think we should mess with the AVFilterContext this way, there
should be better ways to mark a buffer as an OpenCL buffer.

Also keep in mind that when you request a frame, the frame request is
passed to the next filter. If the next filter is an OpenCL filter, it
should return an OpenCL buffer, which could be marked such that the
receiving filter knows that it's an OpenCL buffer and can avoid
GPU<->CPU memcpy operations.
-- 
FFmpeg = Fundamentalist and Free Muttering Prodigious Earthshaking Geisha