[MPlayer-cvslog] r27650 - in trunk: DOCS/man/en/mplayer.1 DOCS/man/fr/mplayer.1 libvo/vo_png.c

[Uoti Urpala]

On Sat, 2008-09-20 at 15:35 +0200, Benjamin Zores wrote:
> Uoti Urpala wrote:
> >> Log:
> >> add outdir sub-option to vo png
> > 
> >> +#define BUFLENGTH 512
> > 
> >> +static void png_mkdir(char *buf, int verbose) {
> > 
> > IMO it would  be preferable to require the directory to exist rather
> > than add all this filesystem code to an individual VO. You could as well
> > allow it to be an arbitrary filename prefix instead of a directory, so
> > with the prefix '/tmp/test' you'd get "/tmp/test00000005.png" etc.
> > 
> >> -    snprintf (buf, 100, "%08d.png", ++framenum);
> >> +    snprintf (buf, 100, "%s/%08d.png", png_outdir, ++framenum);
> > 
> > Above you define a longer BUFLENGTH, but this still uses the same 100
> > byte limit which can be low enough to cause problems in practice.
> > Avoiding a static limit here completely shouldn't be too hard.
> 
> I don't even have thinked about all of this and won't.
> I've just copy/pasted code from vo_jpeg.c so that all picture vo have 
> the same level of common features.

Making copies of a function that large is questionable. You didn't add
the subdirectory options so they still don't have the same features. I'm
not sure whether those subdirectory options are useful in practice;
however they explain having the directory creation code, which looks
like overkill just to avoid having the user create the single directory
like now in vo_png.

That BUFLENGTH vs 100 issue is not shared by vo_jpeg.

Before trying to unify the options between random MPlayer parts you
really need to think which if any of the existing designs makes sense,
as MPlayer has lots of badly designed crap that should be removed rather
than "standardized".