[FFmpeg-devel] [PATCH] avfilter/xpsnr: avoid division by zero

[Jan Ekström]

On Mon, Jan 27, 2025 at 9:23 PM Marton Balint <cus at passwd.hu> wrote:
>
>
>
> On Mon, 27 Jan 2025, Gyan Doshi wrote:
>
> > The ref input may have its frame rate unset, which would then lead to
> > SIGFPE.
> >
> > Related to #11428
> > ---
> > libavfilter/vf_xpsnr.c | 3 ++-
> > 1 file changed, 2 insertions(+), 1 deletion(-)
> >
> > diff --git a/libavfilter/vf_xpsnr.c b/libavfilter/vf_xpsnr.c
> > index 1b2c2a7c2c..8f86c188c5 100644
> > --- a/libavfilter/vf_xpsnr.c
> > +++ b/libavfilter/vf_xpsnr.c
> > @@ -568,7 +568,8 @@ static int config_input_ref(AVFilterLink *inlink)
> >     s->max_error_64 = (1 << s->depth) - 1; /* conventional limit */
> >     s->max_error_64 *= s->max_error_64;
> >
> > -    s->frame_rate = il->frame_rate.num / il->frame_rate.den;
> > +    // Avoid division by zero
> > +    s->frame_rate = il->frame_rate.den ? (il->frame_rate.num / il->frame_rate.den) : 25;
>
> I kind of prefer 0 instead of 25, as far as I see the code does not care,
> and 0 is better than an arbitrary value.

I do agree with the sentiment, but I think at least for myself the
real question is whether we should attempt to get "correct results" or
not, and thus either fail if a frame rate is not properly set, or at
the very least warn loudly that the results may be incorrect
(interpreted as less than 32fps) if the fps value is left unset. Also
I do wonder if we should utilize time base if frame rate is not set,
as I see that 1/0 is utilized for "undefined/variable rate".

Best regards,
Jan