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) 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 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 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 -- Michael
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..)
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?
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).
4. always store chapter_start/len simplification
Not entirely sure what you mean but my impression is that this is ok.
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?
7. dissalow width or height == 0
:)
8. add leading coeff to the crc polynom
No objection. Rich
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
On Wed, Mar 01, 2006 at 01:53:21AM +0100, Michael Niedermayer wrote:
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 ...
I think it was just this one.
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
I missed this 1sec in here. I'm against ANY condition that adds a physical time unit into NUT. It's unnecessary and limits usage to a particular timescale (and also forces different muxing if time is rescaled).
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
Replace 1 sec by max_pts_diff in header and I will not adamantly object, but I don't necessarily like it.
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 ...
Yes, I agree it's cleaner to only have one bitstream format.
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
And likewise there's no way to correct past audio if you accidentally said the wrong thing, or past video if you accidentally showed the wrong thing! Historical revisionism is not part of a container's job. Should we add a feature to let streams patch-over past audio that they want to censor, so that when you remux the file to permenant storage the original audio is overwritten?? :(((( I agree with making it possible to correct [errors but more importantly omissions in] data while it's still relevant (i.e. _during_ the frame), but being able to arbitrarily replace any past data is blatently incorrect IMO. After the end time of the chapter, the demuxer should _know_ that it has the final, absolute, last-word values for the info. Otherwise you have to read eternally into the future to make sure it doesn't change later on, even if you only want to keep this one clip out of the file... If you're basing the clip to extract on the info packets themselves, then you can't even know what clip you want until you read unboundedly into the future, since its length or start might change! My intent with the stream semantics is that the packets actually mean something, locally, without reading the whole file. With the system I proposed, you know absolutely when a chapter has ended and when you have the final correct data for it. Rich
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
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
On Wed, Mar 01, 2006 at 01:50:44PM +0100, Michael Niedermayer wrote:
On Wed, Mar 01, 2006 at 06:15:44AM +0200, Oded Shimon wrote:
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
The current NUT has info streams and info packets always apply globally, so it makes sense in current NUT... Hmm, I do suggest a rule "all info packets with chapter_id>=0 MUST come after main headers" (and not arbitrarily between frames...)
@@ -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?
There are stream flags and nut flags. nut flags are 'msb coded size', 'invalid frame code' and 'coded stream flags'. The checksum flag sounds like a nut flag. (stream flags go on to the caller api, nut flags stay within the nut demuxer)
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
Actually, I'd go even further to suggest this as a per stream setting...
@@ -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
Ah, ok that's good, didn't think of that. It still leave the problem of reading index (and truely, I dislike the added complexity for the questionable gain.) You said this "simplifies" building the index, because forward_ptr is unknown, and a 2 pass solution is messy. You solution for building the index requires building it MANY times.. Although I don't see either actions as messy at all, the first pass could be done using a "bit bucket" memory buffer, and the second pass would be to file out. though you might as well build it in memory alltogether, this is only for the muxer, nobody cares about 80kb memory usage for muxing... The 2 big problems I still have with this new index - index_ptr, and raising complexity of utility that copies index to begginning of file. Yes, both are rather "rediculous" and solvable issues, but WHY have these issues at all... We already disucssed that the likelyhood of loosing a little data in the index without loosing all of it is very little. Rich gave up his "advanced syncpoints" because they raised complexity greatly for very questionable gain... Index repetition is very useful, because you can have the index in 2 edges of the file, but IMO index splitting is hardly useful at all, you might as well just repeat the index twice in the end. And, I'll continue being redundant here, in noting that loosing the index is no great trajedy!... Even in cdroms, seeking without index was just fine! Admittedly slightly slower, but still in the vecinity of ~1 second per seek. Is this really worth it. - ods15
Hi On Wed, Mar 01, 2006 at 03:26:12PM +0200, Oded Shimon wrote:
On Wed, Mar 01, 2006 at 01:50:44PM +0100, Michael Niedermayer wrote:
On Wed, Mar 01, 2006 at 06:15:44AM +0200, Oded Shimon wrote:
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
The current NUT has info streams and info packets always apply globally, so it makes sense in current NUT... Hmm, I do suggest a rule "all info packets with chapter_id>=0 MUST come after main headers" (and not arbitrarily between frames...)
i stongly object against info packet ordering rules which depend on the info packet contents [...]
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
Actually, I'd go even further to suggest this as a per stream setting...
disagree [...] -- Michael
On Wed, Mar 01, 2006 at 02:48:20PM +0100, Michael Niedermayer wrote:
Hi
On Wed, Mar 01, 2006 at 03:26:12PM +0200, Oded Shimon wrote:
On Wed, Mar 01, 2006 at 01:50:44PM +0100, Michael Niedermayer wrote:
On Wed, Mar 01, 2006 at 06:15:44AM +0200, Oded Shimon wrote:
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
The current NUT has info streams and info packets always apply globally, so it makes sense in current NUT... Hmm, I do suggest a rule "all info packets with chapter_id>=0 MUST come after main headers" (and not arbitrarily between frames...)
i stongly object against info packet ordering rules which depend on the info packet contents
And I'm strongly against arbitrary ordering of info packets for the reason I described in the other email: finding the info that's relevant to local content is O(totalfilesize). In fact with a live stream with no end, you _never_ know you have the correct info! Rich
Hi On Wed, Mar 01, 2006 at 09:00:49AM -0500, Rich Felker wrote:
On Wed, Mar 01, 2006 at 02:48:20PM +0100, Michael Niedermayer wrote:
Hi
On Wed, Mar 01, 2006 at 03:26:12PM +0200, Oded Shimon wrote:
On Wed, Mar 01, 2006 at 01:50:44PM +0100, Michael Niedermayer wrote:
On Wed, Mar 01, 2006 at 06:15:44AM +0200, Oded Shimon wrote:
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
The current NUT has info streams and info packets always apply globally, so it makes sense in current NUT... Hmm, I do suggest a rule "all info packets with chapter_id>=0 MUST come after main headers" (and not arbitrarily between frames...)
i stongly object against info packet ordering rules which depend on the info packet contents
And I'm strongly against arbitrary ordering of info packets for the reason I described in the other email: finding the info that's relevant to local content is O(totalfilesize). In fact with a live stream with no end, you _never_ know you have the correct info!
the muxer must place the info sanely, the demuxer should NEVER go and search for info packets, they must be placed where needed [...] -- Michael
On Wed, Mar 01, 2006 at 03:25:35PM +0100, Michael Niedermayer wrote:
Hi
On Wed, Mar 01, 2006 at 09:00:49AM -0500, Rich Felker wrote:
On Wed, Mar 01, 2006 at 02:48:20PM +0100, Michael Niedermayer wrote:
Hi
On Wed, Mar 01, 2006 at 03:26:12PM +0200, Oded Shimon wrote:
On Wed, Mar 01, 2006 at 01:50:44PM +0100, Michael Niedermayer wrote:
On Wed, Mar 01, 2006 at 06:15:44AM +0200, Oded Shimon wrote:
> 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
The current NUT has info streams and info packets always apply globally, so it makes sense in current NUT... Hmm, I do suggest a rule "all info packets with chapter_id>=0 MUST come after main headers" (and not arbitrarily between frames...)
i stongly object against info packet ordering rules which depend on the info packet contents
And I'm strongly against arbitrary ordering of info packets for the reason I described in the other email: finding the info that's relevant to local content is O(totalfilesize). In fact with a live stream with no end, you _never_ know you have the correct info!
the muxer must place the info sanely,
This is exactly what I want in the requirements.
the demuxer should NEVER go and search for info packets, they must be placed where needed
After they're relevant is never where they're needed. My rule was just describing how to put them where they're needed. Rich
On Wed, Mar 01, 2006 at 01:50:44PM +0100, Michael Niedermayer wrote:
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
I still favor having info streams separate for the reasons I explained in the other email.
@@ -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?
This flag pertains to the NUT structure, not the frame contents, so it belongs in the other flags if at all. Personally I don't like this approach and prefer the old way of doing syncpoint checksums as long as you can make it sufficiently extensible to your liking.
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
Why is it 2* ? Why not just max_distance?
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
Yes, and IMO that's problematic.. The rule was very simple and good.
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
Is a checksum of 1 byte really useful? IMO not. Considering that with your proposal a frame header for a 1-meg packet could still be just 1 byte (easy with CBR), this seems like it will not work. This is one reason I prefer the method of combining with syncpoints. Rich
Hi On Wed, Mar 01, 2006 at 08:59:31AM -0500, Rich Felker wrote: [...]
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
Why is it 2* ? Why not just max_distance?
the syncpoint distance is a tradeoff between resync ability after ANY error and overhead the value here is a tradeoff between detecting rare damaged framesize and overhead so it makes sense to have this larger, and why not yet another variable, well if we cant decide what value is good how should a poor muxer? [...]
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
Is a checksum of 1 byte really useful?
yes [...] -- Michael
On Wed, Mar 01, 2006 at 03:32:37PM +0100, Michael Niedermayer wrote:
Hi
On Wed, Mar 01, 2006 at 08:59:31AM -0500, Rich Felker wrote: [...]
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
Why is it 2* ? Why not just max_distance?
the syncpoint distance is a tradeoff between resync ability after ANY error and overhead
the value here is a tradeoff between detecting rare damaged framesize and overhead so it makes sense to have this larger, and why not yet another variable, well if we cant decide what value is good how should a poor muxer?
That's a poor argument, by the same logic we could hard code in spec max_distance, max_pts_distance, and the entire frame code table... I say make it another variable, reccommend in spec it should be no more than ~2*max_distance ... (also, a per stream variable. I think it is best like this) - ods15
Hi On Wed, Mar 01, 2006 at 05:06:26PM +0200, Oded Shimon wrote:
On Wed, Mar 01, 2006 at 03:32:37PM +0100, Michael Niedermayer wrote:
Hi
On Wed, Mar 01, 2006 at 08:59:31AM -0500, Rich Felker wrote: [...]
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
Why is it 2* ? Why not just max_distance?
the syncpoint distance is a tradeoff between resync ability after ANY error and overhead
the value here is a tradeoff between detecting rare damaged framesize and overhead so it makes sense to have this larger, and why not yet another variable, well if we cant decide what value is good how should a poor muxer?
That's a poor argument, by the same logic we could hard code in spec max_distance, max_pts_distance, and the entire frame code table...
I say make it another variable, reccommend in spec it should be no more than ~2*max_distance ... (also, a per stream variable. I think it is best like this)
my vote: NO [...] -- Michael
On Wed, Mar 01, 2006 at 03:32:37PM +0100, Michael Niedermayer wrote:
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
Is a checksum of 1 byte really useful?
yes
A 1 byte message with a checksum is theoretically the same as a 5 byte startcode. However, it may actually be worse in practice. Rich
On Wed, Mar 01, 2006 at 06:15:44AM +0200, Oded Shimon wrote:
On Tue, Feb 28, 2006 at 10:34:22PM +0100, Michael Niedermayer wrote:
+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...
Actually, I still have an issue with this... By a single byte of damage ('N') you can cause either the player to skip the entire file, or to have to cache arbitrary amount of MB of the file, or to seek back.. Am I missing something here? (You have to read all the data at least once to see that the checksum is bogus, and by then you have either skipped 700mb or you you cached it) - ods15
Hi On Sun, Mar 05, 2006 at 09:19:34AM +0200, Oded Shimon wrote:
On Wed, Mar 01, 2006 at 06:15:44AM +0200, Oded Shimon wrote:
On Tue, Feb 28, 2006 at 10:34:22PM +0100, Michael Niedermayer wrote:
+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...
Actually, I still have an issue with this... By a single byte of damage ('N') you can cause either the player to skip the entire file, or to have to cache arbitrary amount of MB of the file, or to seek back.. Am I missing something here? (You have to read all the data at least once to see that the checksum is bogus, and by then you have either skipped 700mb or you you cached it)
add a checksum which just covers startcode + forward ptr if forward_ptr is > 64k or dissalow arbitrary sized packets (=split index, info and stream headers) i also vote again for droping the variable checksum size threshold for frame headers and fixing it to something like 64k ... [...] -- Michael
On Tue, Feb 28, 2006 at 10:34:22PM +0100, Michael Niedermayer wrote:
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
BTW, just wanted to note, you DO know that vorbis headers are usually ~4kb and can actually be any arbitrary size, so the stream headers and just about any nut packet can be any arbitrary size... So you need to support it anyway, regardless of index... - ods15
On Wed, Mar 01, 2006 at 04:02:59PM +0200, Oded Shimon wrote:
On Tue, Feb 28, 2006 at 10:34:22PM +0100, Michael Niedermayer wrote:
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
BTW, just wanted to note, you DO know that vorbis headers are usually ~4kb and can actually be any arbitrary size, so the stream headers and just about any nut packet can be any arbitrary size... So you need to support it anyway, regardless of index...
Nonsense. The buffer containing the stream headers and packets is provided by the caller (possibly as different unit in an embedded system) and thus never requires an allocation by the muxer. If your implementation uses an allocation and copies it to do the write, this is an implementation issue of yours, not any general problem. Same goes for many other things. You can know the size without buffering the contents yourself. If there are cases where this does not apply, it's a bug in the spec and should be fixed. But issues with your implementation are not bugs in the spec, unless they reflect that an ideal implementation would be extremely difficult or overly complex. Rich
participants (3)
-
Michael Niedermayer -
Oded Shimon -
Rich Felker