[FFmpeg-devel] [PATCH] avutil/log: Replace addresses in log output with simple ids

[Marvin Scholz]

On 6 Mar 2025, at 20:27, 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 19:59
>> 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 19:16, 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:49
>>>> 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: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
>
> [..]
>
>>>>>>
>>>>>> 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.

Right, I do think it might be useful to have helpers to make log formatting easier for library users (especially because as the final
consumer you can then decide where to put some eventual state needed), and maybe also some to make it easier to obtain logging details from
AVClass as this is right now somewhat hard to get right and I had to check the source code last time to figure out how its done.

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

Yeah, I think the end goal should be to have a way to not have a global log callback, however that would require quite a bit of
redesign. (This is especially useful if you run independent FFmpeg tasks in different threads and want to get separate logs for
each of them.)

A sort of hacky way right now to do this is use of thread local storage and then "demultiplex" the messages in the log callback
based on that. However that only works as long as the message does not come from a "child" thread internally spawned by FFmpeg.

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