Mailing List archive

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

[linux-dvb] Re: [PATCH?] proposed rewrite of skystar2 filters



Vincenzo Di Massa wrote:
Il mer, 2003-12-03 alle 09:22, Niklas Peinecke ha scritto:

Wolfgang Thiel wrote:

On Tue, Dec 02, 2003 at 08:15:18PM +0100, Holger Waechtler wrote:


Wolfgang Thiel wrote:


On Tue, Dec 02, 2003 at 12:12:02PM +0100, Niklas Peinecke wrote:
...

do you think the patch is ready for CVS? Don't have a skystar2 here, so I can't test it myself...

No. His patch isn't ready, IMHO. It's very nice, but I don't think it's
ready: as soon as adding hw_filters=0 (and making this the default, and
only requesting his code with hw_filters=1), this patch is fine, and should
be added to CVS, but not now, when there is no option 'hw_filters=0' yet.

Just my 2 cents,
Wolfgang


OK, here is another patch. I incorporated the enable_hw_filters parameter and also implemented ref counting for every pid. I defaulted it to 1 (see my previous post for the reason). Setting it to 0 disables the filters alltogether by just calling open_whole_bandwidth (I'm not sure if this is what you were intending, feel free to make any changes you like).

Also there are now _two_ ref counters for open_whole_bandwidth: one triggered by overflow (i.e. all hw filters in use) and one by special pid 0x2000. This is necessary to avoid the situation that you can close down the ts (opened by overflow) by removing a 0x2000 that was not added.

Niklas
	
Hi Niklas,
in your code you do refcounting in this way:
	// check if the pid is already present
	for(i=0; i<adapter->pid_count; i++)
		if((adapter->pid_list[i]&0x1fff)==pid) {
			adapter->pid_list[i]=((((adapter->pid_list[i]>>13)+1)<<13)|pid);
				// we do ref counting in the high 3 bits
			return 1;
		}
I think it is best to:
	// check if the pid is already present
	for(i=0; i<adapter->pid_count; i++)
		if((adapter->pid_list[i]&0x1fff)==pid) {
			if (((adapter->pid_list[i]&0xe000)>>13)==7)
				return -1; //  This is unlikely to happen, but we only have 7 slots. What should we do? Ignoring it sounds incorrect to me!
			adapter->pid_list[i]=((((adapter->pid_list[i]>>13)+1)<<13)|pid);
				// we do ref counting in the high 3 bits
			return 1;
		}


Looks like a good idea, I just thought it would be unlikely to have more than 8 slots (note that the first is actually 0), but it's safer to catch the overflow.

Niklas



--
Info:
To unsubscribe send a mail to ecartis@linuxtv.org with "unsubscribe linux-dvb" as subject.



Home | Main Index | Thread Index