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

[Michael Niedermayer]

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

its not that bad because 32bit/32bit * 64bit is more restricted than
an arbitrary double

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

What does censorship reveal? It reveals fear. -- Julian Assange
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20120120/77fdadfd/attachment.asc>