[PATCH] Implement non-positional parameters parsing and parametric expressions for the crop filter arguments
Hi, this greatly boosts the crop filter exressivity. Regards.
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. 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. [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Its not that you shouldnt use gotos but rather that you should write readable code and code with gotos often but not always is less readable
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? Regards.
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
participants (2)
-
Michael Niedermayer -
Stefano Sabatini