[FFmpeg-devel] [PATCH]: Avoid duplicate last entry in the pass log file for the last frame
Michael Niedermayer
michaelni
Thu Nov 4 01:25:23 CET 2010
On Wed, Nov 03, 2010 at 03:28:01PM -0700, Thierry Foucu wrote:
> 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.
i now realize that you write into the memory, this is not allowed, neither
in ffmpeg.c nor is it a reasonable operation in untils.c
fix the encoder or leave it for someone else to fix properly
[...]
--
Michael GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB
Old school: Use the lowest level language in which you can solve the problem
conveniently.
New school: Use the highest level language in which the latest supercomputer
can solve the problem without the user falling asleep waiting.
-------------- 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/20101104/27e17e07/attachment.pgp>
More information about the ffmpeg-devel
mailing list