[FFmpeg-soc] [PATCH] Implement non-positional parameters parsing and parametric expressions for the crop filter arguments

[Michael Niedermayer]

On Mon, Oct 05, 2009 at 12:37:27PM +0200, Stefano Sabatini wrote:
> On date Monday 2009-10-05 02:19:44 +0200, Michael Niedermayer encoded:
> > On Sun, Oct 04, 2009 at 10:43:21PM +0200, Stefano Sabatini wrote:
> > > Hi,
> > > 
> > > this greatly boosts the crop filter exressivity.
> > > 
> > > Regards.
> > 
> > >  doc/vfilters.texi         |   48 +++++++++++++++++-
> > >  libavfilter/vf_crop.c     |  120 ++++++++++++++++++++++++++++++++++++++++------
> > >  tests/codec-regression.sh |   10 +--
> > >  3 files changed, 155 insertions(+), 23 deletions(-)
> > > c62412ad839dea362fbf46cb89f6d671c53e35b6  crop-use-parse-opts.patch
> > > Index: ffmpeg-vfilters/ffmpeg/doc/vfilters.texi
> > > ===================================================================
> > > --- ffmpeg-vfilters.orig/ffmpeg/doc/vfilters.texi	2009-10-04 22:21:32.000000000 +0200
> > > +++ ffmpeg-vfilters/ffmpeg/doc/vfilters.texi	2009-10-04 22:38:22.000000000 +0200
> > > @@ -72,11 +72,53 @@
> > >  @section crop
> > >  
> > >  @example
> > > -./ffmpeg -i in.avi -vfilters "crop=0:0:-1:240" out.avi
> > > +./ffmpeg -i in.avi -vfilters "crop= x=0 : y=0: out_w= in_w/2" out.avi
> > >  @end example
> > >  
> > > -Crop the input video to x:y:width:height.
> > > -The -1 in width or height means that dimension in the input video.
> > > +Crop the input video.
> > > +
> > > +Parameters are specified as a list of non-positional key=value pairs
> > > +separated by ``:''.
> > 
> > i like to note that this will lead to rejection of the patch if its
> > submitted for inclusion in main svn, like it happened with vf_scale.
> > The old syntax is clean and sufficient, the new is a mess.
> 
> What you call "mess" is what I call "features".
> 
> Having an expressive syntax, which lets for example to specify: "crop
> at the center of the input area, an area with in_w/w and in_h/2" is a
> feature which I want to support, also I found that syntax quite
> readable (being explicit and thus more verbose doesn't necessarily
> mean less readable).
> 
> Then I agree that the code may be simpler, but that depends on the
> libraries.
>  
> > It is possible to allow optionally specifying the left hand sides, that
> > is "x=" but that should not be mandatory and it should not be done now
> > but after the current code that IS already fine is in main svn.
> 
> Fine to discuss that after integration in SVN.
> 

> So I assume is already OK to post a patch to put the crop filter
> in SVN?

how could it not be ok?
it of course is ...
only a review can tell us how close the code is to being acceptable


[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Breaking DRM is a little like attempting to break through a door even
though the window is wide open and the only thing in the house is a bunch
of things you dont want and which you would get tomorrow for free anyway
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-soc/attachments/20091005/b33db3c9/attachment.pgp>