| Message ID | 1519661345-16991-1-git-send-email-t.rapp@noa-archive.com |
|---|---|
| State | Superseded |
| Headers | show |
On Mon, Feb 26, 2018 at 05:09:04PM +0100, Tobias Rapp wrote: > Move time string formatting into inline function. Also fixes out_time > sign prefix for progress report. > > Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> > --- > fftools/ffmpeg.c | 48 +++++++++++++++++++++++++++++++----------------- > 1 file changed, 31 insertions(+), 17 deletions(-) > > diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > index 32caa4b..0097a7d 100644 > --- a/fftools/ffmpeg.c > +++ b/fftools/ffmpeg.c > @@ -1518,6 +1518,27 @@ static int reap_filters(int flush) > return 0; > } > > +static inline char *pts_to_hms_str(char *buf, int64_t pts, unsigned int digits) char buf[AV_TS_MAX_STRING_SIZE] or the buf size should be passed too, in fact this might be better anyway > +{ > + const char *hours_sign; > + int hours, mins; > + double secs; > + > + if (pts == AV_NOPTS_VALUE) { > + snprintf(buf, AV_TS_MAX_STRING_SIZE, "N/A"); > + } else { > + hours_sign = (pts < 0) ? "-" : ""; > + secs = (double)FFABS(pts) / AV_TIME_BASE; > + mins = (int)secs / 60; > + secs = secs - mins * 60; > + hours = mins / 60; > + mins %= 60; This is not the same code, also with double it can produce inexact results and results differing between platforms [...]
On 27.02.2018 01:12, Michael Niedermayer wrote: > On Mon, Feb 26, 2018 at 05:09:04PM +0100, Tobias Rapp wrote: >> Move time string formatting into inline function. Also fixes out_time >> sign prefix for progress report. >> >> Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> >> --- >> fftools/ffmpeg.c | 48 +++++++++++++++++++++++++++++++----------------- >> 1 file changed, 31 insertions(+), 17 deletions(-) >> >> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c >> index 32caa4b..0097a7d 100644 >> --- a/fftools/ffmpeg.c >> +++ b/fftools/ffmpeg.c >> @@ -1518,6 +1518,27 @@ static int reap_filters(int flush) >> return 0; >> } >> >> +static inline char *pts_to_hms_str(char *buf, int64_t pts, unsigned int digits) > > char buf[AV_TS_MAX_STRING_SIZE] > > or the buf size should be passed too, in fact this might be better anyway Will change. >> +{ >> + const char *hours_sign; >> + int hours, mins; >> + double secs; >> + >> + if (pts == AV_NOPTS_VALUE) { >> + snprintf(buf, AV_TS_MAX_STRING_SIZE, "N/A"); >> + } else { >> + hours_sign = (pts < 0) ? "-" : ""; >> + secs = (double)FFABS(pts) / AV_TIME_BASE; >> + mins = (int)secs / 60; >> + secs = secs - mins * 60; >> + hours = mins / 60; >> + mins %= 60; > > This is not the same code, also with double it can produce inexact > results and results differing between platforms I changed secs to double to handle the cases with different number of sub-second digits more easily. Would it be OK to output two digits after the decimal point in both cases? The progress report contains the precise out_time_ms value anyway. Regards, Tobias
On Tue, Feb 27, 2018 at 08:49:19AM +0100, Tobias Rapp wrote: > On 27.02.2018 01:12, Michael Niedermayer wrote: > >On Mon, Feb 26, 2018 at 05:09:04PM +0100, Tobias Rapp wrote: > >>Move time string formatting into inline function. Also fixes out_time > >>sign prefix for progress report. > >> > >>Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> > >>--- > >> fftools/ffmpeg.c | 48 +++++++++++++++++++++++++++++++----------------- > >> 1 file changed, 31 insertions(+), 17 deletions(-) > >> > >>diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c > >>index 32caa4b..0097a7d 100644 > >>--- a/fftools/ffmpeg.c > >>+++ b/fftools/ffmpeg.c > >>@@ -1518,6 +1518,27 @@ static int reap_filters(int flush) > >> return 0; > >> } > >>+static inline char *pts_to_hms_str(char *buf, int64_t pts, unsigned int digits) > > > >char buf[AV_TS_MAX_STRING_SIZE] > > > >or the buf size should be passed too, in fact this might be better anyway > > Will change. > > >>+{ > >>+ const char *hours_sign; > >>+ int hours, mins; > >>+ double secs; > >>+ > >>+ if (pts == AV_NOPTS_VALUE) { > >>+ snprintf(buf, AV_TS_MAX_STRING_SIZE, "N/A"); > >>+ } else { > >>+ hours_sign = (pts < 0) ? "-" : ""; > >>+ secs = (double)FFABS(pts) / AV_TIME_BASE; > >>+ mins = (int)secs / 60; > >>+ secs = secs - mins * 60; > >>+ hours = mins / 60; > >>+ mins %= 60; > > > >This is not the same code, also with double it can produce inexact > >results and results differing between platforms > > I changed secs to double to handle the cases with different number of > sub-second digits more easily. Would it be OK to output two digits after the > decimal point in both cases? The progress report contains the precise > out_time_ms value anyway. iam not sure iam guessing correctly what you mean by "both cases" you mean if its unneeded as in .00 ? I guess that would be ok [...]
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index 32caa4b..0097a7d 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -1518,6 +1518,27 @@ static int reap_filters(int flush) return 0; } +static inline char *pts_to_hms_str(char *buf, int64_t pts, unsigned int digits) +{ + const char *hours_sign; + int hours, mins; + double secs; + + if (pts == AV_NOPTS_VALUE) { + snprintf(buf, AV_TS_MAX_STRING_SIZE, "N/A"); + } else { + hours_sign = (pts < 0) ? "-" : ""; + secs = (double)FFABS(pts) / AV_TIME_BASE; + mins = (int)secs / 60; + secs = secs - mins * 60; + hours = mins / 60; + mins %= 60; + snprintf(buf, AV_TS_MAX_STRING_SIZE, "%s%02d:%02d:%0*.*f", + hours_sign, hours, mins, digits+3, digits, secs); + } + return buf; +} + static void print_final_stats(int64_t total_size) { uint64_t video_size = 0, audio_size = 0, extra_size = 0, other_size = 0; @@ -1649,7 +1670,7 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti int64_t pts = INT64_MIN + 1; static int64_t last_time = -1; static int qp_histogram[52]; - int hours, mins, secs, us; + char buf_pts[AV_TS_MAX_STRING_SIZE] = {0}; int ret; float t; @@ -1751,25 +1772,15 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti nb_frames_drop += ost->last_dropped; } - secs = FFABS(pts) / AV_TIME_BASE; - us = FFABS(pts) % AV_TIME_BASE; - mins = secs / 60; - secs %= 60; - hours = mins / 60; - mins %= 60; - bitrate = pts && total_size >= 0 ? total_size * 8 / (pts / 1000.0) : -1; speed = t != 0.0 ? (double)pts / AV_TIME_BASE / t : -1; if (total_size < 0) snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), - "size=N/A time="); + "size=N/A "); else snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), - "size=%8.0fkB time=", total_size / 1024.0); - if (pts < 0) - snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "-"); + "size=%8.0fkB ", total_size / 1024.0); snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), - "%02d:%02d:%02d.%02d ", hours, mins, secs, - (100 * us) / AV_TIME_BASE); + "time=%s ", pts_to_hms_str(buf_pts, pts, 2)); if (bitrate < 0) { snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),"bitrate=N/A"); @@ -1781,9 +1792,12 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti if (total_size < 0) av_bprintf(&buf_script, "total_size=N/A\n"); else av_bprintf(&buf_script, "total_size=%"PRId64"\n", total_size); - av_bprintf(&buf_script, "out_time_ms=%"PRId64"\n", pts); - av_bprintf(&buf_script, "out_time=%02d:%02d:%02d.%06d\n", - hours, mins, secs, us); + if (pts == AV_NOPTS_VALUE) { + av_bprintf(&buf_script, "out_time_ms=N/A\n"); + } else { + av_bprintf(&buf_script, "out_time_ms=%"PRId64"\n", pts); + } + av_bprintf(&buf_script, "out_time=%s\n", pts_to_hms_str(buf_pts, pts, 6)); if (nb_frames_dup || nb_frames_drop) snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), " dup=%d drop=%d",
Move time string formatting into inline function. Also fixes out_time sign prefix for progress report. Signed-off-by: Tobias Rapp <t.rapp@noa-archive.com> --- fftools/ffmpeg.c | 48 +++++++++++++++++++++++++++++++----------------- 1 file changed, 31 insertions(+), 17 deletions(-)