[FFmpeg-cvslog] r15653 - trunk/libavcodec/rawenc.c

[Michael Niedermayer]

On Tue, Oct 21, 2008 at 01:56:14PM +0200, Benoit Fouet wrote:
> Michael Niedermayer wrote:
> > On Tue, Oct 21, 2008 at 11:17:17AM +0200, Benoit Fouet wrote:
> >   
> >> Hi Mike,
> >>
> >> Mike Melanson wrote:
> >>     
> >>> Mike Melanson wrote:
> >>>
> >>>       
> >>>> benoit wrote:
> >>>>
> >>>>         
> >>>>> Author: benoit
> >>>>> Date: Mon Oct 20 09:35:17 2008
> >>>>> New Revision: 15653
> >>>>>
> >>>>> Log:
> >>>>> Copy pts for each raw encoded frame.
> >>>>> Patch by Andrew Wason rectalogic rectalogic com
> >>>>> Fixes issue 676
> >>>>>
> >>>>>           
> >>>> Fix one issue, break 75 FATE tests. Why did this change break 75 FATE tests?
> >>>>
> >>>>         
> >>> BTW, the next FATE script has trouble transferring stdout/stderr to the
> >>> database. However, the PowerPC configurations are still running the old
> >>> script. See test results such as this for CSCD:
> >>>
> >>> http://fate.multimedia.cx/index.php?test_result=4558904
> >>>
> >>>
> >>>       
> >> I tried this one with the same command line as yours, it fails in the
> >> same way...
> >> removing -vsync 0 made the command work, but output is slightly different.
> >>
> >> my conclusion while tracking the issue are:
> >> in ffmpeg.c, when we use vsync 0, the formula to calculate pts for
> >> output depends on the following line:
> >> 866: ost->sync_opts= lrintf(get_sync_ipts(ost) / av_q2d(enc->time_base));
> >>
> >> I added the following trace:
> >> printf("%g %g => %g\n", get_sync_ipts(ost), av_q2d(enc->time_base),
> >> get_sync_ipts(ost) / av_q2d(enc->time_base));
> >> and what I got is, for the last two frames:
> >> 0.56 0.032 => 17.5, so ost->sync_opts will be set to 18
> >> 0.59 0.032 => 18.4375, so ost->sync_opts will be set to 18 too
> >>
> >> that will lead to having, when computing packet timestamp, a dts equal
> >> to the current dts, and so an error raising in compute_pkt_fields2
> >>
> >> I don't know what a good solution could be, though. If anyone has an
> >> idea on what to do now, I'm all ears.
> >>     
> >
> > Well, the timestamping code needs to be looked over
> > first, does the demuxer set timestamps correctly? (printing them out and
> > looking should show if they look ok or not)
> > My gut feeling says many demuxers are buggy in this respect
> >
> >   
> 
> I'll try to look at that when the regression tests are fixed for FATE.
> 
> > still the true fix for this issue irrespective of above might be to use
> > something like:
> > (taken from ffmpeg.c stream copy case)
> >             if(av_q2d(icodec->time_base) > av_q2d(ist->st->time_base) && av_q2d(ist->st->time_base) < 1.0/1000)
> >                 codec->time_base = icodec->time_base;
> >             else
> >                 codec->time_base = ist->st->time_base;
> >
> > instead of where r_frame_rate is used when vsync=0
> >
> > for vsync!=0 the code that uses the guessed framerate/timebase is likely the
> > best, for vsync==0 guessing the timebase is asking for trouble because no
> > framedroping is done one can never gurantee that the guess will work.
> >
> >
> >   
> 
> I'm really not sure I understood correctly what you meant, so the
> attached fix might be totally wrong...
> anyway, you'll find it attached.

no, thats not what i meant, i meant changing 
            rfps      = ic->streams[i]->r_frame_rate.num;
            rfps_base = ic->streams[i]->r_frame_rate.den;
when vsync==0

but this is untested i may very well be missing something

[...]

-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

No snowflake in an avalanche ever feels responsible. -- Voltaire
-------------- next part --------------
A non-text attachment was scrubbed...
Name: not available
Type: application/pgp-signature
Size: 189 bytes
Desc: Digital signature
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20081021/c6b14158/attachment.pgp>