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

[Benoit Fouet]

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.
this will lead to changing output of the FATE test I'm looking at at the
moment, but I guess this would be case anyway...

>>> stderr:
>>> av_interleaved_write_frame(): Error while opening file
>>>
>>> Then there are many tests that have wildly different timestamps, such as
>>> h264-conformance-ba3_sva_c here:
>>> http://fate.multimedia.cx/index.php?test_result=4558940
>>>
>>>       
>> BTW, in the meantime, should this commit be reverted, or not ?
>>     
>
> if this is not fixed in 2-3 days then it should be reverted until it is
>
>   

if I'm quick enough to understand, this might be fixed before this
deadline :)

-- 
Benoit Fouet
Purple Labs S.A.
www.purplelabs.com

-------------- next part --------------
An embedded and charset-unspecified text was scrubbed...
Name: ffmpeg.c.diff
URL: <http://lists.mplayerhq.hu/pipermail/ffmpeg-cvslog/attachments/20081021/6faa947d/attachment.asc>