Klaus Schmidinger wrote:
> Anssi Hannula wrote:
>> diff -Nurp -x '*~' vdr-1.4.0/device.c vdr-1.4.0-fix/device.c
>> --- vdr-1.4.0/device.c 2006-04-14 17:34:43.000000000 +0300
>> +++ vdr-1.4.0-fix/device.c 2006-05-12 06:31:20.000000000 +0300
>> @@ -627,7 +627,7 @@ eSetChannelResult cDevice::SetChannel(co
>> // use the card that actually can receive it and transfer data from
>> there:
>>
>> if (NeedsTransferMode) {
>> - cDevice *CaDevice = GetDevice(Channel, 0, &NeedsDetachReceivers);
>> + cDevice *CaDevice = GetDevice(Channel, Setup.PrimaryLimit,
>> &NeedsDetachReceivers);
>> if (CaDevice && CanReplay()) {
>> cStatus::MsgChannelSwitch(this, 0); // only report status if
>> we are actually going to switch the channel
>> if (CaDevice->SetChannel(Channel, false) == scrOk) { //
>> calling SetChannel() directly, not SwitchChannel()!
>> @@ -1158,7 +1158,7 @@ int cDevice::Ca(void) const
>>
>> int cDevice::Priority(void) const
>> {
>> - int priority = IsPrimaryDevice() ? Setup.PrimaryLimit - 1 :
>> DEFAULTPRIORITY;
>> + int priority = ActualDevice() == this ? Setup.PrimaryLimit - 1 :
>> DEFAULTPRIORITY;
>> for (int i = 0; i < MAXRECEIVERS; i++) {
>> if (receiver[i])
>> priority = max(receiver[i]->priority, priority);
>> @@ -1183,6 +1183,8 @@ int cDevice::ProvidesCa(const cChannel *
>>
>> bool cDevice::Receiving(bool CheckAny) const
>> {
>> + if (this == cTransferControl::ReceiverDevice())
>> + return true;
>> for (int i = 0; i < MAXRECEIVERS; i++) {
>> if (receiver[i] && (CheckAny || receiver[i]->priority >= 0)) //
>> cReceiver with priority < 0 doesn't count
>> return true;
>
>
> I'm afraid this patch has a nasty side effect.
>
> Asssume the following scenario:
>
> - two devices, primary is 1
> - all channels are available on both devices, except for channels
> 10, 11 and 12 which can only be received by device 2
> - channels 10 and 11 are on the same transponder, channel 12 is on a
> different one
> - switch directly to channel 10 -> transfer mode starts
> - press "Up" to switch to channel 11, but that channel is skipped and
> it switches to channel 12
>
> So I guess I can't accept this patch.
Thank you for taking a look.
However, I don't see why there is such a problem in the above scenario:
- let's consider PrimaryLimit is 0 (the default)
- SwitchChannel() gets called (device.c line 581)
- PrimaryDevice()->ProvidesChannel() returns false (line 591)
- GetDevice() is called (line 281)
- for the device 1 ProvidesChannel() returns false (line 288)
- ProvidesChannel() is called for device 2 (dvbdevice.c line 767)
- Priority = 0, this->Priority() = -1, so hasPriority = true
- ProvidesChannel() result = true is set in line 785
- ProvidesChannel() return true and ndr = false (device.c line 288)
- (device[i]->Receiving() && !ndr) evaluates true (line 290)
- pri = 0 so device 2 is selected
- SwitchChannel() calls another SwitchChannel (line 601)
- SwitchChannel() calls SetChannel (line 568)
- and so on
Hopefully I'll have time to test this scenario later today.
BTW, isn't the PrimaryLimit there so that liveview would not be
distracted when a recording with (priority < primarylimit) is trying to
start? Why is it used for primary device exclusively, then? If the
liveview is in transfer-mode, shouldn't that be protected as well?
If you think it should be used for primary device only, only the change
in Receiving() in my patch is necessary. If you think it should indeed
be used for whatever device the user currently is watching, there are
some more places where transfermode-liveview priority is assumed 0, when
it should be PrimaryLimit (at least in device.c line 591 call to
GetDevice()).
Or if you still think the PrimaryLimit adds only complexity for not much
of real value, then remove it ;) and assume 0 for it's value everywhere.
--
Anssi Hannula
