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

[Marvin Scholz]

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
>> 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?
>

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...

>
> Thanks,
> sw
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request at ffmpeg.org with subject "unsubscribe".