Re: [FFmpeg-devel] [PATCH] all: Don't set AVClass.item_name to its default value

[Zhao Zhili <quinkblack@foxmail.com>]

> 在 2023年12月29日，上午7:42，Marton Balint <cus@passwd.hu> 写道：
> 
> 
> 
>> On Mon, 25 Dec 2023, Anton Khirnov wrote:
>> 
>> Quoting Zhao Zhili (2023-12-25 10:27:59)
>>> > On Dec 25, 2023, at 16:38, Anton Khirnov <anton@khirnov.net> wrote:
>>> > > Quoting Kacper Michajlow (2023-12-24 11:41:52)
>>> >> On Fri, 22 Dec 2023 at 14:57, Anton Khirnov <anton@khirnov.net> wrote:
>>> >>> >>> Quoting Andreas Rheinhardt (2023-12-22 14:48:45)
>>> >>>> Avoids relocations.
>>> >>>> >>>> Signed-off-by: Andreas Rheinhardt <andreas.rheinhardt@outlook.com>
>>> >>>> ---
>>> >>> >>> Maybe mention that it's not needed after
>>> >>> acf63d5350adeae551d412db699f8ca03f7e76b9.
>>> >> >> This is not the only user of this API, no?
>>> >> >> I have a question for my own curiosity. This is ABI (and API) breaking
>>> >> change,
>>> > > It is not. This item was not guaranteed to be set, which was actually
>>> > the reason I wrote the patch that this one refers to.
>>> There is no problem to relax a restriction inside libavutil. However, since there is
>>> no explicit documentation on whether item_name can be null or not, user may make
>>> incorrect assumptions and depend on item_name not being null. I don’t think break
>>> user’s code suddenly is a good idea, although we can say it’s break since the beginning.
>> 
>> My point is that the restriction never existed. There were always
>> AVClass instances that did not set that pointer. There were fewer of
>> them, but they did exist.
> 
> I could just as easily make the argument that these AVClasses were not following the documentation. AVClass is documented in avutil/log.h. We should document if a field can be NULL, this was clearly not done here. Because of that, we should assume what most of the codebase assumed.
> 
> Considering the amount of stuff this breaks, this patch should be reverted,

Agree.

> AVClass->item_name deperecated, item_name_func added and explicitly documented that it may be NULL.

Consider the beneficial of a nullable item_name_func isn't that dramatic, I’m not sure if it worth to force user to do such change. Fix those few avclass instance is user friendly in my opinion.

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

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