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

[Stefano Sabatini]

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? Which kind of generalization do you have in mind
for transpose(), such that the function may be used in rotate90? I can
drop this patch but then I need to know how I can reuse the
transpose code in rotate90.
-- 
FFmpeg = Fundamentalist Fabulous MultiPurpose Elastic God