[linux-dvb] dvb-kernel section demux, my stab at it

[Emard <emard@softhome.net>]

HI Ralph at all


Here's my stab at the section demux code. Here is old code
revised and slightly improved (but not rewritten, not cleaned), 

The changes here are aiming to preserve same code logic, while
enable collecting of more section data (it now collects some 
boundary condition data that would have been rejected before).

Theoretically it should have less packet loss but practically
it didn't solve network problem (the improvement, if any, was
not significant in my case).

Something bigger is still missing. (maybe we need to implement
ip-dvb from latest draft of May 2003 ??).

However there are some improvements in the code and I added
extra comments, it was running without any oops for me.

Emard

--- dvb_demux.c.orig	Sun Nov  9 14:43:56 2003
+++ dvb_demux.c	Sun Nov  9 14:44:20 2003
@@ -232,60 +232,103 @@
 	if (!(count = payload(buf)))
 		return -1;
 
-	p = 188-count;
+	p = 188-count; // payload length, measuerd from end of TS packet
 
+	// monitor the continuity counter
 	cc = buf[3] & 0x0f;
 	ccok = ((feed->cc+1) & 0x0f) == cc ? 1 : 0;
 	feed->cc = cc;
 
-	if (buf[1] & 0x40) { // PUSI set
-		// offset to start of first section is in buf[p] 
-		if (p+buf[p]>187) // trash if it points beyond packet
+	if (buf[1] & 0x40) { // PUSI set -> new section starts in this TS packet
+		// offset to start of new section is in buf[p] 
+		if (p+buf[p]>187) // trash if it points beyond packet p+buf+1 > 188
 			return -1;
 
-		if (buf[p] && ccok) { // rest of previous section?
+		if (buf[p] && ccok) { // nonzero buf[p] -> rest of previous section?
 			// did we have enough data in last packet to calc length?
+
+			// here we process last packet from previous section
+
+			// tmp: how many additional bytes are needed
+			//      to be able to calc length
 			int tmp = 3 - sec->secbufp;
 
 			if (tmp > 0 && tmp != 3) {
-				if (p + tmp >= 187)
+
+				// dicard of pointer points beyond the TS packet end
+				if (p + tmp > 187)  // p + 1 + tmp > 188 -> p + tmp > 187
 					return -1;
 
+				// copy additional bytes
 				demux->memcopy (feed, sec->secbuf+sec->secbufp,
 					       buf+p+1, tmp);
 
+				// just calculate length, do not advance secbufp yet
 				sec->seclen = section_length(sec->secbuf);
 
+				// discard previous section if it indicates length of
+				// more than section max length of 4096 bytes
+				// (including section header)
 				if (sec->seclen > 4096) 
-					return -1;
+					goto newsection; // go to new section :)
 			}
 
+			// rest: how many bytes are needed to complete the previous section
 			rest = sec->seclen - sec->secbufp;
 
-			if (rest == buf[p] && sec->seclen) {
+			if(rest < 0)
+				return -1;
+
+			// if buf[p] contains the missing part of the previous section
+			// this is previous section's last packet and here we rely
+			// on the fact that transmitter should correctly indicate
+			// with PUSI and pointer
+			// the start of the very first new section in this TS packet
+			// that is following the previous section,
+			// not the e.g. second or some later section after the previous one.
+			// 
+		        // so if bytes fit and previous section already has nonzero length,
+			// feed the previous section
+			// we can also try with rest <= buf[p] in if()
+			if (rest <= buf[p] && sec->seclen) {
 				demux->memcopy (feed, sec->secbuf + sec->secbufp,
-					       buf+p+1, buf[p]);
-				sec->secbufp += buf[p];
+					       buf+p+1, rest);
+				sec->secbufp += rest;
 				dvb_dmx_swfilter_section_feed(feed);
+				// to be on safe side, reset the section buffer offset and crc
+				sec->crc_val = ~0;
+				sec->secbufp = 0;
 			}
 		}
 
-		p += buf[p] + 1; 		// skip rest of last section
+	newsection:;
+		// set the pointer p to the beginning of new section
+		p += buf[p] + 1; 		// skip rest of previous section
 		count = 188 - p;
 
 		while (count > 0) {
 
 			sec->crc_val = ~0;
+			sec->secbufp = 0;  // reset buffer pointer
+
+			// enough data to determine sec length?
+			if (count > 2)
+				sec->seclen = section_length(buf+p);
+
+			if (count > 2 && sec->seclen <= count) {
+				// section length is shorter or eqal to TS packet length
 
-			if ((count>2) && // enough data to determine sec length?
-			    ((sec->seclen = section_length(buf+p)) <= count)) {
-				if (sec->seclen>4096) 
+				// standard discarding of too long section
+				if (sec->seclen > 4096) 
 					return -1;
 
 				demux->memcopy (feed, sec->secbuf, buf+p,
 					       sec->seclen);
 
 				sec->secbufp = sec->seclen;
+
+				// advance the pointer and loop to find
+				// some more sections in this TS packet
 				p += sec->seclen;
 				count = 188 - p;
 
@@ -293,12 +336,12 @@
 
 				// filling bytes until packet end?
 				if (count && buf[p]==0xff) 
-					count=0;
+					count=0; // exit loop
 
 			} else { // section continues to following TS packet
 				demux->memcopy(feed, sec->secbuf, buf+p, count);
-				sec->secbufp+=count;
-				count=0;
+				sec->secbufp += count; // advance pointer
+				count=0; // exit loop
 			}
 		}
 
@@ -307,20 +350,28 @@
 
 	// section continued below
 	if (!ccok)
-		return -1;
+		return -1; // no continuty discard section 
 
 	if (!sec->secbufp) // any data in last ts packet?
 		return -1;
 
-	// did we have enough data in last packet to calc section length?
+	// if we don't have enough data in last packet to calc section length
+	// attempt to copy additional bytes
 	if (sec->secbufp < 3) {
+		// tmp: how many additional bytes are needed
+		//      to be able to calc length
 		int tmp = 3 - sec->secbufp;
 		
-		if (tmp>count)
-			return -1;
+		if (tmp > count)
+		{
+			// do not discard it so early; copy what we have and let it
+			// to be continued in the next TS packet
+			demux->memcopy (feed, sec->secbuf + sec->secbufp, buf+p, count);
+			sec->secbufp += count;
+			return 0;
+		}
 
 		sec->crc_val = ~0;
-
 		demux->memcopy (feed, sec->secbuf + sec->secbufp, buf+p, tmp);
 
 		sec->seclen = section_length(sec->secbuf);
@@ -329,6 +380,9 @@
 			return -1;
 	}
 
+	// now we are processing TS packet that entirely 
+	// belongs to one section; therefore we don't need to loop here
+	// rest: how many bytes are missing to complete the section
 	rest = sec->seclen - sec->secbufp;
 
 	if (rest < 0)
@@ -337,7 +391,11 @@
 	if (rest <= count) {	// section completed in this TS packet
 		demux->memcopy (feed, sec->secbuf + sec->secbufp, buf+p, rest);
 		sec->secbufp += rest;
+		if(sec->secbufp != sec->seclen)
+			printk("dvb section demux: internal arithmetic error\n"); 
 		dvb_dmx_swfilter_section_feed(feed);
+		sec->secbufp = 0;
+		sec->crc_val = ~0;
 	} else 	{	// section continues in following ts packet
 		demux->memcopy (feed, sec->secbuf + sec->secbufp, buf+p, count);
 		sec->secbufp += count;