[FFmpeg-devel] [PATCH] avutil/parseutils: only accept full us duration, do not accept mss duration

[Marton Balint]

On Fri, 9 Mar 2018, Marton Balint wrote:

>
>
> On Thu, 8 Mar 2018, Hendrik Leppkes wrote:
>
>> On Thu, Mar 8, 2018 at 9:47 AM, Tobias Rapp <t.rapp at noa-archive.com> wrote:
>>> On 08.03.2018 00:14, Hendrik Leppkes wrote:
>>>>
>>>> On Thu, Mar 8, 2018 at 12:05 AM, Aurelien Jacobs <aurel at gnuage.org> 
> wrote:
>>>>>
>>>>> On Wed, Mar 07, 2018 at 11:45:03PM +0100, Marton Balint wrote:
>>>>>>
>>>>>>
>>>>>>
>>>>>> On Wed, 7 Mar 2018, Aurelien Jacobs wrote:
>>>>>>
>>>>>>> On Tue, Mar 06, 2018 at 01:02:48AM +0100, Marton Balint wrote:
>>>>>>>>
>>>>>>>> Accepting 'u' suffix for a time specification is neither intuitive 
> nor
>>>>>>>> consistent (now that we don't accept m).
>>>>>>>
>>>>>>>
>>>>>>> The 'm' SI prefix is still accepted in various time options, and the
>>>>>>> 'u'
>>>>>>> prefix is still accepted in those options even after your patch, so 
> you
>>>>>>> can't really argue that this patch improve consistency.
>>>>>>> (eg. -black_min_duration 5ms is still accepted).
>>>>>>> So this will surprise nobody that I don't like this patch.
>>>>>>
>>>>>>
>>>>>> This really is a cursed topic, I am not sure I follow, after the patch:
>>>>>>
>>>>>> 5ms is accepted
>>>>>> 5us is accepted
>>>>>> 5m is not accepted
>>>>>> 5u is not accepted
>>>>>
>>>>>
>>>>> That is only for AV_OPT_TYPE_DURATION.
>>>>> All other numeric options type still accept SI prefix without unit.
>>>>> This include various time options such as black_min_duration.
>>>>> So after the patch, for black_min_duration:
>>>>>
>>>>> 5m is accepted
>>>>> 5u is accepted
>>>>>
>>>>
>>>> Because those use AV_OPT_TYPE_DOUBLE, a generic type without any
>>>> possiblity of a unit.
>>>> Ideally those should all be transitioned to AV_OPT_TYPE_DURATION if
>>>> they refer to a time for consistency, and not the actual time-type
>>>> bend to reflect some generic type. Why would we have the duration type
>>>> if its just a copy of the double type anyway?
>>>
>>>
>>> As a user with technical background I find it logical that
>>> AV_OPT_TYPE_DURATION is a superset of AV_OPT_TYPE_DOUBLE. Any
>>> double-formatted string is treated as a value in seconds. Additionally
>>> AV_OPT_TYPE_DURATION accepts HH:MM:SS formatted strings.
>>>
>>> So I think Aurelien has a point here: Why should we enforce a unit suffix
>>> for AV_OPT_TYPE_DURATION and loose the superset property?
>>>
>>
>> Because something like "3m" for a time property is ambigious and
>> confusing. I would personally never expect that to mean "3ms", and as
>> this thread has shown here, I would not be alone with that
>> interpretation.
>
> Ok, it seems there is no consensus/decision, but keeping it how it is 
> locks us in accepting the prefix-only variants because of compatibility. 
> So I will push this soon unless I get an explicit reject from somebody.

Pushed.

Regards,
Marton