[MPlayer-dev-eng] NUT cleanup

[Michael Niedermayer]

Hi

On Tue, Sep 06, 2005 at 07:41:56PM +0300, Oded Shimon wrote:
[...]
> > [...]
> > 
> > > @@ -292,6 +301,8 @@
> > >  	0xDD672F23E64EULL + (((uint64_t)('N'<<8) + 'X')<<48)
> > >  info_startcode
> > >  	0xAB68B596BA78ULL + (((uint64_t)('N'<<8) + 'I')<<48)
> > > +end_startcode
> > > +	0xE8154EDB2A7CULL + (((uint64_t)('N'<<8) + 'E')<<48)
> > 
> > what is the end_startcode good for? i dont remember any arguments against
> > simply storing a 64bit index_ptr at the end except its "bleh" which well
> > i dont really care about, i prefer a nut spec which is bleh over one which
> > is complicated and bloated
> > btw, FAQ: 
> > Q: why didnt you just store a 64bit int at the end?
> > A: because its bleh
> > Q2: but dont u see that now we have to search backward for that startcode,
> > then parse th vlc forward, keep track of where the vlc started and subtract
> > its value from that to find the index?
> > A2: ?
> 
> No, It's a bigger problem - I realized, if you just put a 64bit int at the 
> end, how the hell do you know if you have the end or not? Say I have an 
> incomplete file, The end data could be any random junk, and i'd have no way 
> of knowing if i really have the entire file or not!

1. check if the 64bit ptr points within the file (check needed no matter how
its stored anyway)
2. seek there and check if the index_startcode is there (again we need to
check this for all variants anyway)
-> so IMHO the 64bit variant is much simpler


> So I discussed it with Rich, and I at first wanted a simple
> end_startcode f(64)
> index_ptr     u(64)
> 
> But he made a pretty damn good point, index_ptr can NEVER be bigger than 
> log128(filesize) . That's 5 bytes for a 4GB file! It's certainely not the 
> most expensive backward seek... The demuxer can just start searching from 
> 'log128(filesize) + 8' bytes from end of file, and be sure it's safe.
> Once I find the start code I just keep the current position, read vlc and 
> go backwards... I considered mentioning the log128 trick in the spec.

well you would at least have to forbid 0x80 padding for this trick to be
guranteed to work


> 
> > >  version
> > >  	NUT version. The current value is 2.
> > > @@ -327,6 +335,8 @@
> > >  	3	metadata
> > >  	Note: the remaining values are reserved and MUST NOT be used
> > >  	     a demuxer MUST ignore streams with reserved classes
> > > +	Note: stream_class MUST be bigger or equal to the stream_class of
> > > +	the previous stream.
> > 
> > what is this good for? yeah i know it is in the current spec and just
> > moved but still i dont like it ...
> 
> It has a slight muxer implementation advantage - in my muxer it decides 
> global timestamp by looking at 'stream[0].last_pts' - with this limitation, 
> i can be sure that the first stream is the most "important" one...
> And, well, I don't see it as a big limitation.. not much of a big advantage 
> either though.

many muxers will have to remap stream ids because they are user / input file
supplied, many muxers simple will ignore the rule ...


> 
> > > @@ -478,9 +478,15 @@
> > >  
> > >  checksum
> > >  	adler32 checksum
> > > +	checksum is calculated for the area pointed to by forward_ptr not
> > > +	including the checksum itself (from first byte after the
> > > +	forward_ptr until last byte before the checksum).
> > 
> > somehow i think forward_ptr should be included in the checksum, dunno 
> 
> It makes more "sense", but it's harder to implement. Also it really won't 
> protect you any more from corruption..

ok, if rich agrees iam fine with this though iam slightly in favor of having
the forward_ptr in the checksumed area


> 
> > > -index_timestamp
> > > -	value of the timestamp of a keyframe relative to the last keyframe
> > > +last_pts
> > > +	The starting pts of the last frame
> > > +
> > > +index_pts
> > > +	value of the pts of a keyframe relative to the last keyframe
> > >  	stored in this index
> > >  
> > >  index_position
> > 
> > ok, but we must clarify what the last frame is, think about IBP which is 
> > stored in IPB order, B is stored last but P has a larger pts
> 
> Well, I really considered this last_pts thing as a mere cosmetic issue, to 
> know the total length of a file. Didn't realize it would get so 
> complicated. :) IMO it's just fine if it's the pts of LAST FRAME, whatever 
> it may be, even if it's not the "biggest" pts in the video. (basically it's 
> whatever pts the muxer and demuxer have to keep internally anyway for pts 
> calculations)

i think it should be the biggest pts if we do store it at all


> 
> > > @@ -601,6 +614,10 @@
> > >  headers starting at offset 2^x for each x players SHOULD end their search from
> > >  a particular offset when any startcode is found (including syncpoint)
> > >  
> > > +			Semantic requirements
> > > +
> > > +If more than one stream of a given stream class is present, each one MUST
> > > +have info tags specifying disposition, and if applicable, language.
> > 
> > ok
> 
> I didn't check thoroughly, was there any chunk you skipped from the last 
> mail?...

yes, i skiped all the ones which where "identical" to the ones i commented upon
if i skiped some more then that was unintended

[...]
-- 
Michael