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

[Michael Niedermayer]

On Sun, Sep 03, 2023 at 02:25:07AM +0200, Stefano Sabatini wrote:
> 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

ok

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

as long as theres a pointer to a log context that should be enough.
A log context can modify the log level

passing a seperate and mandatory level offset per call would be a bit ugly

thx

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Any man who breaks a law that conscience tells him is unjust and willingly 
accepts the penalty by staying in jail in order to arouse the conscience of 
the community on the injustice of the law is at that moment expressing the 
very highest respect for law. - Martin Luther King Jr
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 195 bytes
Desc: not available
URL: <https://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20230903/4c843df6/attachment.sig>