[MPlayer-dev-eng] NUT cleanup

[Michael Niedermayer]

Hi

On Tue, Sep 06, 2005 at 01:20:30PM +0300, Oded Shimon wrote:
[...]
> This is all that's left that you haven't ok'ed... ok this stuff, anf I'll 
> apply the patch:
> (this is NOT a valid patch, just chunks from the previous patch...)

[...]

> @@ -234,19 +239,21 @@
>          
>  sync_point:
>  	frame_startcode				f(64)
> -		global_timestamp		v
> +		global_pts			v

i object against changing global_timestamp to global_pts, pts/dts has no
meaning in respect to the global timestamps of sync points
if you dont agree yet, think about the interleaing rule, its per dts
so how do we change global_pts -> global_dts which we would need for
correct interleaving?


[...]

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


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


[...]

>  
> -coded_timestamp
> -	if coded_timestamp < (1<<msb_timestamp_shift) then it is an lsb
> -	timestamp, otherwise it is a full timestamp + (1<<msb_timestamp_shift)
> -	lsb timestamps are converted to full timesamps by:
> -	mask = (1<<msb_timestamp_shift)-1;
> -	delta= last_timestamp - mask/2
> -	timestamp= ((timestamp_lsb-delta)&mask) + delta
> -	a full timestamp MUST be used if there is no reference timestamp
> -	available after the last frame_startcode with the current stream_id
> +coded_pts
> +	if coded_pts < (1<<msb_pts_shift) then it is an lsb
> +	pts, otherwise it is a full pts + (1<<msb_pts_shift)
> +	lsb pts is converted to a full pts by:
> +	mask = (1<<msb_pts_shift)-1;
> +	delta= last_pts - mask/2
> +	pts= ((pts_lsb-delta)&mask) + delta

ok


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



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


[...]

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

[...]
-- 
Michael