| Message ID | 20180622080338.99498-2-wangcao@google.com |
|---|---|
| State | New |
| Headers | show |
Friendly ping. Thanks!
On Fri, Jun 22, 2018 at 04:03:38PM +0800, Wang Cao wrote: > Make ffmpeg to output stats for each video/audio streams and each ouptut file ffmpeg output log in print_report. The report of video/audio sizes is clear now as previously all output video/audio sizes were combined to report and it is unclear such stats is for one output files or aggregates for all output files. > > Signed-off-by: Wang Cao <wangcao@google.com> > --- > fftools/ffmpeg.c | 52 +++++++++++++++++++++++++++++++++--------------- > 1 file changed, 36 insertions(+), 16 deletions(-) This changes the printed values ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -qscale 2 -vframes 3 -an test2.avi frame= 3 fps=0.0 q=2.0 Lsize= 51kB time=00:00:00.12 bitrate=3495.7kbits/s speed= 12x video:51kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 11.128063% vs. frame= 3 fps=0.0 q=2.0 Lsize= 57kB time=00:00:00.12 bitrate=3884.7kbits/s speed=11.2x video:51kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB muxing overhead: 11.128063% the file is the same [...]
> > This changes the printed values > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -qscale 2 -vframes 3 -an > test2.avi > frame= 3 fps=0.0 q=2.0 Lsize= 51kB time=00:00:00.12 > bitrate=3495.7kbits/s speed= 12x > video:51kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB > muxing overhead: 11.128063% > vs. > frame= 3 fps=0.0 q=2.0 Lsize= 57kB time=00:00:00.12 > bitrate=3884.7kbits/s speed=11.2x > video:51kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB > muxing overhead: 11.128063% > the file is the same Thanks for pointing this out! I have noticed the change in the stats but I think the change is correct. Before my patch, the bitrate calculation is based on total_size as in > oc = output_files[0]->ctx; > total_size = avio_size(oc->pb); If I understand this correctly, the size of whole file will be used to calculate the bitrate which considers probably header overhead. I changed it to > total_size = ost->data_size; Therefore only the size of the stream under consideration will be used for calculation. Please correct me if my understanding is wrong. Thanks! On Fri, Jun 29, 2018 at 1:58 PM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Fri, Jun 22, 2018 at 04:03:38PM +0800, Wang Cao wrote: > > Make ffmpeg to output stats for each video/audio streams and each ouptut > file ffmpeg output log in print_report. The report of video/audio sizes is > clear now as previously all output video/audio sizes were combined to > report and it is unclear such stats is for one output files or aggregates > for all output files. > > > > Signed-off-by: Wang Cao <wangcao@google.com> > > --- > > fftools/ffmpeg.c | 52 +++++++++++++++++++++++++++++++++--------------- > > 1 file changed, 36 insertions(+), 16 deletions(-) > > This changes the printed values > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -qscale 2 -vframes 3 -an > test2.avi > > frame= 3 fps=0.0 q=2.0 Lsize= 51kB time=00:00:00.12 > bitrate=3495.7kbits/s speed= 12x > video:51kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB > muxing overhead: 11.128063% > > vs. > > frame= 3 fps=0.0 q=2.0 Lsize= 57kB time=00:00:00.12 > bitrate=3884.7kbits/s speed=11.2x > video:51kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB > muxing overhead: 11.128063% > > the file is the same > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > If a bugfix only changes things apparently unrelated to the bug with no > further explanation, that is a good sign that the bugfix is wrong. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Wed, Jul 11, 2018 at 03:47:45PM -0700, Wang Cao wrote: > > > > This changes the printed values > > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -qscale 2 -vframes 3 -an > > test2.avi > > frame= 3 fps=0.0 q=2.0 Lsize= 51kB time=00:00:00.12 > > bitrate=3495.7kbits/s speed= 12x > > video:51kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB > > muxing overhead: 11.128063% > > vs. > > frame= 3 fps=0.0 q=2.0 Lsize= 57kB time=00:00:00.12 > > bitrate=3884.7kbits/s speed=11.2x > > video:51kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB > > muxing overhead: 11.128063% > > the file is the same > > > Thanks for pointing this out! I have noticed the change in the stats but I > think the change is correct. Before my patch, the bitrate calculation is > based on total_size as in > > > oc = output_files[0]->ctx; > > total_size = avio_size(oc->pb); > > If I understand this correctly, the size of whole file will be used to > calculate the bitrate which considers probably header overhead. > > I changed it to > > > total_size = ost->data_size; > > Therefore only the size of the stream under consideration will be used for > calculation. > > Please correct me if my understanding is wrong. Thanks! Do i understand correctly that you suggest to remove the display of the filesize and file bitrate ? And in its place would be a display of a randomly picked stream bitrate ? [...]
Sorry I probably confused you a little bit. I was not suggesting removing anything. I tried to convince you that there is some issue with the bitrate calculation. From your original comments: > This changes the printed values > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -qscale 2 -vframes 3 -an > test2.avi > frame= 3 fps=0.0 q=2.0 Lsize= 51kB time=00:00:00.12 > bitrate=3495.7kbits/s speed= 12x > video:51kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB > muxing overhead: 11.128063% > vs. > frame= 3 fps=0.0 q=2.0 Lsize= 57kB time=00:00:00.12 > bitrate=3884.7kbits/s speed=11.2x > video:51kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB > muxing overhead: 11.128063% > the file is the same After my change: > frame= 3 fps=0.0 q=2.0 Lsize= 51kB time=00:00:00.12 > bitrate=3495.7kbits/s speed= 12x Before my change(master branch) > frame= 3 fps=0.0 q=2.0 Lsize= 57kB time=00:00:00.12 > bitrate=3884.7kbits/s speed=11.2x The size is different (51 vs 57) because I display the stream size instead of the total file size. I think stream size is more suitable in this context with "frame, fps, time". For bitrate, I use the stream size to calculate it and I think it's more accurate. On Thu, Jul 12, 2018 at 5:01 AM Michael Niedermayer <michael@niedermayer.cc> wrote: > On Wed, Jul 11, 2018 at 03:47:45PM -0700, Wang Cao wrote: > > > > > > This changes the printed values > > > ./ffmpeg -i ~/videos/matrixbench_mpeg2.mpg -qscale 2 -vframes 3 -an > > > test2.avi > > > frame= 3 fps=0.0 q=2.0 Lsize= 51kB time=00:00:00.12 > > > bitrate=3495.7kbits/s speed= 12x > > > video:51kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB > > > muxing overhead: 11.128063% > > > vs. > > > frame= 3 fps=0.0 q=2.0 Lsize= 57kB time=00:00:00.12 > > > bitrate=3884.7kbits/s speed=11.2x > > > video:51kB audio:0kB subtitle:0kB other streams:0kB global headers:0kB > > > muxing overhead: 11.128063% > > > the file is the same > > > > > > Thanks for pointing this out! I have noticed the change in the stats but > I > > think the change is correct. Before my patch, the bitrate calculation is > > based on total_size as in > > > > > oc = output_files[0]->ctx; > > > total_size = avio_size(oc->pb); > > > > If I understand this correctly, the size of whole file will be used to > > calculate the bitrate which considers probably header overhead. > > > > I changed it to > > > > > total_size = ost->data_size; > > > > Therefore only the size of the stream under consideration will be used > for > > calculation. > > > > Please correct me if my understanding is wrong. Thanks! > > Do i understand correctly that you suggest to remove the display of the > filesize and file bitrate ? > And in its place would be a display of a randomly picked stream bitrate ? > > > [...] > -- > Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB > > If you fake or manipulate statistics in a paper in physics you will never > get a job again. > If you fake or manipulate statistics in a paper in medicin you will get > a job for life at the pharma industry. > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
2018-07-12 20:56 GMT+02:00, Wang Cao <doubleecao@gmail.com>: > After my change: > >> frame= 3 fps=0.0 q=2.0 Lsize= 51kB time=00:00:00.12 >> bitrate=3495.7kbits/s speed= 12x > > Before my change(master branch) > >> frame= 3 fps=0.0 q=2.0 Lsize= 57kB time=00:00:00.12 >> bitrate=3884.7kbits/s speed=11.2x > > The size is different (51 vs 57) because I display the stream > size instead of the total file size. I think stream size > is more suitable in this context with "frame, fps, time". > For bitrate, I use the stream size to calculate it and I think > it's more accurate. I believe users will expect that "size" shows the file size, especially since this was done for more than a decade. Carl Eugen
> > I believe users will expect that "size" shows the file size, especially > since this was done for more than a decade. If this is the convention, I believe it should be kept as it was. On the other hand, I believe "bitrate" should be calculated through stream size not the file size. On Thu, Jul 12, 2018 at 2:18 PM Carl Eugen Hoyos <ceffmpeg@gmail.com> wrote: > 2018-07-12 20:56 GMT+02:00, Wang Cao <doubleecao@gmail.com>: > > > After my change: > > > >> frame= 3 fps=0.0 q=2.0 Lsize= 51kB time=00:00:00.12 > >> bitrate=3495.7kbits/s speed= 12x > > > > Before my change(master branch) > > > >> frame= 3 fps=0.0 q=2.0 Lsize= 57kB time=00:00:00.12 > >> bitrate=3884.7kbits/s speed=11.2x > > > > The size is different (51 vs 57) because I display the stream > > size instead of the total file size. I think stream size > > is more suitable in this context with "frame, fps, time". > > For bitrate, I use the stream size to calculate it and I think > > it's more accurate. > > I believe users will expect that "size" shows the file size, especially > since this was done for more than a decade. > > Carl Eugen > _______________________________________________ > ffmpeg-devel mailing list > ffmpeg-devel@ffmpeg.org > http://ffmpeg.org/mailman/listinfo/ffmpeg-devel >
On Thu, Jul 12, 2018 at 04:31:11PM -0700, Wang Cao wrote: > > > > I believe users will expect that "size" shows the file size, especially > > since this was done for more than a decade. > > If this is the convention, I believe it should be kept as it was. On the > other hand, I believe > "bitrate" should be calculated through stream size not the file size. if theres a file with 5 video streams it doesnt make sense to summarize teh file with the bitrate of 1 stream Also the user has no indication of this change, the number is just different theres nothing informing him that it is no longer the files bitrate [...]
> if theres a file with 5 video streams it doesnt make sense to summarize > teh file with the bitrate of 1 stream > Also the user has no indication of this change, the number is just different > theres nothing informing him that it is no longer the files bitrate I was thinking to output stats for each video/audio stream one line by another. It seems this would be a huge change to users who are not aware. Keeping the old file bitrate looks good to me. Can I extend this to output file bitrate for each output file?
On 17-07-2018 02:38 AM, Wang Cao wrote: > I was thinking to output stats for each video/audio stream one line by > another. It seems this would be a huge change to users who are not > aware. Keeping the old file bitrate looks good to me. Can I extend > this to output file bitrate for each output file? Why not keep the current stats as is, and add an option for verbose stats? At the next major release, we can consider making them the default. Per-stream bitrate may be useful for those encoding HLS/DASH variants. Regards, Gyan
diff --git a/fftools/ffmpeg.c b/fftools/ffmpeg.c index d4ac6903cc..321f7a6017 100644 --- a/fftools/ffmpeg.c +++ b/fftools/ffmpeg.c @@ -1530,17 +1530,27 @@ static int reap_filters(int flush) return 0; } -static void print_final_stats(int64_t total_size) +static void print_final_stats(void) { uint64_t video_size = 0, audio_size = 0, extra_size = 0, other_size = 0; uint64_t subtitle_size = 0; uint64_t data_size = 0; + int64_t total_size; float percent = -1.0; int i, j; int pass1_used = 1; + AVFormatContext *oc; for (i = 0; i < nb_output_streams; i++) { OutputStream *ost = output_streams[i]; + if (i > 0 && ost->file_index != output_streams[i-1]->file_index) { + video_size = 0; + audio_size = 0; + extra_size = 0; + other_size = 0; + subtitle_size = 0; + data_size = 0; + } switch (ost->enc_ctx->codec_type) { case AVMEDIA_TYPE_VIDEO: video_size += ost->data_size; break; case AVMEDIA_TYPE_AUDIO: audio_size += ost->data_size; break; @@ -1552,7 +1562,13 @@ static void print_final_stats(int64_t total_size) if ( (ost->enc_ctx->flags & (AV_CODEC_FLAG_PASS1 | AV_CODEC_FLAG_PASS2)) != AV_CODEC_FLAG_PASS1) pass1_used = 0; - } + + // Print stats for each output file. + if (i == nb_output_streams-1 || ost->file_index != output_streams[i+1]->file_index) { + oc = output_files[ost->file_index]->ctx; + total_size = avio_size(oc->pb); + if (total_size <= 0) // FIXME improve avio_size() so it works with non seekable output too + total_size = avio_tell(oc->pb); if (data_size && total_size>0 && total_size >= data_size) percent = 100.0 * (total_size - data_size) / data_size; @@ -1568,6 +1584,8 @@ static void print_final_stats(int64_t total_size) else av_log(NULL, AV_LOG_INFO, "unknown"); av_log(NULL, AV_LOG_INFO, "\n"); + } + } /* print verbose per-stream stats */ for (i = 0; i < nb_input_files; i++) { @@ -1651,7 +1669,6 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti { AVBPrint buf, buf_script; OutputStream *ost; - AVFormatContext *oc; int64_t total_size; AVCodecContext *enc; int frame_number, vid, i; @@ -1680,13 +1697,6 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti t = (cur_time-timer_start) / 1000000.0; - - oc = output_files[0]->ctx; - - total_size = avio_size(oc->pb); - if (total_size <= 0) // FIXME improve avio_size() so it works with non seekable output too - total_size = avio_tell(oc->pb); - vid = 0; av_bprint_init(&buf, 0, AV_BPRINT_SIZE_AUTOMATIC); av_bprint_init(&buf_script, 0, 1); @@ -1697,12 +1707,13 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti if (!ost->stream_copy) q = ost->quality / (float) FF_QP2LAMBDA; - if (vid && enc->codec_type == AVMEDIA_TYPE_VIDEO) { + if (is_last_report && vid && enc->codec_type == AVMEDIA_TYPE_VIDEO) { av_bprintf(&buf, "q=%2.1f ", q); av_bprintf(&buf_script, "stream_%d_%d_q=%.1f\n", ost->file_index, ost->index, q); } - if (!vid && enc->codec_type == AVMEDIA_TYPE_VIDEO) { + if (is_last_report || !is_last_report && !vid) { + if (enc->codec_type == AVMEDIA_TYPE_VIDEO) { float fps; frame_number = ost->frame_number; @@ -1759,9 +1770,11 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti if (av_stream_get_end_pts(ost->st) != AV_NOPTS_VALUE) pts = FFMAX(pts, av_rescale_q(av_stream_get_end_pts(ost->st), ost->st->time_base, AV_TIME_BASE_Q)); - if (is_last_report) + if (is_last_report) { nb_frames_drop += ost->last_dropped; - } + } + + total_size = ost->data_size; secs = FFABS(pts) / AV_TIME_BASE; us = FFABS(pts) % AV_TIME_BASE; @@ -1815,8 +1828,15 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti av_bprintf(&buf_script, "speed=%4.3gx\n", speed); } + if (is_last_report) + av_bprintf(&buf, "\n"); + else if (enc->codec_type == AVMEDIA_TYPE_AUDIO) + av_bprintf(&buf, "\r"); + } + } + if (print_stats || is_last_report) { - const char end = is_last_report ? '\n' : '\r'; + const char end = '\r'; if (print_stats==1 && AV_LOG_INFO > av_log_get_level()) { fprintf(stderr, "%s %c", buf.str, end); } else @@ -1841,7 +1861,7 @@ static void print_report(int is_last_report, int64_t timer_start, int64_t cur_ti } if (is_last_report) - print_final_stats(total_size); + print_final_stats(); } static void ifilter_parameters_from_codecpar(InputFilter *ifilter, AVCodecParameters *par)
Make ffmpeg to output stats for each video/audio streams and each ouptut file ffmpeg output log in print_report. The report of video/audio sizes is clear now as previously all output video/audio sizes were combined to report and it is unclear such stats is for one output files or aggregates for all output files. Signed-off-by: Wang Cao <wangcao@google.com> --- fftools/ffmpeg.c | 52 +++++++++++++++++++++++++++++++++--------------- 1 file changed, 36 insertions(+), 16 deletions(-)