[FFmpeg-devel] [PATCH]: Avoid duplicate last entry in the pass log file for the last frame
Thierry Foucu
tfoucu
Wed Nov 3 19:00:36 CET 2010
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.
> 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?
Index: ffmpeg.c
===================================================================
--- ffmpeg.c (revision 25651)
+++ ffmpeg.c (working copy)
@@ -1252,6 +1252,7 @@
/* if two pass, output log */
if (ost->logfile && enc->stats_out) {
fprintf(ost->logfile, "%s", enc->stats_out);
+ enc->stats_out[0] = 0;
}
}
}
where after we output the stats in the do_video_out function, we reset the
stats_out?
>
> [...]
> --
> Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
>
> Breaking DRM is a little like attempting to break through a door even
> though the window is wide open and the only thing in the house is a bunch
> of things you dont want and which you would get tomorrow for free anyway
>
> -----BEGIN PGP SIGNATURE-----
> Version: GnuPG v1.4.10 (GNU/Linux)
>
> iEYEARECAAYFAkzRncUACgkQYR7HhwQLD6uhyACfR52S28QxdnhXeJzwzeD0mzaQ
> uDIAoIiBPzJp+r29D2f8YHeq9CYLj174
> =JIG3
> -----END PGP SIGNATURE-----
>
> _______________________________________________
> ffmpeg-devel mailing list
> ffmpeg-devel at mplayerhq.hu
> https://lists.mplayerhq.hu/mailman/listinfo/ffmpeg-devel
>
More information about the ffmpeg-devel
mailing list