[FFmpeg-devel] [PATCH v2 1/2] fftools/ffmpeg: fix progress log message in case pts is not available

[Tobias Rapp]

On 27.02.2018 19:03, Michael Niedermayer wrote:
> 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 at noa-archive.com>
>>>> ---
>>>>   fftools/ffmpeg.c | 48 +++++++++++++++++++++++++++++++-----------------
>>>>   1 file changed, 31 insertions(+), 17 deletions(-)
>>>>
>>> [...]
>>>> +{
>>>> +    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

There are two places within print_report() that output 
hour/minute/seconds-formatted time. One is using HH:MM:SS.ZZ format and 
the other one is using HH:MM:SS.ZZZZZZ format. Would it be OK to output 
HH:MM:SS.ZZ (two digits after the decimal separator) in both places like 
in the attached patch version?

Regards,
Tobias

-------------- next part --------------
From d2141a259d733e9ff8e76f0e33b3e2e449adde14 Mon Sep 17 00:00:00 2001
From: Tobias Rapp <t.rapp at noa-archive.com>
Date: Mon, 26 Feb 2018 16:58:24 +0100
Subject: [PATCH v3] fftools/ffmpeg: fix progress log message in case pts is
 not available

Move time string formatting into inline function. Also fixes out_time
sign prefix for progress report.

Signed-off-by: Tobias Rapp <t.rapp at 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 3a45f43..f3598ff 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, size_t buf_size, int64_t pts)
+{
+    const char *hours_sign;
+    int hours, mins, secs, usecs;
+
+    if (pts == AV_NOPTS_VALUE) {
+        snprintf(buf, buf_size, "N/A");
+    } else {
+        hours_sign = (pts < 0) ? "-" : "";
+        secs = FFABS(pts) / AV_TIME_BASE;
+        usecs = FFABS(pts) % AV_TIME_BASE;
+        mins = secs / 60;
+        secs %= 60;
+        hours = mins / 60;
+        mins %= 60;
+        snprintf(buf, buf_size, "%s%02d:%02d:%02d.%02d",
+                 hours_sign, hours, mins, secs, (100 * usecs) / AV_TIME_BASE);
+    }
+    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, sizeof(buf_pts), pts));
 
     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, sizeof(buf_pts), pts));
 
     if (nb_frames_dup || nb_frames_drop)
         snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), " dup=%d drop=%d",
-- 
2.7.4