[NUT-devel] Suggestions [PATCH]

[Michael Niedermayer]

Hi

On Tue, Feb 28, 2006 at 05:51:31PM -0500, Rich Felker wrote:
> On Tue, Feb 28, 2006 at 10:34:22PM +0100, Michael Niedermayer wrote:
> > Hi
> > 
> > the Attached patch does the following:
> > 
> > 1. cleanup packets so all have forward_ptr
> > significantly improves extendability and simplifies stuff
> > slightly larger overhead (~0.006% which is negligible)
> 
> I thought we came up with some reason why it didn't work or didn't
> make sense for syncpoints (with the following frame header included..)

hmm, i remember the issue with the checksum over the syncpoint AND frame 
header but that is fixed below, dunno if there was another issue ...


> 
> > 2. split checksum into frame and syncpoint checksum
> > simplification
> > overhead changes by + 4byte for every packet >64kb (<0.006% worst case)
> > overhead changes by - syncpoint size + 4 for every packet with dts_diff > 1sec
> > overall this could even reduce the overhead, the worst case increase is also
> > negligible
> 
> Oh, I guess this would be it... But I don't see immediately from the
> patch how your change works. Can you explain?

add forward_ptr to syncpoint and move the checksum to the syncpoint
add a flag to the framecode table which indicates that the frameheader has 
its own checksum
add a rule which says that this flag must be set if packet > 64kb or dts
difference to last dts of last packet > 1 sec

for the case that there are many dts_diff > 1sec packets this has lower
overhead if there are few dts_diff >1sec packets but many >64k then its
worse but either way its cleaner&simpler IMHO


> 
> > 3. split index
> > simplification for lavf muxer
> >     currently we just write the data out seek back and update the forward
> >     pointer, this is safe as long as the packet is < the io buffer size in
> >     lavf, if its larger we need to alloc&realloc a buffer and work within that
> >     or do 2 passes to find the forward ptr value, both are more messy
> > index_ptr becomes unneccesary by this
> > improves error resistance
> 
> > +Each index packet SHOULD be <4kb, that way a demuxer can simply:
> > +for(x=fileend-8192; ; x-=8192){
> > +    search index startcode
> > +    if(failure)
> > +        break;
> > +    read_index
> > +}
> 
> Not acceptable. This requires numerous media seeks to read the index!
> There must be a way to find the offset for the beginning of the index
> after just one seek (the minimum number possible for streamable
> writing).

agree, ill suggest something very similar to how it was before my patch


> 
> > 4. always store chapter_start/len
> > simplification
> 
> Not entirely sure what you mean but my impression is that this is ok.

id like to remove the special case for the global info packets which
dont have start/len, i mean set them to 0,0 or whatever but why
use a different bitstream format ...
this obviously is a very minor issue ...


> 
> > 5. change file structure definion so extendability is restored
> 
> OK in theory; I haven't checked closely.
> 
> > 6. drop info streams
> > i see no sense in them ...
> 
> >_<
> Can the semantics I wanted be duplicated exactly with info packets
> only? Without storing a redundant irrelevant info packet?
> 

hmm probably, but iam not sure if iam happy about these semantics, which
exactly do you mean? packets must be within chapter_start ... end in the
stream? thats exactly why i dont like info_streams, theres no way to
correct a past mistake

[...]
-- 
Michael