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

[Michael Niedermayer]

On Wed, Feb 28, 2018 at 09:47:15AM +0100, Tobias Rapp wrote:
> 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?

iam not sure if the reduction of precission is a problem for some use case or not
But such a change doesnt belong in a patch factorizing the code.
It should be done seperately, if its changed

[...]

thx
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

There will always be a question for which you do not know the correct answer.
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 181 bytes
Desc: not available
URL: <http://ffmpeg.org/pipermail/ffmpeg-devel/attachments/20180301/7e594cc8/attachment.sig>