Message ID | 427A34A1.3010403@cadsoft.de |
---|---|
State | New |
Headers |
Received: from tiger.cadsoft.de ([217.7.101.210]) by www.linuxtv.org with esmtp (Exim 4.34) id 1DThor-0003oe-P4 for vdr@linuxtv.org; Thu, 05 May 2005 16:59:17 +0200 Received: from raven.cadsoft.de (raven.cadsoft.de [217.7.101.211]) by tiger.cadsoft.de (8.12.7/8.12.7) with ESMTP id j45EwiMC027424 for <vdr@linuxtv.org>; Thu, 5 May 2005 16:59:01 +0200 Received: from [192.168.100.10] (hawk.cadsoft.de [192.168.100.10]) by raven.cadsoft.de (8.12.7/8.12.7) with ESMTP id j45EwhXY014559 for <vdr@linuxtv.org>; Thu, 5 May 2005 16:58:44 +0200 Message-ID: <427A34A1.3010403@cadsoft.de> Date: Thu, 05 May 2005 16:58:41 +0200 From: Klaus Schmidinger <Klaus.Schmidinger@cadsoft.de> Organization: CadSoft Computer GmbH User-Agent: Mozilla Thunderbird 1.0.2 (X11/20050317) X-Accept-Language: en MIME-Version: 1.0 To: vdr@linuxtv.org Subject: Re: [vdr] vdr-1.3.23: BUG: cPesAssembler dropped data resulting in delayed crash of VDR References: <42754C90.7050501@gmx.de> In-Reply-To: <42754C90.7050501@gmx.de> Content-Type: text/plain; charset=ISO-8859-1; format=flowed Content-Transfer-Encoding: 7bit X-BeenThere: vdr@linuxtv.org X-Mailman-Version: 2.1.5 Precedence: list Reply-To: Klaus Schmidinger's VDR <vdr@linuxtv.org> List-Id: Klaus Schmidinger's VDR <vdr.linuxtv.org> List-Unsubscribe: <http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr>, <mailto:vdr-request@linuxtv.org?subject=unsubscribe> List-Archive: <http://www.linuxtv.org/pipermail/vdr> List-Post: <mailto:vdr@linuxtv.org> List-Help: <mailto:vdr-request@linuxtv.org?subject=help> List-Subscribe: <http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr>, <mailto:vdr-request@linuxtv.org?subject=subscribe> X-List-Received-Date: Thu, 05 May 2005 14:59:18 -0000 Status: O X-Status: X-Keywords: X-UID: 1943 |
Commit Message
Klaus Schmidinger
May 5, 2005, 2:58 p.m. UTC
Reinhard Nissl wrote: > Hi, > > while recording the radio channel "Bayern 1" on Astra 19.2E from 0900 to > 1000 this morning, I've discoverd that VDR crashed at about 0932. > > After debugging the whole afternoon I've recently found the bug that > caused this crash. It's cPesAssembler that drops some data in certain > circumstances. The crash has nothing todo with recording this channel, > but with the transfer mode I'm using with vdr-xine, as it makes use of > cPesAssembler. Just curious: how can you use cPesAssembler? It's local to device.c?! > BTW: To reproduce this crash just tune to the mentioned channel in > transfer mode and wait for about 33 minutes. > > The bug has to do with cPesAssembler's length member, as it conforms to > this equation: > > length == 0 || length >=4 > > It's therefore impossible for cPesAssembler to store any fragments of > less than 4 bytes. But the problem is, that these bytes have modified > it's shift register "tag" which leads to wrong synchronisation on the > next fragment. > > For this channel, it seems to happen that cPesAssembler sees data blocks > that end with the sequence 00 00 01. These 3 bytes are stored in > cPesAssembler's "tag" member. But when the next data block arrives, > these bytes are ignored, as "length" is still 0, and further data is > dropped, as it needs to sychronize on the next start code (00 00 01). > > When the next fragment is to be stored, it results in "data" beeing > something like that > > 00 00 01 00 00 01 c0 ... > > As a result, the expected length evaluates to 6 bytes, but "length" > containes already more than that (in the above case it was 79). This > finally leads to "Rest" beeing -73 which then causes a crash in memcpy. > > The attached patch fixes this issue by introducing a new variable > "hasFragment". It helps to recognize that 1 to 3 bytes are already > stored in cPesAssembler. Feel free to drop this new variable and use > "length" instead. Where length is checked for beeing 0, it should be > checked for beeing < 4 and where "hasFragment" is set to true, "length" > should be set to any value != 0 and < 4. Thanks for debugging this pretty subtle problem. I'd rather go with using only 'length' for this and would suggest the following patch instead: Can you please verify if this does the same as your patch? Klaus
Comments
Hi, Klaus Schmidinger wrote: >> while recording the radio channel "Bayern 1" on Astra 19.2E from 0900 >> to 1000 this morning, I've discoverd that VDR crashed at about 0932. >> >> After debugging the whole afternoon I've recently found the bug that >> caused this crash. It's cPesAssembler that drops some data in certain >> circumstances. The crash has nothing todo with recording this channel, >> but with the transfer mode I'm using with vdr-xine, as it makes use of >> cPesAssembler. > > Just curious: how can you use cPesAssembler? It's local to device.c?! Isn't it "automagically" used when a device (like cXineDevice) is operating in transfer mode ;-) >> BTW: To reproduce this crash just tune to the mentioned channel in >> transfer mode and wait for about 33 minutes. >> >> The bug has to do with cPesAssembler's length member, as it conforms >> to this equation: >> >> length == 0 || length >=4 >> >> It's therefore impossible for cPesAssembler to store any fragments of >> less than 4 bytes. But the problem is, that these bytes have modified >> it's shift register "tag" which leads to wrong synchronisation on the >> next fragment. >> >> For this channel, it seems to happen that cPesAssembler sees data >> blocks that end with the sequence 00 00 01. These 3 bytes are stored >> in cPesAssembler's "tag" member. But when the next data block arrives, >> these bytes are ignored, as "length" is still 0, and further data is >> dropped, as it needs to sychronize on the next start code (00 00 01). >> >> When the next fragment is to be stored, it results in "data" beeing >> something like that >> >> 00 00 01 00 00 01 c0 ... >> >> As a result, the expected length evaluates to 6 bytes, but "length" >> containes already more than that (in the above case it was 79). This >> finally leads to "Rest" beeing -73 which then causes a crash in memcpy. >> >> The attached patch fixes this issue by introducing a new variable >> "hasFragment". It helps to recognize that 1 to 3 bytes are already >> stored in cPesAssembler. Feel free to drop this new variable and use >> "length" instead. Where length is checked for beeing 0, it should be >> checked for beeing < 4 and where "hasFragment" is set to true, >> "length" should be set to any value != 0 and < 4. > > Thanks for debugging this pretty subtle problem. > I'd rather go with using only 'length' for this and would suggest > the following patch instead: > > --- device.c 2005/02/27 13:55:15 1.99 > +++ device.c 2005/05/05 14:48:01 > @@ -34,7 +34,7 @@ > int ExpectedLength(void) { return PacketSize(data); } > static int PacketSize(const uchar *data); > int Length(void) { return length; } > - const uchar *Data(void) { return data; } > + const uchar *Data(void) { return data; } // only valid if Length() >= 4 > void Reset(void); > void Put(uchar c); > void Put(const uchar *Data, int Length); > @@ -76,7 +76,7 @@ > > void cPesAssembler::Put(uchar c) > { > - if (!length) { > + if (length < 4) { > tag = (tag << 8) | c; > if ((tag & 0xFFFFFF00) == 0x00000100) { > if (Realloc(4)) { > @@ -84,6 +84,8 @@ > length = 4; > } > } > + else if (length < 3) > + length++; > } > else if (Realloc(length + 1)) > data[length++] = c; > @@ -91,7 +93,7 @@ > > void cPesAssembler::Put(const uchar *Data, int Length) > { > - while (!length && Length > 0) { > + while (length < 4 && Length > 0) { > Put(*Data++); > Length--; > } > > Can you please verify if this does the same as your patch? I've checked the patch and it looks good. Then I've applied the patch and checked the binary by playing the above mentioned recording: everything's fine! Bye.
Reinhard Nissl wrote: > Hi, > > Klaus Schmidinger wrote: > >>> while recording the radio channel "Bayern 1" on Astra 19.2E from 0900 >>> to 1000 this morning, I've discoverd that VDR crashed at about 0932. >>> >>> After debugging the whole afternoon I've recently found the bug that >>> caused this crash. It's cPesAssembler that drops some data in certain >>> circumstances. The crash has nothing todo with recording this >>> channel, but with the transfer mode I'm using with vdr-xine, as it >>> makes use of cPesAssembler. >> >> >> Just curious: how can you use cPesAssembler? It's local to device.c?! > > > Isn't it "automagically" used when a device (like cXineDevice) is > operating in transfer mode ;-) Sure - but it sounded as if you were using it _explicitly_. > ... >> Can you please verify if this does the same as your patch? > > > I've checked the patch and it looks good. Then I've applied the patch > and checked the binary by playing the above mentioned recording: > everything's fine! Thanks. Klaus
Hi, my problem: after playing dts-cds with the mp3-plugin (0.9.10 or 0.9.12 under vdr 1.3.22) there will not any sound in the live-modus. Only switching to a channel with more than one sound track und change there to DD the sound will appear again. Is there an easy way to solve this problem ? Greeting Burkhardt
I demand that Burkhardt Petermann may or may not have written...
> my problem: after playing dts-cds with the mp3-plugin [...]
What does this have to do with cPesAssembler bugginess?
On 06 May 2005 "Burkhardt Petermann" <bpetermann@xyware.de> wrote: > my problem: after playing dts-cds with the mp3-plugin (0.9.10 or 0.9.12 > under vdr 1.3.22) there will not any sound in the live-modus. > Only switching to a channel with more than one sound track und change there > to DD the sound will appear again. > Is there an easy way to solve this problem ? Well, it depends :-) In past days the mp3 plugin used some special tricks to work around this problem. AFAIK the new 261d firm takes care of this, so the code has been removed. So either you're not using a current firmware or the 261d firmware cannot handle dts sound correctly. You can give the option BROKEN_PCM=1 while compiling the plugin. This reenables the pcm work around. May be this helps. Regards.
Hi, > Well, it depends :-) > > In past days the mp3 plugin used some special tricks to work > around this problem. AFAIK the new 261d firm takes care of this, > so the code has been removed. > > So either you're not using a current firmware or the 261d > firmware cannot handle dts sound correctly. > > You can give the option BROKEN_PCM=1 while compiling the plugin. > This reenables the pcm work around. May be this helps. > > Regards. > thank you, but I'm using the firmware 261d ... Greetings Burkhardt
Hi, forgotten: also I've tried to set BROKEN_PCM and this is not a solution for me. My dvddriver is the cvs-snapshot form 2005-03-10 ... Greetings Burkhardt > -----Original Message----- > From: vdr-bounces@linuxtv.org [mailto:vdr-bounces@linuxtv.org]On Behalf > Of Burkhardt Petermann > Sent: Friday, May 06, 2005 8:13 PM > To: Klaus Schmidinger's VDR > Subject: RE: [vdr] mp3-plugin and dts-CDs > > > Hi, > > > Well, it depends :-) > > > > In past days the mp3 plugin used some special tricks to work > > around this problem. AFAIK the new 261d firm takes care of this, > > so the code has been removed. > > > > So either you're not using a current firmware or the 261d > > firmware cannot handle dts sound correctly. > > > > You can give the option BROKEN_PCM=1 while compiling the plugin. > > This reenables the pcm work around. May be this helps. > > > > Regards. > > > thank you, but I'm using the firmware 261d ... > Greetings > Burkhardt > > _______________________________________________ > vdr mailing list > vdr@linuxtv.org > http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr >
On 06 May 2005 "Burkhardt Petermann" <bpetermann@xyware.de> wrote: > forgotten: also I've tried to set BROKEN_PCM and this is not a solution for > me. > My dvddriver is the cvs-snapshot form 2005-03-10 ... I cannot test this myself as I don't have a dts cd. Can you upload a sample somewhere? Regards.
Hi Stefan, you can download some samples from http://www.sr.se/multikanal/index.stm and burn them to a cd. Greetings Burkhardt > On 06 May 2005 "Burkhardt Petermann" <bpetermann@xyware.de> wrote: > > > forgotten: also I've tried to set BROKEN_PCM and this is not a > solution for > > me. > > My dvddriver is the cvs-snapshot form 2005-03-10 ... > > I cannot test this myself as I don't have a dts cd. > Can you upload a sample somewhere? > > Regards. >
Hi i like to know if this slowing of vdr develop means that vdr-1.4 is near, or a simply pause for holidays and deserved rest... thanks for your work... kikko77
On 06 May 2005 "Burkhardt Petermann" <bpetermann@xyware.de> wrote: > you can download some samples from http://www.sr.se/multikanal/index.stm and > burn them to a cd. I downloaded some samples from there and I cannot reproduce your problem. The sounds returns everytime after playback. I played the samples from harddisc but this shouldn't matter. Using dvb-kernel (for kernel 2.4) cvs from 3.12.2004 and firmware 261d (probably 1.0rc5). >> > My dvddriver is the cvs-snapshot form 2005-03-10 ... Are you sure about the firmware version? Regards.
Hi, my output from driver loading: DVB: AV7111(0) - firm f0240009, rtsl b0250018, vid 71010068, app 8000261d I've testet nearly all DVB-drivers (also with kernel 2.4) and the problem will not disappear. Greetings Burkhardt > I downloaded some samples from there and I cannot reproduce your > problem. The sounds returns everytime after playback. I played > the samples from harddisc but this shouldn't matter. > > Using dvb-kernel (for kernel 2.4) cvs from 3.12.2004 and firmware > 261d (probably 1.0rc5). > > >> > My dvddriver is the cvs-snapshot form 2005-03-10 ... > > Are you sure about the firmware version? > > Regards. > > -- > Stefan Huelswitt > s.huelswitt@gmx.de | http://www.muempf.de/ > > _______________________________________________ > vdr mailing list > vdr@linuxtv.org > http://www.linuxtv.org/cgi-bin/mailman/listinfo/vdr >
tinconn@virgilio.it wrote: > Hi > i like to know if this slowing of vdr develop means that vdr-1.4 is > near, or a simply pause for holidays and deserved rest... Pretty much both. I'm working towards a stable 1.4, but didn't have time to spare in the past few weeks. Greetings Klaus
Hi, now I found the reason: It's the extern decoder (creative DDTS-100) which has a problem with the new firmware and vdr 1.3.2x (with vdr 1.2.6 and AC3overDVB-Patch there isn't the problem). If I change the decoder-input-channel und go then back --> sound ... Next time I'll test my greater AV-Preamp (Tag AV30R) ... Greetings Burkhardt > -----Original Message----- > From: vdr-bounces@linuxtv.org [mailto:vdr-bounces@linuxtv.org]On Behalf > Of Burkhardt Petermann > Sent: Saturday, May 07, 2005 10:46 AM > To: Klaus Schmidinger's VDR > Subject: RE: [vdr] mp3-plugin and dts-CDs > > > Hi, > my output from driver loading: > DVB: AV7111(0) - firm f0240009, rtsl b0250018, vid 71010068, app 8000261d > I've testet nearly all DVB-drivers (also with kernel 2.4) and the problem > will not disappear. > Greetings > Burkhardt
On Fri, May 06, 2005 at 05:58:42PM +0000, Stefan Huelswitt wrote: > On 06 May 2005 "Burkhardt Petermann" <bpetermann@xyware.de> wrote: > > > my problem: after playing dts-cds with the mp3-plugin (0.9.10 or 0.9.12 > > under vdr 1.3.22) there will not any sound in the live-modus. > > Only switching to a channel with more than one sound track und change there > > to DD the sound will appear again. > > Is there an easy way to solve this problem ? > > Well, it depends :-) > > In past days the mp3 plugin used some special tricks to work > around this problem. AFAIK the new 261d firm takes care of this, > so the code has been removed. > > So either you're not using a current firmware or the 261d > firmware cannot handle dts sound correctly. All DTS samples I have around can be handled by 261d. Werner
--- device.c 2005/02/27 13:55:15 1.99 +++ device.c 2005/05/05 14:48:01 @@ -34,7 +34,7 @@ int ExpectedLength(void) { return PacketSize(data); } static int PacketSize(const uchar *data); int Length(void) { return length; } - const uchar *Data(void) { return data; } + const uchar *Data(void) { return data; } // only valid if Length() >= 4 void Reset(void); void Put(uchar c); void Put(const uchar *Data, int Length); @@ -76,7 +76,7 @@ void cPesAssembler::Put(uchar c) { - if (!length) { + if (length < 4) { tag = (tag << 8) | c; if ((tag & 0xFFFFFF00) == 0x00000100) { if (Realloc(4)) { @@ -84,6 +84,8 @@ length = 4; } } + else if (length < 3) + length++; } else if (Realloc(length + 1)) data[length++] = c; @@ -91,7 +93,7 @@ void cPesAssembler::Put(const uchar *Data, int Length) { - while (!length && Length > 0) { + while (length < 4 && Length > 0) { Put(*Data++); Length--; }