[FFmpeg-devel] [PATCH] Set loglevel with strings in ffmpeg.c.

[Stefano Sabatini]

On date Friday 2009-06-12 00:46:12 +0200, Michael Niedermayer encoded:
> On Fri, Jun 12, 2009 at 12:27:18AM +0200, Stefano Sabatini wrote:
> > On date Thursday 2009-06-11 18:48:59 -0300, Ramiro Polla encoded:
> > > On Thu, Jun 11, 2009 at 6:30 PM, Michael Niedermayer<michaelni at gmx.at> wrote:
> > > > On Thu, Jun 11, 2009 at 02:08:41PM -0300, Ramiro Polla wrote:
> > > >> $subj makes it more user-friendly, i.e.:
> > > >> ffmpeg -loglevel info -i input output
> > > >>
> > > >> Ramiro Polla
> > > >
> > > >> ?ffmpeg.c | ? 34 ++++++++++++++++++++++++++++++++--
> > > >> ?1 file changed, 32 insertions(+), 2 deletions(-)
> > > >> b69c3b8cd2469ff83024c20d90ce7b5da1fe5117 ?loglevel.diff
> > > >
> > > > iam not sure if this is worth it but i am sure ffmpeg.c is the wrong place
> > > > ffplay & ffserver should also support that
> > > 
> > > libavutil/options.c maybe?
> > 
> > In order to set an AVOption, that option needs to correspond to a
> > field of a context with an AVClass, which is not the case, so
> > cmdutils.[hc] is the place.
> 
> exactly
> 
> 
> > 
> > > I don't want to spend much more time on this patch, but maybe Stefano
> > > wants to pick it up =)
> > 
> > I always enjoy to improve the interface and move code from apps to
> > utils code ;-), check attached.
> 
> i sometimes feel that you enjoy driving me crazy ;)

That's not intentional, but looks like I'm very talented for it ;-).

> anyway the patch is ok when you split it into cosmetic move
> and functional changes also minor comment below

Yep.
  
> [...]
> > Index: cmdutils.c
> > ===================================================================
> > --- cmdutils.c	(revision 19156)
> > +++ cmdutils.c	(working copy)
> > @@ -213,6 +213,42 @@
> >      return 0;
> >  }
> >  
> > +int opt_loglevel(const char *opt, const char *arg)
> > +{
> > +    const struct { const char *name; int level; } const log_levels[] = {
> > +        { "quiet"  , AV_LOG_QUIET   },
> > +        { "panic"  , AV_LOG_PANIC   },
> > +        { "fatal"  , AV_LOG_FATAL   },
> > +        { "error"  , AV_LOG_ERROR   },
> > +        { "warning", AV_LOG_WARNING },
> > +        { "info"   , AV_LOG_INFO    },
> > +        { "verbose", AV_LOG_VERBOSE },
> > +        { "debug"  , AV_LOG_DEBUG   },
> > +    };
> > +    char *endptr;
> > +    int i, level;
> > +
> > +    for (i = 0; i < FF_ARRAY_ELEMS(log_levels); i++) {
> > +        if (!strcmp(log_levels[i].name, arg)) {
> > +            level = log_levels[i].level;
> > +            goto set_log_level;
> > +        }
> > +    }
> > +
> > +    level = strtol(arg, &endptr, 10);
> > +    if (*endptr) {
> > +        fprintf(stderr, "Invalid loglevel \"%s\". "
> > +                "Possible levels are numbers or:\n", arg);
> > +        for (i = 0; i < FF_ARRAY_ELEMS(log_levels); i++)
> > +            fprintf(stderr, "\"%s\"\n", log_levels[i].name);
> > +        exit(1);
> > +    }
> > +
> > +set_log_level:
> > +    av_log_set_level(level);
> > +    return 0;
> > +}
> 
> as much as i like gotos, this one can be done by just replacing the
> goto with the 2 lines without increasing the number of lines at all

Check attached.
-- 
FFmpeg = Fast and Freak Mega Prodigious Extravagant Goblin
-------------- next part --------------
A non-text attachment was scrubbed...
Name: ffmpeg-loglevel.patch
Type: text/x-diff
Size: 2425 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090615/714766f8/attachment.patch>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: move-loglevel-to-cmdutils.patch
Type: text/x-diff
Size: 3519 bytes
Desc: not available
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20090615/714766f8/attachment-0001.patch>