On 02.03.2018 23:14, wm4 wrote:
On Fri, 2 Mar 2018 22:48:07 +0100
Michael Niedermayer <mich...@niedermayer.cc> wrote:
On Fri, Mar 02, 2018 at 09:07:06AM +0100, Tobias Rapp wrote:
On 01.03.2018 22:08, Michael Niedermayer wrote:
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.r...@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
Factorizing the code *and* keeping the exact same behavior in both cases is
pointless in my eyes as it just increases the amount and complexity of code
for low benefit.
You could first make the formatting the same and then factorize it.
not saying that should be done, just that this should be the easy was to
factor it out
So please either consider this v3 patch or the original one posted in
https://ffmpeg.org/pipermail/ffmpeg-devel/2018-February/225775.html
iam not against this, but it seems others where
My comments were meant as minor suggestion for improvement (as I
stated, unfortunately only in my second reply), not a demand to rewrite
everything. If he just wants to push that first patch, fine with me.
I agree that factorization would make sense but it seems no agreement
could be found on how exactly it should be implemented. So just pushed
v1 of the patch which fixes the progress output strings followed by the
AVBPrint clean-up patch.
Thanks,
Tobias
_______________________________________________
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
