[FFmpeg-devel] [PATCH v2] Add option to log timing

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Moritz Barsnick
> Sent: Tuesday, September 17, 2019 11:07 PM
> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH v2] Add option to log timing
> 
> On Tue, Sep 17, 2019 at 00:07:37 +0000, Soft Works wrote:
> > This commit adds two logging flags: 'timing' and 'datetiming'.
> 
> I like the whole idea. I haven't tested yet, but I will in a moment.
> 
> > Usage:
> > ffmpeg -loglevel +timing
> > or
> > ffmpeg -loglevel +datetiming
> 
> Have you considered what happens if you do $ ffmpeg -loglevel
> +timing+datetiming ?

It will print date + time.

> (It just seems a bit weird having one flag being a superset of another.
> I don't mind otherwise.)

You're correct, It's a superset. I found it more convenient to do it like this because:
- It would make little sense printing only the date for each line
- I wanted to avoid requiring a user to specify two separate flags to get both date and time

But I wouldn't mind changing this to independent flags

> You also need to add documentation for these options to doc/fftools-
> common-opts.texi (section "@item -loglevel").

Damn, this is going to get more work than expected ;-)

> libavutil/log.h is part of the API, IIUC. So you should bump libavutil's micro
> version, and even add an entry into doc/APIchanges.

OK.

> > +        && ((flags & AV_LOG_PRINT_TIME) || (flags &
> AV_LOG_PRINT_DATETIME)))
> > +        format_date_now(&part[4], flags & AV_LOG_PRINT_DATETIME);
> 
> According to the style of the other calls, you could express "&part[4]"
> as "part+4".

I won't argue - conformity wins of course.


Thanks for looking into this, I'll wait for other comments before updating the patch.

softworkz