[FFmpeg-devel] SW's Patchsets Overview

[FFmpeg-devel] SW's Patchsets Overview

[softworkz .]

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:


avformat/hls demuxer: Add WebVTT subtitle support
GitHub:    https://github.com/ffstaging/FFmpeg/pull/53
Patchwork: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=13981


avformat/id3v2: Support null-separated multi-value properties

GitHub:    https://github.com/ffstaging/FFmpeg/pull/54
Patchwork: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=14046


avcodec/dvbsubdec: Fix conditions for fallback to default resolution

GitHub:    https://github.com/ffstaging/FFmpeg/pull/57/files
Patchwork: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=14038
Note:      Pathwork builds were all broken at that time


avformat/dump: Print stream start offsets for input streams

GitHub:    https://github.com/ffstaging/FFmpeg/pull/58
Patchwork: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=14039


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


Improve HWDeviceContext logging

GitHub:    https://github.com/ffstaging/FFmpeg/pull/61
Patchwork: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=14206


print_graphs: Complete Filtergraph Printing
=> actually, this is just refactoring ffprobe output writing

GitHub:    https://github.com/ffstaging/FFmpeg/pull/52
Patchwork: https://patchwork.ffmpeg.org/project/ffmpeg/list/?series=14165


Please let me know about any questions or concerns or when you need more time for anything to review.
Excepting the last one I'll be waiting at least 14 more days, so there's nothing to worry atm.

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

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

[Marton Balint]

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

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.

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

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

Regards,
Marton
_______________________________________________
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".

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

[softworkz .]

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

Thank you,
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".

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

[Marton Balint]

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

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

[softworkz .]

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

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

[Michael Niedermayer]

[-- Attachment #1.1: Type: text/plain, Size: 6532 bytes --]

Hi

On Sun, Apr 06, 2025 at 09:12:00PM +0000, softworkz . wrote:
> 
> 
> > -----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.

In the long run maybe some
    int instance_name_offset;
in AVClass that points to a place in each instance where the name is stored
This can then be set when the objects are allocated or initialized and could
also be overridden by the user application, storing other names

would everyone agree with that ?

thx

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Dictatorship naturally arises out of democracy, and the most aggravated
form of tyranny and slavery out of the most extreme liberty. -- Plato

[-- Attachment #1.2: signature.asc --]
[-- Type: application/pgp-signature, Size: 195 bytes --]

[-- Attachment #2: Type: text/plain, Size: 251 bytes --]

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

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Michael Niedermayer
> Sent: Mittwoch, 9. April 2025 00:25
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] SW's Patchsets Overview
> 
> Hi
> 
> On Sun, Apr 06, 2025 at 09:12:00PM +0000, softworkz . wrote:
> >
> >
> > > -----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.
> 
> In the long run maybe some
>     int instance_name_offset;
> in AVClass that points to a place in each instance where the name is
> stored
> This can then be set when the objects are allocated or initialized and
> could
> also be overridden by the user application, storing other names

Do you mean this as a generalized alternative to the item_name function like used here https://github.com/ffstaging/FFmpeg/pull/61/files#diff-8622a9ccb5f7c11364d8cf5be7ee49928bf3d9c007774a3b138f5e4af9046d84R140-R150 for logging e.g. "D3D11VA" instead of just "AVHWDeviceContext"?

I had thought about an instance counter - which would be more reliable than inferring from the pointer-to-pointer address - but the problem is that there's no global base method for creating    AVClass instances where this could be implemented...

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

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

[Nicolas George]

Marton Balint (HE12025-04-06):
> 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.

I do not. The more I think on it, the more I consider this whole
endeavour is completely misguided.

One of our guiding principles is that the console output of our
command-line tools should be, by default, usable by experienced users.
This is why we reject proposals to hide the banner by default, and this
is why we should not do this either.

Regards,

-- 
  Nicolas George
_______________________________________________
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".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of
> Nicolas George
> Sent: Montag, 7. April 2025 11:14
> To: FFmpeg development discussions and patches <ffmpeg-devel@ffmpeg.org>
> Subject: Re: [FFmpeg-devel] SW's Patchsets Overview
> 
> Marton Balint (HE12025-04-06):
> > 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.
> 
> I do not. The more I think on it, the more I consider this whole
> endeavour is completely misguided.
> 
> One of our guiding principles is that the console output of our
> command-line tools should be, by default, usable by experienced users.
> This is why we reject proposals to hide the banner by default, and this
> is why we should not do this either.
> 
> Regards,
> 
> --
>   Nicolas George
> _______________________________________________


Hi Nicolas,

had a bad sleep? 

Misguided? 

Our "guiding principles" involve outputting tons of meaningless numbers?

The analogon you are trying to draw is comparing apples and oranges. Besides - no matter how experienced a user may be - as long as the one doesn't have access to any kind of debugging tool, those memory addresses are largely useless (excepting the small value of discriminability that my original patchset is retaining in the form of "simple IDs").
Beyond those use cases, anybody claiming those numbers being of value for oneself, being an experienced user, would be nothing more than a pretender.

sw


PS: As far as I'm concerned: I did have a bad sleep. Next time I'll try to be friendly again, even though it's hard after reading such nonsense.





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

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

[Marton Balint]

On Mon, 7 Apr 2025, Nicolas George wrote:

> Marton Balint (HE12025-04-06):
>> 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.
>
> I do not. The more I think on it, the more I consider this whole
> endeavour is completely misguided.
>
> One of our guiding principles is that the console output of our
> command-line tools should be, by default, usable by experienced users.
> This is why we reject proposals to hide the banner by default, and this
> is why we should not do this either.

Not showing pointer addresses also has benefits, such as easier 
diffability of output, or better human readability. It depends on actual 
use case which is "useful", so a logging flag completely makes sense to me 
to show or hide it, depending on what the user wants.

As for the question of the default behaviour, I don't have a strong 
opinion, both can make sense, maybe I would keep the existing behaviour 
for the library, but change the default for the cli tools to no-pointer 
output.

Regards,
Marton
_______________________________________________
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".

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

[softworkz .]

> -----Original Message-----
> From: ffmpeg-devel <ffmpeg-devel-bounces@ffmpeg.org> On Behalf Of Marton
> Balint

Hi Marton,

> As for the question of the default behaviour, I don't have a strong
> opinion, both can make sense, maybe I would keep the existing behaviour
> for the library, but change the default for the cli tools to no-pointer
> output.

I like the idea; I think I'll pick it up for the next revision.

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

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

[Nicolas George]

Marton Balint (HE12025-04-09):
> Not showing pointer addresses also has benefits, such as easier diffability
> of output, or better human readability. It depends on actual use case which
> is "useful", so a logging flag completely makes sense to me to show or hide
> it, depending on what the user wants.

Sure, it has second order benefits: it makes a few things a little
easier. For example if you want to remove the address to diff the output
of multiple runs, you can just pipe through sed; you are already piping
to get the diff anyway.

But the problems it causes, not being able to identify the source of a
warning, are first order.

> As for the question of the default behaviour, I don't have a strong opinion,
> both can make sense, maybe I would keep the existing behaviour for the
> library, but change the default for the cli tools to no-pointer output.

Even in the CLI tool, this change is harmful as soon as there are
multiple instances of the same component, like two scale filters.

Detecting that situation to enable the flag would not be worth the
effort and would clash with the principle of least surprise.

Stop with the sunk costs, just drop this misfeature.

Regards,

-- 
  Nicolas George
_______________________________________________
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".