[FFmpeg-devel] [PATCH v5 2/3] fftools/opt_common: add time and datetime log flags

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Tobias Rapp
> Sent: Friday, February 7, 2025 2:22 PM
> To: ffmpeg-devel at ffmpeg.org
> Subject: Re: [FFmpeg-devel] [PATCH v5 2/3] fftools/opt_common: add
> time and datetime log flags
> 
> On 07/02/2025 12:27, Soft Works wrote:
> 
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel<ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> >> Tobias Rapp
> >> Sent: Friday, February 7, 2025 11:42 AM
> >> To:ffmpeg-devel at ffmpeg.org
> >> Subject: Re: [FFmpeg-devel] [PATCH v5 2/3] fftools/opt_common: add
> >> time and datetime log flags
> >>
> >> On 07/02/2025 08:57, softworkz wrote:
> >>
> >>> From: softworkz<softworkz at hotmail.com>
> >>>
> >>> This commit adds two logging flags: 'time' and 'datetime'.
> >>>
> >>> Usage:
> >>>
> >>> ffmpeg -loglevel +time
> >>>
> >>> or
> >>>
> >>> ffmpeg -loglevel +datetime
> >>>
> >>> Setting av_log_set_flags(0) in term_exit in ffmpeg.c prevents
> >>> timing to be printed when exiting.
> >>>
> >>> Signed-off-by: softworkz<softworkz at hotmail.com>
> >>> ---
> >>>    fftools/ffmpeg.c     |  1 +
> >>>    fftools/opt_common.c | 12 ++++++++++++
> >>>    2 files changed, 13 insertions(+)
> >>>
> >>> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> >>> index dc321fb4a2..f4c717afaa 100644
> >>> --- a/fftools/ffmpeg.c
> >>> +++ b/fftools/ffmpeg.c
> >>> @@ -130,6 +130,7 @@ static void term_exit_sigsafe(void)
> >>>
> >>>    void term_exit(void)
> >>>    {
> >>> +    av_log_set_flags(0);
> >>>        av_log(NULL, AV_LOG_QUIET, "%s", "");
> >>>        term_exit_sigsafe();
> >>>    }
> >> If I understand the purpose of AV_LOG_QUIET correctly the correct
> way
> >> would be to skip writing time/datetime information in the log
> writer
> >> itself (part of patch #1 in this patch-set) if level is <=
> >> AV_LOG_QUIET,
> >> rather than clearing the flags here.
> >>
> >> Regards,
> >> Tobias
> >
> > Hi Tobias,
> >
> > my understanding of AV_LOG_QUIET would be that when program code
> logs a message with that level, it will be printed even when the user
> has "-loglevel quiet".
> > I would wonder why such log messages shouldn't be prefixed with the
> time (normally).
> > To me it appears that the special case is rather the log invocation
> on exit which prints - well: nothing.
> >
> > I'd rather question why it is not printing the log level for quiet.
> Is it really intended or was it only done to make it possible to
> print this empty message on exit?
> >
> > Do I see it wrong? Then I'll change it of course.
> >
> > Thanks a lot for reviewing!
> >
> > sw
> 
> The documentation for AV_LOG_QUIET in log.h says "Print no output"

This is the user-facing definition. When a user configures quiet, it means that ffmpeg prints no output. But on the side of the code, specifying AV_LOG_QUIET doesn't mean "print no output" but the opposite ("print it, no matter what!").


> from grepping through code it seems the implementation matches that
> definition. The purpose of av_log(NULL, AV_LOG_QUIET, "%s", "") is
> described with the AV_LOG_SKIP_REPEATED flag, see
> https://ffmpeg.org/doxygen/trunk/group__lavu__log.html#ga6cdf5cd331b1
> 7e80e8308124f05a6db8

Hm, not sure whether this has been the original purpose of AV_LOG_QUIET. 
And one could argue that consumers of avutil might use this log level for printing non-empty messages. That's what the snippet above would have preserved working.
But the quirky part has already happened (not introducing av_log_flush() or something), so - as a whole - it can't be easily remedied anyway.


> So I suggest to just add a "(level > AV_LOG_QUIET)" clause to the
> added
> lines in forma_line(), similar to how it is done with the loglevel
> prefix in that function some lines below:
> 
> if (*print_prefix && (level > AV_LOG_QUIET) && (flags &
> (AV_LOG_PRINT_TIME | AV_LOG_PRINT_DATETIME)))
>      format_date_now(&part[4], flags & AV_LOG_PRINT_DATETIME);

Fine. Done for the next version,
thank you very much!

sw