Re: [FFmpeg-devel] [PATCH] ffmpeg: add 100ms delay for first stats
On 23-12-2020 04:03 pm, Nicolas George wrote:
Gyan Doshi (12020-12-23):
It's void av_dump_format(...)
Seems too trivial to change it , or add + initialize yet another global
variable just for this.
Exactly. Dumping the output is a global affair, it can warrant a global
variable. Something like this:
of->header_written = 1;
av_dump_format(of->ctx, file_index, of->ctx->url, 1);
+ output_dumped++;
...
+ if (output_dumped >= nb_output_files) {
...
+ }
That way, the initial stat is printed as early as possible, but will not
mangle the dump. And if the dump is very late, it is acceptable to have
the 500ms timeout too:
+ if (output_dumped >= nb_output_files || blah >= 50) {
Ok. Will work on this.
But I still think I should post an immediate fix that handles it for 90%
of cases.
Thanks,
Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] ffmpeg: add 100ms delay for first stats
Gyan Doshi (12020-12-23):
> It's void av_dump_format(...)
>
> Seems too trivial to change it , or add + initialize yet another global
> variable just for this.
Exactly. Dumping the output is a global affair, it can warrant a global
variable. Something like this:
of->header_written = 1;
av_dump_format(of->ctx, file_index, of->ctx->url, 1);
+ output_dumped++;
...
+ if (output_dumped >= nb_output_files) {
...
+ }
That way, the initial stat is printed as early as possible, but will not
mangle the dump. And if the dump is very late, it is acceptable to have
the 500ms timeout too:
+ if (output_dumped >= nb_output_files || blah >= 50) {
Regards,
--
Nicolas George
signature.asc
Description: PGP signature
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] ffmpeg: add 100ms delay for first stats
On 23-12-2020 03:53 pm, Nicolas George wrote: Gyan Doshi (12020-12-23): That's not the issue. It's the mangled line - the output from lavf/dump.c. mangled with stats line AV_NOPTS_VALUE is an acceptable value and already checked for at line 1815. As is <=0 output size. Indicating to the user that ffmpeg processing has stalled at the beginning is a valid and acceptable outcome, even a necessary one. The delay here is just to avoid the mangling. Some hardcoded value is the best that I see we can do, but if you have an analytical solution, I'm all ears. Ah, I see. But in that case, the proper solution seems obvious: set a flag when av_dump_format() has returned, and delay the printing until that flag is set. Or am I still missing something? It's void av_dump_format(...) Seems too trivial to change it , or add + initialize yet another global variable just for this. Regards, Gyan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] ffmpeg: add 100ms delay for first stats
Gyan Doshi (12020-12-23): > That's not the issue. It's the mangled line - the output from lavf/dump.c. > mangled with stats line > > AV_NOPTS_VALUE is an acceptable value and already checked for at line 1815. > As is <=0 output size. > > Indicating to the user that ffmpeg processing has stalled at the beginning > is a valid and acceptable outcome, even a necessary one. > The delay here is just to avoid the mangling. Some hardcoded value is the > best that I see we can do, but if you have an analytical solution, I'm all > ears. Ah, I see. But in that case, the proper solution seems obvious: set a flag when av_dump_format() has returned, and delay the printing until that flag is set. Or am I still missing something? Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] ffmpeg: add 100ms delay for first stats
On 23-12-2020 03:35 pm, Nicolas George wrote: Gyan Doshi (12020-12-23): Which piece of info or object would indicate that? At the stage of this check, I don't see any available. Read Michael's mail: Output #0, avi, to 'file.avi':= 0kB time=-577014:32:22.77 bitrate= -0.0kbits/s speed=N/A The problem is obviously the time. The large negative number looks like it was converted from AV_NOPTS_VALUE, so test if the PTS being printed is AV_NOPTS_VALUE or not. That's not the issue. It's the mangled line - the output from lavf/dump.c. mangled with stats line AV_NOPTS_VALUE is an acceptable value and already checked for at line 1815. As is <=0 output size. Indicating to the user that ffmpeg processing has stalled at the beginning is a valid and acceptable outcome, even a necessary one. The delay here is just to avoid the mangling. Some hardcoded value is the best that I see we can do, but if you have an analytical solution, I'm all ears. Regards, Gyan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] ffmpeg: add 100ms delay for first stats
Gyan Doshi (12020-12-23): > Which piece of info or object would indicate that? At the stage of this > check, I don't see any available. Read Michael's mail: Output #0, avi, to 'file.avi':= 0kB time=-577014:32:22.77 bitrate= -0.0kbits/s speed=N/A The problem is obviously the time. The large negative number looks like it was converted from AV_NOPTS_VALUE, so test if the PTS being printed is AV_NOPTS_VALUE or not. > Earlier, it was hardcoded to 500 ms. I chose something more apt for 2020, > but 500 ms is fine as well. If you fix it, fix it properly, please. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] ffmpeg: add 100ms delay for first stats
On 23-12-2020 03:12 pm, Nicolas George wrote: Gyan Doshi (12020-12-23): Avoids breaking output file dump report. --- fftools/ffmpeg.c | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) This looks broken. Why 100ms? Because on your setup, 80ms is not enough but 100ms is? On a slower system, it would need to be 120 or 200. Test if the info to be printed is already valid. Which piece of info or object would indicate that? At the stage of this check, I don't see any available. Earlier, it was hardcoded to 500 ms. I chose something more apt for 2020, but 500 ms is fine as well. Regards, Gyan ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] ffmpeg: add 100ms delay for first stats
Gyan Doshi (12020-12-23): > Avoids breaking output file dump report. > --- > fftools/ffmpeg.c | 3 ++- > 1 file changed, 2 insertions(+), 1 deletion(-) This looks broken. Why 100ms? Because on your setup, 80ms is not enough but 100ms is? On a slower system, it would need to be 120 or 200. Test if the info to be printed is already valid. Regards, -- Nicolas George signature.asc Description: PGP signature ___ ffmpeg-devel mailing list ffmpeg-devel@ffmpeg.org https://ffmpeg.org/mailman/listinfo/ffmpeg-devel To unsubscribe, visit link above, or email ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
Re: [FFmpeg-devel] [PATCH] ffmpeg: add 100ms delay for first stats
On 23-12-2020 10:28 am, Gyan Doshi wrote:
Avoids breaking output file dump report.
---
fftools/ffmpeg.c | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c
index 2c0820aacf..449da484d9 100644
--- a/fftools/ffmpeg.c
+++ b/fftools/ffmpeg.c
@@ -1699,7 +1699,8 @@ static void print_report(int is_last_report, int64_t
timer_start, int64_t cur_ti
if (last_time == -1) {
last_time = cur_time;
}
-if ((cur_time - last_time) < stats_period && !first_report)
+if (((cur_time - last_time) < stats_period && !first_report) ||
+((cur_time - last_time) < 10 && first_report))
return;
last_time = cur_time;
}
Looks fixed here. Can anyone else do a quick check?
Thanks,
Gyan
___
ffmpeg-devel mailing list
ffmpeg-devel@ffmpeg.org
https://ffmpeg.org/mailman/listinfo/ffmpeg-devel
To unsubscribe, visit link above, or email
ffmpeg-devel-requ...@ffmpeg.org with subject "unsubscribe".
