[FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log output with simple ids

[Soft Works]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Marvin
> Scholz
> Sent: Donnerstag, 6. März 2025 18:49
> To: FFmpeg development discussions and patches <ffmpeg-devel at ffmpeg.org>
> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log
> output with simple ids
> 
> 
> 
> On 6 Mar 2025, at 18:44, Soft Works wrote:
> 
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> Marvin
> >> Scholz
> >> Sent: Donnerstag, 6. März 2025 18:38
> >> To: FFmpeg development discussions and patches <ffmpeg-
> devel at ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in
> log
> >> output with simple ids
> >>
> >>
> >>
> >> On 6 Mar 2025, at 18:02, Soft Works wrote:
> >>
> >>>> -----Original Message-----
> >>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
> >>>> Nicolas George
> >>>> Sent: Donnerstag, 6. März 2025 11:09
> >>>> To: FFmpeg development discussions and patches <ffmpeg-
> >> devel at ffmpeg.org>
> >>>> Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses
> in
> >> log
> >>>> output with simple ids
> >>>>
> >>>> Soft Works (HE12025-03-05):
> >>>>> Sorry. So - seriously: what would be your recipe then?
> >>>>
> >>>> I see not just a little of non-trivial code for a very minor
> feature,
> >>>
> >>> Whether trivial or non-trivial, it's definitely just very little
> code.
> >>>
> >>>> that might be a hint that it would be best to let it go.
> >>>
> >>> This is not a helpful comment. I'm trying hard to be friendly and
> >> productive and I think it's not asked too much to at least try doing
> as
> >> well.
> >>>
> >>>
> >>>> Also, if somebody is debugging a program using the libraries, the
> >>>> pointers are relevant for that program. For that reason, I think
> the
> >>>> change is a bad idea in the library.
> >>>
> >>> It's a valid point, I have acknowledged that already and added a log
> >> flag in V2 which allows to control it.
> >>>
> >>> As a further compromise, we could also enable it by default in case
> >> when DEBUG is defined, how about that?
> >>>
> >>> Generally, debugging is important without doubt, but it doesn't mean
> >> that Millions of users need to see something in the output which is
> only
> >> ever relevant to developers - that's the premise of this patchset.
> >>>
> >>> And even as a developer, those addresses are interesting only in a
> >> very narrow range of cases.
> >>> These addresses have been a major pain point for myself and many
> >> others over years when comparing logfiles. Even the best diffing
> >> algorithms are getting confused by these addresses and I think this
> >> patchset provides a huge benefit for both, users and developers in
> the
> >> future, making their work a lot easier.
> >>>
> >>>
> >>>> On the other hand, you could do that change in the fftools. The
> point
> >>>> about pointers being relevant does not apply for them, and they can
> >> have
> >>>> as much global state as they want.
> >>>
> >>> You know that it's not easily possible to do it from within fftools
> >> because all libs are logging directly to avutil, so it's not quite
> clear
> >> to me what you are up to.
> >>> Do you mean something like a int(* av_log_format_prefix)(...)
> callback
> >> that fftools could register to?
> >>>
> >>
> >> First of all I want to say I like the idea of having cleaner logs,
> >> but...
> >>
> >> IMHO "complex" logging formatting should be handled by fftools
> >> especially if
> >> they need global state. Even though thats not the case right now, but
> >> just
> >> like Nicolas I also would prefer to not add even more global state
> for
> >> logging
> >> to the library...
> >>
> >> All the fancy log formatting should be done in a log callback in the
> >> fftools and the default library logging callback should just be a
> very
> >> basic
> >> one, is my opinion on this.
> >
> > That's all fine and probably reasonable. But is it fair to block a
> small change because some major rework would be desired at some point?
> >
> > When that change will be made, it will of course move out this little
> change as well.
> >
> > But are you really saying that this small change cannot be made
> because you don't like the general way of the current implementation?
> >
> 
> Just to be clear, I am not blocking this, just wanted to give my
> perspective on the topic.
> So if others think its fine and want it, lets go for it.

Thanks - and sorry for my retardation, I just realized why it's bad to do it this way with regards to library usage. I'll add a callback so that fftools can do the prefix formatting.

For the idea of moving all the formatting to fftools, wouldn't this be a major breakage for library consumers who are used to getting the log output formatted like now?


Thanks,
sw