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

[Anton Khirnov]

Quoting Michael Niedermayer (2020-01-24 13:04:08)
> On Tue, Jan 21, 2020 at 07:44:46PM +0100, Anton Khirnov wrote:
> > Quoting Michael Niedermayer (2020-01-21 15:43:48)
> > > 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.
> > 
> > The problem is that right now, flags is only used by the default log
> > callback. The behaviour of the default log callback is not specified by
> > the API, so it can be changed later without much trouble. With this
> > patch, the function of flags is hardcoded into the API, making its
> > future removal significantly harder.
> 
> I dont really see this concern. Because if you disable the flag "today"
> you break the API as it is documented, the flag is documented to
> affect the message repeation.
> With this patch, disabling it still breaks a bunch of message repeating
> behavior, so to me this looks like its basically the same.

The flag is documented, but the situations where it applies are not.
Currently, it only applies to the default log callback. It has no effect
whatsoever on users who use their own callback. With your patch, its
influence spreads into the core API. Since I see the flag as something
to be removed, I would prefer it were not done.

> 
> But what do you suggest ?
> 
> We could send all the repeated _once() messages to the callback and leave it
> to the callback to drop them. Just needs a way to tag them as repeats
> 
> We could move the (no)repeat flag to each context but this feels unwieldy
> and feels like it solves a problem noone had. Because noone ever asked
> AFAIR that they wanted to change repeating behavior on a per context base.
> This is probably mostly used by developers wanting to check for "all"
> messages. Or users produding bug reports which also would ideally have
> no dropped messages.
> 
> I can also just drop the use of the flag entirely from the patch and just
> leave this as a unconditional _once() log. It feels a bit like a missing
> feature though because as a devleoper for debuging a simple switch to
> see all repeats seems usefull.

I would say that log_once() should be used only for messages that are
meaningful just once (per context). It then makes no sense to log them
multiple times. Otherwise normal logging should be used.

-- 
Anton Khirnov