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

["Jan Ekström" <jeebjp@gmail.com>]

On Mon, Jan 27, 2025 at 9:23 PM Marton Balint <cus@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
_______________________________________________
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".