[FFmpeg-devel] [PATCH] add ME_T/ESA to avcodec.h

[Michael Niedermayer]

On Fri, Jun 06, 2008 at 01:49:04AM +0100, Robert Swain wrote:
> 2008/6/5 Michael Niedermayer <michaelni at gmx.at>:
> > On Thu, Jun 05, 2008 at 10:27:46AM -0700, Baptiste Coudurier wrote:
> >> Michael Niedermayer wrote:
> >> > On Thu, Jun 05, 2008 at 09:42:06AM +0100, Robert Swain wrote:
> >> >> 2008/6/5 Michael Niedermayer <michaelni at gmx.at>:
> >> >>> On Wed, Jun 04, 2008 at 11:55:23PM +0100, Robert Swain wrote:
> >> >>>> On 4 Jun 2008, at 21:56, Baptiste Coudurier wrote:
> >> >>>>> Michael Niedermayer wrote:
> >> >>>>>> Me too, ive a half finished per codec defaults change locally ...
> >> >>>>> Awesome !
> >> >>>> I made some patches for this that didn't quite work that used the
> >> >>>> 'wrong' approach. Baptiste said they should use AVOption instead but I
> >> >>>> was unaware of this API at the time and by the time I'd finished
> >> >>>> writing it only to be told it would need rewriting, I didn't have the
> >> >>>> motivation to fix it up. :)
> >> >>>>
> >> >>>> If you want to look at what I did, they are patches 0001-0004* here:
> >> >>>>
> >> >>>> http://www.swains.plus.com/superdump/ffmpeg/patches/
> >> >>>>
> >> >>>> I would appreciate if you made what you've done available too as I may
> >> >>>> have some criticisms! :)
> >> >>> Code below,
> >> >>> just put -vpre anime-hq on the command line and have a file with the path
> >> >>> ~/.ffmpeg/mpeg4-anime-hq.ffpreset
> >> >>> with all your AVOptoion key=value stuff in it
> >> >>> similar for other codecs.
> >> >>>
> >> >>> minor known bug, -vpre must be after -vcodec
> >> >>>
> >> >>> Iam planning to commit the code if there are no objections ...
> >> >> This is presets code, not per codec defaults, but it's good. :)
> >> >
> >> > Well, defaults are one preset IMHO
> >> > It would be easy just to do the equivalent of '-vpre default' when no
> >> > other -vpre is specified.
> >> > I suspect that would be much simpler than your code, also defaults would
> >> > be in a user editable file and not bloat the library itself ...
> >> >
> >>
> >> I had the idea that some codecs would need to override default
> >> AVOptions, more specifically min and max ranges, like libx264 or dnxhd
> >> for qmin/qmax.
> >>
> >> For example DNxHD supports qmax up to 1024, and FFmpeg cannot use this
> >
> >> possibility atm (needed to encode /dev/random for example), should I add
> >> bump qmax max value to 1024 ?
> >
> > yes
> >
> >
> >>
> >> We might add a mechanism where codecs could override ranges and default
> >> value.
> >
> > yes ...
> >
> > but ranges depend on more than just the codec, they also depend on other
> > values, not every combination is always allowed.
> >
> > Simply adding a AVCodec.check_values() could check all that ...
> > the presets suggested by me could handle defaults ...
> 
> We (M?ns, Baptiste and myself) seem to agree that hardcoding default
> values as well as the separate issue of extending to handle hardcoded
> ranges and whatever else people think might be useful is preferable to
> user-editable defaults. I think hardcoding is better because it avoids
> any potential issues with users having altered the default values and
> blaming FFmpeg as a consequence. 

Quite vague argument, i now could try to guess what you mean exactly and
reply but i think it would be better if you would elaborate on
"any potential issues"

Also a user can always edit the source, and i wouldnt bet that distros
and people compiling windows binaries for the masses wont do it ...
Its a lot easier to ask a user to post the default-x264.ffpreset file
or check that it matches svn in these cases

Besides if defaults would be in a editable file, it would become part of
bugreports.html to provide that file if it differs from svn and of faq
to check that file first, we could even print its content with a sufficient
verbose level and ask peopl to use that in bugreports

mplayer also supports user editable configuration files and alot of
people use it and are happy with it.
I really think you guys are going in the completely wrong direction
here
adding code duplication to _avoid_ supporting a usefull feature.

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

It is dangerous to be right in matters on which the established authorities
are wrong. -- Voltaire
-------------- 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-devel/attachments/20080607/6bc46510/attachment.pgp>