Quoting Michael Niedermayer (2020-01-29 12:25:42) > On Tue, Jan 28, 2020 at 07:38:45PM +0100, Anton Khirnov wrote: > > 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. > > I cannot think of a single case for which this would be true. > > All cases i can think of for which i intended to use log_once() for, and for > some of these users have asked for this. In one case theres a offer to pay > for it by a user. So this is a real case with real intrerrest behind > is where the messages do carry information in each instance (for a developer). > but are annoying to the user. > > So, we sure can use log_once() with no flag to turn the "once" off. And let > the developer who needs it off edit the source and rebuild but its not correct > to say that the messages are not meaningful after their first occurance. > A developer looking into some issue with a file would want to know if a > header error occurs once or on every frame or maybe every time a vissual > issue appears. > > Maybe we could have log_once() not suppress the messages after their first > occurance but reduce them to DEBUG or TRACE level. > What do you think ?
Yes, a reduced loglevel sounds better to me. The only issue with that the name no longer matches the functionality - since it no longer logs just once - but I can't think of a better name (maybe log_display_once(), but that's ugly and not much more descriptive) and it's probably not a big deal. No other objections from me. -- Anton Khirnov _______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
