On Tue, Jul 02, 2019 at 12:28:53AM +0200, Ulf Zibis wrote: > Thanks Michael for your review, answers inline ... > > Am 01.07.19 um 17:16 schrieb Michael Niedermayer: > >> timestamp.h | 78 > >> ++++++++++++++++++++++++++++++++++++++++++++++++++++++++---- > >> 1 file changed, 73 insertions(+), 5 deletions(-) > >> 3d1275421b1b8d4952815151158c7af954d38ca6 timestamp_2.patch > >> From 709cb4662132deff2d17a8700f58fa91b118c56d Mon Sep 17 00:00:00 2001 > >> From: Ulf Zibis <ulf.zi...@cosoco.de> > >> Date: 29.06.2019, 17:52:06 > >> > >> avutil/timestamp: added 2 new print formats > >> > >> diff --git a/libavutil/timestamp.h b/libavutil/timestamp.h > >> index e082f01..b94e5ce 100644 > >> --- a/libavutil/timestamp.h > >> +++ b/libavutil/timestamp.h > >> @@ -48,14 +48,14 @@ > >> } > >> > >> /** > >> - * Convenience macro, the return value should be used only directly in > >> + * Convenience macro. The return value should be used only directly in > >> * function arguments but never stand-alone. > >> */ > >> -#define av_ts2str(ts) av_ts_make_string((char[AV_TS_MAX_STRING_SIZE]){0}, > >> ts) > >> +#define av_ts2str(ts) > >> av_ts_make_string((char[AV_TS_MAX_STRING_SIZE]){'\0'}, ts) > >> > >> /** > >> * Fill the provided buffer with a string containing a timestamp time > >> - * representation. > >> + * representation in seconds. > >> * > >> * @param buf a buffer with size in bytes of at least > >> AV_TS_MAX_STRING_SIZE > >> * @param ts the timestamp to represent > >> @@ -70,9 +70,77 @@ > >> } > >> > >> /** > >> - * Convenience macro, the return value should be used only directly in > >> + * Convenience macro. The return value should be used only directly in > >> * function arguments but never stand-alone. > >> */ > > Unrelated change, belongs in a seperate patch > The following change I think should be part of the patch, as it helps to > distinguish between the existing timestamp functions and my new ones: > - * representation. > + * representation in seconds. > The other above changes are cosmetic and can go in a separate patch. But > would you endorse them?
Iam not a english composer ... > > >> -#define av_ts2timestr(ts, tb) > >> av_ts_make_time_string((char[AV_TS_MAX_STRING_SIZE]){0}, ts, tb) > >> +#define av_ts2timestr(ts, tb) > >> av_ts_make_time_string((char[AV_TS_MAX_STRING_SIZE]){'\0'}, ts, tb) > >> + > >> +/** > >> + * Fill the provided buffer with a string containing a timestamp time > >> + * representation in minutes and seconds. > >> + * > >> + * @param buf a buffer with size in bytes of at least > >> AV_TS_MAX_STRING_SIZE > >> + * @param ts the timestamp to represent > >> + * @param tb the timebase of the timestamp > >> + * @return the buffer in input > >> + */ > >> +static inline char *av_ts_make_minute_string(char *buf, int64_t ts, > >> AVRational *tb) > >> +{ > >> + if (ts == AV_NOPTS_VALUE) snprintf(buf, AV_TS_MAX_STRING_SIZE, > >> "NOPTS"); > >> + else { > >> + double time = av_q2d(*tb) * ts; > > If this could be done without float/double that would be preferred as it > > avoids inaccuracies / slight differences between platforms > > I too thought on that, but the existing functions too rely on > float/double. Should I anyway provide a solution with integer arithmetic? its indepedant of your patch but i think all these should use integers unless its too messy thx [...] -- Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB Observe your enemies, for they first find out your faults. -- Antisthenes
signature.asc
Description: PGP signature
_______________________________________________ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".