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.c    Mon Jun 01 05:22:37 
> 2009 -0300
> +++ b/linux/drivers/media/dvb/dvb-core/dvb_demux.c    Mon 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.h    Mon Jun 01 05:22:37 
> 2009 -0300
> +++ b/linux/drivers/media/dvb/dvb-core/dvb_demux.h    Mon 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

Reply via email to