[Ffmpeg-devel] Changing "-vstats" option behaviour

[Stefano Sabatini]

On date Wednesday 2007-04-18 09:15:00 +0200, Benoit Fouet encoded:
> Hi,
> 
> Stefano Sabatini wrote:
> 
> [snip]
> > Hope this is OK.
> >
> > Cheers
> >   
> > ------------------------------------------------------------------------
> >
> > Index: ffmpeg.c
> > ===================================================================
> > --- ffmpeg.c	(revision 8745)
> > +++ ffmpeg.c	(working copy)
> >   
> [snip]
> > @@ -3449,6 +3432,15 @@
> >      }
> >  }
> >  
> > +static void opt_vstats_file (const char *arg)
> > +{
> > +    vstats_file = fopen(arg ,"w");
> > +    if (!vstats_file) {
> >   
> you surely meant fvstats, no ?
> this code won't even compile.

Ehr... sorry again.

> 
> > +        perror("fopen");
> > +        exit(1);
> > +    }
> > +}
> > +
> >  static void opt_video_bsf(const char *arg)
> >  {
> >      AVBitStreamFilterContext *bsfc= av_bitstream_filter_init(arg); //FIXME split name and args for filter at '='
> > @@ -3610,7 +3602,7 @@
> >      { "deinterlace", OPT_BOOL | OPT_EXPERT | OPT_VIDEO, {(void*)&do_deinterlace},
> >        "deinterlace pictures" },
> >      { "psnr", OPT_BOOL | OPT_EXPERT | OPT_VIDEO, {(void*)&do_psnr}, "calculate PSNR of compressed frames" },
> > -    { "vstats", OPT_BOOL | OPT_EXPERT | OPT_VIDEO, {(void*)&do_vstats}, "dump video coding statistics to file" },
> > +    { "vstats_file", HAS_ARG | OPT_EXPERT | OPT_VIDEO, {(void*)opt_vstats_file}, "dump video coding statistics to file", "file" },
> >   
> isn't it also possible to keep vstats option, and internally call
> opt_vstats_file with the right filename ?
> that way, we keep what exists and offer a new option...
> any thoughts ?

It would be possible keeping the old option and the global flag
do_vstats, setting in opt_vstats_file the flag do_vstats to 1, then
putting in do_video_stats:

if (!fvstats && do_vstats) {
    \\ do_vstats is setted but not the file where to put the video_stats
    \\ open vstats_HHMMSS.log and get the fvstats file handler point to it 
    char filename[40];
    time_t today2;
    struct tm *today;

    today2 = time(NULL);
    today = localtime(&today2);
    snprintf(filename, sizeof(filename), "vstats_%02d%02d%02d.log", today->tm_hour,
    today->tm_min,
    today->tm_sec);
   
    fvstats = fopen(filename,"w");
    if (!fvstats) {
        perror("fopen");
        exit(1);
    }
}

...

Anyway I find confusing to have two different options for this (rather
than having two ways to do a thing, it's better to keep just one
method, the more general), and the above solution isn't completely
safe (-vstats_file generic-filename -vstats will print to
generic-filename rather than to the vstats_HHMMSS.log file).

Cheers
-- 
Stefano Sabatini
Linux user number 337176 (see http://counter.li.org)
-------------- next part --------------
A non-text attachment was scrubbed...
Name: vstats_file.patch
Type: text/x-diff
Size: 3717 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20070418/9a877856/attachment.patch>