Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Marvin Scholz <epirat07@gmail.com>
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
Date: Thu, 06 Mar 2025 18:49:19 +0100
Message-ID: <EBDDC16F-FCC5-4BAA-9727-9B39E2EEDF68@gmail.com> (raw)
In-Reply-To: <DM8P223MB036565CFBB87B9BE20B893B5BACA2@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM>



On 6 Mar 2025, at 18:44, Soft Works wrote:

>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Marvin
>> Scholz
>> Sent: Donnerstag, 6. März 2025 18:38
>> 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 18:02, Soft Works wrote:
>>
>>>> -----Original Message-----
>>>> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
>>>> Nicolas George
>>>> Sent: Donnerstag, 6. März 2025 11:09
>>>> 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
>>>>
>>>> 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@ffmpeg.org
> https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>
> To unsubscribe, visit link above, or email
> ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

To unsubscribe, visit link above, or email
ffmpeg-devel-request@ffmpeg.org with subject "unsubscribe".

  reply	other threads:[~2025-03-06 17:49 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-03-05 15:38 softworkz
2025-03-05 15:40 ` Nicolas George
2025-03-05 15:45   ` Soft Works
2025-03-05 15:48   ` Soft Works
2025-03-06 10:08     ` Nicolas George
2025-03-06 17:02       ` Soft Works
2025-03-06 17:38         ` Marvin Scholz
2025-03-06 17:44           ` Soft Works
2025-03-06 17:49             ` Marvin Scholz [this message]
2025-03-06 18:16               ` Soft Works
2025-03-06 18:58                 ` Marvin Scholz
2025-03-06 19:27                   ` Soft Works
2025-03-06 20:01                     ` Marvin Scholz
2025-03-06 20:48                       ` Soft Works
2025-03-06 20:56       ` Soft Works
2025-03-05 15:42 ` Soft Works
2025-03-05 16:23 ` Gyan Doshi
2025-03-05 16:30   ` Soft Works
2025-03-05 17:14     ` Gyan Doshi
2025-03-05 18:19 ` [FFmpeg-devel] [PATCH v2 0/3] " ffmpegagent
2025-03-05 18:19   ` [FFmpeg-devel] [PATCH v2 1/3] " softworkz
2025-03-05 18:19   ` [FFmpeg-devel] [PATCH v2 2/3] fftools/opt_common: add memaddresses log flag softworkz
2025-03-05 18:19   ` [FFmpeg-devel] [PATCH v2 3/3] doc/fftools-common-opts: document " softworkz
2025-03-06 10:04   ` [FFmpeg-devel] [PATCH v2 0/3] avutil/log: Replace addresses in log output with simple ids Nicolas George
2025-03-06 16:38     ` Soft Works
2025-03-06 16:43       ` Nicolas George
2025-03-06 17:05         ` Soft Works
2025-03-06 17:38           ` Soft Works

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=EBDDC16F-FCC5-4BAA-9727-9B39E2EEDF68@gmail.com \
    --to=epirat07@gmail.com \
    --cc=ffmpeg-devel@ffmpeg.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel

This inbox may be cloned and mirrored by anyone:

	git clone --mirror https://master.gitmailbox.com/ffmpegdev/0 ffmpegdev/git/0.git

	# If you have public-inbox 1.1+ installed, you may
	# initialize and index your mirror using the following commands:
	public-inbox-init -V2 ffmpegdev ffmpegdev/ https://master.gitmailbox.com/ffmpegdev \
		ffmpegdev@gitmailbox.com
	public-inbox-index ffmpegdev

Example config snippet for mirrors.


AGPL code for this site: git clone https://public-inbox.org/public-inbox.git