[RESEND PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-ci_init-fix-new
Mauro, please pull fixed tree: http://udev.netup.ru/hg/v4l-dvb-aospan-ci_init-fix-new/ This is very urgent fix. Thanks to Dan Carpentererro...@gmail.com for patch. Thanks ! On 05.06.2010 16:05, Dan Carpenter wrote: videobuf_dvb_register_bus() returns negative error codes on failure. This was introduced in e4425eab6b2: V4L/DVB: cx23885: Check register errors. Signed-off-by: Dan Carpentererro...@gmail.com --- I don't have the hardware to test this, but it looks reversed. diff --git a/drivers/media/video/cx23885/cx23885-dvb.c b/drivers/media/video/cx23885/cx23885-dvb.c index 0a199d7..bf7c328 100644 --- a/drivers/media/video/cx23885/cx23885-dvb.c +++ b/drivers/media/video/cx23885/cx23885-dvb.c @@ -991,7 +991,7 @@ static int dvb_register(struct cx23885_tsport *port) ret = videobuf_dvb_register_bus(port-frontends, THIS_MODULE, port, dev-pci-dev, adapter_nr, 0, cx23885_dvb_fe_ioctl_override); - if (!ret) + if (ret 0) return ret; /* init CI MAC */ -- Abylai Ospanaos...@netup.ru NetUP Inc. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-ci_init-fix-new
Mauro, please pull fixed tree: http://udev.netup.ru/hg/v4l-dvb-aospan-ci_init-fix-new/ Thanks to Dan Carpentererro...@gmail.com. thanks ! On 05.06.2010 16:05, Dan Carpenter wrote: videobuf_dvb_register_bus() returns negative error codes on failure. This was introduced in e4425eab6b2: V4L/DVB: cx23885: Check register errors. Signed-off-by: Dan Carpentererro...@gmail.com --- I don't have the hardware to test this, but it looks reversed. diff --git a/drivers/media/video/cx23885/cx23885-dvb.c b/drivers/media/video/cx23885/cx23885-dvb.c index 0a199d7..bf7c328 100644 --- a/drivers/media/video/cx23885/cx23885-dvb.c +++ b/drivers/media/video/cx23885/cx23885-dvb.c @@ -991,7 +991,7 @@ static int dvb_register(struct cx23885_tsport *port) ret = videobuf_dvb_register_bus(port-frontends, THIS_MODULE, port, dev-pci-dev, adapter_nr, 0, cx23885_dvb_fe_ioctl_override); - if (!ret) + if (ret 0) return ret; /* init CI MAC */ -- Abylai Ospanaos...@netup.ru NetUP Inc. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-tscheck
Hello Mauro, On Wed, 2010-02-24 at 02:33 -0300, Mauro Carvalho Chehab wrote: Please pull change: http://udev.netup.ru/hg/v4l-dvb-aospan-tscheck/rev/0fdeb662c7f6 Allow to enable TS continuity and TEI check on loaded module. Current dvb_demux_tscheck processing doesn't allow to enable check on loaded module. dvb_demux_tscheck can be enabled only when loading module ( dvb_dmx_init should be called to enable dvb_demux_tscheck ). This patch fix this issue. NACK. It is not safe to use vmalloc at dvb_dmx_swfilter_packet(), as this function can be called during interrupt time, and vmalloc can sleep. ok, you are right. I have modified this patch. Please check it again: http://udev.netup.ru/hg/v4l-dvb-aospan-tscheck/rev/3d42e560c88d in this patch cnt_storage always allocated and used only when dvb_demux_tscheck assigned. Is this acceptable ? Thanks. -- Abylai Ospan aos...@netup.ru NetUP Inc. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-get_frontend
Mauro, Please pull change: http://udev.netup.ru/hg/v4l-dvb-aospan-get_frontend 1. configurable IRQ from CAM. IRQ from CAM disabled by default. In some environment enabled IRQ can cause of machine freeze. 2. get_frontend for STV0900 realized. Thanks. -- Abylai Ospan aos...@netup.ru NetUP Inc. signature.asc Description: This is a digitally signed message part
[PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-tscheck
Mauro, Please pull change: http://udev.netup.ru/hg/v4l-dvb-aospan-tscheck/rev/0fdeb662c7f6 Allow to enable TS continuity and TEI check on loaded module. Current dvb_demux_tscheck processing doesn't allow to enable check on loaded module. dvb_demux_tscheck can be enabled only when loading module ( dvb_dmx_init should be called to enable dvb_demux_tscheck ). This patch fix this issue. Thanks. -- Abylai Ospan aos...@netup.ru NetUP Inc. signature.asc Description: This is a digitally signed message part
[PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-cut3.0
Mauro, Please pull change: http://udev.netup.ru/hg/v4l-dvb-aospan-cut3.0/rev/19d25fe524a5 STV0900 Cut 3.0 AGC2 fix for NetUP Dual DVB-S2-CI card. Thanks. -- Abylai Ospan aos...@netup.ru NetUP Inc. signature.asc Description: This is a digitally signed message part
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Abylai Ospan wrote: On Sat, 2010-01-16 at 11:18 -0200, Mauro Carvalho Chehab wrote: Abylai Ospan wrote: Mauro, Please pulll change: http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan-22fix/rev/fc3e44f30da3 22-kHz set_tone fix for NetUP Dual DVB-S2-CI card. 22kHz logic controlled by demod. This patch modified after discussion with Oliver. This version is acceptable for both side ... Thanks. Your site seems to be down: abort: error: No route to host Please send me a pull request when the site returns. Please try again. Should work. Thanks. You forgot to sign it. You can simply reply to this email with your SOB, as I'll need to add Olvier's ack on it also. Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Hello, On Fri, 2010-01-22 at 15:34 -0200, Mauro Carvalho Chehab wrote: You forgot to sign it. You can simply reply to this email with your SOB, as I'll need to add Olvier's ack on it also. fixed. http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan-22fix/rev/058d68daed91 -- Abylai Ospan aos...@netup.ru NetUP Inc. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Abylai Ospan wrote: Hello, On Fri, 2010-01-22 at 15:34 -0200, Mauro Carvalho Chehab wrote: You forgot to sign it. You can simply reply to this email with your SOB, as I'll need to add Olvier's ack on it also. fixed. http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan-22fix/rev/058d68daed91 Applied on -git, thanks. PS.: Douglas should update -hg tree when he have time. Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
On Fri, 2010-01-22 at 17:31 -0200, Mauro Carvalho Chehab wrote: Abylai Ospan wrote: Hello, On Fri, 2010-01-22 at 15:34 -0200, Mauro Carvalho Chehab wrote: You forgot to sign it. You can simply reply to this email with your SOB, as I'll need to add Olvier's ack on it also. fixed. http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan-22fix/rev/058d68daed91 Applied on -git, thanks. PS.: Douglas should update -hg tree when he have time. ok, thanks. -- Abylai Ospan aos...@netup.ru NetUP Inc. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Hello, On Fri, Jan 22, 2010 at 5:31 PM, Mauro Carvalho Chehab mche...@redhat.com wrote: Abylai Ospan wrote: Hello, On Fri, 2010-01-22 at 15:34 -0200, Mauro Carvalho Chehab wrote: You forgot to sign it. You can simply reply to this email with your SOB, as I'll need to add Olvier's ack on it also. fixed. http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan-22fix/rev/058d68daed91 Applied on -git, thanks. PS.: Douglas should update -hg tree when he have time. Applied on -hg tree, thanks Cheers, Douglas -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Abylai Ospan wrote: On Sat, 2010-01-16 at 11:18 -0200, Mauro Carvalho Chehab wrote: Abylai Ospan wrote: Mauro, Please pulll change: http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan-22fix/rev/fc3e44f30da3 22-kHz set_tone fix for NetUP Dual DVB-S2-CI card. 22kHz logic controlled by demod. This patch modified after discussion with Oliver. This version is acceptable for both side ... Thanks. Your site seems to be down: abort: error: No route to host Please send me a pull request when the site returns. Please try again. Should work. Thanks. Patch is ok. Signed-off-by: Oliver Endriss o.endr...@gmx.de Oliver -- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Abylai Ospan wrote: Mauro, Please pulll change: http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan-22fix/rev/fc3e44f30da3 22-kHz set_tone fix for NetUP Dual DVB-S2-CI card. 22kHz logic controlled by demod. This patch modified after discussion with Oliver. This version is acceptable for both side ... Thanks. Your site seems to be down: abort: error: No route to host Please send me a pull request when the site returns. Cheers, Mauro. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
On Sat, 2010-01-16 at 11:18 -0200, Mauro Carvalho Chehab wrote: Abylai Ospan wrote: Mauro, Please pulll change: http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan-22fix/rev/fc3e44f30da3 22-kHz set_tone fix for NetUP Dual DVB-S2-CI card. 22kHz logic controlled by demod. This patch modified after discussion with Oliver. This version is acceptable for both side ... Thanks. Your site seems to be down: abort: error: No route to host Please send me a pull request when the site returns. Please try again. Should work. Thanks. -- Abylai Ospan aos...@netup.ru NetUP Inc. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Hello Oliver, There is no reason why we have to use the set_tone routine of the stv0900. You can combine - set_tone of LNBH24 with - stv090x_send_diseqc_msg of stv0900 without losing anything. Before a DiSEqC message will be sent, the tone will be disabled. Then the stv090x_send_diseqc_msg of the stv0900 can control the 22kHz tone generator of the LNBH24 using EXTM or DSQIN. yes, you right. I think lnbp21_set_tone will be used if set_tone is not defined previously (set_tone == NULL). Is this correct ? No, sorry. set_tone had to be added, because the stv0900 can also operate in DiSEqC envelope mode (connected to DSQIN of the LNBH24). In this mode set_tone of the LNBH24 has to be used. ok, you right. But seems like fe-ops.set_tone = lnbp21_set_tone; override .set_tone = stv090x_set_tone, and stv090x_set_tone never used. Is this right ? Also please check our logic. We set override_clear=LNBH24_TEN in cx23885-dvb.c i.e. we disabling 22kHz logic in LNBH24 and all 22kHz logic driven from one place ( stv0900 ). This more clear for debugging (hardware and software ). This problem is reported by one of our customer. Your changeset 13673:75331e740f61 is broke 22khz tone functionality on our cards. We need to find compromise. Reverting the patch is not a problem ( in this case we need to remove override_clear=LNBH24_TEN). -- Abylai Ospan aos...@netup.ru NetUP Inc. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Hi, Abylai Ospan wrote: Hello Oliver, There is no reason why we have to use the set_tone routine of the stv0900. You can combine - set_tone of LNBH24 with - stv090x_send_diseqc_msg of stv0900 without losing anything. Before a DiSEqC message will be sent, the tone will be disabled. Then the stv090x_send_diseqc_msg of the stv0900 can control the 22kHz tone generator of the LNBH24 using EXTM or DSQIN. yes, you right. I think lnbp21_set_tone will be used if set_tone is not defined previously (set_tone == NULL). Is this correct ? No, sorry. set_tone had to be added, because the stv0900 can also operate in DiSEqC envelope mode (connected to DSQIN of the LNBH24). In this mode set_tone of the LNBH24 has to be used. ok, you right. But seems like fe-ops.set_tone = lnbp21_set_tone; override .set_tone = stv090x_set_tone, and stv090x_set_tone never used. Is this right ? Correct. Also please check our logic. We set override_clear=LNBH24_TEN in cx23885-dvb.c i.e. we disabling 22kHz logic in LNBH24 and all 22kHz logic driven from one place ( stv0900 ). Well, if you specify override_clear=LNBH24_TEN then we have a problem. Possible solutions: a) Do not set LNBH24_TEN in override_clear, or b) Modify your patch to something like that: if (!(override_clear LNBH24_TEN)) fe-ops.set_tone = lnbp21_set_tone; Both will fix the issue. This more clear for debugging (hardware and software ). This problem is reported by one of our customer. Your changeset 13673:75331e740f61 is broke 22khz tone functionality on our cards. Ok, I did not expect that someone would set LNBH24_TEN in override_clear. ;-) We need to find compromise. Reverting the patch is not a problem ( in this case we need to remove override_clear=LNBH24_TEN). No problem. Both solutions above are ok for me. Maybe b) is more robust, because it allows override_clear=LNBH24_TEN to be used. I leaveit to you: Choose the one you like. Regards, Oliver -- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Mauro, Please pulll change: http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan-22fix/rev/fc3e44f30da3 22-kHz set_tone fix for NetUP Dual DVB-S2-CI card. 22kHz logic controlled by demod. This patch modified after discussion with Oliver. This version is acceptable for both side ... Thanks. -- Abylai Ospan aos...@netup.ru NetUP Inc. signature.asc Description: This is a digitally signed message part
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Hello, On Thu, 2010-01-14 at 14:44 +0100, Oliver Endriss wrote: This more clear for debugging (hardware and software ). This problem is reported by one of our customer. Your changeset 13673:75331e740f61 is broke 22khz tone functionality on our cards. Ok, I did not expect that someone would set LNBH24_TEN in override_clear. ;-) yes, this not trivial to trace all override definitions :) We need to find compromise. Reverting the patch is not a problem ( in this case we need to remove override_clear=LNBH24_TEN). No problem. Both solutions above are ok for me. Maybe b) is more robust, because it allows override_clear=LNBH24_TEN to be used. I leaveit to you: Choose the one you like. ok. second is choosen - it's no broke yours and our logic. -- Abylai Ospan aos...@netup.ru NetUP Inc. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Mauro, Please pulll change: http://udev.netup.ru/hg/v4l-dvb-aospan-22fix/ 22-kHz set_tone fix for NetUP Dual DVB-S2-CI card. Description: 22-kHz tone can be driven in two ways: 1. LNBH24 can produce 22kHz continuous tone when TEN=1 ( 22 KHz tone output is always activated ). 2. LNBH24 can reproduce 22kHz tone timings from DSQIN or EXTM pin's when TEN=0. From LNBH24 datasheet: In order to improve design flexibility an external tone input pin is available (EXTM). The EXTM is a Logic input pin which activates the 22 kHz tone output, on the VoTX pin, by using the LNBH24 integrated tone generator (similar to the DSQIN pin function). In fact, the output tone waveform characteristics will always be internally controlled by the LNBH24 tone generator and the EXTM signal will be used as a timing control for DiSEqC tone data encoding on the VoTX output. In NetUP Dual DVB-S2-CI card 22kHz tone timings on EXTM pin produced by STV0900 demod: .set_tone = stv0900_set_tone redefine to set_tone = lnbp21_set_tone is not correct for NetUP Dual DVB-S2-CI card. -- Abylai Ospan aos...@netup.ru NetUP Inc. P.S. Also I think diseqc doesn't work when TEN bit enabled in LNBH24 (22kHz tone can't be modulated by demod), but need to check ... signature.asc Description: This is a digitally signed message part
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Hi, Abylai Ospan wrote: Please pulll change: http://udev.netup.ru/hg/v4l-dvb-aospan-22fix/ 22-kHz set_tone fix for NetUP Dual DVB-S2-CI card. NAK. Description: 22-kHz tone can be driven in two ways: 1. LNBH24 can produce 22kHz continuous tone when TEN=1 ( 22 KHz tone output is always activated ). 2. LNBH24 can reproduce 22kHz tone timings from DSQIN or EXTM pin's when TEN=0. From LNBH24 datasheet: In order to improve design flexibility an external tone input pin is available (EXTM). The EXTM is a Logic input pin which activates the 22 kHz tone output, on the VoTX pin, by using the LNBH24 integrated tone generator (similar to the DSQIN pin function). In fact, the output tone waveform characteristics will always be internally controlled by the LNBH24 tone generator and the EXTM signal will be used as a timing control for DiSEqC tone data encoding on the VoTX output. In NetUP Dual DVB-S2-CI card 22kHz tone timings on EXTM pin produced by STV0900 demod: .set_tone = stv0900_set_tone redefine to set_tone = lnbp21_set_tone is not correct for NetUP Dual DVB-S2-CI card. Why is it not correct? Please explain. Afaics it does not matter, whether the demod or the LNBH24 controls the 22kHz tone. With your patch, lnbp21_set_tone would never be used... CU Oliver -- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-22fix
Hi, Abylai Ospan wrote: redefine to set_tone = lnbp21_set_tone is not correct for NetUP Dual DVB-S2-CI card. Why is it not correct? Please explain. Afaics it does not matter, whether the demod or the LNBH24 controls the 22kHz tone. With your patch, lnbp21_set_tone would never be used... LNBH24 can _continuously_ produce 22kHz tone. This way acceptable if you only want to switch LNB's oscillator to high band. If you want to send diseqc command you need to modulate this 22kHz tone. LNBH24 can't do this but STV0900 demod can. There is no reason why we have to use the set_tone routine of the stv0900. You can combine - set_tone of LNBH24 with - stv090x_send_diseqc_msg of stv0900 without losing anything. Before a DiSEqC message will be sent, the tone will be disabled. Then the stv090x_send_diseqc_msg of the stv0900 can control the 22kHz tone generator of the LNBH24 using EXTM or DSQIN. I have created some demonstration from oscilloscope on NetUP Dual DVB-S2-CI card in attachment ( sorry, russian interface :). Blue is a EXTM pin, yellow is a coaxial connector. As you can see STV0900 sends 11 period's and LNBH24 reproduce it to coaxial output. This is so colled diseqc modulated mode. Scheme of modulated signals you can find in diseqc bus specification - http://www.eutelsat.com/satellites/pdf/Diseqc/Reference% 20docs/bus_spec.pdf on Figure 1: DiSEqC Modulation Scheme. See above. The 22kHz output of the LNBH24 is a logical 'or' of the TEN flag and the control inputs. I don't see any problem here. I think lnbp21_set_tone will be used if set_tone is not defined previously (set_tone == NULL). Is this correct ? No, sorry. set_tone had to be added, because the stv0900 can also operate in DiSEqC envelope mode (connected to DSQIN of the LNBH24). In this mode set_tone of the LNBH24 has to be used. When I modified the lnbp21 module, I considered both cases. I could not (and cannot) see any problem here. Please revert the patch. CU Oliver -- VDR Remote Plugin 0.4.0: http://www.escape-edv.de/endriss/vdr/ 4 MByte Mod: http://www.escape-edv.de/endriss/dvb-mem-mod/ Full-TS Mod: http://www.escape-edv.de/endriss/dvb-full-ts-mod/ -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PULL] http://udev.netup.ru/hg/v4l-dvb-aospan-ci-irq/
Mauro, Please pull change: http://udev.netup.ru/hg/v4l-dvb-aospan-ci-irq/ configurable IRQ mode on NetUP Dual DVB-S2 CI; IRQ from CAM processing (CI interface works faster). with this patch CI works 7x times faster in our benchmarks. Thanks. -- Abylai Ospan aos...@netup.ru NetUP Inc. signature.asc Description: This is a digitally signed message part
Re: [RESEND PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Hello Mauro,
Please pull fixed patch from:
http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan/rev/1d956b581b02
I have fixed pointed issues. Strange that ./v4l/scripts/checkpatch.pl don't
show me this errors before. Now checkpatch shows 0 errors/warnings.
P.S.
In some stripped environments running additiona user-level programs is
difficult. In this case this debug possibility very useful.
On Saturday 31 October 2009 13:45:17 Mauro Carvalho Chehab wrote:
Em Sun, 11 Oct 2009 21:09:24 +0400
aos...@netup.ru aos...@netup.ru escreveu:
Hello Mauro,
On Friday 09 October 2009 16:01:58 Mauro Carvalho Chehab wrote:
Wouldn't be be better to just use dvbtraffic userspace apps for it?
this feature very usable for debugging. Please add it.
Also,
your patch has some coding style issues.
please point to this issues. I will fix.
# HG changeset patch
# User Abylay Ospan aos...@netup.ru
# Date 1253802277 -14400
# Node ID 711d9630876ffd81196c1032ce77825d5b433cc8
# Parent a798c751f06d60335fbbad9fdca8c41f1298d228
TS speed check. Logging transport stream speed in Kbits per second
Signed-off-by: Abylay Ospan aos...@netup.ru
--- a/linux/drivers/media/dvb/dvb-core/dvb_demux.c Wed Sep 23 10:21:53
2009 +0200 +++ b/linux/drivers/media/dvb/dvb-core/dvb_demux.c Thu Sep
24
18:24:37 2009 +0400 @@ -42,6 +42,11 @@
module_param(dvb_demux_tscheck, int, 0644);
MODULE_PARM_DESC(dvb_demux_tscheck,
enable transport stream continuity and TEI check);
+
+static int dvb_demux_speedcheck;
+module_param(dvb_demux_speedcheck, int, 0644);
+MODULE_PARM_DESC(dvb_demux_speedcheck,
+ enable transport stream speed check);
#define dprintk_tscheck(x...) do { \
if (dvb_demux_tscheck printk_ratelimit())\
@@ -385,6 +390,37 @@
struct dvb_demux_feed *feed;
u16 pid = ts_pid(buf);
int dvr_done = 0;
+ struct timespec cur_time, delta_time;
+ u64 speed_bytes, speed_timedelta;
+
+ if (dvb_demux_speedcheck) {
+ demux-speed_pkts_cnt++;
+
+ /* show speed every SPEED_PKTS_INTERVAL packets */
+ if (!(demux-speed_pkts_cnt%SPEED_PKTS_INTERVAL)) {
You need to add spaces between each argument of the % operator
+ cur_time = current_kernel_time();
+
+ if (demux-speed_last_time.tv_sec != 0
+ demux-speed_last_time.tv_nsec != 0) {
+ delta_time = timespec_sub(cur_time,
+ demux-speed_last_time);
+ speed_bytes = (u64)demux-speed_pkts_cnt*188*8;
You need to add spaces between each argument of the * operator
+ /* convert to 1024 basis */
+ speed_bytes = 1000*div64_u64(speed_bytes,
+ 1024);
You need to add spaces between each argument of the * operator
+ speed_timedelta =
+ (u64)timespec_to_ns(delta_time);
+ speed_timedelta = div64_u64(speed_timedelta,
+ 100); /* nsec - usec */
+ printk(KERN_INFO TS speed %llu Kbits/sec \n,
+ div64_u64(speed_bytes,
+ speed_timedelta));
+ };
+
+ demux-speed_last_time = cur_time;
+ demux-speed_pkts_cnt = 0;
+ };
+ };
if (dvb_demux_tscheck) {
if (!demux-cnt_storage)
--- a/linux/drivers/media/dvb/dvb-core/dvb_demux.h Wed Sep 23 10:21:53
2009 +0200 +++ b/linux/drivers/media/dvb/dvb-core/dvb_demux.h Thu Sep
24
18:24:37 2009 +0400 @@ -44,6 +44,7 @@
#define DVB_DEMUX_MASK_MAX 18
#define MAX_PID 0x1fff
+#define SPEED_PKTS_INTERVAL 5
struct dvb_demux_filter {
struct dmx_section_filter filter;
@@ -132,6 +133,9 @@
spinlock_t lock;
uint8_t *cnt_storage; /* for TS continuity check */
+
+ struct timespec speed_last_time; /* for TS speed check */
+ uint32_t speed_pkts_cnt; /* for TS speed check */
};
int dvb_dmx_init(struct dvb_demux *dvbdemux);
Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Em Sun, 11 Oct 2009 21:09:24 +0400
aos...@netup.ru aos...@netup.ru escreveu:
Hello Mauro,
On Friday 09 October 2009 16:01:58 Mauro Carvalho Chehab wrote:
Wouldn't be be better to just use dvbtraffic userspace apps for it?
this feature very usable for debugging. Please add it.
Also,
your patch has some coding style issues.
please point to this issues. I will fix.
# HG changeset patch
# User Abylay Ospan aos...@netup.ru
# Date 1253802277 -14400
# Node ID 711d9630876ffd81196c1032ce77825d5b433cc8
# Parent a798c751f06d60335fbbad9fdca8c41f1298d228
TS speed check. Logging transport stream speed in Kbits per second
Signed-off-by: Abylay Ospan aos...@netup.ru
--- a/linux/drivers/media/dvb/dvb-core/dvb_demux.cWed Sep 23 10:21:53
2009 +0200
+++ b/linux/drivers/media/dvb/dvb-core/dvb_demux.cThu Sep 24 18:24:37
2009 +0400
@@ -42,6 +42,11 @@
module_param(dvb_demux_tscheck, int, 0644);
MODULE_PARM_DESC(dvb_demux_tscheck,
enable transport stream continuity and TEI check);
+
+static int dvb_demux_speedcheck;
+module_param(dvb_demux_speedcheck, int, 0644);
+MODULE_PARM_DESC(dvb_demux_speedcheck,
+ enable transport stream speed check);
#define dprintk_tscheck(x...) do { \
if (dvb_demux_tscheck printk_ratelimit())\
@@ -385,6 +390,37 @@
struct dvb_demux_feed *feed;
u16 pid = ts_pid(buf);
int dvr_done = 0;
+ struct timespec cur_time, delta_time;
+ u64 speed_bytes, speed_timedelta;
+
+ if (dvb_demux_speedcheck) {
+ demux-speed_pkts_cnt++;
+
+ /* show speed every SPEED_PKTS_INTERVAL packets */
+ if (!(demux-speed_pkts_cnt%SPEED_PKTS_INTERVAL)) {
You need to add spaces between each argument of the % operator
+ cur_time = current_kernel_time();
+
+ if (demux-speed_last_time.tv_sec != 0
+ demux-speed_last_time.tv_nsec != 0) {
+ delta_time = timespec_sub(cur_time,
+ demux-speed_last_time);
+ speed_bytes = (u64)demux-speed_pkts_cnt*188*8;
You need to add spaces between each argument of the * operator
+ /* convert to 1024 basis */
+ speed_bytes = 1000*div64_u64(speed_bytes,
+ 1024);
You need to add spaces between each argument of the * operator
+ speed_timedelta =
+ (u64)timespec_to_ns(delta_time);
+ speed_timedelta = div64_u64(speed_timedelta,
+ 100); /* nsec - usec */
+ printk(KERN_INFO TS speed %llu Kbits/sec \n,
+ div64_u64(speed_bytes,
+ speed_timedelta));
+ };
+
+ demux-speed_last_time = cur_time;
+ demux-speed_pkts_cnt = 0;
+ };
+ };
if (dvb_demux_tscheck) {
if (!demux-cnt_storage)
--- a/linux/drivers/media/dvb/dvb-core/dvb_demux.hWed Sep 23 10:21:53
2009 +0200
+++ b/linux/drivers/media/dvb/dvb-core/dvb_demux.hThu Sep 24 18:24:37
2009 +0400
@@ -44,6 +44,7 @@
#define DVB_DEMUX_MASK_MAX 18
#define MAX_PID 0x1fff
+#define SPEED_PKTS_INTERVAL 5
struct dvb_demux_filter {
struct dmx_section_filter filter;
@@ -132,6 +133,9 @@
spinlock_t lock;
uint8_t *cnt_storage; /* for TS continuity check */
+
+ struct timespec speed_last_time; /* for TS speed check */
+ uint32_t speed_pkts_cnt; /* for TS speed check */
};
int dvb_dmx_init(struct dvb_demux *dvbdemux);
Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Hello Mauro, On Friday 09 October 2009 16:01:58 Mauro Carvalho Chehab wrote: Wouldn't be be better to just use dvbtraffic userspace apps for it? this feature very usable for debugging. Please add it. Also, your patch has some coding style issues. please point to this issues. I will fix. Em Thu, 24 Sep 2009 18:41:08 +0400 Abylai Ospan aos...@netup.ru escreveu: Mauro, Please pulll changes from: http://udev.netup.ru/hg/v4l-dvb-aospan/rev/711d9630876f Show speed of transport stream in Kbit/sec: for example, data obtained from DVB-S2 transponder from Eutelsat W4: Jun 27 12:06:01 udev TS speed 58608 Kbits/sec Jun 27 12:06:03 udev TS speed 58422 Kbits/sec Jun 27 12:06:04 udev TS speed 58608 Kbits/sec for DVB-S1 transponder from the same satellite: Jun 27 12:07:00 udev TS speed 37108 Kbits/sec Jun 27 12:07:02 udev TS speed 37089 Kbits/sec Jun 27 12:07:04 udev TS speed 37202 Kbits/sec this feature can be enabled using following command: echo 1 /sys/module/dvb_core/parameters/dvb_demux_speedcheck and disabled by following command: echo 0 /sys/module/dvb_core/parameters/dvb_demux_speedcheck Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [RESEND PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Hi Abylai, Wouldn't be be better to just use dvbtraffic userspace apps for it? Also, your patch has some coding style issues. Em Thu, 24 Sep 2009 18:41:08 +0400 Abylai Ospan aos...@netup.ru escreveu: Mauro, Please pulll changes from: http://udev.netup.ru/hg/v4l-dvb-aospan/rev/711d9630876f Show speed of transport stream in Kbit/sec: for example, data obtained from DVB-S2 transponder from Eutelsat W4: Jun 27 12:06:01 udev TS speed 58608 Kbits/sec Jun 27 12:06:03 udev TS speed 58422 Kbits/sec Jun 27 12:06:04 udev TS speed 58608 Kbits/sec for DVB-S1 transponder from the same satellite: Jun 27 12:07:00 udev TS speed 37108 Kbits/sec Jun 27 12:07:02 udev TS speed 37089 Kbits/sec Jun 27 12:07:04 udev TS speed 37202 Kbits/sec this feature can be enabled using following command: echo 1 /sys/module/dvb_core/parameters/dvb_demux_speedcheck and disabled by following command: echo 0 /sys/module/dvb_core/parameters/dvb_demux_speedcheck Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[RESEND PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Mauro, Please pulll changes from: http://udev.netup.ru/hg/v4l-dvb-aospan/rev/711d9630876f Show speed of transport stream in Kbit/sec: for example, data obtained from DVB-S2 transponder from Eutelsat W4: Jun 27 12:06:01 udev TS speed 58608 Kbits/sec Jun 27 12:06:03 udev TS speed 58422 Kbits/sec Jun 27 12:06:04 udev TS speed 58608 Kbits/sec for DVB-S1 transponder from the same satellite: Jun 27 12:07:00 udev TS speed 37108 Kbits/sec Jun 27 12:07:02 udev TS speed 37089 Kbits/sec Jun 27 12:07:04 udev TS speed 37202 Kbits/sec this feature can be enabled using following command: echo 1 /sys/module/dvb_core/parameters/dvb_demux_speedcheck and disabled by following command: echo 0 /sys/module/dvb_core/parameters/dvb_demux_speedcheck -- Abylai Ospan aos...@netup.ru NetUP Inc. -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Mauro, Please pulll changes: http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan/rev/8b0c92aec325 vmalloc to kmalloc changed. vmalloc don't work (kernel oops hapens) when dvb_dmx_swfilter_packets called from IRQ handler ( for example in budget-core ). -- Abylai Ospan aos...@netup.ru NetUP Inc. smime.p7s Description: S/MIME cryptographic signature
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
On Sun, 2009-06-28 at 15:25 +0400, Abylai Ospan wrote: Mauro, Please pulll changes: http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan/rev/8b0c92aec325 vmalloc to kmalloc changed. Since MAX_PID is only 8 kB and kmalloc can handle up to 128 kB, that seems OK. vmalloc don't work (kernel oops hapens) when dvb_dmx_swfilter_packets called from IRQ handler ( for example in budget-core ). No one should call dvb_dmx_swfilter_packets() from an IRQ handler. That function does too much work to finish in a short amount of time. I see budget-core.c:vpeirq(), actually gets called from a tasklet (softirq context) though. Here's a LKML thread where Andrew Morton says it's unsafe to call __vmalloc() from IRQ contexts: http://lkml.org/lkml/2008/11/17/23 Since tasklets run in an interrupt context, with many (all?) of the limitations of a IRQ handler, I suspect Andrew's comments may apply to tasklets as well. What really needs to be fixed is linux/drivers/media/dvb/ttpci/budget-core.c:vpeirq() should not call dvb_dmx_swfilter_packets() from a tasklet context, but instead from a work handler handler context. That means converting from a tasklet to a work handler. A change to the dvb-core to use kmalloc() is just avoiding the work of coverting from tasklets to workqueues. It looks like the move to replace tasklets with workqueues is at least 2 years old: http://lwn.net/Articles/239633/ BTW, it looks like av7110.c has a similar problem using a tasklet to call dvb_dmx_swfilter_packets(). Regards, Andy -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Abylai Ospan wrote: В Вск, 28/06/2009 в 08:52 -0400, Andy Walls пишет: BTW, it looks like av7110.c has a similar problem using a tasklet to call dvb_dmx_swfilter_packets(). seems like videbuf usage is more prefer. In this case dvb_dmx_swfilter called in separate thread videobuf_dvb_thread. That won't work for the av7110 driver until videobuf-dvb supports using hardware PID filters. Regards, Andreas -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
[PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Mauro, Please pulll changes: 1. correct init values in cnt_storage for TS continuity check: http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan/rev/f753dd0f27c9 2. Show speed of transport stream in Kbit/sec: http://udev.netup.ru/cgi-bin/hgwebdir.cgi/v4l-dvb-aospan/rev/91923e3471ac for example, data obtained from DVB-S2 transponder from Eutelsat W4: Jun 27 12:06:01 udev TS speed 58608 Kbits/sec Jun 27 12:06:03 udev TS speed 58422 Kbits/sec Jun 27 12:06:04 udev TS speed 58608 Kbits/sec for DVB-S1 transponder from the same satellite: Jun 27 12:07:00 udev TS speed 37108 Kbits/sec Jun 27 12:07:02 udev TS speed 37089 Kbits/sec Jun 27 12:07:04 udev TS speed 37202 Kbits/sec this feature can be enabled using following command: echo 1 /sys/module/dvb_core/parameters/dvb_demux_speedcheck and disabled by following command: echo 0 /sys/module/dvb_core/parameters/dvb_demux_speedcheck -- Abylai Ospan aos...@netup.ru NetUP Inc. smime.p7s Description: S/MIME cryptographic signature
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Mauro,
Please pull new changes -
http://udev.netup.ru/hg/v4l-dvb-aospan/rev/db2114ac07ed
+#define dprintk_tscheck(x...) do { \
+ if (dvb_demux_tscheck printk_ratelimit()) printk(x); } while (0)
checkpatch.pl is not perfect. The better is to break the macro as I've shown
on
my previous email, breaking one statement per line:
done.
No need for an empty line here. Please remove to keep all vars together.
done.
+ int cnt_pid;
unsigned cnt_pid;
changed to pid var. Thanks for Andreas Oberritter.
+
+ if (dvb_demux_tscheck) {
+
No need for an empty line here.
done.
Please add a define for 0x1fff and use the define, instead of using a magic
value at vmalloc, like:
#define MAX_PID 0x1ffe
done.
You need to add a check to see if the vmalloc really worked.
Also, if you don't have memory for the first packet, it doesn't make sense to
keep insisting on allocating memory. Better just to disable the check. So, I
would code it like:
done.
+ /* check pkt counter */
+ cnt_pid = ((buf[1] 0x1f)8) | buf[2];
+
+ if (cnt_pid != 0x1fff) {
if (cnt_pid = MAX_PID) {
changed for if (cnt_pid MAX_PID) because PID=0x1FFF should be
ignored ( padding NULL packets - described in
http://en.wikipedia.org/wiki/MPEG_transport_stream ).
+ if ((buf[3] 0xf) != demux-cnt_storage[cnt_pid])
+ dprintk_tscheck(TS packet counter mismatch.
PID=0x%x expected 0x%x got 0x%x\n,\
+ cnt_pid,
demux-cnt_storage[cnt_pid], buf[3] 0xf);
Please, don't add the backslash. Also, in order to have it 80-line compliant,
you could break the strings as:
done.
In order to work with the lack of memory, you'll need a label here:
no_dvb_demux_tscheck
done.
--
Abylai Ospan aos...@netup.ru
NetUP Inc.
smime.p7s
Description: S/MIME cryptographic signature
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Em Wed, 27 May 2009 22:15:17 +0400 Abylai Ospan aos...@netup.ru escreveu: Mauro, Please pull from http://udev.netup.ru/hg/v4l-dvb-aospan/rev/e90b61dfd602 Transport Stream continuity check. Very usefull for debugging when broken transport stream received. Pulled, thanks. +/* + * uncomment this define for transport stream packets continuity counter check + * #define DVB_DEMUX_SECTION_TS_COUNTER_CHECK + */ It wouldn't be better to add it as a Kconfig or as a module option, in order to be easy for people to enable it? Also, it may make sense to change it to use printk_ratelimit, to avoid it to produce too much printk errors. Cheers, Mauro -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Mauro, В Пнд, 01/06/2009 в 04:52 -0300, Mauro Carvalho Chehab пишет: + * uncomment this define for transport stream packets continuity counter check + * #define DVB_DEMUX_SECTION_TS_COUNTER_CHECK + */ It wouldn't be better to add it as a Kconfig or as a module option, in order to be easy for people to enable it? module option seems more flexible. Also, it may make sense to change it to use printk_ratelimit, to avoid it to produce too much printk errors. yes, this right. Please pulll new changes - http://udev.netup.ru/hg/v4l-dvb-aospan/rev/4acf883807a8 1. Module option dvb_demux_tscheck added. Can be changed on fly ( by echo 1 /sys/module/dvb_core/parameters/dvb_demux_tscheck). Check disabled by default. 2. printk_ratelimit used. 3. DVB_DEMUX_SECTION_TS_COUNTER_CHECK define removed -- Abylai Ospan aos...@netup.ru NetUP Inc. smime.p7s Description: S/MIME cryptographic signature
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Em Mon, 01 Jun 2009 13:02:56 +0400
Abylai Ospan aos...@netup.ru escreveu:
Mauro,
В Пнд, 01/06/2009 в 04:52 -0300, Mauro Carvalho Chehab пишет:
+ * uncomment this define for transport stream packets continuity counter
check
+ * #define DVB_DEMUX_SECTION_TS_COUNTER_CHECK
+ */
It wouldn't be better to add it as a Kconfig or as a module option, in
order to
be easy for people to enable it?
module option seems more flexible.
Also, it may make sense to change it to use printk_ratelimit, to avoid it
to produce
too much printk errors.
yes, this right.
Please pulll new changes -
http://udev.netup.ru/hg/v4l-dvb-aospan/rev/4acf883807a8
# HG changeset patch
# User Abylay Ospan aos...@netup.ru
# Date 1243846636 -14400
# Node ID 4acf883807a8f9fcd82f7c45ee61513955b6a82b
# Parent d55ec37bebfa91dfa0586393551382ba37355b56
TS continuity check: show error message when discontinuity detected or TEI
flag detected in header
Signed-off-by: Abylay Ospan aos...@netup.ru
--- a/linux/drivers/media/dvb/dvb-core/dvb_demux.cMon Jun 01 05:22:37
2009 -0300
+++ b/linux/drivers/media/dvb/dvb-core/dvb_demux.cMon Jun 01 12:57:16
2009 +0400
@@ -37,6 +37,13 @@
** #define DVB_DEMUX_SECTION_LOSS_LOG to monitor payload loss in the syslog
*/
// #define DVB_DEMUX_SECTION_LOSS_LOG
+
+static int dvb_demux_tscheck;
+module_param(dvb_demux_tscheck, int, 0644);
+MODULE_PARM_DESC(dvb_demux_tscheck, enable transport stream continuity and
TEI check);
+
+#define dprintk_tscheck(x...) do { if (dvb_demux_tscheck
printk_ratelimit()) printk(x); } while (0)
Please check your patch against kernel codingstyle with checkpatch.pl. There
are some CodingStyle violations here and on the code bellow.
In this specific case, break it into one statement per line, like:
#define dprintk_tscheck(x...) do { \
if (dvb_demux_tscheck printk_ratelimit())\
printk(x); \
} while (0)
+
/**
* static inlined helper functions
@@ -375,6 +382,24 @@
struct dvb_demux_feed *feed;
u16 pid = ts_pid(buf);
int dvr_done = 0;
+
+ int cnt_pid;
+
+ if(dvb_demux_tscheck){
CodingStyle.
+ /* check pkt counter */
+ cnt_pid = ( (buf[1] 0x1f)8 ) | buf[2];
CodingStyle.
+
+ if(cnt_pid != 0x1fff){
CodingStyle.
+ if( buf[1] 0x80 )
CodingStyle.
+ dprintk_tscheck(TEI detected. PID=0x%x
data1=0x%x\n, cnt_pid, buf[1] );
CodingStyle.
+
+ if( (buf[3] 0xf) != demux-cnt_storage[cnt_pid] )
CodingStyle.
+ dprintk_tscheck(TS packet counter mismatch.
PID=0x%x expected 0x%x got 0x%x\n, cnt_pid, demux-cnt_storage[cnt_pid],
buf[3] 0xf );
CodingStyle.
+
+ demux-cnt_storage[cnt_pid] = ( (buf[3] 0xf) + 1)
0xf;
CodingStyle.
+ };
+ /* end check */
+ };
list_for_each_entry(feed, demux-feed_list, list_head) {
if ((feed-pid != pid) (feed-pid != 0x2000))
--- a/linux/drivers/media/dvb/dvb-core/dvb_demux.hMon Jun 01 05:22:37
2009 -0300
+++ b/linux/drivers/media/dvb/dvb-core/dvb_demux.hMon Jun 01 12:57:16
2009 +0400
@@ -128,6 +128,8 @@
struct mutex mutex;
spinlock_t lock;
+
+ uint8_t cnt_storage[0x1fff]; /// for TS continuity check
Please, don't add the penalty of increasing the size of the struct to this
amount of size if debug is disabled. You should instead add a pointer here and
allocate it dynamically only if debug is enabled.
};
int dvb_dmx_init(struct dvb_demux *dvbdemux);
Also, as the previous changeset were already merged, do your patch against the
current tree
Cheers,
Mauro
--
To unsubscribe from this list: send the line unsubscribe linux-media in
the body of a message to majord...@vger.kernel.org
More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
В Пнд, 01/06/2009 в 06:38 -0300, Mauro Carvalho Chehab пишет: Please check your patch against kernel codingstyle with checkpatch.pl. There are some CodingStyle violations here and on the code bellow. Please pull new changes - http://udev.netup.ru/hg/v4l-dvb-aospan/rev/0d3e6c0695cc 1. checkpatch.pl don't show warnings/errors ( except printk line length ). 2. cnt_storage array dinamically allocated before doing checks. We can't allocate it in dvb_dmx_init routine because module option dvb_demux_tscheck can be enabled on running system ( not in module startup only ). -- Abylai Ospan aos...@netup.ru NetUP Inc. smime.p7s Description: S/MIME cryptographic signature
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Em Mon, 01 Jun 2009 17:42:24 +0400
Abylai Ospan aos...@netup.ru escreveu:
В Пнд, 01/06/2009 в 06:38 -0300, Mauro Carvalho Chehab пишет:
Please check your patch against kernel codingstyle with checkpatch.pl. There
are some CodingStyle violations here and on the code bellow.
Please pull new changes -
http://udev.netup.ru/hg/v4l-dvb-aospan/rev/0d3e6c0695cc
1. checkpatch.pl don't show warnings/errors ( except printk line
length ).
2. cnt_storage array dinamically allocated before doing checks. We can't
allocate it in dvb_dmx_init routine because module option
dvb_demux_tscheck can be enabled on running system ( not in module
startup only ).
Ok, we're close to a final version.
TS continuity check: show error message when discontinuity detected or TEI flag
detected in header
Signed-off-by: Abylay Ospan aos...@netup.ru
--- a/linux/drivers/media/dvb/dvb-core/dvb_demux.cMon Jun 01 05:22:37
2009 -0300
+++ b/linux/drivers/media/dvb/dvb-core/dvb_demux.cMon Jun 01 17:36:33
2009 +0400
@@ -37,6 +37,15 @@
** #define DVB_DEMUX_SECTION_LOSS_LOG to monitor payload loss in the syslog
*/
// #define DVB_DEMUX_SECTION_LOSS_LOG
+
+static int dvb_demux_tscheck;
+module_param(dvb_demux_tscheck, int, 0644);
+MODULE_PARM_DESC(dvb_demux_tscheck, \
+ enable transport stream continuity and TEI check);
Please remove the backslash on the above line. Just do:
MODULE_PARM_DESC(dvb_demux_tscheck,
enable transport stream continuity and TEI check);
+
+#define dprintk_tscheck(x...) do { \
+ if (dvb_demux_tscheck printk_ratelimit()) printk(x); } while (0)
checkpatch.pl is not perfect. The better is to break the macro as I've shown on
my previous email, breaking one statement per line:
#define dprintk_tscheck(x...) do { \
if (dvb_demux_tscheck printk_ratelimit())\
printk(x); \
} while (0)
This allows a faster reading of the line.
PS.: The defines are the only places at the source code where the backslashes
are needed.
+
/**
* static inlined helper functions
@@ -375,6 +384,29 @@
struct dvb_demux_feed *feed;
u16 pid = ts_pid(buf);
int dvr_done = 0;
+
No need for an empty line here. Please remove to keep all vars together.
+ int cnt_pid;
unsigned cnt_pid;
+
+ if (dvb_demux_tscheck) {
+
No need for an empty line here.
+ if (!demux-cnt_storage)
+ demux-cnt_storage = vmalloc(0x1fff);
Please add a define for 0x1fff and use the define, instead of using a magic
value at vmalloc, like:
#define MAX_PID 0x1ffe
...
if (!demux-cnt_storage)
demux-cnt_storage = vmalloc(MAX_PID + 1);
You need to add a check to see if the vmalloc really worked.
Also, if you don't have memory for the first packet, it doesn't make sense to
keep insisting on allocating memory. Better just to disable the check. So, I
would code it like:
if (!demux-cnt_storage) {
WARN(Couldn't allocate memory for TS/TEI check.
Disabling it\n);
dvb_demux_tscheck = 0;
goto no_dvb_demux_tscheck;
}
+
+ /* check pkt counter */
+ cnt_pid = ((buf[1] 0x1f)8) | buf[2];
+
+ if (cnt_pid != 0x1fff) {
if (cnt_pid = MAX_PID) {
This will make clear that there's no chance of writing outside the array limit,
after changing cnt_pid to unsigned.
+ if (buf[1] 0x80)
+ dprintk_tscheck(TEI detected. PID=0x%x
data1=0x%x\n, cnt_pid, buf[1]);
+
+ if ((buf[3] 0xf) != demux-cnt_storage[cnt_pid])
+ dprintk_tscheck(TS packet counter mismatch.
PID=0x%x expected 0x%x got 0x%x\n,\
+ cnt_pid,
demux-cnt_storage[cnt_pid], buf[3] 0xf);
Please, don't add the backslash. Also, in order to have it 80-line compliant,
you could break the strings as:
dprintk_tscheck(TS packet counter mismatch.
PID=0x%x expected 0x%x got
0x%x\n,
cnt_pid,
demux-cnt_storage[cnt_pid],
buf[3] 0xf);
+
+ demux-cnt_storage[cnt_pid] = ((buf[3] 0xf) + 1)0xf;
+ };
+ /* end check */
+ };
In order to work with the lack of memory, you'll need a label here:
no_dvb_demux_tscheck:
Another alternative would be to convert the debug logic into a static function.
The source code will look cleaner, and the gcc optimizer will tranform it into
inline and produce the same binary. Something like:
if (demux-cnt_storage)
Re: [PULL] http://udev.netup.ru/hg/v4l-dvb-aospan
Hi, I missed the original mail, so I am replying to this one. See below. @@ -375,6 +384,29 @@ struct dvb_demux_feed *feed; u16 pid = ts_pid(buf); int dvr_done = 0; + No need for an empty line here. Please remove to keep all vars together. +int cnt_pid; unsigned cnt_pid; There's no need for cnt_pid at all, because pid already contains the PID value. Please add a define for 0x1fff and use the define, instead of using a magic value at vmalloc, like: #define MAX_PID 0x1ffe One would assume MAX_PID to be 0x1fff. Maybe it would be better to have two definitions to improve readability. for dvb_dmx_swfilter_packet(): #define NULL_PID 0x1fff for vmalloc: #define MAX_PID 0x1fff or #define NUM_PIDS 0x2000 ... and use (NUM_PIDS - 1) to make clear that no memory for the NULL PID is allocated. Regards, Andreas -- To unsubscribe from this list: send the line unsubscribe linux-media in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
