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

[Michael Niedermayer]

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


> 
> 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?
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?

[...]
-- 
Michael     GnuPG fingerprint: 9FF2128B147EF6730BADF133611EC787040B0FAB

Republics decline into democracies and democracies degenerate into
despotisms. -- Aristotle
-------------- 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/20090226/3ad91315/attachment.pgp>