[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 19:59
> 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 19:16, 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: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

[..]

> >>>>
> >>>> 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?
> >
> 
> Yeah, one of the reasons why I did not really do any work on this yet as
> I am really
> not sure whats the best way to go about this to not be too surprising
> for existing
> library users...

I think, people _do_ want the formatting even when using the libs directly, so while separating formatting from the plain logging might make sense, it would probably still need to be in avutil - otherwise most consumers would probably be annoyed and have to copy the formatting code from fftools to their own code base (or similar) - which wouldn't be a win for anybody.
I see why the global state is bad with regards to this patchset's feature (and V3 will remedy), but avoidance of global state would be much easier and also make more sense if there was some kind of "local state" as a replacement, so that people could for example have separate log outputs when performing separate and independent operations.
I suppose that's not easy to achieve with the current architecture?

sw