[FFmpeg-devel] [PATCH 1/5] avfilter/af_afade: fix crossfade duration maximum value

[Marton Balint]

On Mon, 1 Oct 2018, Paul B Mahol wrote:

> On 10/1/18, Marton Balint <cus at passwd.hu> wrote:
>>
>>
>> On Mon, 1 Oct 2018, Paul B Mahol wrote:
>>
>>> On 10/1/18, Marton Balint <cus at passwd.hu> wrote:
>>>>
>>>>
>>>> On Mon, 1 Oct 2018, Paul B Mahol wrote:
>>>>
>>>>> On 10/1/18, Marton Balint <cus at passwd.hu> wrote:
>>>>>>
>>>>>>
>>>>>> On Mon, 1 Oct 2018, Paul B Mahol wrote:
>>>>>>
>>>>>>> On 10/1/18, Marton Balint <cus at passwd.hu> wrote:
>>>>>>>>
>>>>>>>>
>>>>>>>> On Mon, 1 Oct 2018, Paul B Mahol wrote:
>>>>>>>>
>>>>>>>>> On 9/30/18, Marton Balint <cus at passwd.hu> wrote:
>>>>>>>>>> Signed-off-by: Marton Balint <cus at passwd.hu>
>>>>>>>>>> ---
>>>>>>>>>>  libavfilter/af_afade.c | 4 ++--
>>>>>>>>>>  1 file changed, 2 insertions(+), 2 deletions(-)
>>>>>>>>>>
>>>>>>>>>
>>>>>>>>> Why?
>>>>>>>>>
>>>>>>>>> Value is too big.
>>>>>>>>
>>>>>>>> AV_OPT_TYPE_DURATION is microsecond precision, therefore 60 is 60
>>>>>>>> usecs.
>>>>>>>> Probably you meant 60 seconds as limit. That is what the patch fixes.
>>>>>>>
>>>>>>> I do not think so. Duration is interpreted as seconds, look how
>>>>>>> variable is used in code.
>>>>>>
>>>>>> As far as I see duration is rescaled from AV_TIME_BASE (microseconds)
>>>>>> to
>>>>>> sample_rate time base:
>>>>>>
>>>>>> if (s->duration)
>>>>>>       s->nb_samples = av_rescale(s->duration, outlink->sample_rate,
>>>>>> AV_TIME_BASE);
>>>>>>
>>>>>> Duration is parsed as seconds (can be fractional as well), but
>>>>>> internally
>>>>>> it is stored as microseconds (int64). Another example is the f_realtime
>>>>>> filter where the sleep time limit default according to the
>>>>>> documentation
>>>>>> is 2 seconds and it is expressed as 2000000 in the code.
>>>>>
>>>>> Yes, and current patch is wrong. Simply exceding max value is not proper
>>>>> fix.
>>>>
>>>> What is the proper fix then?
>>>
>>> Interpreting duration correctly, like trim filters do.
>>
>> Somebody help me, because I truly don't get it.
>>
>> 1) OPT_TYPE_DURATION default value is given in the .i64 field in
>> microseconds.
>>
>> 2) OPT_TYPE_DURATION min/max value is given in the min/max fields in
>> microseconds.
>>
>> Do you disagree with any of these? If yes, then what do you suggest?
>
> No, I do not disagree with any of those above. But patch is incomplete.

Post/push a better patch then please.

Thanks,
Marton