[FFmpeg-devel] [PATCH 1/2] avutil/log: Add av_log_once() for printing a message just once per instance

[Michael Niedermayer]

On Tue, Jan 21, 2020 at 12:24:50PM +0100, Anton Khirnov wrote:
> Quoting Michael Niedermayer (2020-01-16 17:51:28)
> > Compared to ad-hoc if(printed) ... code this allows the user to disable
> > it with a flag and see all repeated messages, it is also simpler
> 
> That flag is global state - it should be deprecated and removed, not
> embedded further into the API.

When the flag is replaced by a non global solution every of its uses
would be replaced too.

Until such a non global API exists, this is the only way the user can
indicate her choice of which log messages to print.
Code should honor the existing API and user preferrance.


> 
> > 
> > TODO: APIChanges & version bump
> > 
> > Signed-off-by: Michael Niedermayer <michael at niedermayer.cc>
> > ---
> >  libavutil/log.c | 15 +++++++++++++++
> >  libavutil/log.h | 19 +++++++++++++++++++
> >  2 files changed, 34 insertions(+)
> > 
> > diff --git a/libavutil/log.c b/libavutil/log.c
> > index e8a0db7716..7646e18cfa 100644
> > --- a/libavutil/log.c
> > +++ b/libavutil/log.c
> > @@ -372,6 +372,21 @@ void av_log(void* avcl, int level, const char *fmt, ...)
> >      va_end(vl);
> >  }
> >  
> > +void av_log_once(void* avcl, int level, int *state, const char *fmt, ...)
> > +{
> > +    if (!((flags & AV_LOG_SKIP_REPEATED) && *state)) {
> > +        AVClass* avc = avcl ? *(AVClass **) avcl : NULL;
> > +        va_list vl;
> > +        va_start(vl, fmt);
> > +        if (avc && avc->version >= (50 << 16 | 15 << 8 | 2) &&
> > +            avc->log_level_offset_offset && level >= AV_LOG_FATAL)
> > +            level += *(int *) (((uint8_t *) avcl) + avc->log_level_offset_offset);
> 
> This logic is duplicated with av_log(). 

> Why is this not done in av_vlog
> anyway?

I am not aware of a reason, ill post a patch to move it over


> 
> > +        av_vlog(avcl, level, fmt, vl);
> > +        va_end(vl);
> > +        *state = 1;
> > +    }
> > +}
> > +
> >  void av_vlog(void* avcl, int level, const char *fmt, va_list vl)
> >  {
> >      void (*log_callback)(void*, int, const char*, va_list) = av_log_callback;
> > diff --git a/libavutil/log.h b/libavutil/log.h
> > index d9554e609d..618d0d1817 100644
> > --- a/libavutil/log.h
> > +++ b/libavutil/log.h
> > @@ -233,6 +233,25 @@ typedef struct AVClass {
> >   */
> >  void av_log(void *avcl, int level, const char *fmt, ...) av_printf_format(3, 4);
> >  
> > +/**
> > + * Send the specified message to the log once if the level is less than or equal
> > + * to the current av_log_level. By default, all logging messages are sent to
> > + * stderr. This behavior can be altered by setting a different logging callback
> > + * function.
> > + * If AV_LOG_SKIP_REPEATED is set each message (each unique state) will only be
> > + * printed once. If its not set then this behaves live av_log()
> > + * @see av_log
> > + *
> > + * @param avcl A pointer to an arbitrary struct of which the first field is a
> > + *        pointer to an AVClass struct or NULL if general log.
> > + * @param level The importance level of the message expressed using a @ref
> > + *        lavu_log_constants "Logging Constant".
> > + * @param fmt The format string (printf-compatible) that specifies how
> > + *        subsequent arguments are converted to output.
> > + * @param state a variable to keep trak of if a message has already been printed
> > + *        this must be initialized to 0 before the first use.
> 
> We should either demand that the caller ensures this does not get called
> simultaneously with the same state, or make the above check-and-set
> atomic.

I had a variant with atomic locally but it felt a bit messy considering that
probably none of the callers would need it to be atomic.
(messy being that the atomic stuff needs the state explicitly inited)

ill document that each state should not be accessed by 2 threads simultaneously

Thanks

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

The greatest way to live with honor in this world is to be what we pretend
to be. -- Socrates
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20200121/0f8f2a5c/attachment.sig>