[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 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.

Actually patch is fine, sorry.