[FFmpeg-cvslog] r17572 - in trunk/libavcodec: mpeg12.c mpegvideo_parser.c

[Baptiste Coudurier]

Michael Niedermayer wrote:
> On Thu, Feb 26, 2009 at 10:35:31AM -0800, Baptiste Coudurier wrote:
>> On 2/26/2009 5:01 AM, Michael Niedermayer wrote:
>>> On Thu, Feb 26, 2009 at 12:19:51AM -0800, Baptiste Coudurier wrote:
>>>> Michael Niedermayer wrote:
>>>>> On Wed, Feb 25, 2009 at 05:05:59PM -0800, Baptiste Coudurier wrote:
>>>>>> On 2/25/2009 3:41 PM, Michael Niedermayer wrote:
>>>>>>> On Wed, Feb 25, 2009 at 03:00:02PM -0800, Baptiste Coudurier wrote:
>>>>>>>> On 2/25/2009 2:29 PM, Michael Niedermayer wrote:
>>> [...]
>>>> Here are some problems:
>>>>
>>>> ffmpeg.c:
>>>>               if (ist->st->codec->time_base.num != 0) {
>>>>                    ist->next_pts += ((int64_t)AV_TIME_BASE *
>>>>                                    ist->st->codec->time_base.num) /
>>>>                          ist->st->codec->time_base.den;
>>>>               }
>>> this was not correct to begin with
>>>
>>>
>>>> This is no more correct.
>>>>
>>>> There is an issue in compute_pkt_fields2 when muxing:
>>>>
>>>> if(st->time_base.num*1000LL > st->time_base.den){
>>>>        *pnum = st->time_base.num;
>>>>        *pden = st->time_base.den;
>>>> }else if(st->codec->time_base.num*1000LL > st->codec->time_base.den){
>>>>        *pnum = st->codec->time_base.num;
>>>>        *pden = st->codec->time_base.den;
>>> this should be correct. you sniped the repeat_pict compensation
>>> that comes in the immedeatly following line
>>>> [...]
>>>>
>>>>  compute_frame_duration(&num, &den, st, NULL, pkt);
>>>>  if (den && num) {
>>>>     pkt->duration = av_rescale(1, num * (int64_t)st->time_base.den,
>>>> 		 den * (int64_t)st->time_base.num);
>>>>
>>>> st->codec->time_base.den is now *2 but pkt->duration will be 1,
>>>> this is wrong.
>>> thats just because NULL is passed instead of something containing the
>>> repeat_pict value
>>>
>>> before it was wrong for field mpeg and wrong for repeat_first_field
>>> now its wrong for progressive and correct for field mpeg
>>>
>> No, when muxing there is no parser, repeat_pict is useless.
>> I thought it was obvious, I mentioned "when muxing".
> 
> there are repeat_first_field & top_field_first flags in the stream you mux
> they represent different durations and the duration generation code on the
> mxuer side did never work if these
> where 2 out of 3 valid combinations, now a different variant of the 3
> works, that is what i said and your "no" feels very offensive to be honest

Yes, I agree it was offensive, like when you said it was 5 am and you
were wasting your time.

>> If pkt->duration is wrong, muxing in mov is _broken_ and this applies to
>> H.264.
> 
> yes it was broken, its broken for a different case now.
> 
> Do you plan to help me fix the issue?

Of course.

> Do you plan to keep attacking the ffmpeg leader so he wastes the little time
> he has to look into this issue short before a release?

No I don't, I'll test your patch right now.

-- 
Baptiste COUDURIER                              GnuPG Key Id: 0x5C1ABAAA
Key fingerprint                 8D77134D20CC9220201FC5DB0AC9325C5C1ABAAA
checking for life_signs in -lkenny... no
FFmpeg maintainer                                  http://www.ffmpeg.org