[FFmpeg-devel] [PATCH 3/4] Implement and use shareable ff_transpose function.

[Michael Niedermayer]

On Mon, Oct 18, 2010 at 09:45:19AM +0200, Stefano Sabatini wrote:
> On date Monday 2010-10-18 04:09:17 +0200, Michael Niedermayer encoded:
> > On Mon, Oct 18, 2010 at 01:31:22AM +0200, Stefano Sabatini wrote:
> > > On date Sunday 2010-10-17 23:54:28 +0200, Michael Niedermayer encoded:
> > > > On Sun, Oct 17, 2010 at 10:22:30PM +0200, Stefano Sabatini wrote:
> > > > > On date Sunday 2010-10-17 21:57:56 +0200, Michael Niedermayer encoded:
> > > > > > On Sun, Oct 17, 2010 at 01:23:36PM +0200, Stefano Sabatini wrote:
> > > > > > > ---
> > > > > > >  libavfilter/Makefile       |    2 +-
> > > > > > >  libavfilter/transpose.c    |  108 ++++++++++++++++++++++++++++++++++++++++++++
> > > > > > >  libavfilter/transpose.h    |   37 +++++++++++++++
> > > > > > >  libavfilter/vf_transpose.c |   88 +++---------------------------------
> > > > > > >  4 files changed, 152 insertions(+), 83 deletions(-)
> > > > > > >  create mode 100644 libavfilter/transpose.c
> > > > > > >  create mode 100644 libavfilter/transpose.h
> > > > > > 
> > > > > > The only use i can see in spliting transpose out is to do SIMD optims and
> > > > > > i dont see how this would fit in this design
> > > > > 
> > > > > See the rotate90 filter which uses ff_transpose.c, that was for
> > > > > avoiding having two filters in the same vf_ file.
> > > > 
> > > > the filters do almost the same though.
> > > > and one filter could support all cases and the other could call that filter
> > > > for nice rotate=deg / transpose user API
> > > 
> > > I played with the idea of a filter inserting another filter in the
> > > filterchain during init, unfortunately this can't be done during the
> > > init stage since the single filter has no hint regarding the graph
> > > context, and this can't be done during the configuration stage as well
> > > as at that point the query_formats configuration already happened.
> > 
> > i was thinking of using the functions of the filter, like using start/end_frame
> > ... 
> > 
> > 
> > > 
> > > So the only way I figured out to use the functionality of another
> > > filter is to simply use its code.
> > > 
> > > > I dont care strongly if its done with ff_transpose() or one filter calling the
> > > > other but i would like to keep it simple and
> > > > allow SIMD optims to be added because transpose can be done much faster with
> > > > simd. and even with C doing it blockwise might be faster due to cache issues
> > > 
> > > Anyway please let's try to not stall this patch, optimizations can be
> > > done later anyway.
> > 
> > given that this patch is just factoring a bit of code out and that everything
> > could just be implemented in one filter. I dont see anything critical on this
> > patch.
> > 
> > and iam not asking for optimizations, iam asking for code factorization to be
> > done in a way that it doesnt have to be redone when adding optimizations.
> > And the case where all code is in one filter surely is easier to optimize
> > than if its split over 2 filters with a arbitrary case of several (transpose)
> > to be split into a 3rd file.
> 
> Sorry but I have no clue regarding what do you want to achive. Why
> having transpose() in vf_transpose.c rather than in traspose.c may
> help optimizations?

you need cpu detection, you need function pointers that go in a context.
vf_transpose has a context it has a init function
transpose.c has neither.


> Which kind of generalization do you have in mind
> for transpose(),

you cant implement all of th 90 deg rotations by transpose and vflip so theres
code left that is outside transpose.c that too need cpu detect + context +
function pointers + init code. That code btw is hflip


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Good people do not need laws to tell them to act responsibly, while bad
people will find a way around the laws. -- Plato
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101018/b38df0ef/attachment.pgp>