[FFmpeg-devel] [PATCH]: Avoid duplicate last entry in the pass log file for the last frame

[Michael Niedermayer]

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


[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 198 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-devel/attachments/20101103/fc1b2e20/attachment.pgp>