[MPlayer-dev-eng] NUT cleanup

[Oded Shimon]

On Tue, Sep 06, 2005 at 07:05:19PM +0200, Michael Niedermayer wrote:
> 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

Ok, good enough for me... I'd like Rich's approval too though.

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

Didn't think of that.

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

Stream remapping should not be very expensive or hard...
I plan on writing wrapper functions for the libnut muxer functions which 
will make sure the stream conforms more (buffering to make dts right..), 
making a small id-remapping should be near trivial.
(The stream id's and classes will just be checked and then a simple "remap" 
array will do the trick..)

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

/me pokes Rich...

The harder implementation part is that it means i need to re-implement 
get_v for the demuxer where it reads forward_ptr, every byte read 
has to be pushed into the memory buffer - because all of 
forward_ptr needs to be stored in ram as well as the data...

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

If it's that important, yeah, I guess
stream->big_pts = MAX(stream->big_pts,frame->pts)
ain't that tough...

- ods15