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

Stefano Sabatini stefasab at gmail.com
Fri Jan 20 16:13:52 CET 2012


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).

Actually there is also another problem, the size of a double number
representation doesn't have a well defined size, check for example:
http://stackoverflow.com/questions/1701055/what-is-the-maximum-length-in-chars-needed-to-represent-any-double-value

we may format the string to a limited size and add a define
#define AV_TS_MAX_SIZE ...

or the time representation may result truncated.

In this case the usage would be:

char buf1[AV_TS_MAX_SIZE], buf2[AV_TS_MAX_SIZE];
printf("pts1_time:%s pts2_time:%s\n", av_time2str(buf1, pts1, tb1), av_time2str(buf2, pts2, tb2));

which is not as convenient but should be safe.
-- 
FFmpeg = Formidable & Faithless Murdering Philosofic Elfic Ghost


More information about the ffmpeg-devel mailing list