Git Inbox Mirror of the ffmpeg-devel mailing list - see https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
 help / color / mirror / Atom feed
From: Soft Works <softworkz-at-hotmail.com@ffmpeg.org>
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, 6 Mar 2025 19:27:16 +0000
Message-ID: <DM8P223MB03659A2A6D726C35A4C92E0BBACA2@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM> (raw)
In-Reply-To: <378BC58C-7EF8-4637-9CDB-476E30AF03E3@gmail.com>



> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Marvin
> Scholz
> Sent: Donnerstag, 6. März 2025 19:59
> 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 19:16, 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:49
> >> 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: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

[..]

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








_______________________________________________
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 19:27 UTC|newest]

Thread overview: 33+ 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
2025-03-06 18:16               ` Soft Works
2025-03-06 18:58                 ` Marvin Scholz
2025-03-06 19:27                   ` Soft Works [this message]
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
2025-03-06 20:59   ` [FFmpeg-devel] [PATCH v3 0/4] " ffmpegagent
2025-03-06 20:59     ` [FFmpeg-devel] [PATCH v3 1/4] avutil/log: Add callback for context prefix formatting softworkz
2025-03-06 20:59     ` [FFmpeg-devel] [PATCH v3 2/4] fftools/opt_common: add memaddresses log flag softworkz
2025-03-06 20:59     ` [FFmpeg-devel] [PATCH v3 3/4] fftools: Provide a log formatting callback for context prefixes softworkz
2025-03-06 20:59     ` [FFmpeg-devel] [PATCH v3 4/4] doc/fftools-common-opts: document memaddresses log flag softworkz

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=DM8P223MB03659A2A6D726C35A4C92E0BBACA2@DM8P223MB0365.NAMP223.PROD.OUTLOOK.COM \
    --to=softworkz-at-hotmail.com@ffmpeg.org \
    --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