Mailing List archive

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

[linux-dvb] Re: Losless Section Demux version 1.4



> Any similar conditionals (i.e. they are checking for more than 3 bytes to
> calculate section length) also do not directly reject any data.

Hmmm... looking at the code I wasn't able to certainly tell so;
I was thinking it rejects some data and made an attempt to fix 
that what I saw as rejection point in my patch which in the comments 
says this:
// do not discard it so early; copy what we have and let it
// to be continued in the next TS packet

> One boundary condition which (really) was not checked correctly is 
> when PUSI is set, a previous section ends before the next one but
> there also is filling between the end of the previous and the start
> (indicated by the offset) of the new one.
> That's fixed by changing one "==" to "<=".

Can you review those changes if it breaks something and/or needs somewhere 
additionally change == to <= ?

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;

Home | Main Index | Thread Index