> 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;
