[MPlayer-dev-eng] Re: [PATCH] Donald Graft's kerndeint adaptive deinterlacing filter

[Diego Biurrun]

Tobias Diedrich writes:
 > Doh. I somehow managed to forget the commit for the manpage.

OK, I just replied to this commit on mplayer-docs, but then noticed
that Tobias Diedrich is not subscribed to it, so I am repeating this
here.  The comments about filter parameter design are of general
interest, too, so please forgive me..

Tobias Diedrich CVS writes:
 > Modified Files:
 > 	mplayer.1 
 > Log Message:
 > kerndeint documentation

Thanks.  Some comments below.

 > +.PD 1

This one needs to come below the .PD 0, not above.

 > +.TP
 > +.B kerndeint[=map[:order[:thresh[:sharp[:twoway]]]]]
 > +Donald Graft's adaptive kernel deinterlacer.
 > +Deinterlaces parts of video if a configurable threshold is exceeded.
 > +.PD 0
 > +.RSs
 > +.IPs map (0 or 1)

You need to put the argument of .IPs in "" quotes unless it is only
one word.

 > +Paint pixels which exceed the threshold white (default 0).
 > +.IPs order (0 or 1)
 > +Swap fields if 1 (default 0).

Hmm, why does a flag get a value?  The other filters do it by name,
i.e.

  -vf kerndeint=order

instead of

  -vf kerndeint=1

IMHO this is easier to use and it is also more consistent with the
rest of the filters.

 > +.IPs thresh (0 - 255)
 > +Threshold (default 10).

This appears to be the most important parameter and thus the one that
will be set the most.  IMHO it should be the first parameter then, so
that you can do things like

  -vf kerndeint=100

instead of

  -vf kerndeint=0:0:100

Since your filter does not support things like -vf kerndeint=help or
setting parameter values by name, this seems particularly important to
me.

 > +.IPs sharp (0 or 1)
 > +Enable additional sharping (default 0).
 > +.IPs twoway (0 or 1)
 > +Enable twoway sharping (default 0).

It's sharpENing.

Please consider changing your filter.
Regards

Diego