[FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log output with simple ids
Marvin Scholz
epirat07 at gmail.com
Thu Mar 6 19:49:19 EET 2025
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
> 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".
More information about the ffmpeg-devel
mailing list