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

[Stefano Sabatini <stefasab@gmail.com>]

On date Wednesday 2023-09-06 13:13:25 +0200, Anton Khirnov wrote:
> Quoting Stefano Sabatini (2023-09-06 00:59:44)
[...]
> A simple rename is a trivial API change. Almost everything else is not.
> 
> > and therefore should not be committed.
> 
> Yes, the baseline for every API change is that it is undesirable and you
> must supply sufficiently strong arguments to overcome that.
> 
> > 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.
> 

> I do not see the code being made more correct, but Michael observed in
> this thread that it becomes longer and more convoluted.

It is a different construct, not necessarily more convoluted. Also
keep in mind this is needed in order to achive two goals:

1. return a proper error code since we can have two types of failures
here (memalloc or invalid argument)

2. log the invalid argument reason so that the failure reason is in
the log, without having to repeat the validation logic in the caller

> I am not convinced that adding logging to this function is an
> improvement. You have to pass an extra parameter to every call, making
> the code longer and less readable. We do not need a dedicated error
> message for every malloc.

See above, in this case we can have ENOMEM/EINVAL and it should be
possible to distinguish between the two.
_______________________________________________
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".