[Ffmpeg-devel] [PATCH] get rid of AVFrac

[Baptiste Couduurier]

Baptiste Coudurier a ?crit :
> Hi
> 
> Michael Niedermayer wrote:
>> Hi
>>
>> On Mon, Dec 18, 2006 at 11:40:00AM +0100, Baptiste Coudurier wrote:
>>> Hi
>>>
>>> Michael Niedermayer wrote:
>>>> Hi
>>>>
>>>> On Sat, Dec 16, 2006 at 01:29:48AM +0100, Baptiste Coudurier wrote:
>>>>> Hi
>>>>>
>>>>> Michael Niedermayer a ?crit :
>>>>>> Hi
>>>>>>
>>>>>> On Sat, Oct 14, 2006 at 07:01:59PM +0200, Reimar D?ffinger wrote:
>>>>>>> Hello,
>>>>>>> On Fri, Oct 13, 2006 at 09:33:18PM +0200, Michael Niedermayer wrote:
>>>>>>> [...]
>>>>>>>> this doesnt look correct ...
>>>>>>> Hmm.. well, no idea what exactly the old code does and how.
>>>>>>> Here is another try, it's the best I can do without further hints.
>>>>>> [...]
>>>>>>
>>>>>> so what the code does is it simply fills in missing timestamps on
>>>>>> the encoding side and it does so without rounding errors assuming
>>>>>> constant framerate and a few other things allthough
>>>>>> the usefullness of the whole is questionable (the user app should set
>>>>>> the timestamps and not leave it to lav* to guess something)
>>>>>>
>>>>>> [...]
>>>>>>
>>>>>> IMHO drop "support" for missing dts on the encoder side completely it makes
>>>>>> no sense at all, its like sending emails with no To: field
>>>>>>
>>>>> Here is an updated patch which drop support for missing pts or dts on 
>>>>> the encoder side.
>>>>>
>>>>> However ffmpeg.c does not compile anymore, 
>>>> that is a problem, patch rejected :)
>>> Hehe, yes, but is the patch the good approach (get rid of AVFrac and
>>> drop support for missing dts or pts on the encoder side), and therefore
>>> ffmpeg.c must be updated, or should we keep st->pts ?
>>>
>>> If the latter, I would that say AVFrac is not that useless since
>>> ffmpeg.c uses it, and it is exact.
>>>
>>>>> and Im not sure to know 
>>>>> exactly what ffmpeg.c does, could st->pts be updated to use rescaled 
>>>>> st->cur_dts ? Seems that cur_dts would only be 1 frame less that 
>>>>> st->pts.val in that case, maybe Im wrong.
>>>> yes cur_dts and st->pts represent the same thing IIRC ...
>>>> the difference is that st->pts is exact and cur_dts is not, using the later
>>>> will cause terrible AV-desync under the correct circumstances, think about
>>>> a container timebase of 1/1000 and 30fps video, the duration of one frame
>>>> is 33.333... in the container timebase, if you repeatly add 33 you will be
>>>> off by 0.6 seconds per minute ...
>>>>
>>> I see. Now if st->pts is changed to int64_t and uses av_rescale_q from
>>> codec timebase to stream timebase, is it less exact than AVFrac ?
>> that might work ...
>>
> 
> Ok, here is a new attempt. I quickly checked values of pts.val and new
> pts values, they were equal, also with -vsync 0 -r 40.
> 

Checked again, and regression tests fails for asf, ts, mpg.
video pts are equal, audio differs:

asf:
old:
av_write_frame: index 1, pts2:1097 dts2:1097
av_write_frame: index 1, pts2:1123 dts2:1123
av_write_frame: index 1, pts2:1149 dts2:1149
new:
av_write_frame: index 1, pts2:1092 dts2:1092
av_write_frame: index 1, pts2:1118 dts2:1118
av_write_frame: index 1, pts2:1144 dts2:1144

diff seems to increase while encoding, like you were fearing.

ts:
old:
av_write_frame: index 1, pts2:84637 dts2:84637
av_write_frame: index 0, pts2:86400 dts2:82800
av_write_frame: index 1, pts2:86988 dts2:86988
av_write_frame: index 1, pts2:89339 dts2:89339
new:
av_write_frame: index 1, pts2:84636 dts2:84636
av_write_frame: index 0, pts2:86400 dts2:82800
av_write_frame: index 1, pts2:86987 dts2:86987
av_write_frame: index 1, pts2:89338 dts2:89338

So what do you think ? It seems not safe to use int64_t, or my code is 
wrong.

Do we drop pts generation ? Do we keep AVFrac ?

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no