Re: [FFmpeg-devel] SW's Patchsets Overview

["softworkz ." <softworkz-at-hotmail.com@ffmpeg.org>]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Marton
> Balint
> Sent: Sonntag, 6. April 2025 23:05
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] SW's Patchsets Overview
> 
> 
> 
> On Wed, 2 Apr 2025, softworkz . wrote:
> 
> >
> >
> >> -----Original Message-----
> >> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Marton
> >> Balint
> >> Sent: Mittwoch, 2. April 2025 21:45
> >> To: FFmpeg development discussions and patches <ffmpeg-
> devel@ffmpeg.org>
> >> Subject: Re: [FFmpeg-devel] SW's Patchsets Overview
> >>
> >>
> >>
> >> On Wed, 2 Apr 2025, softworkz . wrote:
> >>
> >>> Hello everybody,
> >>>
> >>> with freshly gained push access rights, I want to act responsibly
> and
> >>> carefully, and also avoid unexpected surprises so I'm not going to
> >> rush
> >>> things. Due to that change, I thought it might be good to post an
> >>> overview of the patchsets I am intending to push in the near future:
> >>
> >> Thanks for the heads up.
> >>
> >> [...]
> >>
> >>> avutil/log: Replace addresses in log output with simple ids
> >>>
> >>> GitHub:    https://github.com/ffstaging/FFmpeg/pull/59
> >>> Patchwork:
> >> https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=14094
> >>
> >
> > Hi Marton,
> >
> > thanks a lot for looking at the patchset.
> >
> >> To be honest, I don't like this at all. You duplicate a lot of code
> from
> >> avutil/log, and the implementation has quite a few problems, some of
> >> them not really fixable.
> >
> > Originally, this was a patch against avutil/log. Nicolas objected that
> > it was adding global state and Hendrik (and Nicolas) suggested that I
> > should to this in fftools only - outside of the libs, in a was that
> > fftools get their own logging implementation - with the potential of
> > being able to do other things in the future that wouldn't make sense
> in
> > the lib code. Letting fftools have their own logging implementation of
> > can of course only start from a copy in order to retain existing
> > behavior. On top of that I applied that little change then.
> >
> >
> >> - creating object IDs in the order the objects log something (what if
> >> they do not? What if it depends on loglevel?)
> >> - tracking object IDs based on their address - objects are
> >>    allocated and removed at runtime, it is possible that an address
> will be
> >>    re-used for a different object later on
> >
> > The Ids are not meant to have much more value than the addresses
> > currently shown - with an important difference: They are short and
> > remain the same on repeated execution. Plus: they are counted by
> > AVClass, that give a little additional value, but since they are just
> > "indexing" the addresses, they are in fact prone to the same
> > shortcomings like the addresses themselves, meaning that a re-
> assignment
> > might give you the same id for something different and also different
> > addresses (in consequence the IDs as well) can reference the same
> thing
> > (e.g. with buffer refs).
> >
> >> - linear search of addresses. A long ffmpeg process can constantly
> >> create
> >>    objects during runtime, eventually completely depleting the pool
> and
> >>    causing an extensive search for all future logs.
> >
> > I have considered that case. There is a hard limit from when on no IDs
> > are assigned anymore (all zeros).
> >
> >
> >> So overall I don't think it's worth pursuing this, especially since
> most
> >> users won't care neither about the ID, nor about the address...
> >
> > Let me give two examples of where I find it useful to have those IDs:
> >
> > On startup decoders can be initialized multiple times, like first for
> > probing and then for transcoding. Or when there are multiple streams
> of
> > the same type (codec), the log messages can be confusing when the log
> > output from several identical ones gets mixed up. Being able to see
> > "which is which" is quite of value at times.
> >
> > HW Device context can also get initialized multiple times and knowing
> > which one has shut down already and which hasn't - is helpful. Also,
> in
> > case of complex filtergraphs with multiple derived and reverse-derived
> > hw contexts, one can quickly get lost in understanding the logs.
> >
> >
> > That being said - I don't want to insist on those IDs. We could also
> > just hide the addresses (activatable by a log flag) and I'd still be
> > happy about being able to do logfile diffs in the future without
> trouble
> > 😊
> >
> > In that case, the change could also be made just in avutil/log.
> Probably
> > also depends on what the consensus would be regarding the value of
> > fftools having their own logging implementation - or rather not?
> >
> > I'm open for either direction.
> 
> I think a log flag to completely hide the addresses makes sense, and can
> be implemented cleanly and reliably in avutil/log. I can totally support
> that.
> 
> Thanks,
> Marton
> _______________________________________________

Hi Marton,


thanks a lot for your reply. As nobody has voiced in favor of the simple ID replacement, I'm fine to go that way.

There's one point remaining though: The intention was to hide the addresses by default and allow to enable them via flag. Two earlier commenters were seconding that, a third one not explicitly objecting. It was only said that it must be possible to enable it again by flag.

What is your opinion regarding the default?

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