[FFmpeg-devel] [PATCH] libmpcodecs support

[Aurelien Jacobs]

On Sat, Jan 15, 2011 at 03:45:09AM +0100, Michael Niedermayer wrote:
> On Sat, Jan 15, 2011 at 02:26:40AM +0100, Aurelien Jacobs wrote:
> > On Sat, Jan 15, 2011 at 12:44:26AM +0100, Michael Niedermayer wrote:
> > > On Fri, Jan 14, 2011 at 11:34:28PM +0100, Aurelien Jacobs wrote:
> > > > On Fri, Jan 14, 2011 at 05:37:45AM +0100, Michael Niedermayer wrote:
> > > > > Hi
> > > > > 
> > > > > Attached patchset makes libavfilter support most libmpcodecs filters
> > > > > The libmpcodecs filters are practically unmodified and for keeping it
> > > > > maintainable id like to keep them unmodified.
> > > > > 
> > > > > I will commit this soon but wont enable it yet. That way others can help
> > > > > cleanup the wraper (minus code that for maintainability should stay identical
> > > > > to the original), fix bugs, rename all global functions so they wont conflict
> > > > > with mplayers if they ever include this and generally help.
> > > > > 
> > > > > Please dont bikeshed this to death.
> > > > > 
> > > > > Ahh before i can commit this, libmpcodecs and subdirectories must not get
> > > > > blocked by the tab & trailing whitespace checkin scripts.
> > > > > 
> > > > 
> > > > 
> > > > > From 34072e13989ebed5e76a425b886b5b17756e6d5d Mon Sep 17 00:00:00 2001
> > > > > From: Michael Niedermayer <michaelni at gmx.at>
> > > > > Date: Fri, 14 Jan 2011 05:07:39 +0100
> > > > > Subject: [PATCH 7/7] Enable libmpcodecs support.
> > > > > 
> > > > > ---
> > > > >  libavfilter/Makefile     |   65 ++++++++++++++++++++++++++++++++++++++++++++++
> > > > >  libavfilter/allfilters.c |    1 +
> > > > >  2 files changed, 66 insertions(+), 0 deletions(-)
> > > > > 
> > > > > diff --git a/libavfilter/Makefile b/libavfilter/Makefile
> > > > > index fdb181e..0db4750 100644
> > > > > --- a/libavfilter/Makefile
> > > > > +++ b/libavfilter/Makefile
> > > > > @@ -30,6 +30,7 @@ OBJS-$(CONFIG_FREI0R_FILTER)                 += vf_frei0r.o
> > > > >  OBJS-$(CONFIG_GRADFUN_FILTER)                += vf_gradfun.o
> > > > >  OBJS-$(CONFIG_HFLIP_FILTER)                  += vf_hflip.o
> > > > >  OBJS-$(CONFIG_HQDN3D_FILTER)                 += vf_hqdn3d.o
> > > > > +OBJS-$(CONFIG_MP_FILTER)                     += vf_mp.o
> > > > >  OBJS-$(CONFIG_NOFORMAT_FILTER)               += vf_format.o
> > > > >  OBJS-$(CONFIG_NULL_FILTER)                   += vf_null.o
> > > > >  OBJS-$(CONFIG_OCV_FILTER)                    += vf_libopencv.o
> > > > > @@ -54,6 +55,70 @@ OBJS-$(CONFIG_NULLSRC_FILTER)                += vsrc_nullsrc.o
> > > > >  
> > > > >  OBJS-$(CONFIG_NULLSINK_FILTER)               += vsink_nullsink.o
> > > > >  
> > > > > +
> > > > > +OBJS += libmpcodecs/mp_image.o
> > > > > +OBJS += libmpcodecs/img_format.o
> > > > > +OBJS [...]
> > > > 
> > > > I guess all those libmpcodecs files should also be added to
> > > > OBJS-$(CONFIG_MP_FILTER). I don't think we want them compiled
> > > > unconditionally.
> > > 
> > > i had hoped other people would help but ok fixed locally
> > > 
> > > 
> > > [...]
> > > > IIUC you said that the ultimate goal is to have all those filters
> > > > converted to native libavfilter so that we can drop all the libmpcodecs
> > > > filters. If so, why does this libmpcodecs filter list includes some
> > > > filters which have already a native libavfilter version ?? Namely:
> 
> > > >   vf_cropdetect.o
> 
> quick test says same speed

Good, so no need to include it.

> > > >   vf_dsize.o
> 
> > > >   vf_rectangle.o
> 
> not in svn

I havn't compared the feature set in detail, but vf_dsize looks similar
to setdar, and vf_rectangle looks similar to drawbox.

> > > >   vf_blackframe.o
> > > >   vf_mirror.o
> > > >   vf_rotate.o
> 
> wraped libmpcodec is faster than libavfilter
> 
> testing done by looking at the fps value from ffmpeg, differences where
> significant (on the order of 5%)

That way you can already show one concrete interest about libmpcodecs
wrapper. It allows to detect some week points in libavfilter. Good !

> > > > Also vf_yvu9 is documented as "Deprecated in favor of the software
> > > > scaler". So I doubt it is a good idea to keep it.
> > > 
> > > why is it not droped from mplayer?
> > 
> > Probably because libmpcodec is mostly un-maintained since years, and
> > that the few person touching it fear removing anything, just in case
> > somebody might still be using it.
> > And maybe also to avoid breaking old scripts using "mplayer -vf yvu9".
> 
> suggest on mplayer-dev to remove it, if its really useless it should be
> removed from mplayer not just our copy

I won't waste any of my time trying to fix/cleanup/improve libmpcodecs.
If I had time to work on filters, I would spend it on native
libavfilter.
That's also what makes me fear that this libmpcodecs wrapper will rot
here forever.

> > > vf_screensot needs some API amendments on our side, so its WIP
> > 
> > So it's not ready to go in ffmpeg's repository...
> > 
> > > > I don't think there is any clean API to handle this in libavfilter.
> > > > And I don't think vf_screenshot can have any kind of usefullness inside
> > > > libavfilter.
> > > 
> > > taking screenshoots is useful
> > 
> > Yes, it is definitely useful !
> > But it has nothing to do inside libavfilter !
> > An application which want to take a screenshot will just grab the
> > current picture out of the desired sink of the filter graph and encode
> > it to png (or any codec) using lavc/lavf.
> > This is much more flexible.
> 
> how many applications support it without a explicit filter?
> does ffplay?

Most probably none.
But AFAIU vf_screenshot don't support it either in current libavfilter
framework, so it's not better in any way...

Aurel