[FFmpeg-devel] [PATCH]: Avoid duplicate last entry in the pass log file for the last frame
Thierry Foucu
tfoucu
Wed Nov 3 23:28:01 CET 2010
On Wed, Nov 3, 2010 at 11:54 AM, Michael Niedermayer <michaelni at gmx.at>wrote:
> On Wed, Nov 03, 2010 at 11:00:36AM -0700, Thierry Foucu wrote:
> > On Wed, Nov 3, 2010 at 10:37 AM, Michael Niedermayer <michaelni at gmx.at
> >wrote:
> >
> > > On Wed, Nov 03, 2010 at 09:41:12AM -0700, Thierry Foucu wrote:
> > > > On Tue, Nov 2, 2010 at 7:47 PM, Michael Niedermayer <
> michaelni at gmx.at
> > > >wrote:
> > > >
> > > > > On Tue, Nov 02, 2010 at 04:43:09PM -0700, Thierry Foucu wrote:
> > > > > > On Tue, Nov 2, 2010 at 3:21 PM, Thierry Foucu <tfoucu at gmail.com>
> > > wrote:
> > > > > >
> > > > > > > $subject
> > > > > > >
> > > > > > > Before the patch, i was getting this in the log file:
> > > > > > >
> > > > > > > in:131 out:131 type:2 q:1599 itex:0 ptex:9194 mv:878 misc:1710
> > > fcode:1
> > > > > > > bcode:1 mc-var:47056 var:132365 icount:0 skipcount:1 hbits:42;
> > > > > > > in:132 out:132 type:2 q:1601 itex:436 ptex:9173 mv:931
> misc:1706
> > > > > fcode:1
> > > > > > > bcode:1 mc-var:49302 var:137777 icount:2 skipcount:1 hbits:42;
> > > > > > > in:133 out:133 type:2 q:1600 itex:461 ptex:6090 mv:761
> misc:1622
> > > > > fcode:1
> > > > > > > bcode:1 mc-var:48901 var:133431 icount:3 skipcount:2 hbits:42;
> > > > > > > in:134 out:134 type:2 q:1565 itex:379 ptex:14567 mv:951
> misc:1749
> > > > > fcode:1
> > > > > > > bcode:1 mc-var:57528 var:134434 icount:2 skipcount:1 hbits:42;
> > > > > > > in:135 out:135 type:1 q:1224 itex:96623 ptex:0 mv:0 misc:1007
> > > fcode:1
> > > > > > > bcode:1 mc-var:0 var:143173 icount:300 skipcount:0 hbits:42;
> > > > > > > in:135 out:135 type:1 q:1224 itex:96623 ptex:0 mv:0 misc:1007
> > > fcode:1
> > > > > > > bcode:1 mc-var:0 var:143173 icount:300 skipcount:0 hbits:42;
> > > > > > >
> > > > > > >
> > > > > > > With the patch, I get this:
> > > > > > > in:131 out:131 type:2 q:1599 itex:0 ptex:9194 mv:878 misc:1710
> > > fcode:1
> > > > > > > bcode:1 mc-var:47056 var:132365 icount:0 skipcount:1 hbits:42;
> > > > > > > in:132 out:132 type:2 q:1601 itex:436 ptex:9173 mv:931
> misc:1706
> > > > > fcode:1
> > > > > > > bcode:1 mc-var:49302 var:137777 icount:2 skipcount:1 hbits:42;
> > > > > > > in:133 out:133 type:2 q:1600 itex:461 ptex:6090 mv:761
> misc:1622
> > > > > fcode:1
> > > > > > > bcode:1 mc-var:48901 var:133431 icount:3 skipcount:2 hbits:42;
> > > > > > > in:134 out:134 type:2 q:1565 itex:379 ptex:14567 mv:951
> misc:1749
> > > > > fcode:1
> > > > > > > bcode:1 mc-var:57528 var:134434 icount:2 skipcount:1 hbits:42;
> > > > > > > in:135 out:135 type:1 q:1224 itex:96623 ptex:0 mv:0 misc:1007
> > > fcode:1
> > > > > > > bcode:1 mc-var:0 var:143173 icount:300 skipcount:0 hbits:42;
> > > > > > >
> > > > > > > Note that before the patch the last frame stats are present
> twice.
> > > > > > >
> > > > > > > It seems that at the end of encoding, we output the stats_out
> > > twice:
> > > > > > >
> > > > > > > One at line 1254 in function do_video_out
> > > > > > > 1252 /* if two pass, output log */
> > > > > > > 1253 if (ost->logfile && enc->stats_out) {
> > > > > > > 1254 fprintf(ost->logfile, "%s",
> > > enc->stats_out);
> > > > > > > 1255 }
> > > > > > >
> > > > > > > There is a if statement before that to make sure that the
> return
> > > value
> > > > > of
> > > > > > > avcodec_encode_video is greater than 0
> > > > > > >
> > > > > > > the second time we output the last frame stat is in
> output_packet,
> > > but
> > > > > we
> > > > > > > were not checking to see if the return value of
> > > avcodec_encode_video is
> > > > > > > greater than 0
> > > > > > >
> > > > > > > The patch does that.
> > > > > > >
> > > > > > > Index: ffmpeg.c
> > > > > > >
> ===================================================================
> > > > > > > --- ffmpeg.c (revision 25651)
> > > > > > > +++ ffmpeg.c (working copy)
> > > > > > > @@ -1797,7 +1797,7 @@
> > > > > > > video_size += ret;
> > > > > > > if(enc->coded_frame &&
> > > > > > > enc->coded_frame->key_frame)
> > > > > > > pkt.flags |= AV_PKT_FLAG_KEY;
> > > > > > > - if (ost->logfile &&
> enc->stats_out) {
> > > > > > > + if (ost->logfile && enc->stats_out
> &&
> > > ret
> > > > > > 0)
> > > > > > > {
> > > > > > > fprintf(ost->logfile, "%s",
> > > > > > > enc->stats_out);
> > > > > > > }
> > > > > > > break;
> > > > > > >
> > > > > > >
> > > > > > >
> > > > > > here is another version to fix the problem where i add a if (ret
> > 0)
> > > > > around
> > > > > > the code, instead of just checking the return value for the pass
> log
> > > file
> > > > > > Index: ffmpeg.c
> > > > > >
> ===================================================================
> > > > > > --- ffmpeg.c (revision 25651)
> > > > > > +++ ffmpeg.c (working copy)
> > > > > > @@ -1794,12 +1794,14 @@
> > > > > > fprintf(stderr, "Video encoding
> > > > > failed\n");
> > > > > > ffmpeg_exit(1);
> > > > > > }
> > > > > > + if (ret > 0) {
> > > > > > video_size += ret;
> > > > > > if(enc->coded_frame &&
> > > > > > enc->coded_frame->key_frame)
> > > > > > pkt.flags |= AV_PKT_FLAG_KEY;
> > > > > > if (ost->logfile && enc->stats_out)
> {
> > > > > > fprintf(ost->logfile, "%s",
> > > > > > enc->stats_out);
> > > > > > }
> > > > > > + }
> > > > > > break;
> > > > > > default:
> > > > > > ret=-1;
> > > > > >
> > > > > >
> > > > > > If this is the patch you will prefer, i will email another patch
> > > > > > for indentation (if needed)
> > > > >
> > > > > this change is incorrect, the encoder is buggy
> > > > >
> > > > >
> > > > Hi Michael,
> > > >
> > > > do you mind to tell me why this change is incorrect? I was looking at
> > > > ffmpeg.c and in the function do_video_out, after calling the function
> > > > avcodec_encode_video, we do check for (ret > 0) before logging the
> > > > enc->stats_out to the logfile.
> > >
> > > is that neccesary or does it also work without that check?
> > >
> > >
> > without the ret > 0 check in the do_video_out, we will call write_frame
> when
> > we do not have a frame being encoded.
> > So, i guess, you do need the check in the do_video_out.
>
> write_frame != writing stats. My question was purely about writing stats
>
>
> >
> >
> >
> > > anyway, id like to output 2pass stats at the last frame in ffv1 and not
> > > like
> > > its done once every 32 frames.
> > >
> >
> > I though the 1pass stats are outputted once every frame in the
> do_video_out
> > function call, after we call the function write_frame.
>
>
> >
> > your patch would break that
> > >
> >
> > so, what about that?
>
> thats a hack, and if we do it could be done in the core at least
>
>
what about this. Adding the clear of stats_out in the avcodec_encode_video
function.
Index: libavcodec/utils.c
===================================================================
--- libavcodec/utils.c (revision 25669)
+++ libavcodec/utils.c (working copy)
@@ -567,6 +567,8 @@
}
if(av_image_check_size(avctx->width, avctx->height, 0, avctx))
return -1;
+ if (avctx->stats_out)
+ avctx->stats_out[0] = 0; //to make sure the stat are clear
if((avctx->codec->capabilities & CODEC_CAP_DELAY) || pict){
int ret = avctx->codec->encode(avctx, buf, buf_size, pict);
avctx->frame_number++;
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> it is not once nor twice but times without number that the same ideas make
> their appearance in the world. -- Aristotle
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAkzRr90ACgkQYR7HhwQLD6tkgwCdELO5zQ+03UGxBeTdcox9NoEY
> wmsAoI4NCHR1ZH59sOUSK+Ld4OUsyqfg
> =R0t2
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>
-------------- next part --------------
Index: libavcodec/utils.c
===================================================================
--- libavcodec/utils.c (revision 25669)
+++ libavcodec/utils.c (working copy)
@@ -567,6 +567,8 @@
}
if(av_image_check_size(avctx->width, avctx->height, 0, avctx))
return -1;
+ if (avctx->stats_out)
+ avctx->stats_out[0] = 0; //to make sure the stat are clear
if((avctx->codec->capabilities & CODEC_CAP_DELAY) || pict){
int ret = avctx->codec->encode(avctx, buf, buf_size, pict);
avctx->frame_number++;
More information about the ffmpeg-devel
mailing list