[FFmpeg-devel] [PATCH]opencl: compile kernels separately

Michael Niedermayer michaelni at gmx.at
Wed Oct 30 12:01:57 CET 2013


On Wed, Oct 30, 2013 at 11:07:20AM +0800, Wei Gao wrote:
> 2013/10/30 Lenny Wang <lenny at multicorewareinc.com>
> 
> > Attached new patch after 2nd review.
> >
> > On Tue, Oct 29, 2013 at 3:24 PM, Lenny Wang <lenny at multicorewareinc.com>
> > wrote:
> > > Attached new patch has been cleaned up and modified based on previous
> > > reviewer's comments.  Any comments or questions please just let me
> > > know.
> > >
> > > On Tue, Oct 29, 2013 at 4:19 AM, Thilo Borgmann <thilo.borgmann at mail.de>
> > wrote:
> > >> Am 29.10.13 07:59, schrieb Lenny Wang:
> > >>> Currently opencl kernels in ffmpeg are compiled altogether at
> > >>> initialization stage, most of related data structures are maintained
> > within
> > >>> the opencl framework of ffmpeg.  This is very cumbersome to use and is
> > not
> > >>> efficient.  This patch uses distributed opencl programs/kernels for
> > each
> > >>> filter (or potentially any other component that uses opencl), allowing
> > >>> kernels to be compiled separately at each component's initialization
> > stage.
> > >>>
> > >>> Tests have been conducted successfully on mainstream Nvidia/AMD/Intel
> > >>> platforms with "-vf deshake=opencl=1,unsharp=opencl=1".
> > >>
> > >> Your patch contains trailing whitespaces, a lot of unnecessary and
> > unrelated
> > >> changes. Please clean it up and resubmit it.
> > >>
> > >> -Thilo
> >
> 
> Hi, I reviewed the patch, and still has some bugs on code styles.
> 
> I correct all bugs which I have found and make a patch, is it OK for you?

i suggest to split the patch into a libavutil and a libavfilter
one. This reduces the chances of breaking public API/ABI as the
code must still build (and work) with only the libavutil patch applied.

also i suggest to make sure that your new API will serve future needs
and be easy extendible not requiring major version bumps for changes.
Its likely alot less work to
design it well in the first place than to deal with these API/ABI
compatibility issues later.

old functions that are not part of the new API should be marked as
deprecated, point the reader to the new and be put under #if so they
get automatically disabled with the next major version bump.

also if the new is still meant to be experimental, it might be best
to require a --enable-experimental in configure.
In that case then, random ABI/API changes would be no problem, sadly
its too late for that for the old API, but it could be used for the
new ...

also, if its too hard to support old and new at the same time, one
"hack" would be to support enough of the old API/ABI so building
succeeds and code trying to use it gets a clear & clean error
like return AVERROR(ENOSYS) on old functions.

[...]

Thanks

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Let us carefully observe those good qualities wherein our enemies excel us
and endeavor to excel them, by avoiding what is faulty, and imitating what
is excellent in them. -- Plutarch
-------------- 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/20131030/0cc8c874/attachment.asc>


More information about the ffmpeg-devel mailing list