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

[Stefano Sabatini]

On date Saturday 2023-09-02 22:07:53 +0200, Michael Niedermayer wrote:
> On Fri, Sep 01, 2023 at 08:38:26PM +0200, Stefano Sabatini wrote:
> > On date Friday 2023-09-01 18:54:40 +0200, Michael Niedermayer wrote:
> > > On Thu, Aug 31, 2023 at 07:16:20PM +0200, Stefano Sabatini wrote:
> > [...]
> > > > +/**
> > > > + * Compute and return a normalized Gaussian vector.
> > > > + *
> > > > + * @param vecp: pointer where the computed vector is put in case of
> > > > + *        success
> > > > + * @param standard_deviation the standard deviation used to generate
> > > > + *        the Gaussian vector, must be a non-negative value
> > > > + * @param quality the quality of the generated Gaussian vector, must
> > > > + *        be a non-negative value. It affects the lenght of the generated
> > > > + *        vector. A value equal to 3 corresponds to high quality.
> > > > + * @param log_ctx a pointer to an arbitrary struct of which the first
> > > > + *        field is a pointer to an AVClass struct (used for av_log)
> > > > + *        used for logging, can be NULL
> > > > + *
> > > > + * @return a negative error code on error, non negative otherwise
> > > > + */
> > > > +int sws_get_gaussian_vec(SwsVector **vecp,
> > > > +                         double standard_deviation, double quality,
> > > > +                         void *log_ctx);
> > > 
> > > which of the two do you consider better?
> > > 
> > > First, here the central part we return is the vector
> > > 
> > > SwsVector *gaus_vec = sws_getGaussianVec(NULL, 1, 2);
> > > SwsVector *temp_vec = sws_ConvolveVec(NULL, in_vec, gaus_vec);
> > > sws_averageVec(temp_vec, temp_vec, in_vec);
> > > 
> > > av_free(gaus_vec);
> > > return temp_vec; // Error checking here happens by temp_vec being NULL in all cases of error
> > > 
> > > vs.
> > > 
> > > Second, here the central part we return is the error code
> > > 
> > > SwsVector *gaus_vec = NULL;
> > > SwsVector *temp_vec = NULL;
> > > int err = sws_getGaussianVec(&gaus_vec, 1, 2);
> > > if (err<0)
> > >     goto fail;
> > > 
> > > err = sws_ConvolveVec(&temp_vec, in_vec, gaus_vec);
> > > if (err<0)
> > >     goto fail;
> > > 
> > > err = sws_averageVec(&temp_vec, temp_vec, in_vec);
> > > if (err<0)
> > >     goto fail;
> > 
> > The latter pattern enables differentiation between error codes (ENOMEM
> > or EINVAL) and provides feedback in the log message. With the former
> > you only know if it fails, but you don't know why (relevant in case
> > e.g. we make the parameter tunable by a filter and we don't want to
> > add additional validation and logging at the filter level).
> 

> can the API be designed so that optionally the user could choose to
> only check the error code after several steps ?
> (this would avoid the need for 1 check per call where the fine grained
>  information is not needed)
> I mean similar to the concept of NAN in floating point so that a failure
> can be propagated and only at the end checked.

Well, with the new approach you can do:

SwsVector *gaus_vec, *temp_vec, *avg_vec;

sws_get_gaussian_vec(&gaus_vec, 1, 2);
sws_get_convolution_vec(&temp_vec, in_vec, gaus_vec);
sws_get_average_vec(&avg_vec, temp_vec, in_vec);
 
av_free(gaus_vec);
av_free(temp_vec);
return avg_vec; // Error checking here happens by avg_vec being NULL in all cases of error

If you want to disable the log we could add a log_ctx_offset parameter.