re-use devices in transfer mode

Message ID 46297FC1.8070104@gmail.com
State New
Headers

Commit Message

Anssi Hannula April 21, 2007, 3:06 a.m. UTC
  Hi!

Consider the scenario:
- 2 devices, 1 used in transfer mode on transponder A
- a new recording / streamdev session starts on transponder A

Now, IMO the correct thing to do is start the new receiver in the
transfer-moded device, so that the second device is left free.

However, the usual "use-already-tuned-devices" check in GetDevice() only
checks for device->Receiving(), which does not report transfer-moded
device, resulting in the new receiver being started on second device,
thus both devices being reserved for receiving data from the same
transponder.

Attached is a patch for GetDevice() to check for transfer-moded devices
as well.
  

Comments

Udo Richter April 21, 2007, 10:45 a.m. UTC | #1
Anssi Hannula wrote:
> However, the usual "use-already-tuned-devices" check in GetDevice() only
> checks for device->Receiving(), which does not report transfer-moded
> device, resulting in the new receiver being started on second device,
> thus both devices being reserved for receiving data from the same
> transponder.

Without taking a deeper look into it right now, I think there was some 
trap with detecting transfer mode in case that some streams are received 
additionally to live mode, for example teletext with osdteletext plugin. 
You may want to double-check this. Changes to GetDevice tend to have 
strange side effects.

Cheers,

Udo
  
C.Y.M April 21, 2007, 11:19 a.m. UTC | #2
> Anssi Hannula wrote:
> > However, the usual "use-already-tuned-devices" check in GetDevice() only
> > checks for device->Receiving(), which does not report transfer-moded
> > device, resulting in the new receiver being started on second device,
> > thus both devices being reserved for receiving data from the same
> > transponder.
>
>
Can this patch also be applied to vdr-1.5.1 with a little manual fixing?
Thanks.
  
Anssi Hannula April 21, 2007, 2:26 p.m. UTC | #3
Udo Richter wrote:
> Anssi Hannula wrote:
>> However, the usual "use-already-tuned-devices" check in GetDevice() only
>> checks for device->Receiving(), which does not report transfer-moded
>> device, resulting in the new receiver being started on second device,
>> thus both devices being reserved for receiving data from the same
>> transponder.
> 
> Without taking a deeper look into it right now, I think there was some
> trap with detecting transfer mode in case that some streams are received
> additionally to live mode, for example teletext with osdteletext plugin.
> You may want to double-check this. Changes to GetDevice tend to have
> strange side effects.

I proposed this same patch previously, but it was suggested to instead
of the check for transfer mode to just change the Receiving() to
Receiving(true). I didn't see any caveats back then, so I agreed.
Unfortunately, that resulted in problems with situations that you describe.

However, this original version of the patch does explicitely check for a
device in transfer-mode, and does not care about osdteletext receivers.

What could happen is that this would match the transfer modes whose
receiverdevice() is the primary device itself (some situations related
to the ca or ac3, I guess), thus starting the new receiver in the
primary device, which could cause side-effects if the card's bandwidth
is not wide enough. However, even without this patch, when a recording
is already taking place on the primary device, this rule matches and
another recording could be started on the primary device. I don't know
if we should be preventing these from happening, but if we do, I think
we could make the rule as
imp |= !device[i]->Receiving() && (device[i] !=
cTransferControl::ReceiverDevice() || device[i]->IsPrimaryDevice()) ||
ndr // prevents matching local-transfer-moded primary-device
or
imp |= !device[i]->Receiving() && device[i] !=
cTransferControl::ReceiverDevice() || device[i]->IsPrimaryDevice() ||
ndr // addidtionally prevents matching recording primary-devices
  
Anssi Hannula April 21, 2007, 2:29 p.m. UTC | #4
Stone wrote:
> 
>     Anssi Hannula wrote:
>     > However, the usual "use-already-tuned-devices" check in
>     GetDevice() only
>     > checks for device->Receiving(), which does not report transfer-moded
>     > device, resulting in the new receiver being started on second device,
>     > thus both devices being reserved for receiving data from the same
>     > transponder.
> 
> 
> Can this patch also be applied to vdr-1.5.1 with a little manual
> fixing?  Thanks.

I think so, by modifying line 330:
 imp <<= 1; imp |= !device[i]->Receiving() || ndr;
to
 imp <<= 1; imp |= !device[i]->Receiving() && device[i] !=
cTransferControl::ReceiverDevice() || ndr;
  
Klaus Schmidinger April 29, 2007, 1:14 p.m. UTC | #5
On 04/21/07 16:26, Anssi Hannula wrote:
> Udo Richter wrote:
>> Anssi Hannula wrote:
>>> However, the usual "use-already-tuned-devices" check in GetDevice() only
>>> checks for device->Receiving(), which does not report transfer-moded
>>> device, resulting in the new receiver being started on second device,
>>> thus both devices being reserved for receiving data from the same
>>> transponder.
>> Without taking a deeper look into it right now, I think there was some
>> trap with detecting transfer mode in case that some streams are received
>> additionally to live mode, for example teletext with osdteletext plugin.
>> You may want to double-check this. Changes to GetDevice tend to have
>> strange side effects.
> 
> I proposed this same patch previously, but it was suggested to instead
> of the check for transfer mode to just change the Receiving() to
> Receiving(true). I didn't see any caveats back then, so I agreed.
> Unfortunately, that resulted in problems with situations that you describe.
> 
> However, this original version of the patch does explicitely check for a
> device in transfer-mode, and does not care about osdteletext receivers.
> 
> What could happen is that this would match the transfer modes whose
> receiverdevice() is the primary device itself (some situations related
> to the ca or ac3, I guess), thus starting the new receiver in the
> primary device, which could cause side-effects if the card's bandwidth
> is not wide enough. However, even without this patch, when a recording
> is already taking place on the primary device, this rule matches and
> another recording could be started on the primary device. I don't know
> if we should be preventing these from happening, but if we do, I think
> we could make the rule as
> imp |= !device[i]->Receiving() && (device[i] !=
> cTransferControl::ReceiverDevice() || device[i]->IsPrimaryDevice()) ||
> ndr // prevents matching local-transfer-moded primary-device
> or
> imp |= !device[i]->Receiving() && device[i] !=
> cTransferControl::ReceiverDevice() || device[i]->IsPrimaryDevice() ||
> ndr // addidtionally prevents matching recording primary-devices

I'm not sure about changing anything in that area in any 1.4 maintenance patch.

If this gets changed at all, it will be in the 1.5 developer version.
So please provide a patch for that version, possibly including all the
thoughts that have come up in this thread.

Klaus
  

Patch

diff -Nurp -x '*~' vdr-1.4.6/device.c vdr-1.4.6-f2/device.c
--- vdr-1.4.6/device.c	2006-09-03 13:13:25.000000000 +0300
+++ vdr-1.4.6-f2/device.c	2007-04-21 05:58:38.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] == cTransferControl::ReceiverDevice();        // avoid the Transfer Mode receiver device
          imp <<= 8; imp |= min(max(device[i]->Priority() + MAXPRIORITY, 0), 0xFF); // use the device with the lowest priority (+MAXPRIORITY to assure that values -99..99 can be used)