> -----Original Message----- > From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of Marvin > Scholz > Sent: Donnerstag, 6. März 2025 21:02 > To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org> > Subject: Re: [FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log > output with simple ids > > > > On 6 Mar 2025, at 20:27, Soft Works wrote: > > >> -----Original Message----- > >> From: ffmpeg-devel <ffmpeg-devel-boun...@ffmpeg.org> On Behalf Of > Marvin > >> Scholz > >> Sent: Donnerstag, 6. März 2025 19:59 > >> To: FFmpeg development discussions and patches <ffmpeg- > de...@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-boun...@ffmpeg.org> On Behalf Of > >> Marvin > >>>> Scholz > >>>> Sent: Donnerstag, 6. März 2025 18:49 > >>>> To: FFmpeg development discussions and patches <ffmpeg- > >> de...@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-boun...@ffmpeg.org> On Behalf Of > >>>> Marvin > >>>>>> Scholz > >>>>>> Sent: Donnerstag, 6. März 2025 18:38 > >>>>>> To: FFmpeg development discussions and patches <ffmpeg- > >>>> de...@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-boun...@ffmpeg.org> On Behalf > Of > >>>>>>>> Nicolas George > >>>>>>>> Sent: Donnerstag, 6. März 2025 11:09 > >>>>>>>> To: FFmpeg development discussions and patches <ffmpeg- > >>>>>> de...@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. > > Right, I do think it might be useful to have helpers to make log > formatting easier for library users (especially because as the final > consumer you can then decide where to put some eventual state needed), > and maybe also some to make it easier to obtain logging details from > AVClass as this is right now somewhat hard to get right and I had to > check the source code last time to figure out how its done. > > > 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? > > Yeah, I think the end goal should be to have a way to not have a global > log callback, however that would require quite a bit of > redesign.
Alright I thought so, just questionmarked because - with my patterns of thinking being dominated by object-oriented logic - I've been sometimes surprised by approaches in procedural languages to achieve similar things. > A sort of hacky way right now to do this is use of thread local storage > and then "demultiplex" the messages in the log callback > based on that. However that only works as long as the message does not > come from a "child" thread internally spawned by FFmpeg. That's probably only feasible in very specific cases where you (as the consumer) know that it works. The absolute minimum of an approach I can think of would be something like a "context_id", but it would need to (more or less) "auto-propagate" from context to context for all contexts that are being created - which sounds like a major change. So, down to earth, for V3 of the patchset I have added a callback (av_log_formatprefix_callback) and a function to register (av_log_set_formatprefix_callback()) to log.c and the actual formatting (plus global state) is done in fftools now. I realize that this is a much better way. Thanks for the feedback. sw _______________________________________________ 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".
