[FFmpeg-devel] [PATCH] parseutils: accept only full "ms" and "us" prefixes

[Rostislav Pehlivanov]

On 5 March 2018 at 16:47, Aurelien Jacobs <aurel at gnuage.org> wrote:

> On Sat, Mar 03, 2018 at 08:19:43PM +0000, Rostislav Pehlivanov wrote:
> > The commit which added those was pushed prematurely before anyone could
> object
> > to illogical suffixes like just m for milliseconds.
>
> What you call illogical is following the same convention as pretty much
> all numeric parameters in ffmpeg. (bitrate, bufsize, framerate...)
> So it is at least consistent.
>
> > Without this, we'd be locked
> > into never being able to implement the "m" suffix for minutes.
>
> I will always be against using "m" for minute, but you can use "min" if
> you want.
>
> > Signed-off-by: Rostislav Pehlivanov <atomnuker at gmail.com>
> > ---
> >  libavutil/parseutils.c | 10 ++++------
> >  1 file changed, 4 insertions(+), 6 deletions(-)
> >
> > diff --git a/libavutil/parseutils.c b/libavutil/parseutils.c
> > index 44c845577a..1b81757aab 100644
> > --- a/libavutil/parseutils.c
> > +++ b/libavutil/parseutils.c
> > @@ -689,17 +689,15 @@ int av_parse_time(int64_t *timeval, const char
> *timestr, int duration)
> >
> >      if (duration) {
> >          t = dt.tm_hour * 3600 + dt.tm_min * 60 + dt.tm_sec;
> > -        if (*q == 'm') {
> > +        if (q[0] == 'm' && q[1] == 's') {
> >              suffix = 1000;
> >              microseconds /= 1000;
> > -            q++;
> > -        } else if (*q == 'u') {
> > +            q += 2;
> > +        } else if (q[0] == 'u' && q[1] == 's') {
> >              suffix = 1;
> >              microseconds = 0;
> > -            q++;
> > +            q += 2;
> >          }
> > -        if (*q == 's')
> > -            q++;
> >      } else {
> >          int is_utc = *q == 'Z' || *q == 'z';
> >          int tzoffset = 0;
>
> Why do you remove support for using just 's' as a unit without prefix ?
> I've seen no one complaining about it and I can't see any issue with it.
>
> Also I don't like that this patch makes AV_OPT_TYPE_DURATION
> inconsistant with every other numeric parameters, that all accept a SI
> prefix without unit.
> (that include opus_delay for which "3.5" and "3500m" are equivalent)
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at ffmpeg.org
> http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
>

Dropped the "us" requirement and applied the patch.
Keeping the SI requirement where it doesn't make sense (and preventing us
from extending the option to something that does make sense) is worse than
being inconsistent.