device.c: cDevice::GetDevice

Message ID 44A9B08C.6060604@gmail.com
State New
Headers

Commit Message

Anssi Hannula July 4, 2006, 12:04 a.m. UTC
  syrius.ml@no-log.org wrote:
> Hi,

Hi!

> let's consider this setup:
> - one headless computer, 2 dvb-s cards (1 ff, 1 budget) both
>   receiving the exactly the same set of channels
> - this server runs 2 vdr instances: first main one with
>   streamdev-server handling the dvb cards, the other one with
>   streamdev-client
> - both of them are running vdr-xineliboutput plugin so that i can have
>   2 independant outputs(TV) plugged in.
> - main vdr is doing the recording and also being used a http stream
>   server for one client (occasionnaly)
> 
> The channels i'm using are those:
> :BBC
> BBC 1 London;BSkyB:10773:hC56:S28.2E:22000:5000:5001=eng,5002=NAR:5003:0:6301:2:2045:0
> BBC 2 England;BSkyB:10773:hC56:S28.2E:22000:5100:5101=eng,5102=NAR:5103:0:6302:2:2045:0
> BBC THREE;BSkyB:10773:hC56:S28.2E:22000:5200:5201=eng,5202=NAR:5203:0:6319:2:2045:0
> BBC FOUR;BSkyB:10773:hC56:S28.2E:22000:5300:5301=eng,5302=NAR:5303:0:6316:2:2045:0
> :ITV
> ITV1 Granada;BSkyB:10758:vC56:S28.2E:22000:2318:2319=eng:2320:0:10080:2:2044:0
> ITV2;BSkyB:10758:vC56:S28.2E:22000:2314:2315=eng,2363=NAR:2317:0:10070:2:2044:0
> ITV3;BSkyB:10906:vC56:S28.2E:22000:2363:2364=eng,2314=NAR:2365:0:10260:2:2054:0
> ITV4;BSkyB:10758:vC56:S28.2E:22000:2305:2306=eng:2309:0:10072:2:2044:0
> 
> I have added
> esyslog("GetDevice(1) device #%d imp=%x ndr=%d isreceiving=%d isactual=%d isprimary=%d hasdecoder=%d devprio=%d Priority=%d providesca=%d\n",i,imp,ndr,device[i]->Receiving(),device[i]==ActualDevice(),device[i]->IsPrimaryDevice(),device[i]->HasDecoder(),device[i]->Priority(),Priority,device[i]->ProvidesCa(Channel));
> before if (imp < Impact) {
> and added
> esyslog("GetDevice(2) device #%d SELECTED imp=%x\n",i,imp);
> before if (NeedsDetachReceivers)
> 
> first (main) vdr tunes to bbc 1: it uses the budget card:
> Jul  4 00:33:24 dvb vdr: [22230] switching to channel 1
> Jul  4 00:33:24 dvb vdr: [22230] GetDevice(1) device #0 imp=116202 ndr=0 isreceiving=0 isactual=0 isprimary=0 hasdecoder=1 devprio=-1 Priority=0 providesca=2 
> Jul  4 00:33:24 dvb vdr: [22230] GetDevice(2) device #0 SELECTED imp=116202 
> Jul  4 00:33:24 dvb vdr: [22230] INFO: Using software decryption on card 1
> Jul  4 00:33:24 dvb vdr: [22230] GetDevice(1) device #1 imp=106202 ndr=0 isreceiving=0 isactual=0 isprimary=0 hasdecoder=0 devprio=-1 Priority=0 providesca=2 
> Jul  4 00:33:24 dvb vdr: [22230] GetDevice(2) device #1 SELECTED imp=106202 
> 
> 
> then second vdr is launched and tuned to bbc 2: it uses the ff card:
> Jul  4 00:37:05 dvb vdr: [22246] GetDevice(1) device #0 imp=116202 ndr=0 isreceiving=0 isactual=0 isprimary=0 hasdecoder=1 devprio=-1 Priority=0 providesca=2 
> Jul  4 00:37:05 dvb vdr: [22246] GetDevice(2) device #0 SELECTED imp=116202 
> Jul  4 00:37:05 dvb vdr: [22246] GetDevice(1) device #1 imp=146202 ndr=0 isreceiving=0 isactual=1 isprimary=0 hasdecoder=0 devprio=-1 Priority=0 providesca=2 
> Jul  4 00:37:06 dvb vdr: [22246] GetDevice(1) device #0 imp=116202 ndr=0 isreceiving=0 isactual=0 isprimary=0 hasdecoder=1 devprio=-1 Priority=0 providesca=2
> Jul  4 00:37:06 dvb vdr: [22246] GetDevice(2) device #0 SELECTED imp=116202 
> Jul  4 00:37:06 dvb vdr: [22246] GetDevice(1) device #1 imp=146202 ndr=0 isreceiving=0 isactual=1 isprimary=0 hasdecoder=0 devprio=-1 Priority=0 providesca=2 
> Jul  4 00:37:06 dvb vdr: [22246] Streamdev: Setting data connection to 10.0.2.10:48025
> 
> How could I adapt GetDevice so that the second vdr would use the same
> card ? (bbc1 and bbc2 are on the same transponder)
> 
> Because if i start a recording or the http client on itv1, one transfer
> mode is interrupted. It's obviously not what i want. I would have
> expected bbc1 and bbc2 to be handled by only one card letting the
> other one free for something else.
> 
> 
> 
> in cDevice::GetDevice (device.c):
>          imp <<= 1; imp |= !device[i]->Receiving() || ndr;

This is the problem. Devices in transfer-mode return false from
device->Receiving(), so the impact gets increased even with already
tuned devices.

Attached is a patch that takes transfer-moded devices into account.
Please try it.


> could this be replaced by:
>          imp |= ndr;

No.

> are there cases where ndr is True and device[i]->Receiving() is false ?

Yes, when the device in question is in transfer-mode or has other
priority -1 receivers attached.

> if device[i]->Receiving() and ndr is false i guess that would mean
> this device should be selected. (and maybe imp shouldn't be increased
> in that case)

On the contrary, this check is for the situation that the already-tuned
device should be used, even when Receiving() == true.

> or/and what could i change in:
>          imp <<= 1; imp |= device[i] == ActualDevice();
> so that it doesn't increase imp if ActualDevice also provides the
> desired channel ?
> (or maybe this test could be skipped in that case ?)

In that case the (fixed) first check will not increase the impact and
the device will thus be selected, and the impact of this ActualDevice()
check does not matter.

> 
> another ex:
> for a reason or another, 1st vdr tunes to bbc1 and uses first dvd card
> (the ff one)
> then second vdr wants to tune to bbc1 as well: unfortunatly the
> selected device is not the first one but the second one because it
> doesn't have a decoder. (and imp is unconditionnaly increased when the
> card has a decoder)

This is for the same reason I stated above, the first check should
ensure that the already tuned card gets selected.

> 
> I understand selecting the appropriate device is not that quite
> simple.
> Is there a way to make this selection more modular and add a new
> setting to select between different GetDevice behabiors ?

No need for that, this bug just needs to be fixed.
  

Comments

syrius.ml@no-log.org July 4, 2006, 1:10 a.m. UTC | #1
Anssi Hannula <anssi.hannula@gmail.com> writes:

>> in cDevice::GetDevice (device.c):
>>          imp <<= 1; imp |= !device[i]->Receiving() || ndr;
>
> This is the problem. Devices in transfer-mode return false from
> device->Receiving(), so the impact gets increased even with already
> tuned devices.
>
> Attached is a patch that takes transfer-moded devices into account.
> Please try it.

it seems it corrects it.
Thanks a lot !

--
  
Klaus Schmidinger July 22, 2006, 1 p.m. UTC | #2
syrius.ml@no-log.org wrote:
> Anssi Hannula <anssi.hannula@gmail.com> writes:
> 
>>> in cDevice::GetDevice (device.c):
>>>          imp <<= 1; imp |= !device[i]->Receiving() || ndr;
>> This is the problem. Devices in transfer-mode return false from
>> device->Receiving(), so the impact gets increased even with already
>> tuned devices.
>>
>> Attached is a patch that takes transfer-moded devices into account.
>> Please try it.
> 
> it seems it corrects it.

Can you please test whether this also works with

   imp <<= 1; imp |= !device[i]->Receiving(true) || ndr;

instead of

   imp <<= 1; imp |= !device[i]->Receiving() && device[i] != cTransferControl::ReceiverDevice() || ndr;


@Anssi: is there a particular reason why you have limited this to
         devices in Transfer-Mode? With Receiving(true) it would check
         if there is _any_ receiver attached to that device. This would also
         cover other streaming clients.

Klaus
  
Anssi Hannula July 22, 2006, 1:20 p.m. UTC | #3
Klaus Schmidinger wrote:
> syrius.ml@no-log.org wrote:
> 
>> Anssi Hannula <anssi.hannula@gmail.com> writes:
>>
>>>> in cDevice::GetDevice (device.c):
>>>>          imp <<= 1; imp |= !device[i]->Receiving() || ndr;
>>>
>>> This is the problem. Devices in transfer-mode return false from
>>> device->Receiving(), so the impact gets increased even with already
>>> tuned devices.
>>>
>>> Attached is a patch that takes transfer-moded devices into account.
>>> Please try it.
>>
>>
>> it seems it corrects it.
> 
> 
> Can you please test whether this also works with
> 
>   imp <<= 1; imp |= !device[i]->Receiving(true) || ndr;
> 
> instead of
> 
>   imp <<= 1; imp |= !device[i]->Receiving() && device[i] !=
> cTransferControl::ReceiverDevice() || ndr;
> 
> 
> @Anssi: is there a particular reason why you have limited this to
>         devices in Transfer-Mode? With Receiving(true) it would check
>         if there is _any_ receiver attached to that device. This would also
>         cover other streaming clients.

Not really, I just didn't think of Receiving(true) at the time :/
Yep, it is probably better to use Receiving(true), so that all priority
-1 receivers will match.

Note that streaming clients are already caught with Receiving() as afaik
their priority is >= 0, so this modification will only affect
transfer-mode and other receivers with priority -1.
  

Patch

--- vdr-1.4.1/device.c.old	2006-07-04 02:42:40.000000000 +0300
+++ vdr-1.4.1/device.c	2006-07-04 02:47:45.000000000 +0300
@@ -292,7 +292,7 @@  cDevice *cDevice::GetDevice(const cChann
          // to their individual severity, where the one listed first will make the most
          // difference, because it results in the most significant bit of the result.
          uint imp = 0;
-         imp <<= 1; imp |= !device[i]->Receiving() || ndr;                         // use receiving devices if we don't need to detach existing receivers
+         imp <<= 1; imp |= !device[i]->Receiving() && device[i] != cTransferControl::ReceiverDevice() || ndr; // use receiving devices if we don't need to detach existing receivers
          imp <<= 1; imp |= device[i]->Receiving();                                 // avoid devices that are receiving
          imp <<= 1; imp |= device[i] == ActualDevice();                            // avoid the actual device (in case of Transfer Mode)
          imp <<= 1; imp |= device[i]->IsPrimaryDevice();                           // avoid the primary device