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)
No objection...
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
Generally OK with this, some nitpiks later...
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
Dislike. :( It's a complication... It also makes it harder to put the index in the begginning cause you have to change the offset in several packets, not just one...
4. always store chapter_start/len simplification 5. change file structure definion so extendability is restored 6. drop info streams i see no sense in them ... 7. dissalow width or height == 0 8. add leading coeff to the crc polynom
OK.
+note, demuxers MUST be able to deal with new and unknown headers
One thing about this, what if the forward_ptr of this unkown header is 100mb and you just lost half the file... Oh, just noticed, even new headers have checksums, ok then...
+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 ....
+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?
@@ -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?
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 ?
+ 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 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.
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...
@@ -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(x=fileend-8192; ; x-=8192){ + search index startcode + if(failure) + break; + read_index +}
Also, as Rich noted, this is a bad way to find an index... - ods15