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

[Gyan Doshi]

On 2025-01-28 01:44 am, Jan Ekström wrote:
> 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".

Does PSNR expect an attributes match between the two inputs - 
resolution, framerate... ? If yes, then can we set it to the main input 
framerate. If that is unset, then we assume <32 and log a warning.

Regards,
Gyan