[FFmpeg-devel] [PATCH] lsws/swscale.h: introduce sws_get_gaussian_vec

[Stefano Sabatini]

On date Tuesday 2023-09-05 13:19:00 +0200, Anton Khirnov wrote:
> Quoting Stefano Sabatini (2023-09-01 20:28:33)
> > On date Friday 2023-09-01 17:50:56 +0200, Anton Khirnov wrote:
> > > Quoting Stefano Sabatini (2023-08-31 17:06:06)
> > > > On date Saturday 2023-08-26 17:15:36 +0200, Anton Khirnov wrote:
> > > > > Quoting Stefano Sabatini (2023-08-26 14:23:28)
> > > > > > Use in place of sws_getGaussianVec.
> > > > > > 
> > > > > > The new function enable better log handling, and provide better naming
> > > > > > for the variance variable, now named standard_deviation to reflect the
> > > > > > meaning of the parameter.
> > > > > 
> > > > 
> > > > > Logging to NULL does not seem like an improvement to me.
> > > > 
> > > > Adding the log_ctx.
> > > > 
> > > > > Renaming a function parameter does not require an API break.
> > > > 
> > > > The main point was improving the naming of the variable, but while at
> > > > it I'm also adding the logging context and providing a return code to
> > > > specify an error failure, and moving to snake_case convention which is
> > > > the one used by the new API additions.
> > > 
> > > As I already said above - function parameter names in a prototype are
> > > purely cosmetic and have no effect on anything besides doxygen. You can
> > > change them at will and even remove them entirely without breaking API
> > > or ABI.
> > > 
> > 
> > > The other reasons do not strike me as strong enough to warrant an API
> > > break.
> > 

> > I disagree on this: the function is probably only used internally and
> > by libavfilter, and the migration is trivial enough so should cause no
> > disruption anyway.
> 

> The migration is not trivial for someone who is not familiar with the
> code (such as a distro package maintainer), since the new function has a
> different signature. I really do not think we should break APIs for
> frivolous reasons, which includes cosmetic ones.

Following this logic every API change should be considered not trivial
for someone not familiar with the code, and therefore should not be
committed. Also there is no evidence that external components are
using this function.

Besides the naming change, there are ergonomic and functional changes
making the behavior of the code more correct.

(Also probably it would be worth moving the naming of all the
remaining functions to snake_case to provide a consistent API - and
reduce the cognitive overload on the API user).