[NUT-devel] Suggestions [PATCH]

[Michael Niedermayer]

Hi

On Wed, Mar 01, 2006 at 06:15:44AM +0200, Oded Shimon wrote:
[...]
> > +prolog
> > +        startcode                               f(64)
> > +        forward_ptr                             v
> > +
> > +epilog
> > +        reserved_bytes
> > +        checksum                                u(32)
> 
> I slightlydislikes the names :) It sounds like a book... How about 
> packet_header and packet_footer ....

ok


> 
> > +file:
> > +    file_id_string
> > +    while(!eof){
> > +        prolog, main_header, epilog
> > +        reserved_headers
> >          for(i=0; i<stream_count; i++){
> > -            stream_header
> > +            prolog, stream_header, epilog
> > +            reserved_headers
> >          }
> >          while(next_code == info_startcode){
> > -            info_packet
> > +            prolog, info_packet, epilog
> > +            reserved_headers
> >          }
> > -        if(next_code == index_startcode){
> > -            index
> > +        while(next_code == index_startcode){
> > +            prolog, index_packet, epilog
> > +            reserved_headers
> >          }
> >          if (!eof) while(next_code != main_startcode){
> > -            if(next_code == syncpoint_startcode)
> > -                syncpoint
> > +            if(next_code == syncpoint_startcode){
> > +                prolog, syncpoint, epilog
> > +            }
> >              frame
> > +            reserved_headers
> >          }
> >      }
> 
> Does this mean info packets can only come after headers?... What if the 
> headers are large and the radio station wants to switch songs, repeat the 
> entire headers and then the info packets?

the current nut has this limit too, iam perfectly fine with removing that,
but IIRC rich at some point in the past wanted info only after stream headers
or something like that


> 
> > @@ -476,12 +500,15 @@
> >        1  is_key             if set, frame is keyframe
> >        2  end_of_relevance   if set, stream has no relevance on
> >                              presentation. (EOR)
> > +      4  has_checksum       if set then the frame header contains a checksum
> 
> Maybe this should be a NUT-flag?

NUT-flag?


> 
> >      EOR frames MUST be zero-length and must be set keyframe.
> >      All streams SHOULD end with EOR, where the pts of the EOR indicates the
> >      end presentation time of the final frame.
> >      An EOR set stream is unset by the first content frames.
> >      EOR can only be unset in streams with zero decode_delay .
> > +    has_checksum must be set if the frame is larger then 64kb or its
> 
> 2*max_distance ?

ok


> 
> > +    dts differs by more then 1 second from the dts of the last frame
> 
> Can we use last_pts/max_pts_distance ? What I liked so much about that 

ok


> solution is that it worked even after a seek or damage, it required no 
> additional variable or context.
> Also, does the syncpoint rule of max_pts_distance still apply? or is it 
> just this checksum rule.

the syncpoint max_pts_distance rule doesnt apply anymore


> 
> >  checksum
> >      crc32 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).
> 
> You should probably mention what area the checksum of frame headers 
> cover...

ok


> 
> > @@ -729,21 +743,30 @@
> >  If an index is written anywhere in the file, it MUST be written at end of
> >  file as well.
> >  
> > +Each index packet SHOULD be <4kb, that way a demuxer can simply:
> 
> This is extremely hard to enforce, the index has 2 parts, the syncpoint 
> positions and the keyframe pts stuff. The keyframe pts is usually pretty 
> small so you could just stop when you reach 4kb when writing syncpoint 
> positions, and hope that the other part doesn't grow past 4kb...

for(i=0; i<n; i+=step){
    while(build index(i, step, 4kb))
        step>>=1;
    write out
}

build & write out can trivially be replaced by write & seek back within an IO
buffer

[...]
-- 
Michael