[MPlayer-dev-eng] [PATCH] demuxer and MP_NOPTS_VALUE

[Uoti Urpala]

On Sat, 2007-04-21 at 08:30 +0200, Christian Aistleitner wrote:
> let me repeat (and clarify) my interpretation of MP_NOPTS_VALUE and its  
> connection to correct_pts:
> 
> If correct_pts is set, MP_NOPTS_VALUE has to be used as pts value for  
> packets where no valid pts can be found/determined.

Yes. Note however that ds->pts which you mentioned is not a packet
timestamp. Exactly what it is depends on the case... as I said it's kind
of a hack. Its most important use is in old-style audio pts calculation.

> If correct_pts is not set, either MP_NOPTS_VALUE or 0 has to be used as  
> pts value for packets where no valid pts can be found/determined.
> 
> Is there a preference for either MP_NOPTS_VALUE or 0 in the latter setting?

Eventually only MP_NOPTS_VALUE should be used. I'm not 100% sure whether
there is still some code (besides that check you mentioned which
explicitly changes based on correct_pts) which would only work with 0.
The simplest way is to create packets with new_demux_packet() and not
set pts if it's not known; the field will be initialized to a suitable
value.

> >> However, libmpdemux/demuxer.c initializes ds->pts to 0 and not
> >> MP_NOPTS_VALUE.
> >
> > I think it shouldn't matter much what the initialization value is. It
> > might be used in some error case but 0 is probably no worse then. The
> > whole ds->pts field is kind of a hack.
> 
> I think for people wanting to read code, it is important go give sound  
> initial values. If my above interpretation is correct, then setting  
> ds->pts to 0 regardless of correct_pts is misleading (at least it has been  
> for me).

As mentioned above ds->pts isn't a packet timestamp.

> >>  Furthermore, fill_buffers handling does not respect pts of
> >> MP_NOPTS_VALUE if correct_pts is not set.
> >
> > Maybe it would be nicer if it always tested for MP_NOPTS_VALUE, but it
> > shouldn't matter with current code.
> 
> Actually, I wanted to post a patch to libmpdemux_mpg.c which corrects its  
> pts handling if no pts can be found (and a follow up patch, which imploves  
> edl handling with -oac copy). I cannot submit my patch, if there is no  
> well defined way to indicate missing pts values. So even if it does not  
> matter for the current code, it might matter for future code.

Setting to 0/MP_NOPTS_VALUE depending on correct_pts should work. Maybe
it would be ok to move to using MP_NOPTS_VALUE only, but that would
require some checking first (for example I haven't checked whether
MEncoder would break further if MP_NOPTS_VALUE was used - correct_pts is
always false under MEncoder).

> >> Is my understanding of MP_NOPTS_VALUE flawed?
> >
> > I think it's weird to talk about "understanding of MP_NOPTS_VALUE". It's
> > just a #define and there's not much to understand about it in isolation.
> > Not all uses of it need to have exactly the same semantics.
> 
> Are you serious?

MP_NOPTS_VALUE is basically just "a value not used for real pts". It
could be for example a possible return value from a function parsing a
timestamp from a commandline option. Semantics for that case would
obviously differ from usage in packet timestamps.

Maybe you really mean something like "understanding packet timestamps".