Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg: fix progress log message in case pts is not available
On Mon, 26 Feb 2018 14:47:30 +0100
Tobias Rapp wrote:
> On 26.02.2018 14:02, wm4 wrote:
> > On Mon, 26 Feb 2018 13:14:58 +0100
> > Tobias Rapp wrote:
> >
> >> Also fixes sign prefix for progress report.
> >>
> >> Signed-off-by: Tobias Rapp
> >> ---
> >> fftools/ffmpeg.c | 25 +
> >> 1 file changed, 17 insertions(+), 8 deletions(-)
> >>
> >> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> >> index 32caa4b..51f27bf 100644
> >> --- a/fftools/ffmpeg.c
> >> +++ b/fftools/ffmpeg.c
> >> @@ -1650,6 +1650,7 @@ static void print_report(int is_last_report, int64_t
> >> timer_start, int64_t cur_ti
> >> static int64_t last_time = -1;
> >> static int qp_histogram[52];
> >> int hours, mins, secs, us;
> >> +const char *hours_sign;
> >> int ret;
> >> float t;
> >>
> >> @@ -1757,6 +1758,7 @@ static void print_report(int is_last_report, int64_t
> >> timer_start, int64_t cur_ti
> >> secs %= 60;
> >> hours = mins / 60;
> >> mins %= 60;
> >> +hours_sign = (pts < 0) ? "-" : "";
> >>
> >> bitrate = pts && total_size >= 0 ? total_size * 8 / (pts / 1000.0) :
> >> -1;
> >> speed = t != 0.0 ? (double)pts / AV_TIME_BASE / t : -1;
> >> @@ -1765,11 +1767,13 @@ static void print_report(int is_last_report,
> >> int64_t timer_start, int64_t cur_ti
> >>"size=N/A time=");
> >> elsesnprintf(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), "-");
> >> -snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
> >> - "%02d:%02d:%02d.%02d ", hours, mins, secs,
> >> - (100 * us) / AV_TIME_BASE);
> >> +if (pts == AV_NOPTS_VALUE) {
> >> +snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "N/A");
> >> +} else {
> >> +snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
> >> + "%s%02d:%02d:%02d.%02d ", hours_sign, hours, mins, secs,
> >> + (100 * us) / AV_TIME_BASE);
> >> +}
> >>
> >> if (bitrate < 0) {
> >> snprintf(buf + strlen(buf), sizeof(buf) -
> >> strlen(buf),"bitrate=N/A");
> >> @@ -1781,9 +1785,14 @@ 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");
> >> elseav_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");
> >> +av_bprintf(&buf_script, "out_time=N/A\n");
> >> +} else {
> >> +av_bprintf(&buf_script, "out_time_ms=%"PRId64"\n", pts);
> >> +av_bprintf(&buf_script, "out_time=%s%02d:%02d:%02d.%06d\n",
> >> + hours_sign, hours, mins, secs, us);
> >> +}
> >>
> >> if (nb_frames_dup || nb_frames_drop)
> >> snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), " dup=%d
> >> drop=%d",
> >
> > Could use av_ts2str(), although that would return different formatting.
>
> I would prefer to not change the formatting, av_ts2str just prints the
> number of seconds as a float while the current HH:MM:SS.ZZ format is
> more user friendly, IMHO.
Yeah, sure. I don't insist on anything either - I just think it'd be
good if you'd consider moving this formatting into a av_ts2str() style
function/macro, which can be defined locally in ffmpeg.c.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg: fix progress log message in case pts is not available
On 26.02.2018 14:02, wm4 wrote:
On Mon, 26 Feb 2018 13:14:58 +0100
Tobias Rapp wrote:
Also fixes sign prefix for progress report.
Signed-off-by: Tobias Rapp
---
fftools/ffmpeg.c | 25 +
1 file changed, 17 insertions(+), 8 deletions(-)
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 32caa4b..51f27bf 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1650,6 +1650,7 @@ static void print_report(int is_last_report, int64_t
timer_start, int64_t cur_ti
static int64_t last_time = -1;
static int qp_histogram[52];
int hours, mins, secs, us;
+const char *hours_sign;
int ret;
float t;
@@ -1757,6 +1758,7 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti
secs %= 60;
hours = mins / 60;
mins %= 60;
+hours_sign = (pts < 0) ? "-" : "";
bitrate = pts && total_size >= 0 ? total_size * 8 / (pts / 1000.0) : -1;
speed = t != 0.0 ? (double)pts / AV_TIME_BASE / t : -1;
@@ -1765,11 +1767,13 @@ static void print_report(int is_last_report, int64_t
timer_start, int64_t cur_ti
"size=N/A time=");
elsesnprintf(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), "-");
-snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
- "%02d:%02d:%02d.%02d ", hours, mins, secs,
- (100 * us) / AV_TIME_BASE);
+if (pts == AV_NOPTS_VALUE) {
+snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "N/A");
+} else {
+snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
+ "%s%02d:%02d:%02d.%02d ", hours_sign, hours, mins, secs,
+ (100 * us) / AV_TIME_BASE);
+}
if (bitrate < 0) {
snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),"bitrate=N/A");
@@ -1781,9 +1785,14 @@ 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");
elseav_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");
+av_bprintf(&buf_script, "out_time=N/A\n");
+} else {
+av_bprintf(&buf_script, "out_time_ms=%"PRId64"\n", pts);
+av_bprintf(&buf_script, "out_time=%s%02d:%02d:%02d.%06d\n",
+ hours_sign, hours, mins, secs, us);
+}
if (nb_frames_dup || nb_frames_drop)
snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), " dup=%d
drop=%d",
Could use av_ts2str(), although that would return different formatting.
I would prefer to not change the formatting, av_ts2str just prints the
number of seconds as a float while the current HH:MM:SS.ZZ format is
more user friendly, IMHO.
Or maybe do something similar and put that code into a new function or
macro, so you don't have to repeat all those awful string buffer
management expressions in the first hunk.
If you refer to the "buf + strlen(buf), sizeof(buf) - strlen(buf)"
expressions the print_report() function is full of those. I agree that
switching buf to AVBPrint would improve the code -- this could be a
follow-up patch, if desired.
Regards,
Tobias
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
Re: [FFmpeg-devel] [PATCH] fftools/ffmpeg: fix progress log message in case pts is not available
On Mon, 26 Feb 2018 13:14:58 +0100
Tobias Rapp wrote:
> Also fixes sign prefix for progress report.
>
> Signed-off-by: Tobias Rapp
> ---
> fftools/ffmpeg.c | 25 +
> 1 file changed, 17 insertions(+), 8 deletions(-)
>
> diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
> index 32caa4b..51f27bf 100644
> --- a/fftools/ffmpeg.c
> +++ b/fftools/ffmpeg.c
> @@ -1650,6 +1650,7 @@ static void print_report(int is_last_report, int64_t
> timer_start, int64_t cur_ti
> static int64_t last_time = -1;
> static int qp_histogram[52];
> int hours, mins, secs, us;
> +const char *hours_sign;
> int ret;
> float t;
>
> @@ -1757,6 +1758,7 @@ static void print_report(int is_last_report, int64_t
> timer_start, int64_t cur_ti
> secs %= 60;
> hours = mins / 60;
> mins %= 60;
> +hours_sign = (pts < 0) ? "-" : "";
>
> bitrate = pts && total_size >= 0 ? total_size * 8 / (pts / 1000.0) : -1;
> speed = t != 0.0 ? (double)pts / AV_TIME_BASE / t : -1;
> @@ -1765,11 +1767,13 @@ static void print_report(int is_last_report, int64_t
> timer_start, int64_t cur_ti
> "size=N/A time=");
> elsesnprintf(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), "-");
> -snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
> - "%02d:%02d:%02d.%02d ", hours, mins, secs,
> - (100 * us) / AV_TIME_BASE);
> +if (pts == AV_NOPTS_VALUE) {
> +snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), "N/A");
> +} else {
> +snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),
> + "%s%02d:%02d:%02d.%02d ", hours_sign, hours, mins, secs,
> + (100 * us) / AV_TIME_BASE);
> +}
>
> if (bitrate < 0) {
> snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf),"bitrate=N/A");
> @@ -1781,9 +1785,14 @@ 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");
> elseav_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");
> +av_bprintf(&buf_script, "out_time=N/A\n");
> +} else {
> +av_bprintf(&buf_script, "out_time_ms=%"PRId64"\n", pts);
> +av_bprintf(&buf_script, "out_time=%s%02d:%02d:%02d.%06d\n",
> + hours_sign, hours, mins, secs, us);
> +}
>
> if (nb_frames_dup || nb_frames_drop)
> snprintf(buf + strlen(buf), sizeof(buf) - strlen(buf), " dup=%d
> drop=%d",
Could use av_ts2str(), although that would return different formatting.
Or maybe do something similar and put that code into a new function or
macro, so you don't have to repeat all those awful string buffer
management expressions in the first hunk.
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
http://ffmpeg.org/mailman/listinfo/ffmpeg-devel
