[FFmpeg-devel] [PATCH v7 1/3] avutil/log: Add log flag AV_LOG_PRINT_MEMADDRESSES

[Andreas Rheinhardt]

softworkz .:
> 
> 
>> -----Original Message-----
>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>> Andreas Rheinhardt
>> Sent: Mittwoch, 9. April 2025 10:12
>> To: ffmpeg-devel at ffmpeg.org
>> Subject: Re: [FFmpeg-devel] [PATCH v7 1/3] avutil/log: Add log flag
>> AV_LOG_PRINT_MEMADDRESSES
>>
>> softworkz .:
>>>
>>>
>>>> -----Original Message-----
>>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>>>> softworkz .
>>>> Sent: Mittwoch, 9. April 2025 09:51
>>>> To: FFmpeg development discussions and patches <ffmpeg-
>> devel at ffmpeg.org>
>>>> Subject: Re: [FFmpeg-devel] [PATCH v7 1/3] avutil/log: Add log flag
>>>> AV_LOG_PRINT_MEMADDRESSES
>>>>
>>>>
>>>>
>>>>> -----Original Message-----
>>>>> From: ffmpeg-devel <ffmpeg-devel-bounces at ffmpeg.org> On Behalf Of
>>>>> Andreas Rheinhardt
>>>>> Sent: Mittwoch, 9. April 2025 09:28
>>>>> To: ffmpeg-devel at ffmpeg.org
>>>>> Subject: Re: [FFmpeg-devel] [PATCH v7 1/3] avutil/log: Add log flag
>>>>> AV_LOG_PRINT_MEMADDRESSES
>>>>>
>>>>> softworkz:
>>>>>> From: softworkz <softworkz at hotmail.com>
>>>>>>
>>>>>> which is controls prefix formatting. With this flag set, the prefix
>>>> is
>>>>>> printed including the memory address, otherwise it is omitted.
>>>>>> In libavutil, the flag is set by default, retaining the previous
>>>>>> behavior. fftools remove the flag as default.
>>>>>
>>>>> The implementation of this flag is counter to the usual one: It is
>>>>> enabled by default, but every av_log_set_flags() that does not set
>>>>> AV_LOG_PRINT_MEMADDRESSES implicitly disables printing memory
>>>> addresses.
>>>>
>>>> Hi Andreas,
>>>>
>>>> thanks for looking at the patchset.
>>>
>>> [..]
>>>
>>>>> AV_LOG_PRINT_MEMADDRESS seems better given that every av_log() will
>>>>> likely only print one memaddress.
>>>>
>>>> It can be more than a single address per log line since it also
>> prints
>>>> the parent's memory address in case it specifies
>>>> parent_log_context_offset.
>>
>> I know. That's why my statement contained a "likely".
> 
> Sorry, of course you know. My bad.
> 
> 
>> https://github.com/ffstaging/FFmpeg/blob/02eda84bf2fcf0db7793872204b0f56
>>>> 4a6557232/libavutil/log.c#L329-L337
>>>
>>> But the reason - from my thinking - why I made it plural is because
>> it's a global flag that controls whether all memory addresses ( <- pl.)
>> should be printed or not. If it was a flag that is conveyed as part of a
>> log invocation (like av_log), controlling whether the memory address
>> should be printed for that single log line, then I'd think it should be
>> singular - probably even despite the nit regarding the parent address.
>>>
>>> Does it make sense? 😊
>>>
>>
>> I get your thinking, but it is not consistent with the other flags:
>> There is no AV_LOG_PRINT_TIMES after all.
> 
> Okay, singular it will be!
> 
> 
>>> Do you think it would make more sense to negate the flag semantic?
>>
>> Yes.
> 
> Fine, let's do it that way round. To be clear, you only mean to negate the internal logic of that flag, i.e. making it AV_LOG_NO_PRINT_MEMADDRESS internally,

The flag is public, not internal.

 but for the fftools CLI, it will still be:
> 
> ./ffmpeg -v memaddress
> 
> to enable it, right?

Given that the default for the fftools is different from the default for
lavu, it makes sense to handle it differently.

- Andreas