[FFmpeg-devel] [PATCH] Implement a cmdutils functions for listing available elements

[Stefano Sabatini]

On date Wednesday 2009-11-18 23:04:38 +0100, Michael Niedermayer encoded:
> On Wed, Nov 18, 2009 at 09:09:34PM +0100, Stefano Sabatini wrote:
[...]
> > Would be acceptable to define the function only when defining the
> > first handler? I avoided that in the first time because I was trying to
> > minimize each patch.
> 
> The goal is not to minimize each patch, that way each patch would just add one
> byte or remove one byte.
> The goal is to sanely split patches so they can be reviewed with reasonable
> effort.
> One cannot review a function that does not work at all because most of its
> code will be added in several later patches
> 
[...] 
> > having one option would allow tricks of the kind: -list help or -list
> > formats+codecs -list all.
> 
> and for what would these tricks be good for? (except of course making the
> thing much more complex)

For example having -list all would probably avoid the need for keeping
show_formats (or at least it may significantly simplify it).

But then I want to avoid to pollute the option space, having one
extensible option is better than many ones.
 
> > Also if you're trying to keep the -formats option we can do like in
> > the attached patch.
> > 
> > First patch makes the bitstreams listing in -formats show one entry per
> > line, makes it more readable and consistent with the other listing.
> > 
> > Regards.
> > -- 
> > FFmpeg = Forgiving & Faithless Mystic Pitiless Eager Ghost
> 
> >  cmdutils.c |    2 +-
> >  1 file changed, 1 insertion(+), 1 deletion(-)
> > 72784f900330aa0105d9d067b94d7a180be28e37  print-bsf-one-per-line.patch
> > Index: ffmpeg/cmdutils.c
> > ===================================================================
> > --- ffmpeg.orig/cmdutils.c	2009-11-18 20:59:58.000000000 +0100
> > +++ ffmpeg/cmdutils.c	2009-11-18 21:03:14.000000000 +0100
> > @@ -532,7 +532,7 @@
> >  
> >      printf("Bitstream filters:\n");
> >      while((bsf = av_bitstream_filter_next(bsf)))
> > -        printf(" %s", bsf->name);
> > +        printf("%s\n", bsf->name);
> >      printf("\n");
> >  
> >      printf("Supported file protocols:\n");
> 
> ok

Applied.

> remainder rejected
> 
> look we have code like
> function(){
>     print bitstreams
>     print codecs
> }
> 
> your patch should look like:
> -function(){
> +foo(){
>     print bitstreams
> +}
> +bar(){
>     print codecs
> }
> 
> anything else is rejected, please _try_ to change code in a way that is
> reviewable, all these complete rewrites because you split a function in 2
> are completely unacceptable and always rejected without review!

Tried this approach.

Regards.
-- 
FFmpeg = Frenzy Faithless Marvellous Powered Elected Gadget
-------------- next part --------------
A non-text attachment was scrubbed...
Name: impl-opt-list.patch
Type: text/x-diff
Size: 4279 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20091118/6e2fbd50/attachment.patch>