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

[Paul B Mahol]

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.