[FFmpeg-devel] SW's Patchsets Overview

[Marton Balint]

On Wed, 2 Apr 2025, softworkz . wrote:

>
>
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of Marton
>> Balint
>> Sent: Mittwoch, 2. April 2025 21:45
>> To: FFmpeg development discussions and patches <ffmpeg-devel at 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