> -----Original Message-----
> From: [EMAIL PROTECTED] [mailto:libav-user-
> [EMAIL PROTECTED] On Behalf Of Luca Abeni
> Sent: Tuesday, July 08, 2008 12:58
> To: Libav* user questions and discussions
> Subject: Re: [libav-user] How to handle 33-bits rollover in MPEG?
> 
> Hi Michel,
> 
> Michel Bardiaux wrote:
> > Luca Abeni wrote:
> >> Hi Michel,
> >>
> >> Michel Bardiaux wrote:
> >> [...]
> >>> On decoding, av_read_frame gives the timestamps as found, hence
> every 26 hours, the time apparently recycles (but contrary to Monopoly I
> don't get €50 each time :-)).
> >>>
> >>> So, apparently the burden of compensating for the rollover is left
> to the user.
> >> I am not sure if I fully understand the problem, here, but in my
> understanding
> >> there is not much to compensate: if pts_wrap_bits is 33, then the
> MPEG demuxer
> >> generates timestamps on 33 bits, and if you consider only 33 bits
> there should
> >> be no problem.
> >>
> >>
> >>> The sequential reading is relatively easy to handle, I just need to
> remember whether my wrapper for av_read_frame has seen a TS bigger than
> say 1 million; then if it sees a ts near zero, rollover has occurred and
> I just have to add 2exp33. (That would only work for files < 26+ hours,
> but an MPEG that long is bloody unlikely!)
> >>>
> >>> The seek, however, is something else entirely since it uses
> arithmetic on timestamps as if they were plain normal 64bits integers,
> without rollover.
> >> I suspect this is the problem. I do not know what the libavformat's
> behaviour
> >> is supposed to be, but either:
> >> 1) libavformat somehow take care of the rollover, converting
> timestamps from
> >>     pts_wrap_bits to 64 bits, or
> >
> > It does not.
> 
> Ok
> 
> 
> >> 2) libavformat only consider pts_wrap_bits, and in this case using
> 64bit
> >>     arithmetic on timestamps is wrong.
> >
> > Worse: some parts of libavformat (read_pes_header) only consider 33
> > bits,
> 
> read_pes_header is in the demuxer, so I think it is right in considering
> only 33 bits

Yes, *if* one decides to stick with 33 (or rather pts_wrap) bits all the way. 
In which case, it would be better to use a specific struct type rather than 
int64_t, to make sure no 'naïve' arithmetic is done. And in this case, the 
muxer should not complain when the timestamps wrap-around.

If one goes for the other solution, which is full 64-bits timestamps at the 
client API level, then the convention should go all the way down to 
read_pes_header. This is the choice I have made for patches I am currently 
experimenting with. 

> 
> 
> > but other parts (like seeking) expect normal 64-bits arithmetic.
> 
> I think this is the bug (but, again, this is just my understanding - or
> maybe
> misunderstanding... Michael should say something here). In my
> understanding,
> either there should be some code in libavformat which does the 33->64bit
> conversion, 

ITIYM 'for internal purposes only'?


> or the seeking code should not do 64bits arithmetic.

I don’t see how seeking could possibly work on a file containing rollover using 
only 33-bits unsigned arithmetic.

> 
> >
> >>
> >>> From reading The Fine Source, it seems the critical function is
> mpegps_read_pes_header and if I patch that to handle the rollover (ie
> add 2exp33 when appropriate) everything would work just right.
> >> I do not think the muxer should be in charge of handling the
> rollover: in my
> >
> > The *muxer*?
> 
> Sorry, that was a typo. I obviously wanted to write "demuxer"
> 
> 
> >> understanding mpegps_read_pes_header should return 33bit timestamps.
> >> So, I suspect the problem is somewhere else...
> >> But maybe I am just confused ;-)
> >
> > Again: *if* read_pes_header is supposed to work as it does, ie return
> > 33-bits timestamps, then the rest of libavformat should do the same
> 
> or could contain some code (in the generic code, not in the demuxer) to
> do the 33->64 conversion...

If different levels of lavf have to work with different 'tastes' of timestamps, 
then that contract should be documented very carefully. Currently, my feeling 
is that it is just 'we don’t care about rollover'.

> 
> 
> > I dont see how seek_frame could work then! In other words: for
> > av_seek_frame to work, the rollover has to be compensated inside
> > libavformat itself.
> 
> I agree with you, here. I just think that the compensation should be
> done
> in generic code, and not in the mpeg-specific one.

And I can agree with that, *provided* 2 different types, without any 
possibility of casting, are used.

> 
> 
> > Also, as Vladimir, and Michael Conrad, pointed out, if you pass
> > timestamps truncated to 33-bits to av_write_frame, and rollover
> occurs,
> > you get "non-monotonous timestamps" (and harakiri).
> 
> Ok; there is a clear asymmetry between av_read_frame() and
> av_write_frame().
> I do not know if it is intended or not.
> I think we need some authoritative opinion here (Michael?).
> 
_______________________________________________
libav-user mailing list
[email protected]
https://lists.mplayerhq.hu/mailman/listinfo/libav-user

Reply via email to