[FFmpeg-devel] [RFC] lavu PTS printing utils

[Alexander Strasser]

Stefano Sabatini wrote:
> On date Friday 2012-01-27 22:14:35 +0100, Alexander Strasser encoded:
> > Stefano Sabatini wrote:
> > [...]
> > > Updated patch with sample ffmpeg patch, comments:
> > > 
> > > * av_ts2str(), av_ts2time2str() are retained for their usefulness,
> > >   although they should never be used in C++, we assume that C++
> > >   programmers using FFmpeg code have a clue regarding C/C++
> > >   incompatibilites, and they can still use the long av_ts_make*
> > >   functions
> > 
> >   As they are macros and are for convenience only (functionality is still
> > available for C++ programmers), I am OK with that.
> > 
> > > * double precision is reduced to 6 digits, which looks enough for A/V
> > >   applications, could not be enough for scientific computing but again
> > >   given the application domain I don't think this is a much serious
> > >   issue
> > > 
> > > * "ts" is favored over "timestamp" for brevity/convenience reasons
> > > 
> > > * "ts2str" naming is kept in the macro for brevity/convenience reasons
> > 
> >   Fine if you prefer it this way.
> > 
> 
> > [...] 
> > > +static inline char *av_ts_make_string(char *buf, int64_t ts)
> > [...] 
> > > +static inline char *av_ts_make_time_string(char *buf, int64_t ts, AVRational *tb)
> > 
> > NIT:
> >   Here and before I would prefer ts as the first argument, it is also consistent
> > with the naming which is av_ts_*. Like it is saying these functions are acting on
> > a timestamp (I know they can't and don't need to modify it as they only get a copy).
> >
> >   IMHO following that convention makes the order more easy to remember. I would be
> > glad to hear your opinion.
> 
> Procedural C style convenctions usually demand this order of arguments:
> 
> make_thing(dst, src);

  This differs often. Also there is no clear solution then to
settle on one convention and use it consistently. But for me
this is an instance of a different problem, which applies if
you have two equal entities like registers or byte arrays or
other arrays or structures of the same type.

> so in this case that would be:
> av_ts_make_string(char *buf, int64_t ts)
> 
> which is interpreted as:
> buf <= ts
> 
> i.e. like an assignment.
> 
> In case of:
> static inline char *av_ts_make_time_string(char *buf, int64_t ts, AVRational *tb);
> 
> here the timestamp can't be represented by a single entity (indeed it
> needs a tb in order to be interpreted), so I think that preferring:

  You could also say you need a tb to produce an absolute time representation
from a ts.

> static inline char *av_ts_make_time_string(int64_t ts, AVRational *tb, char *buf)
> 
> would be mostly confusing.

  I would have preferred that order.

> Note that here "_ts_" is used like a namespace rather than for
> indicating the "object" of the "operator" denoted by the function, in
> case you're adopting an object-oriented programming style, for which:
> 
> int av_bike_set_shed_color(AVBike *bike, char *color_str);
> 
> would have made much sense.

  The borderline is always kind of fuzzy. Depends on your viewing angle.
I think I could explain myself a bit better but it is getting off topic.

  Anyway I won't insist on anything in this corner case. So it is up
to you to decide.

  Alexander