[FFmpeg-devel] [PATCH v6 2/4] avfilter/vf_framerate: if metadata lavfi.scd.mafd exists, we'll use it first

[Marton Balint]

On Thu, 14 May 2020, Paul B Mahol wrote:

> On 5/14/20, Marton Balint <cus at passwd.hu> wrote:
>>
>>
>> On Thu, 14 May 2020, Marton Balint wrote:
>>
>>>
>>>
>>> On Thu, 14 May 2020, Nicolas George wrote:
>>>
>>>> Marton Balint (12020-05-14):
>>>>> I am not a huge fan of this patch, mafd refers to a score between this
>>> frame
>>>>> and the previous frame, we cannot ensure that there were no additional
>>> frame
>>>>> processing between scdet and this filter which may have duplicated or
>>>>> removed frames. So I'd rather not add this feature.
>>>>
>>>> It can only happen if the user has put filters in the middle. I move we
>>>> trust users who insert such obscure filters to do what they want to, or
>>>> to fix their issues if they have some.
>>>
>>> Fine, I am not blocking this if null pointer deref issues are fixed.
>>>
>>> I think we can also assume that if the metadata exists, it will contain a
>>> valid number, so I suggest this code:
>>>
>>>         e_mafd = av_dict_get(next->metadata, "lavfi.scd.mafd", NULL,
>>> AV_DICT_MATCH_CASE);
>>>         if (e_mafd) {
>>>             mafd = strtod(e_mafd->value, NULL);
>>>         } else {
>>>             s->sad(crnt->data[0], crnt->linesize[0], next->data[0],
>>> next->linesize[0], crnt->width, crnt->height, &sad);
>>>             emms_c();
>>>             mafd = (double)sad * 100.0 / (crnt->width * crnt->height) / (1
>>>
>>> << s->bitdepth);
>>>         }
>>
>> Why this patch was committed without a recent review???
>>
>
> Looks like repeating occurrence? Remove commit rights?
>

I have to agree. Limin's patches can be useful, but they always benefit 
from an extra set of eyes.

Regards,
Marton