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

[Reimar Döffinger]

On Fri, Jan 20, 2012 at 04:13:52PM +0100, Stefano Sabatini wrote:
> On date Tuesday 2012-01-17 22:56:35 +0100, Reimar Döffinger encoded:
> > On Tue, Jan 17, 2012 at 10:44:21PM +0100, Alexander Strasser wrote:
> > > [...]
> > > > +
> > > > +static inline char *av_get_ts_string(char *buf, size_t buf_size, int64_t ts)
> > > > +{
> > > > +    if (ts == AV_NOPTS_VALUE) snprintf(buf, buf_size, "NOPTS");
> > > > +    else                      snprintf(buf, buf_size, "%"PRId64"", ts);
> > > > +    return buf;
> > > > +}
> > > > +
> > > > +#define av_ts2str(ts) av_get_ts_string((char[42]){ 0 }, 42, ts)
> > > > +
> > > > +static inline char* av_get_time_string(char *buf, size_t buf_size, int64_t ts, AVRational *tb)
> > > > +{
> > > > +    if (ts == AV_NOPTS_VALUE) snprintf(buf, buf_size, "NOPTS");
> > > > +    else                      snprintf(buf, buf_size, "%f", av_q2d(*tb) * ts);
> > > > +    return buf;
> > > > +}
> > > > +
> > > > +#define av_time2str(ts, tb) av_get_time_string((char[42]){ 0 }, 42, ts, tb)
> > > 
> > >   The naming of the macros is a bit risky. Especially considering how often we
> > > append numbers to names to introduce a successor. av_time2str2 would be rather
> > > confusing.
> > 
> 
> > Also while I don't care too much about C++, the (char[42]){ 0 } syntax
> > has not made it into even the latest C++ version, so I'd strongly
> > suggest to think at least thrice before adding such a thing to the
> > public API (even though a macro means it won't cause compile errors
> > unless used - but if we want to take advantage of that it should
> > probably be documented).
> > Actually, I think it might be a right-out bad idea to use this
> > at all in public API, because I don't think anyone would accept
> > that
> 
> Agreed, also I don't think that:
> printf("pts1_time:%s pts2_time:%s\n", av_time2str(pts1, tb1), av_time2str(pts2, tb2))
> 
> has defined behavior in this case (evaluation order of arguments is
> undefined in C, and the construct may return an address in the stack
> which to a buffer which may be overwritten by the other function
> call).

No, I don't think that is a problem. They are different variables
which both have the same scope (the current block) so the compiler
can't place them any more at the same place that if you used
char a[42],b[42];

> char buf1[AV_TS_MAX_SIZE], buf2[AV_TS_MAX_SIZE];

Not much of an opinion otherwise, but _TS_ is not that great a name
due to the confusion potential with (MPEG-)TS.