[MPlayer-dev-eng] Patches for NUT

[Oded Shimon]

On Fri, Feb 10, 2006 at 10:18:38PM +0100, Michael Niedermayer wrote:
> On Fri, Feb 10, 2006 at 08:56:27PM +0200, Oded Shimon wrote:
> > Hmm, I used the const you said earlier (0x1EDC6F41) . Seems Ogg uses 
> > 0x04C11DB7 . 
> 
> 0x1EDC6F41 is the Castagnoli / iSCSI CRC
> 0x04C11DB7 is the IEEE one, which seems more popular, so i think we should
> switch to that
> 
> 
> > Byte ordering is global and well defined for NUT ....
> 
> iam not talking about nut but the crc:
> 
> #define G 0x04C11DB7
> for(i=0; i<size; i++){
>     crc ^= buf[i]<<24;
>     for(j=0; j<8; j++)
>         crc= (crc<<1) ^ (G & (crc>>31));
> }
> put_byte(crc>>24); put_byte(crc>>16); put_byte(crc>>8); put_byte(crc);
> 
> vs.
> 
> #define G ((0x04C11DB7>>1) + 0x10000000)
> for(i=0; i<size; i++){
>     crc ^= buf[i];
>     for(j=0; j<8; j++)
>         crc= (crc>>1) ^ (G & -(crc&1));
> }
> put_byte(crc); put_byte(crc>>8); put_byte(crc>>16); put_byte(crc>>24);
> 
> and furthermore you can reverse the bits of 0x04C11DB7 to 0xDB710641 for both
> 
> we should also maybe use a non zero initial value other wise every all zero 
> array will have a matching crc

Heh the Ogg spec mentions nothing except the generator. Well, I understand 
nothing of this, we can choose whichever, just need to say it in the spec.

> > Are you still against crc being immediately following syncpoint startcode? 
> > It's the most natural position for it. Having it after frame header is very 
> > weird, and having it at end of syncpoint is even worse as it is in the 
> > middle of the area it checksums.
> 
> yes, ill explain the reason why
> 1.
> <data> <crc>
> crc is what the crc routine outputs after being feeded with data from left
> to right 1 byte at a time
> if you now run the crc routine over the data and the following 4 bytes of the
> crc too then it will return 0, that obviously wont work if the crc is stored
> somewhere else
> 
> 2. crc have nice burst detection capabilities (every single burst error of
> 32 or less bits is detected and in the case of 0x04C11DB7 every 2 bursts of
> 8  or less bits are detected if they are less then 1mbit apart, every 2 
> bursts of 10 or less bits are detected if they are <10kbit apart, ...
> every 6 bit errors will be detected as long as they are confined to a area
> of ~200bits
> 
> now these things depend on the mathematically correct placement of the 
> crc checksum, if you place it at the begin they will very likely no longer
> hold

Ok then, I guess we will have it at end of frame header. It's a bit bizzare 
as frame header is dependant on being after syncpoint or not.

> > I'm also still a bit hesitating about the overhead of adding 4 bytes per 
> > syncpoint, but I guess it's ok.
> 
> well, suggest an alternative ...
> IMHO 4byte checksum vs. nothing is not hard to decide, the problems the 
> "nothing" variant causes are too big ...

Yeah, I have no alternatives.

> > One more issue - now that index does not mark EOF as it can appear 
> > anywhere, the index_ptr comes out of nowhere, the demuxer can even mistake 
> > it for being a frame startcode (It could even appear as a NUT startcode!).
> > So I suggest making a end_startcode before the index_ptr...
> 
> just put index_ptr into the index, right before the checksum

That's.. a bit odd... Will all indexes have an index_ptr? Or only the one 
at EOF?
Also it makes it impossible to add reserved bytes in the index.


How about this patch then?...

- ods15
-------------- next part --------------
Index: DOCS/tech/mpcf.txt
===================================================================
RCS file: /cvsroot/mplayer/main/DOCS/tech/mpcf.txt,v
retrieving revision 1.101
diff -u -r1.101 mpcf.txt
--- DOCS/tech/mpcf.txt	10 Feb 2006 10:42:40 -0000	1.101
+++ DOCS/tech/mpcf.txt	11 Feb 2006 05:58:44 -0000
@@ -219,6 +219,9 @@
     }
     for(i=0; i<reserved_count[frame_code]; i++)
         reserved                        v
+    if (after_syncpoint){
+        syncpoint_checksum              u(32)
+    }
     data
 
 index:
@@ -558,6 +561,11 @@
     including the checksum itself (from first byte after the
     forward_ptr until last byte before the checksum).
 
+syncpoint_checksum
+    crc32 checksum
+    checksum covers from first byte after syncpoint startcode until last 
+    byte before the syncpoint_checksum.
+
 back_ptr_div8
     back_ptr = back_ptr_div8 * 8 + 7
     back_ptr must point to a position within 8 bytes of a syncpoint