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
> You really insist on accepting '5u'?
I'm not insisting on this (even if I prefer it), but I'm saying that
your patch is *not* removing '5u' support at least for *some* time
options, so it is actually decreasing consistency.
ffmpeg-devel mailing list